Re: [PATCH net] ipvlan: fix addr hash list corruption

[Jiri Benc <jbenc@redhat.com>]

On Tue, 24 Mar 2015 16:16:38 -0700, Mahesh Bandewar wrote:
> Well, we already have hlist_unhashed().The following patch should fix
> the duplicate addition as well as deletion. Please give it a try.

Good idea, it's surely better than adding a new boolean.

However, I'm wondering that when there's apparently no problem with the
addresses being on the hash list when interface is down, what's the
point in clearing the hash list in the ndo_stop handler and
repopulating it in ndo_open?

The following patch fixes the problem, too, and as a bonus removes code:

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 4f4099d5603d..03def841b1a9 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -149,10 +149,6 @@ static int ipvlan_open(struct net_device *dev)
 	else
 		dev->flags &= ~IFF_NOARP;
 
-	if (ipvlan->ipv6cnt > 0 || ipvlan->ipv4cnt > 0) {
-		list_for_each_entry(addr, &ipvlan->addrs, anode)
-			ipvlan_ht_addr_add(ipvlan, addr);
-	}
 	return dev_uc_add(phy_dev, phy_dev->dev_addr);
 }
 
@@ -166,11 +162,6 @@ static int ipvlan_stop(struct net_device *dev)
 	dev_mc_unsync(phy_dev, dev);
 
 	dev_uc_del(phy_dev, phy_dev->dev_addr);
-
-	if (ipvlan->ipv6cnt > 0 || ipvlan->ipv4cnt > 0) {
-		list_for_each_entry(addr, &ipvlan->addrs, anode)
-			ipvlan_ht_addr_del(addr, !dev->dismantle);
-	}
 	return 0;
 }
 

What do you think? Should I submit this formally?

 Jiri

-- 
Jiri Benc