Il giorno 21/lug/2012 15:22, "Stuart Henderson" <s...@spacehopper.org> ha
scritto:
>
> 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.
Here's one: ok for me.
Ciao,
David
>
> 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 */
