From: David Ahern <dsahern@gmail.com>
To: Wei Wang <weiwan@google.com>, Ido Schimmel <idosch@idosch.org>
Cc: Cong Wang <xiyou.wangcong@gmail.com>,
John Stultz <john.stultz@linaro.org>,
Martin KaFai Lau <kafai@fb.com>,
lkml <linux-kernel@vger.kernel.org>,
Network Development <netdev@vger.kernel.org>,
Linux USB List <linux-usb@vger.kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Felipe Balbi <felipe.balbi@linux.intel.com>
Subject: Re: unregister_netdevice: waiting for eth0 to become free. Usage count = 1
Date: Sun, 13 Aug 2017 10:24:53 -0600 [thread overview]
Message-ID: <61d95fe2-edfd-9fc4-5e21-5b96e4e03c9f@gmail.com> (raw)
In-Reply-To: <CAEA6p_Cngs9Nx9OZ15oRMu_43oZTYHjzcw2vzgemfEyh_g0nNA@mail.gmail.com>
On 8/12/17 1:42 PM, Wei Wang wrote:
> Hi Ido,
>
>>> - 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().
>>
>
> Yes.. Agree. But one difference is that if the route is removed from
> addrconf_ifdown(), dst_dev_put() won't be called to release the
> devices before doing dst_release(). It is OK if dst_release() sees the
> refcnt on dst already drops to 0 and directly destroys the dst. But I
> think it will cause problem if at the time, the dst is still held by
> some other users because then the refcnt on the device going down will
> not get released.
> That's why I think we should remove the dst with either dst->dev ==
> going down dev or rt6->rt6i_idev->dev == going down dev from the fib6
> tree always because there, we always call dst_dev_put() to release the
> device.
>
>> 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.
>>
>
> Right. That is expected as the route is already removed from the tree.
>
>> 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)?
>>
>
> Yes. possible. But as I explained earlier, I still think we should
> also remove routes with rt6->rt6i_idev->dev == going down dev from the
> tree.
Looking at my patch to move host routes from loopback to device with the
address, I have this:
@@ -2789,7 +2808,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 ||
+ (netdev_unregistering(dev) && rt->rt6i_idev->dev == dev)) &&
rt != adn->net->ipv6.ip6_null_entry &&
(rt->rt6i_nsiblings == 0 ||
(dev && netdev_unregistering(dev)) ||
next prev parent reply other threads:[~2017-08-13 16:24 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-07 21:05 unregister_netdevice: waiting for eth0 to become free. Usage count = 1 John Stultz
2017-08-07 21:15 ` John Stultz
2017-08-09 23:34 ` Cong Wang
2017-08-09 23:44 ` John Stultz
2017-08-10 0:36 ` Wei Wang
2017-08-10 0:44 ` John Stultz
2017-08-10 1:26 ` John Stultz
[not found] ` <CALAqxLXY-kE2N__eDq514cLycR0-QRpyOCMWpdttO6fgovE2rg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-08-10 1:36 ` Wei Wang
2017-08-10 5:41 ` Wei Wang
2017-08-10 18:12 ` John Stultz
2017-08-10 20:06 ` Wei Wang
2017-08-11 16:48 ` Cong Wang
2017-08-11 17:25 ` Wei Wang
[not found] ` <CAEA6p_CooRGNOQN3fosF0JLqmd9aOTuNHKNPE8cbrzDH6c176w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-08-12 0:10 ` Wei Wang
2017-08-12 0:19 ` David Ahern
2017-08-12 0:25 ` Wei Wang
[not found] ` <CAEA6p_CxiwXzQHgLA_Oma0pfayYReGc5QmJkV=Td9M1Kdqwbzg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-08-12 3:37 ` David Ahern
2017-08-12 19:29 ` Wei Wang
2017-08-12 0:31 ` John Stultz
2017-08-12 0:46 ` Wei Wang
2017-08-12 3:07 ` John Stultz
2017-08-12 19:28 ` Wei Wang
2017-08-12 19:29 ` Wei Wang
2017-08-12 18:01 ` Ido Schimmel
2017-08-12 19:42 ` Wei Wang
2017-08-13 16:24 ` David Ahern [this message]
2017-08-13 20:56 ` Wei Wang
[not found] ` <CAEA6p_BtBdPviFVRC4UoJchbz7eH9fM=KDihYA3zUQebmNBpdA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-08-13 23:08 ` David Ahern
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=61d95fe2-edfd-9fc4-5e21-5b96e4e03c9f@gmail.com \
--to=dsahern@gmail.com \
--cc=davem@davemloft.net \
--cc=felipe.balbi@linux.intel.com \
--cc=idosch@idosch.org \
--cc=john.stultz@linaro.org \
--cc=kafai@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=weiwan@google.com \
--cc=xiyou.wangcong@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).