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(&nd6_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, &ifp->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, &ifnet, 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 = MAX(timeout, pltime_expire / 2);
-
-       nd6_rs_output_set_timo(timeout);
+       nd6_rs_output_set_timo(nd6_rs_output_timeout);
 }
 
 void


-- 
I'm not entirely sure you are real.

Reply via email to