Re: network device reference leak with net-next

[John Fastabend <john.r.fastabend@intel.com>]

On 11/15/2010 11:10 AM, Stephen Hemminger wrote:
> On Mon, 15 Nov 2010 20:04:00 +0100
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
>> Le lundi 15 novembre 2010 à 10:56 -0800, Stephen Hemminger a écrit :
>>> This is a new regression (doesn't exist with 2.6.36)
>>>
>>> If I shutdown KVM instance with Virt manager, the virtual
>>> interfaces in the bridge aren't getting cleaned up because
>>> of leftover reference count.
>>>
>>>
>>> [ 9781.050474] unregister_netdevice: waiting for vnet1 to become free. Usage count = 1
>>> [ 9785.143400] virbr4: port 3(vnet6) entering forwarding state
>>> [ 9785.177194] virbr4: port 3(vnet6) entering disabled state
>>> [ 9785.201129] device vnet6 left promiscuous mode
>>> [ 9785.201135] virbr4: port 3(vnet6) entering disabled state
>>> [ 9791.286950] unregister_netdevice: waiting for vnet1 to become free. Usage count = 1
>>> [ 9795.461526] unregister_netdevice: waiting for vnet6 to become free. Usage count = 1
>>> [ 9801.523398] unregister_netdevice: waiting for vnet1 to become free. Usage count = 1
>>> --
>>
>> Is the refcount stay forever to 1, or eventually reaches 0 ?
>>
>>
>>
> 
> Stays 1 for as long as I waited about 10 minutes
> 

Similar refcount error with ixgbe bisected to this patch,

commit 2de795707294972f6c34bae9de713e502c431296
Author: Lorenzo Colitti <lorenzo@google.com>
Date:   Wed Oct 27 18:16:49 2010 +0000

    ipv6: addrconf: don't remove address state on ifdown if the address is being kept

    Currently, addrconf_ifdown does not delete statically configured IPv6
    addresses when the interface is brought down. The intent is that when
    the interface comes back up the address will be usable again. However,
    this doesn't actually work, because the system stops listening on the
    corresponding solicited-node multicast address, so the address cannot
    respond to neighbor solicitations and thus receive traffic. Also, the
    code notifies the rest of the system that the address is being deleted
    (e.g, RTM_DELADDR), even though it is not. Fix it so that none of this
    state is updated if the address is being kept on the interface.

    Tested: Added a statically configured IPv6 address to an interface,
    started ping, brought link down, brought link up again. When link came
    up ping kept on going and "ip -6 maddr" showed that the host was still
    subscribed to there

    Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>


Quick glance looks like an in6_ifa_put is missed?

--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2754,13 +2754,13 @@ Hunk #1, a/net/ipv6/addrconf.c static int addrconf_ifdown(struct net_device *dev, int how)
                        ifa->state = INET6_IFADDR_STATE_DEAD;
                        spin_unlock_bh(&ifa->state_lock);

-                       if (state == INET6_IFADDR_STATE_DEAD) {
-                               in6_ifa_put(ifa);
-                       } else {
+                       if (state != INET6_IFADDR_STATE_DEAD) {
                                __ipv6_ifa_notify(RTM_DELADDR, ifa);
                                atomic_notifier_call_chain(&inet6addr_chain,
                                                           NETDEV_DOWN, ifa);
                        }
+
+                       in6_ifa_put(ifa);
                        write_lock_bh(&idev->lock);
                }
        }

With a quick hack the above seems to resolve the issue but I'll need to review the ipv6 stuff to be sure this is sane.

Thanks,
John.