Re: unregister_netdevice: waiting for eth0 to become free. Usage count = 1
On 8/13/17 2:56 PM, Wei Wang wrote: >> 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)) || > > As you explained earlier, after your patch, all entries in the fib6 > tree will have rt->dst.dev be the same as rt->rt6i_idev->dev except > those ones created by p6_rt_cache_alloc() and ip6_rt_pcpu_alloc(). > Then the above newly added check is mainly to catch those cached dst > entries (created by ip6_rt_cached_alloc()). right? > And it is required because __ipv6_ifa_notify() -> ip6_del_rt() won't > take care of those cached dst entries. > > Then I think I should wait for your patches to get merged before > submitting my patch? no. your patch will need to go back to 4.12; my changes will not be appropriate for that. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: unregister_netdevice: waiting for eth0 to become free. Usage count = 1
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)) || -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: unregister_netdevice: waiting for eth0 to become free. Usage count = 1
On 8/11/17 6:25 PM, Wei Wang wrote: > By "a patch to fix that" do you mean after your patch, for every rt6, > rt6->rt6i_idev will be the same as rt6->dst.dev? FIB entries should have them the same device with my patch. The copies done (ip6_rt_cache_alloc and ip6_rt_pcpu_alloc) will have to set dst.dev to loopback or VRF device for RTF_LOCAL routes; it's the only way to get local traffic to work and this is similar to what IPv4 does. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: unregister_netdevice: waiting for eth0 to become free. Usage count = 1
On 8/11/17 6:10 PM, 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. Yes, host routes and anycast routes. I have a patch to fix that but it is held up on a few VRF test cases failing. Hopefully I can get that figured out next week. These unrelated routes against the loopback device have been a source of a number of problems (e.g. take down 'lo' and all of IPv6 networking stops for that namespace). -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
