Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752231AbdHMQY5 (ORCPT ); Sun, 13 Aug 2017 12:24:57 -0400 Received: from mail-pf0-f173.google.com ([209.85.192.173]:35965 "EHLO mail-pf0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751024AbdHMQYz (ORCPT ); Sun, 13 Aug 2017 12:24:55 -0400 Subject: Re: unregister_netdevice: waiting for eth0 to become free. Usage count = 1 To: Wei Wang , Ido Schimmel References: <20170812180129.GA31700@splinter> Cc: Cong Wang , John Stultz , Martin KaFai Lau , lkml , Network Development , Linux USB List , "David S. Miller" , Felipe Balbi From: David Ahern Message-ID: <61d95fe2-edfd-9fc4-5e21-5b96e4e03c9f@gmail.com> Date: Sun, 13 Aug 2017 10:24:53 -0600 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2354 Lines: 55 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)) ||