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).
send fewer router solicitations
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?
diff --git nd6_rtr.c nd6_rtr.c
index 6ca25f0..4ac978f 100644
--- nd6_rtr.c
+++ nd6_rtr.c
@@ -75,6 +75,7 @@ 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 *);
@@ -283,30 +284,63 @@ 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 */
- nd6_rs_output_timeout *= 2;
- if (nd6_rs_output_timeout > ND6_RS_OUTPUT_INTERVAL)
- nd6_rs_output_timeout = ND6_RS_OUTPUT_INTERVAL;
+ timeout = nd6_rs_output_timeout * 2;
TAILQ_FOREACH(ifp, , if_list) {
if (ISSET(ifp->if_flags, IFF_RUNNING) &&
ISSET(ifp->if_xflags, IFXF_AUTOCONF6)) {
- ia6 = in6ifa_ifpforlinklocal(ifp, IN6_IFF_TENTATIVE);
- if (ia6 != NULL)
- nd6_rs_output(ifp, ia6);
+ t = nd6_rs_next_pltime_timo(ifp);
+ if (t == ND6_INFINITE_LIFETIME || t <
+ ND6_RS_OUTPUT_INTERVAL) {
+ ia6 = in6ifa_ifpforlinklocal(ifp,
+ IN6_IFF_TENTATIVE);
+ if (ia6 != NULL)
+ nd6_rs_output(ifp, ia6);
+ }
+
+ pltime_expire = MIN(pltime_expire, t);
}
}
- nd6_rs_output_set_timo(nd6_rs_output_timeout);
+ if (pltime_expire != ND6_INFINITE_LIFETIME)
+ timeout = MAX(timeout, pltime_expire / 2);
+
+ nd6_rs_output_set_timo(timeout);
}
void
--
I'm not entirely sure you are real.
