Re: send fewer router solicitations
On Mon, Sep 26, 2016 at 10:16:04PM +0100, Stuart Henderson wrote: > On 2016/09/26 20:14, Florian Obser wrote: > > On Wed, Sep 21, 2016 at 01:23:25PM +0100, Stuart Henderson wrote: > > > > > There's a problem with this: we lose the exponential backoff for the > > > quick timer. Say you have v6 at home and enable autoconf on your laptop > > > then move to a network without v6 - this results in you spamming the > > > network with a multicast frame every second, which does not make you > > > a good network citizen especially if that's on a wireless lan. > > > > > [...] > > > I think the current state is quite a lot worse than the previous one > > > (even though it's better on networks that *do* have v6), so I'm wondering > > > if it might be better to revert if it's complicated to fix here (there > > > was a different plan for sending RS in the future anyway wasn't there?) > > > > does this help? > > If not, I guess we should back it out > > > > diff --git nd6_rtr.c nd6_rtr.c > > index a7529c9..3c23365 100644 > > --- nd6_rtr.c > > +++ nd6_rtr.c > > @@ -328,7 +328,8 @@ nd6_rs_output_timo(void *ignored_arg) > > t = nd6_rs_next_pltime_timo(ifp); > > if (t == ND6_INFINITE_LIFETIME || t < > > ND6_RS_OUTPUT_INTERVAL) { > > - timeout = ND6_RS_OUTPUT_QUICK_INTERVAL; > > + if (t == ND6_INFINITE_LIFETIME) > > + timeout = ND6_RS_OUTPUT_QUICK_INTERVAL; > > ia6 = in6ifa_ifpforlinklocal(ifp, > > IN6_IFF_TENTATIVE); > > if (ia6 != NULL) > > Same behaviour with this. The problem is that it's just setting > to a fixed ND6_RS_OUTPUT_QUICK_INTERVAL which isn't getting backed > off anywhere. To fix it I think we'd at least need another global > (if not a per-interface variable) that can be used as the "current > quick_timeout" and back that off or reset it as necessary, but > my attempts to do that thus far haven't been successful. > then let's revert. OK? diff --git nd6_rtr.c nd6_rtr.c index 93e2f4c..a9fb3f5 100644 --- nd6_rtr.c +++ nd6_rtr.c @@ -75,7 +75,6 @@ int rt6_deleteroute(struct rtentry *, void *, unsigned int); void nd6_addr_add(void *); void nd6_rs_output_timo(void *); -u_int32_t nd6_rs_next_pltime_timo(struct ifnet *); void nd6_rs_output_set_timo(int); void nd6_rs_output(struct ifnet *, struct in6_ifaddr *); void nd6_rs_dev_state(void *); @@ -284,64 +283,30 @@ nd6_rs_output_set_timo(int timeout) timeout_add_sec(_rs_output_timer, nd6_rs_output_timeout); } -u_int32_t -nd6_rs_next_pltime_timo(struct ifnet *ifp) -{ - struct ifaddr *ifa; - struct in6_ifaddr *ia6; - u_int32_t pltime_expires = ND6_INFINITE_LIFETIME; - - TAILQ_FOREACH(ifa, >if_addrlist, ifa_list) { - if (ifa->ifa_addr->sa_family != AF_INET6) - continue; - - ia6 = ifatoia6(ifa); - if (ia6->ia6_lifetime.ia6t_pltime == ND6_INFINITE_LIFETIME || - IFA6_IS_DEPRECATED(ia6) || IFA6_IS_INVALID(ia6)) - continue; - - pltime_expires = MIN(pltime_expires, - ia6->ia6_lifetime.ia6t_pltime); - } - - return pltime_expires; -} - void nd6_rs_output_timo(void *ignored_arg) { struct ifnet *ifp; struct in6_ifaddr *ia6; - u_int32_t pltime_expire = ND6_INFINITE_LIFETIME, t; - int timeout = ND6_RS_OUTPUT_INTERVAL; if (nd6_rs_timeout_count == 0) return; if (nd6_rs_output_timeout < ND6_RS_OUTPUT_INTERVAL) /* exponential backoff if running quick timeouts */ - timeout = nd6_rs_output_timeout * 2; + nd6_rs_output_timeout *= 2; + if (nd6_rs_output_timeout > ND6_RS_OUTPUT_INTERVAL) + nd6_rs_output_timeout = ND6_RS_OUTPUT_INTERVAL; TAILQ_FOREACH(ifp, , if_list) { if (ISSET(ifp->if_flags, IFF_RUNNING) && ISSET(ifp->if_xflags, IFXF_AUTOCONF6)) { - t = nd6_rs_next_pltime_timo(ifp); - if (t == ND6_INFINITE_LIFETIME || t < - ND6_RS_OUTPUT_INTERVAL) { - timeout = ND6_RS_OUTPUT_QUICK_INTERVAL; - ia6 = in6ifa_ifpforlinklocal(ifp, - IN6_IFF_TENTATIVE); - if (ia6 != NULL) - nd6_rs_output(ifp, ia6); - } - - pltime_expire = MIN(pltime_expire, t); + ia6 = in6ifa_ifpforlinklocal(ifp, IN6_IFF_TENTATIVE); + if (ia6 != NULL) + nd6_rs_output(ifp, ia6); } } - if (pltime_expire != ND6_INFINITE_LIFETIME) - timeout =
Re: send fewer router solicitations
On 2016/09/26 20:14, Florian Obser wrote: > On Wed, Sep 21, 2016 at 01:23:25PM +0100, Stuart Henderson wrote: > > > There's a problem with this: we lose the exponential backoff for the > > quick timer. Say you have v6 at home and enable autoconf on your laptop > > then move to a network without v6 - this results in you spamming the > > network with a multicast frame every second, which does not make you > > a good network citizen especially if that's on a wireless lan. > > > [...] > > I think the current state is quite a lot worse than the previous one > > (even though it's better on networks that *do* have v6), so I'm wondering > > if it might be better to revert if it's complicated to fix here (there > > was a different plan for sending RS in the future anyway wasn't there?) > > does this help? > If not, I guess we should back it out > > diff --git nd6_rtr.c nd6_rtr.c > index a7529c9..3c23365 100644 > --- nd6_rtr.c > +++ nd6_rtr.c > @@ -328,7 +328,8 @@ nd6_rs_output_timo(void *ignored_arg) > t = nd6_rs_next_pltime_timo(ifp); > if (t == ND6_INFINITE_LIFETIME || t < > ND6_RS_OUTPUT_INTERVAL) { > - timeout = ND6_RS_OUTPUT_QUICK_INTERVAL; > + if (t == ND6_INFINITE_LIFETIME) > + timeout = ND6_RS_OUTPUT_QUICK_INTERVAL; > ia6 = in6ifa_ifpforlinklocal(ifp, > IN6_IFF_TENTATIVE); > if (ia6 != NULL) Same behaviour with this. The problem is that it's just setting to a fixed ND6_RS_OUTPUT_QUICK_INTERVAL which isn't getting backed off anywhere. To fix it I think we'd at least need another global (if not a per-interface variable) that can be used as the "current quick_timeout" and back that off or reset it as necessary, but my attempts to do that thus far haven't been successful.
Re: send fewer router solicitations
On Wed, Sep 21, 2016 at 01:23:25PM +0100, Stuart Henderson wrote: > There's a problem with this: we lose the exponential backoff for the > quick timer. Say you have v6 at home and enable autoconf on your laptop > then move to a network without v6 - this results in you spamming the > network with a multicast frame every second, which does not make you > a good network citizen especially if that's on a wireless lan. > [...] > I think the current state is quite a lot worse than the previous one > (even though it's better on networks that *do* have v6), so I'm wondering > if it might be better to revert if it's complicated to fix here (there > was a different plan for sending RS in the future anyway wasn't there?) does this help? If not, I guess we should back it out diff --git nd6_rtr.c nd6_rtr.c index a7529c9..3c23365 100644 --- nd6_rtr.c +++ nd6_rtr.c @@ -328,7 +328,8 @@ nd6_rs_output_timo(void *ignored_arg) t = nd6_rs_next_pltime_timo(ifp); if (t == ND6_INFINITE_LIFETIME || t < ND6_RS_OUTPUT_INTERVAL) { - timeout = ND6_RS_OUTPUT_QUICK_INTERVAL; + if (t == ND6_INFINITE_LIFETIME) + timeout = ND6_RS_OUTPUT_QUICK_INTERVAL; ia6 = in6ifa_ifpforlinklocal(ifp, IN6_IFF_TENTATIVE); if (ia6 != NULL) -- I'm not entirely sure you are real.
Re: send fewer router solicitations
On 2016/09/02 10:37, Florian Obser wrote: > Our kernel based rtsol code is like this little child. We bring up > the interface, send our first solicitation and get an advertisment > back with a pltime of a week or so. > > We lean back, quite happy that we can do v6 now, but after 60 seconds > we wake up, oh shit, better check if that prefix is still valid. and > so on and on. > > This is particularly annoying if you try to debug ICMPv6 with tcpdump > in a network with a lot of openbsd machines. > > To stop naddy from pestering me about this at every hackathon (rightly > so!), let's base the timeout on the prefixes pltime. ;) There's a problem with this: we lose the exponential backoff for the quick timer. Say you have v6 at home and enable autoconf on your laptop then move to a network without v6 - this results in you spamming the network with a multicast frame every second, which does not make you a good network citizen especially if that's on a wireless lan. 310 void 311 nd6_rs_output_timo(void *ignored_arg) 312 { 313 struct ifnet *ifp; 314 struct in6_ifaddr *ia6; 315 u_int32_t pltime_expire = ND6_INFINITE_LIFETIME, t; 316 int timeout = ND6_RS_OUTPUT_INTERVAL; 318 if (nd6_rs_timeout_count == 0) 319 return; 320 321 if (nd6_rs_output_timeout < ND6_RS_OUTPUT_INTERVAL) 322 /* exponential backoff if running quick timeouts */ 323 timeout = nd6_rs_output_timeout * 2; I've tried a few things along the lines of these here instead if (nd6_rs_output_timeout < ND6_RS_OUTPUT_INTERVAL) { nd6_rs_output_timeout *= 2; timeout = nd6_rs_output_timeout; } but haven't hit on a working combination yet. I think the current state is quite a lot worse than the previous one (even though it's better on networks that *do* have v6), so I'm wondering if it might be better to revert if it's complicated to fix here (there was a different plan for sending RS in the future anyway wasn't there?) 324 325 TAILQ_FOREACH(ifp, , if_list) { 326 if (ISSET(ifp->if_flags, IFF_RUNNING) && 327 ISSET(ifp->if_xflags, IFXF_AUTOCONF6)) { 328 t = nd6_rs_next_pltime_timo(ifp); 329 if (t == ND6_INFINITE_LIFETIME || t < 330 ND6_RS_OUTPUT_INTERVAL) { 331 timeout = ND6_RS_OUTPUT_QUICK_INTERVAL; 332 ia6 = in6ifa_ifpforlinklocal(ifp, 333 IN6_IFF_TENTATIVE); 334 if (ia6 != NULL) 335 nd6_rs_output(ifp, ia6); 336 } 337 338 pltime_expire = MIN(pltime_expire, t); 339 } 340 } 341 if (pltime_expire != ND6_INFINITE_LIFETIME) 342 timeout = MAX(timeout, pltime_expire / 2); 343 344 nd6_rs_output_set_timo(timeout); 345 }
Re: send fewer router solicitations
On Fri, Sep 02, 2016 at 05:49:22PM +0100, Stuart Henderson wrote: > On 2016/09/02 10:37, Florian Obser wrote: > > To stop naddy from pestering me about this at every hackathon (rightly > > so!), let's base the timeout on the prefixes pltime. ;) > > Just a thought, are we going to need to cap this to the RDNSS time too when > we start caring about that? > I will not parse the DNS options in the kernel. See CVE-2014-3954 ;) I imagine when the time comes that we care we have the router advertisement parsing in userland and solicitations will again be send from userland, too. -- I'm not entirely sure you are real.
Re: send fewer router solicitations
On 2016/09/02 10:37, Florian Obser wrote: > To stop naddy from pestering me about this at every hackathon (rightly > so!), let's base the timeout on the prefixes pltime. ;) Just a thought, are we going to need to cap this to the RDNSS time too when we start caring about that?
Re: send fewer router solicitations
On 2016/09/02 10:37, Florian Obser wrote: > Our kernel based rtsol code is like this little child. We bring up > the interface, send our first solicitation and get an advertisment > back with a pltime of a week or so. > > We lean back, quite happy that we can do v6 now, but after 60 seconds > we wake up, oh shit, better check if that prefix is still valid. and > so on and on. > > This is particularly annoying if you try to debug ICMPv6 with tcpdump > in a network with a lot of openbsd machines. > > To stop naddy from pestering me about this at every hackathon (rightly > so!), let's base the timeout on the prefixes pltime. ;) > > OK? I agree with doing this, but I think this makes it more important that we do something to expire old prefixes when changing networks (i.e. change of link state).