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).
