diff mbox

[net] ipvlan: fix addr hash list corruption

Message ID CAF2d9jh+VozezamrEK_6vfTvY8LZSzw9n-bJjSX3G11RXmfz3w@mail.gmail.com
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Mahesh Bandewar (महेश बंडेवार) March 24, 2015, 11:16 p.m. UTC 
On Tue, Mar 24, 2015 at 10:06 AM, Jiri Benc <jbenc@redhat.com> wrote:
> On Tue, 24 Mar 2015 13:00:37 -0400 (EDT), David Miller wrote:
>> From: Jiri Benc <jbenc@redhat.com>
>> Date: Mon, 23 Mar 2015 22:10:19 +0100
>>
>> > @@ -504,7 +504,8 @@ static void ipvlan_link_delete(struct net_device *dev, struct list_head *head)
>> >
>> >     if (ipvlan->ipv6cnt > 0 || ipvlan->ipv4cnt > 0) {
>> >             list_for_each_entry_safe(addr, next, &ipvlan->addrs, anode) {
>> > -                   ipvlan_ht_addr_del(addr, !dev->dismantle);
>> > +                   if (netif_running(dev))
>> > +                           ipvlan_ht_addr_del(addr, !dev->dismantle);
>> >                     list_del_rcu(&addr->anode);
>> >             }
>> >     }
>>
>> This is so error prone, because you are depending upon so many implementation
>> details to infer a boolean state "is this address hashed".
>>
>> So just add the boolean state to struct ipvl_addr, and manage it in
>> the ipvlan_ht_addr_{add,del}() code.
>
> I had that originally but then decided to go with smaller memory
> footprint. Which obviously doesn't matter much here. I'll send v2 with
> the boolean state.
>
Hi Jiri,

Well, we already have hlist_unhashed().The following patch should fix
the duplicate addition as well as deletion. Please give it a try.

 }

Thanks,
--mahesh..
>  Jiri
>
> --
> Jiri Benc
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Miller March 25, 2015, 1:18 a.m. UTC | #1 
From: Mahesh Bandewar <maheshb@google.com>
Date: Tue, 24 Mar 2015 16:16:38 -0700

> Well, we already have hlist_unhashed().The following patch should fix
> the duplicate addition as well as deletion. Please give it a try.

Yes, this looks a _lot_ better.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
index 2a175006028b..8a542b9340c4 100644
--- a/drivers/net/ipvlan/ipvlan_core.c
+++ b/drivers/net/ipvlan/ipvlan_core.c
@@ -81,12 +81,13 @@  void ipvlan_ht_addr_add(struct ipvl_dev *ipvlan,
struct ipvl_addr *addr)
        hash = (addr->atype == IPVL_IPV6) ?
               ipvlan_get_v6_hash(&addr->ip6addr) :
               ipvlan_get_v4_hash(&addr->ip4addr);
-       hlist_add_head_rcu(&addr->hlnode, &port->hlhead[hash]);
+       if (hlist_unhashed(&addr->hlnode))
+               hlist_add_head_rcu(&addr->hlnode, &port->hlhead[hash]);
 }

 void ipvlan_ht_addr_del(struct ipvl_addr *addr, bool sync)
 {
-       hlist_del_rcu(&addr->hlnode);
+       hlist_del_init_rcu(&addr->hlnode);
        if (sync)
                synchronize_rcu();