netdev - Re: [RFC PATCH net-next 03/19] ipv6: Clear nexthop flags upon netdev up

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180103074418.GA761@splinter>
Date:   Wed, 3 Jan 2018 09:44:18 +0200
From:   Ido Schimmel <idosch@...sch.org>
To:     David Ahern <dsahern@...il.com>
Cc:     Ido Schimmel <idosch@...lanox.com>, netdev@...r.kernel.org,
        davem@...emloft.net, roopa@...ulusnetworks.com,
        nicolas.dichtel@...nd.com, mlxsw@...lanox.com
Subject: Re: [RFC PATCH net-next 03/19] ipv6: Clear nexthop flags upon netdev
 up

Hi David,

On Tue, Jan 02, 2018 at 09:20:47AM -0700, David Ahern wrote:
> On 12/31/17 9:14 AM, Ido Schimmel wrote:
> > Previous patch marked nexthops with the 'dead' and 'linkdown' flags.
> > Clear these flags when the netdev comes back up.
> > 
> > Signed-off-by: Ido Schimmel <idosch@...lanox.com>
> > ---
> >  include/net/ip6_route.h |  1 +
> >  net/ipv6/addrconf.c     |  3 +++
> >  net/ipv6/route.c        | 29 +++++++++++++++++++++++++++++
> >  3 files changed, 33 insertions(+)
> > 
> > diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
> > index 18e442ea93d8..caad39198c2a 100644
> > --- a/include/net/ip6_route.h
> > +++ b/include/net/ip6_route.h
> > @@ -169,6 +169,7 @@ void rt6_ifdown(struct net *net, struct net_device *dev);
> >  void rt6_mtu_change(struct net_device *dev, unsigned int mtu);
> >  void rt6_remove_prefsrc(struct inet6_ifaddr *ifp);
> >  void rt6_clean_tohost(struct net *net, struct in6_addr *gateway);
> > +void rt6_sync_up(struct net_device *dev, unsigned int nh_flags);
> >  
> >  static inline const struct rt6_info *skb_rt6_info(const struct sk_buff *skb)
> >  {
> > diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> > index ed06b1190f05..b6405568ed7b 100644
> > --- a/net/ipv6/addrconf.c
> > +++ b/net/ipv6/addrconf.c
> > @@ -3484,6 +3484,9 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event,
> >  			if (run_pending)
> >  				addrconf_dad_run(idev);
> >  
> > +			/* Device has an address by now */
> > +			rt6_sync_up(dev, RTNH_F_DEAD);
> > +
> 
> Seems like this should be in the NETDEV_UP section, say after
> addrconf_permanent_addr.

Unless the `keep_addr_on_down` sysctl is set, then at this stage the
netdev doesn't have an IP address and we shouldn't clear the dead flag
just yet.

This is consistent with IPv4 that clears the dead flag from nexthops in
a multipath route only if the nexthop device has an IP address. When the
last IPv4 address is removed from a netdev all the routes using it are
flushed and there's nothing to clear upon NETDEV_UP.

Assuming you're OK with that, I can reword the commit message to make it
clearer.