On 2012/07/20 14:03, LEVAI Daniel wrote: > Hi! > > It just crashed, didn't do anything fancy with it. I was connected with > ssh to the machine when suddenly I got disconnected.
http://marc.info/?l=openbsd-misc&m=134072894816852&w=2 http://marc.info/?l=openbsd-misc&m=134149317104627&w=2 http://arp.thrush.com/openbsd/pppoe/sys_net_if_spppsubr_c.diff Here's a version of the patch without the extra debug printf/logs. I don't see any regressions with this, I've manually triggered the code a couple of times, but don't really want to keep dropping my ADSL for a better test as the telco is likely to drop my sync speed if I do that. Any OKs? I think this is safe enough to make release. Index: if_spppsubr.c =================================================================== RCS file: /cvs/src/sys/net/if_spppsubr.c,v retrieving revision 1.96 diff -u -p -r1.96 if_spppsubr.c --- if_spppsubr.c 28 Jan 2012 12:14:45 -0000 1.96 +++ if_spppsubr.c 21 Jul 2012 13:10:24 -0000 @@ -398,7 +398,7 @@ HIDE void sppp_qflush(struct ifqueue *if int sppp_update_gw_walker(struct radix_node *rn, void *arg, u_int); void sppp_update_gw(struct ifnet *ifp); HIDE void sppp_set_ip_addrs(void *, void *); -HIDE void sppp_clear_ip_addrs(struct sppp *sp); +HIDE void sppp_clear_ip_addrs(void *, void *); HIDE void sppp_set_phase(struct sppp *sp); /* our control protocol descriptors */ @@ -3014,6 +3014,10 @@ struct sppp_set_ip_addrs_args { u_int32_t hisaddr; }; +struct sppp_clear_ip_addrs_args { + struct sppp *sp; +}; + HIDE void sppp_ipcp_tlu(struct sppp *sp) { @@ -3096,9 +3100,22 @@ sppp_ipcp_tls(struct sppp *sp) HIDE void sppp_ipcp_tlf(struct sppp *sp) { + struct ifnet *ifp = &sp->pp_if; + struct sppp_clear_ip_addrs_args *args; + + args = malloc(sizeof(*args), M_TEMP, M_NOWAIT); + if (args == NULL) + return; + + args->sp = sp; + if (sp->ipcp.flags & (IPCP_MYADDR_DYN|IPCP_HISADDR_DYN)) /* Some address was dynamic, clear it again. */ - sppp_clear_ip_addrs(sp); + if (workq_add_task(NULL, 0, sppp_clear_ip_addrs, args, NULL)) { + free(args, M_TEMP); + printf("%s: workq_add_task failed, cannot clear " + "addresses\n", ifp->if_xname); + } /* we no longer need LCP */ sp->lcp.protos &= ~(1 << IDX_IPCP); @@ -4760,16 +4777,27 @@ sppp_set_ip_addrs(void *arg1, void *arg2 } /* - * Clear IP addresses. Must be called at splnet. + * Work queue task clearing addresses from process context. + * Clear IP addresses. */ HIDE void -sppp_clear_ip_addrs(struct sppp *sp) +sppp_clear_ip_addrs(void *arg1, void *arg2) { + struct sppp_clear_ip_addrs_args *args = arg1; + struct sppp *sp = args->sp; struct ifnet *ifp = &sp->pp_if; + int debug = ifp->if_flags & IFF_DEBUG; struct ifaddr *ifa; struct sockaddr_in *si; struct sockaddr_in *dest; + int s; + + /* Arguments are now on local stack so free temporary storage. */ + free(args, M_TEMP); + + s = splsoftnet(); + u_int32_t remote; if (sp->ipcp.flags & IPCP_HISADDR_DYN) remote = sp->ipcp.saved_hisaddr; @@ -4792,6 +4820,7 @@ sppp_clear_ip_addrs(struct sppp *sp) } if (ifa && si) { + int error; struct sockaddr_in new_sin = *si; in_ifscrub(ifp, ifatoia(ifa)); @@ -4800,10 +4829,17 @@ sppp_clear_ip_addrs(struct sppp *sp) if (sp->ipcp.flags & IPCP_HISADDR_DYN) /* replace peer addr in place */ dest->sin_addr.s_addr = sp->ipcp.saved_hisaddr; - if (!in_ifinit(ifp, ifatoia(ifa), &new_sin, 0, 0)) + if (!(error = in_ifinit(ifp, ifatoia(ifa), &new_sin, 0, 0))) dohooks(ifp->if_addrhooks, 0); + if (debug && error) { + log(LOG_DEBUG, SPP_FMT "sppp_clear_ip_addrs: in_ifinit " + " failed, error=%d\n", SPP_ARGS(ifp), error); + splx(s); + return; + } sppp_update_gw(ifp); } + splx(s); } Index: pf.c =================================================================== RCS file: /cvs/src/sys/net/pf.c,v retrieving revision 1.808 diff -u -p -r1.808 pf.c --- pf.c 10 Jul 2012 17:33:48 -0000 1.808 +++ pf.c 21 Jul 2012 13:10:24 -0000 @@ -2526,7 +2526,7 @@ pf_send_tcp(const struct pf_rule *r, sa_ m->m_pkthdr.pf.flags |= PF_TAG_GENERATED; m->m_pkthdr.pf.tag = rtag; m->m_pkthdr.rdomain = rdom; - if (r && r->set_prio[0] != PF_PRIO_NOTSET) + if (r && (r->scrub_flags & PFSTATE_SETPRIO)) m->m_pkthdr.pf.prio = r->set_prio[0]; #ifdef ALTQ @@ -2650,7 +2650,7 @@ pf_send_icmp(struct mbuf *m, u_int8_t ty m0->m_pkthdr.pf.flags |= PF_TAG_GENERATED; m0->m_pkthdr.rdomain = rdomain; - if (r && r->set_prio[0] != PF_PRIO_NOTSET) + if (r && (r->scrub_flags & PFSTATE_SETPRIO)) m0->m_pkthdr.pf.prio = r->set_prio[0]; #ifdef ALTQ @@ -3279,11 +3279,9 @@ pf_rule_to_actions(struct pf_rule *r, st if (r->max_mss) a->max_mss = r->max_mss; a->flags |= (r->scrub_flags & (PFSTATE_NODF|PFSTATE_RANDOMID| - PFSTATE_SETTOS|PFSTATE_SCRUB_TCP)); - if (r->set_prio[0] != PF_PRIO_NOTSET) - a->set_prio[0] = r->set_prio[0]; - if (r->set_prio[1] != PF_PRIO_NOTSET) - a->set_prio[1] = r->set_prio[1]; + PFSTATE_SETTOS|PFSTATE_SCRUB_TCP|PFSTATE_SETPRIO)); + a->set_prio[0] = r->set_prio[0]; + a->set_prio[1] = r->set_prio[1]; } #define PF_TEST_ATTRIB(t, a) \ @@ -3319,7 +3317,6 @@ pf_test_rule(struct pf_pdesc *pd, struct u_int8_t icmptype = 0, icmpcode = 0; bzero(&act, sizeof(act)); - act.set_prio[0] = act.set_prio[1] = PF_PRIO_NOTSET; bzero(sns, sizeof(sns)); act.rtableid = pd->rdomain; SLIST_INIT(&rules); @@ -6886,11 +6883,11 @@ done: pf_tag_packet(pd.m, s->tag, s->rtableid[pd.didx]); if (pqid || (pd.tos & IPTOS_LOWDELAY)) { qid = s->pqid; - if (s->set_prio[1] != PF_PRIO_NOTSET) + if (s->state_flags & PFSTATE_SETPRIO) pd.m->m_pkthdr.pf.prio = s->set_prio[1]; } else { qid = s->qid; - if (s->set_prio[0] != PF_PRIO_NOTSET) + if (s->state_flags & PFSTATE_SETPRIO) pd.m->m_pkthdr.pf.prio = s->set_prio[0]; } } else { @@ -6898,11 +6895,11 @@ done: r->set_tos); if (pqid || (pd.tos & IPTOS_LOWDELAY)) { qid = r->pqid; - if (r->set_prio[1] != PF_PRIO_NOTSET) + if (r->scrub_flags & PFSTATE_SETPRIO) pd.m->m_pkthdr.pf.prio = r->set_prio[1]; } else { qid = r->qid; - if (r->set_prio[0] != PF_PRIO_NOTSET) + if (r->scrub_flags & PFSTATE_SETPRIO) pd.m->m_pkthdr.pf.prio = r->set_prio[0]; } } Index: pf_ioctl.c =================================================================== RCS file: /cvs/src/sys/net/pf_ioctl.c,v retrieving revision 1.253 diff -u -p -r1.253 pf_ioctl.c --- pf_ioctl.c 8 Jul 2012 07:58:09 -0000 1.253 +++ pf_ioctl.c 21 Jul 2012 13:10:24 -0000 @@ -1088,10 +1088,9 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a error = EINVAL; if (rule->rt && !rule->direction) error = EINVAL; - if ((rule->set_prio[0] != PF_PRIO_NOTSET && - rule->set_prio[0] > IFQ_MAXPRIO) || - (rule->set_prio[1] != PF_PRIO_NOTSET && - rule->set_prio[1] > IFQ_MAXPRIO)) + if (rule->scrub_flags & PFSTATE_SETPRIO && + (rule->set_prio[0] > IFQ_MAXPRIO || + rule->set_prio[1] > IFQ_MAXPRIO)) error = EINVAL; if (error) { Index: pfvar.h =================================================================== RCS file: /cvs/src/sys/net/pfvar.h,v retrieving revision 1.365 diff -u -p -r1.365 pfvar.h --- pfvar.h 10 Jul 2012 09:38:22 -0000 1.365 +++ pfvar.h 21 Jul 2012 13:10:24 -0000 @@ -648,7 +648,6 @@ struct pf_rule { #define PF_FLUSH 0x01 #define PF_FLUSH_GLOBAL 0x02 u_int8_t flush; -#define PF_PRIO_NOTSET 0xff u_int8_t set_prio[2]; sa_family_t naf; @@ -840,7 +839,9 @@ struct pf_state { #define PFSTATE_SETTOS 0x0040 #define PFSTATE_RANDOMID 0x0080 #define PFSTATE_SCRUB_TCP 0x0100 +#define PFSTATE_SETPRIO 0x0200 #define PFSTATE_SCRUBMASK (PFSTATE_NODF|PFSTATE_RANDOMID|PFSTATE_SCRUB_TCP) +#define PFSTATE_SETMASK (PFSTATE_SETTOS|PFSTATE_SETPRIO) u_int8_t log; u_int8_t timeout; u_int8_t sync_state; /* PFSYNC_S_x */
