lkml.org 
[lkml]   [2017]   [Aug]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: unregister_netdevice: waiting for eth0 to become free. Usage count = 1
    Hi Wei,

    On Fri, Aug 11, 2017 at 05:10:02PM -0700, Wei Wang wrote:
    > I think we have a potential fix for this issue.
    > Martin and I found that when addrconf_dst_alloc() creates a rt6, it is
    > possible that rt6->dst.dev points to loopback device while
    > rt6->rt6i_idev->dev points to a real device.
    > When the real device goes down, the current fib6 clean up code only
    > checks for rt6->dst.dev and assumes rt6->rt6i_idev->dev is the same.
    > That leaves unreleased refcnt on the real device if rt6->dst.dev
    > points to loopback dev.

    [...]

    > From 2d8861808c2029013f6b6e86120ba6902329145b Mon Sep 17 00:00:00 2001
    > From: Wei Wang <weiwan@google.com>
    > Date: Fri, 11 Aug 2017 16:36:04 -0700
    > Subject: [PATCH 1/2] potential fix for unregister_netdevice()
    >
    > Change-Id: I5d5f6f7a7ad0f5dd769f33487db17ff2570d52ea
    > ---
    > net/ipv6/route.c | 17 ++++++++---------
    > 1 file changed, 8 insertions(+), 9 deletions(-)
    >
    > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
    > index 4d30c96a819d..105922903932 100644
    > --- a/net/ipv6/route.c
    > +++ b/net/ipv6/route.c
    > @@ -417,14 +417,12 @@ static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
    > struct net_device *loopback_dev =
    > dev_net(dev)->loopback_dev;
    >
    > - if (dev != loopback_dev) {
    > - if (idev && idev->dev == dev) {
    > - struct inet6_dev *loopback_idev =
    > - in6_dev_get(loopback_dev);
    > - if (loopback_idev) {
    > - rt->rt6i_idev = loopback_idev;
    > - in6_dev_put(idev);
    > - }
    > + if (idev && idev->dev != loopback_dev) {
    > + struct inet6_dev *loopback_idev =
    > + in6_dev_get(loopback_dev);
    > + if (loopback_idev) {
    > + rt->rt6i_idev = loopback_idev;
    > + in6_dev_put(idev);
    > }
    > }
    > }
    > @@ -2789,7 +2787,8 @@ static int fib6_ifdown(struct rt6_info *rt, void *arg)
    > const struct arg_dev_net *adn = arg;
    > const struct net_device *dev = adn->dev;
    >
    > - if ((rt->dst.dev == dev || !dev) &&
    > + if ((rt->dst.dev == dev || !dev ||
    > + rt->rt6i_idev->dev == dev) &&

    Can you please explain why this line is needed? While host routes aren't
    removed from the FIB by rt6_ifdown() (when dst.dev goes down), they are
    removed later on in addrconf_ifdown().

    With your patch, if I check the return value of ip6_del_rt() in
    __ipv6_ifa_notify() I see that -ENONET is returned. Because the host
    route was already removed by rt6_ifdown(). When the line in question is
    removed from the patch I don't get the error anymore.

    Is it possible that in John's case the host route was correctly removed
    from the FIB and that the unreleased reference was due to a wrong check
    in ip6_dst_ifdown() (which you patched correctly AFAICT)?

    Thanks

    > rt != adn->net->ipv6.ip6_null_entry &&
    > (rt->rt6i_nsiblings == 0 ||
    > (dev && netdev_unregistering(dev)) ||
    > --
    > 2.14.0.434.g98096fd7a8-goog
    >

    \
     
     \ /
      Last update: 2017-08-12 20:12    [W:3.315 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site