* [PATCH] net: Correctly sync addresses from multiple sources to single device @ 2014-01-17 19:52 Vlad Yasevich 2014-01-21 22:53 ` David Miller 2014-01-22 14:18 ` Andrey Dmitrov 0 siblings, 2 replies; 8+ messages in thread From: Vlad Yasevich @ 2014-01-17 19:52 UTC (permalink / raw) To: netdev; +Cc: Vlad Yasevich, Andrey Dmitrov When we have multiple devices attempting to sync the same address to a single destination, each device should be permitted to sync it once. To accomplish this, pass the sync count of the source address to __hw_addr_add_ex(). 'sync_cnt' tracks how many time a given address has been successfully synced. If the address is found in the destination list, but the 'sync_cnt' of the source is 0, then this address has not been synced from this interface and we need to allow the sync operation to succeed thus incrementing reference counts. Reported-by: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> CC: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> --- net/core/dev_addr_lists.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c index ec40a84..bd1b880 100644 --- a/net/core/dev_addr_lists.c +++ b/net/core/dev_addr_lists.c @@ -48,7 +48,8 @@ static int __hw_addr_create_ex(struct netdev_hw_addr_list *list, static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, const unsigned char *addr, int addr_len, - unsigned char addr_type, bool global, bool sync) + unsigned char addr_type, bool global, bool sync, + int sync_count) { struct netdev_hw_addr *ha; @@ -66,7 +67,7 @@ static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, ha->global_use = true; } if (sync) { - if (ha->synced) + if (ha->synced && sync_count) return -EEXIST; else ha->synced = true; @@ -84,7 +85,8 @@ static int __hw_addr_add(struct netdev_hw_addr_list *list, const unsigned char *addr, int addr_len, unsigned char addr_type) { - return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false); + return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false, + 0); } static int __hw_addr_del_entry(struct netdev_hw_addr_list *list, @@ -139,7 +141,7 @@ static int __hw_addr_sync_one(struct netdev_hw_addr_list *to_list, int err; err = __hw_addr_add_ex(to_list, ha->addr, addr_len, ha->type, - false, true); + false, true, ha->sync_count); if (err && err != -EEXIST) return err; @@ -676,7 +678,7 @@ static int __dev_mc_add(struct net_device *dev, const unsigned char *addr, netif_addr_lock_bh(dev); err = __hw_addr_add_ex(&dev->mc, addr, dev->addr_len, - NETDEV_HW_ADDR_T_MULTICAST, global, false); + NETDEV_HW_ADDR_T_MULTICAST, global, false, 0); if (!err) __dev_set_rx_mode(dev); netif_addr_unlock_bh(dev); -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] net: Correctly sync addresses from multiple sources to single device 2014-01-17 19:52 [PATCH] net: Correctly sync addresses from multiple sources to single device Vlad Yasevich @ 2014-01-21 22:53 ` David Miller 2014-01-22 15:30 ` Vlad Yasevich 2014-01-22 14:18 ` Andrey Dmitrov 1 sibling, 1 reply; 8+ messages in thread From: David Miller @ 2014-01-21 22:53 UTC (permalink / raw) To: vyasevic; +Cc: netdev, andrey.dmitrov From: Vlad Yasevich <vyasevic@redhat.com> Date: Fri, 17 Jan 2014 14:52:12 -0500 > When we have multiple devices attempting to sync the same address > to a single destination, each device should be permitted to sync > it once. To accomplish this, pass the sync count of the source > address to __hw_addr_add_ex(). 'sync_cnt' tracks how many time > a given address has been successfully synced. If the address > is found in the destination list, but the 'sync_cnt' of the source > is 0, then this address has not been synced from this interface > and we need to allow the sync operation to succeed thus incrementing > reference counts. > > Reported-by: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> > CC: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> I applied this to net-next since 3.13 just got released, and it doesn't compile. net/core/dev_addr_lists.c: In function ‘__hw_addr_sync_one’: net/core/dev_addr_lists.c:144:26: error: ‘struct netdev_hw_addr’ has no member named ‘sync_count’ ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: Correctly sync addresses from multiple sources to single device 2014-01-21 22:53 ` David Miller @ 2014-01-22 15:30 ` Vlad Yasevich 0 siblings, 0 replies; 8+ messages in thread From: Vlad Yasevich @ 2014-01-22 15:30 UTC (permalink / raw) To: David Miller; +Cc: netdev, andrey.dmitrov On 01/21/2014 05:53 PM, David Miller wrote: > From: Vlad Yasevich <vyasevic@redhat.com> > Date: Fri, 17 Jan 2014 14:52:12 -0500 > >> When we have multiple devices attempting to sync the same address >> to a single destination, each device should be permitted to sync >> it once. To accomplish this, pass the sync count of the source >> address to __hw_addr_add_ex(). 'sync_cnt' tracks how many time >> a given address has been successfully synced. If the address >> is found in the destination list, but the 'sync_cnt' of the source >> is 0, then this address has not been synced from this interface >> and we need to allow the sync operation to succeed thus incrementing >> reference counts. >> >> Reported-by: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> >> CC: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> >> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > > I applied this to net-next since 3.13 just got released, and it doesn't > compile. > > net/core/dev_addr_lists.c: In function ‘__hw_addr_sync_one’: > net/core/dev_addr_lists.c:144:26: error: ‘struct netdev_hw_addr’ has no member named ‘sync_count’ > That does it... Time to add a hook to format_patch to check for dirty index. -vlad ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: Correctly sync addresses from multiple sources to single device 2014-01-17 19:52 [PATCH] net: Correctly sync addresses from multiple sources to single device Vlad Yasevich 2014-01-21 22:53 ` David Miller @ 2014-01-22 14:18 ` Andrey Dmitrov 2014-01-22 15:06 ` Vlad Yasevich 2014-01-22 17:54 ` [PATCH v2] " Vlad Yasevich 1 sibling, 2 replies; 8+ messages in thread From: Andrey Dmitrov @ 2014-01-22 14:18 UTC (permalink / raw) To: Vlad Yasevich; +Cc: netdev, davem, Alexandra N. Kossovsky, Konstantin Ushakov On 01/17/2014 11:52 PM, Vlad Yasevich wrote: > When we have multiple devices attempting to sync the same address > to a single destination, each device should be permitted to sync > it once. To accomplish this, pass the sync count of the source > address to __hw_addr_add_ex(). 'sync_cnt' tracks how many time > a given address has been successfully synced. If the address > is found in the destination list, but the 'sync_cnt' of the source > is 0, then this address has not been synced from this interface > and we need to allow the sync operation to succeed thus incrementing > reference counts. > > Reported-by: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> > CC: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > --- > net/core/dev_addr_lists.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c > index ec40a84..bd1b880 100644 > --- a/net/core/dev_addr_lists.c > +++ b/net/core/dev_addr_lists.c > @@ -48,7 +48,8 @@ static int __hw_addr_create_ex(struct netdev_hw_addr_list *list, > > static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, > const unsigned char *addr, int addr_len, > - unsigned char addr_type, bool global, bool sync) > + unsigned char addr_type, bool global, bool sync, > + int sync_count) > { > struct netdev_hw_addr *ha; > > @@ -66,7 +67,7 @@ static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, > ha->global_use = true; > } > if (sync) { > - if (ha->synced) > + if (ha->synced && sync_count) > return -EEXIST; > else > ha->synced = true; > @@ -84,7 +85,8 @@ static int __hw_addr_add(struct netdev_hw_addr_list *list, > const unsigned char *addr, int addr_len, > unsigned char addr_type) > { > - return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false); > + return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false, > + 0); > } > > static int __hw_addr_del_entry(struct netdev_hw_addr_list *list, > @@ -139,7 +141,7 @@ static int __hw_addr_sync_one(struct netdev_hw_addr_list *to_list, > int err; > > err = __hw_addr_add_ex(to_list, ha->addr, addr_len, ha->type, > - false, true); > + false, true, ha->sync_count); > if (err && err != -EEXIST) > return err; > > @@ -676,7 +678,7 @@ static int __dev_mc_add(struct net_device *dev, const unsigned char *addr, > > netif_addr_lock_bh(dev); > err = __hw_addr_add_ex(&dev->mc, addr, dev->addr_len, > - NETDEV_HW_ADDR_T_MULTICAST, global, false); > + NETDEV_HW_ADDR_T_MULTICAST, global, false, 0); > if (!err) > __dev_set_rx_mode(dev); > netif_addr_unlock_bh(dev); Thanks. The patch was tested with linux-3.12.6. However, while building the kernel I got complains on: > err = __hw_addr_add_ex(to_list, ha->addr, addr_len, ha->type, > - false, true); > + false, true, ha->sync_count); Namely ha (struct netdev_hw_addr) has no field sync_count. It has sync_cnt. So I’ve fixed this when testing the patch. Is it a typo or something intentional? Your patch solves the initial problem, but it looks like it introduces a new one. When interface joins multicast group corresponding MAC address is added to the interface maddr list. It should be removed when socket explicitly leaves the group or is just closed. It’s the case without your patch. However, with the patched kernel address is still there after socket is closed. You can reproduce it with mcast_client binary (mcast_client.c was attached to the first letter in the thread). To be specific: $ ip maddr show eth3 9: eth3 link 33:33:00:00:00:01 users 3 link 01:00:5e:00:00:01 users 3 link 33:33:ff:01:39:7c users 3 link 01:80:c2:00:00:21 users 2 inet 224.0.0.1 inet6 ff02::1:ff01:397c inet6 ff02::1 inet6 ff01::1 $ gcc mcast_client.c -o cl $ sudo ./cl Abort it with Ctrl+c $ ip maddr show eth3 9: eth3 link 33:33:00:00:00:01 users 3 link 01:00:5e:00:00:01 users 3 link 33:33:ff:01:39:7c users 3 link 01:80:c2:00:00:21 users 2 link 01:00:5e:11:58:a8 inet 224.0.0.1 inet6 ff02::1:ff01:397c inet6 ff02::1 inet6 ff01::1 'link 01:00:5e:11:58:a8’ address is still there. Should’ve been removed. Andrey ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] net: Correctly sync addresses from multiple sources to single device 2014-01-22 14:18 ` Andrey Dmitrov @ 2014-01-22 15:06 ` Vlad Yasevich 2014-01-22 17:54 ` [PATCH v2] " Vlad Yasevich 1 sibling, 0 replies; 8+ messages in thread From: Vlad Yasevich @ 2014-01-22 15:06 UTC (permalink / raw) To: Andrey Dmitrov, Vlad Yasevich Cc: netdev, davem, Alexandra N. Kossovsky, Konstantin Ushakov On 01/22/2014 09:18 AM, Andrey Dmitrov wrote: > On 01/17/2014 11:52 PM, Vlad Yasevich wrote: >> When we have multiple devices attempting to sync the same address >> to a single destination, each device should be permitted to sync >> it once. To accomplish this, pass the sync count of the source >> address to __hw_addr_add_ex(). 'sync_cnt' tracks how many time >> a given address has been successfully synced. If the address >> is found in the destination list, but the 'sync_cnt' of the source >> is 0, then this address has not been synced from this interface >> and we need to allow the sync operation to succeed thus incrementing >> reference counts. >> >> Reported-by: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> >> CC: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> >> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> >> --- >> net/core/dev_addr_lists.c | 12 +++++++----- >> 1 file changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c >> index ec40a84..bd1b880 100644 >> --- a/net/core/dev_addr_lists.c >> +++ b/net/core/dev_addr_lists.c >> @@ -48,7 +48,8 @@ static int __hw_addr_create_ex(struct >> netdev_hw_addr_list *list, >> static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, >> const unsigned char *addr, int addr_len, >> - unsigned char addr_type, bool global, bool sync) >> + unsigned char addr_type, bool global, bool sync, >> + int sync_count) >> { >> struct netdev_hw_addr *ha; >> @@ -66,7 +67,7 @@ static int __hw_addr_add_ex(struct >> netdev_hw_addr_list *list, >> ha->global_use = true; >> } >> if (sync) { >> - if (ha->synced) >> + if (ha->synced && sync_count) >> return -EEXIST; >> else >> ha->synced = true; >> @@ -84,7 +85,8 @@ static int __hw_addr_add(struct netdev_hw_addr_list >> *list, >> const unsigned char *addr, int addr_len, >> unsigned char addr_type) >> { >> - return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, >> false); >> + return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, >> false, >> + 0); >> } >> static int __hw_addr_del_entry(struct netdev_hw_addr_list *list, >> @@ -139,7 +141,7 @@ static int __hw_addr_sync_one(struct >> netdev_hw_addr_list *to_list, >> int err; >> err = __hw_addr_add_ex(to_list, ha->addr, addr_len, ha->type, >> - false, true); >> + false, true, ha->sync_count); >> if (err && err != -EEXIST) >> return err; >> @@ -676,7 +678,7 @@ static int __dev_mc_add(struct net_device *dev, >> const unsigned char *addr, >> netif_addr_lock_bh(dev); >> err = __hw_addr_add_ex(&dev->mc, addr, dev->addr_len, >> - NETDEV_HW_ADDR_T_MULTICAST, global, false); >> + NETDEV_HW_ADDR_T_MULTICAST, global, false, 0); >> if (!err) >> __dev_set_rx_mode(dev); >> netif_addr_unlock_bh(dev); > > Thanks. The patch was tested with linux-3.12.6. > > However, while building the kernel I got complains on: > >> err = __hw_addr_add_ex(to_list, ha->addr, addr_len, ha->type, >> - false, true); >> + false, true, ha->sync_count); > > Namely ha (struct netdev_hw_addr) has no field sync_count. It has sync_cnt. > So I’ve fixed this when testing the patch. Is it a typo or something > intentional? > > Your patch solves the initial problem, but it looks like it introduces a > new one. > > When interface joins multicast group corresponding MAC address is added > to the interface maddr list. It should be removed when socket explicitly > leaves the group or is just closed. It’s the case without your patch. > However, with the patched kernel address is still there after socket is > closed. > > You can reproduce it with mcast_client binary (mcast_client.c was > attached to the first letter in the thread). To be specific: > > $ ip maddr show eth3 > 9: eth3 > link 33:33:00:00:00:01 users 3 > link 01:00:5e:00:00:01 users 3 > link 33:33:ff:01:39:7c users 3 > link 01:80:c2:00:00:21 users 2 > inet 224.0.0.1 > inet6 ff02::1:ff01:397c > inet6 ff02::1 > inet6 ff01::1 > > $ gcc mcast_client.c -o cl > $ sudo ./cl > Abort it with Ctrl+c > $ ip maddr show eth3 > 9: eth3 > link 33:33:00:00:00:01 users 3 > link 01:00:5e:00:00:01 users 3 > link 33:33:ff:01:39:7c users 3 > link 01:80:c2:00:00:21 users 2 > link 01:00:5e:11:58:a8 > inet 224.0.0.1 > inet6 ff02::1:ff01:397c > inet6 ff02::1 > inet6 ff01::1 > > 'link 01:00:5e:11:58:a8’ address is still there. Should’ve been removed. You are right. Sync really doesn't deal well with with multiple sources syncing the same address. Looks like we need to keep track of how many time the address has been synced from the different sources. Looks like I have to restore commit 4543fbefe6e06a9e40d9f2b28d688393a299f079. :) -vlad > > Andrey > -- > 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] net: Correctly sync addresses from multiple sources to single device 2014-01-22 14:18 ` Andrey Dmitrov 2014-01-22 15:06 ` Vlad Yasevich @ 2014-01-22 17:54 ` Vlad Yasevich 2014-01-23 21:07 ` David Miller 2014-01-24 11:33 ` Andrey Dmitrov 1 sibling, 2 replies; 8+ messages in thread From: Vlad Yasevich @ 2014-01-22 17:54 UTC (permalink / raw) To: netdev Cc: Vlad Yasevich, Andrey Dmitrov, Alexandra N. Kossovsky, Konstantin Ushakov When we have multiple devices attempting to sync the same address to a single destination, each device should be permitted to sync it once. To accomplish this, pass the 'sync_cnt' of the source address when adding the addresss to the lower device. 'sync_cnt' tracks how many time a given address has been succefully synced. This way, we know that if the 'sync_cnt' passed in is 0, we should sync this address. Also, turn 'synced' member back into the counter as was originally done in commit 4543fbefe6e06a9e40d9f2b28d688393a299f079. net: count hw_addr syncs so that unsync works properly. It tracks how many time a given address has been added via a 'sync' operation. For every successfull 'sync' the counter is incremented, and for ever 'unsync', the counter is decremented. This makes sure that the address will be properly removed from the the lower device when all the upper devices have removed it. Reported-by: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> CC: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> CC: Alexandra N. Kossovsky <Alexandra.Kossovsky@oktetlabs.ru> CC: Konstantin Ushakov <Konstantin.Ushakov@oktetlabs.ru> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> --- net/core/dev_addr_lists.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c index ec40a84..9974f48 100644 --- a/net/core/dev_addr_lists.c +++ b/net/core/dev_addr_lists.c @@ -38,7 +38,7 @@ static int __hw_addr_create_ex(struct netdev_hw_addr_list *list, ha->type = addr_type; ha->refcount = 1; ha->global_use = global; - ha->synced = sync; + ha->synced = sync ? 1 : 0; ha->sync_cnt = 0; list_add_tail_rcu(&ha->list, &list->list); list->count++; @@ -48,7 +48,8 @@ static int __hw_addr_create_ex(struct netdev_hw_addr_list *list, static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, const unsigned char *addr, int addr_len, - unsigned char addr_type, bool global, bool sync) + unsigned char addr_type, bool global, bool sync, + int sync_count) { struct netdev_hw_addr *ha; @@ -66,10 +67,10 @@ static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, ha->global_use = true; } if (sync) { - if (ha->synced) + if (ha->synced && sync_count) return -EEXIST; else - ha->synced = true; + ha->synced++; } ha->refcount++; return 0; @@ -84,7 +85,8 @@ static int __hw_addr_add(struct netdev_hw_addr_list *list, const unsigned char *addr, int addr_len, unsigned char addr_type) { - return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false); + return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false, + 0); } static int __hw_addr_del_entry(struct netdev_hw_addr_list *list, @@ -101,7 +103,7 @@ static int __hw_addr_del_entry(struct netdev_hw_addr_list *list, ha->global_use = false; if (sync) - ha->synced = false; + ha->synced--; if (--ha->refcount) return 0; @@ -139,7 +141,7 @@ static int __hw_addr_sync_one(struct netdev_hw_addr_list *to_list, int err; err = __hw_addr_add_ex(to_list, ha->addr, addr_len, ha->type, - false, true); + false, true, ha->sync_cnt); if (err && err != -EEXIST) return err; @@ -676,7 +678,7 @@ static int __dev_mc_add(struct net_device *dev, const unsigned char *addr, netif_addr_lock_bh(dev); err = __hw_addr_add_ex(&dev->mc, addr, dev->addr_len, - NETDEV_HW_ADDR_T_MULTICAST, global, false); + NETDEV_HW_ADDR_T_MULTICAST, global, false, 0); if (!err) __dev_set_rx_mode(dev); netif_addr_unlock_bh(dev); -- 1.8.4.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] net: Correctly sync addresses from multiple sources to single device 2014-01-22 17:54 ` [PATCH v2] " Vlad Yasevich @ 2014-01-23 21:07 ` David Miller 2014-01-24 11:33 ` Andrey Dmitrov 1 sibling, 0 replies; 8+ messages in thread From: David Miller @ 2014-01-23 21:07 UTC (permalink / raw) To: vyasevic; +Cc: netdev, andrey.dmitrov, Alexandra.Kossovsky, Konstantin.Ushakov From: Vlad Yasevich <vyasevic@redhat.com> Date: Wed, 22 Jan 2014 12:54:15 -0500 > When we have multiple devices attempting to sync the same address > to a single destination, each device should be permitted to sync > it once. To accomplish this, pass the 'sync_cnt' of the source > address when adding the addresss to the lower device. 'sync_cnt' > tracks how many time a given address has been succefully synced. > This way, we know that if the 'sync_cnt' passed in is 0, we should > sync this address. > > Also, turn 'synced' member back into the counter as was originally > done in > commit 4543fbefe6e06a9e40d9f2b28d688393a299f079. > net: count hw_addr syncs so that unsync works properly. > It tracks how many time a given address has been added via a > 'sync' operation. For every successfull 'sync' the counter is > incremented, and for ever 'unsync', the counter is decremented. > This makes sure that the address will be properly removed from > the the lower device when all the upper devices have removed it. > > Reported-by: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> > CC: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> > CC: Alexandra N. Kossovsky <Alexandra.Kossovsky@oktetlabs.ru> > CC: Konstantin Ushakov <Konstantin.Ushakov@oktetlabs.ru> > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> This one compiles, great :-) Applied, thanks Vlad. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] net: Correctly sync addresses from multiple sources to single device 2014-01-22 17:54 ` [PATCH v2] " Vlad Yasevich 2014-01-23 21:07 ` David Miller @ 2014-01-24 11:33 ` Andrey Dmitrov 1 sibling, 0 replies; 8+ messages in thread From: Andrey Dmitrov @ 2014-01-24 11:33 UTC (permalink / raw) To: Vlad Yasevich; +Cc: netdev, Alexandra N. Kossovsky, Konstantin Ushakov On 01/22/2014 09:54 PM, Vlad Yasevich wrote: > When we have multiple devices attempting to sync the same address > to a single destination, each device should be permitted to sync > it once. To accomplish this, pass the 'sync_cnt' of the source > address when adding the addresss to the lower device. 'sync_cnt' > tracks how many time a given address has been succefully synced. > This way, we know that if the 'sync_cnt' passed in is 0, we should > sync this address. > > Also, turn 'synced' member back into the counter as was originally > done in > commit 4543fbefe6e06a9e40d9f2b28d688393a299f079. > net: count hw_addr syncs so that unsync works properly. > It tracks how many time a given address has been added via a > 'sync' operation. For every successfull 'sync' the counter is > incremented, and for ever 'unsync', the counter is decremented. > This makes sure that the address will be properly removed from > the the lower device when all the upper devices have removed it. > > Reported-by: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> > CC: Andrey Dmitrov <andrey.dmitrov@oktetlabs.ru> > CC: Alexandra N. Kossovsky <Alexandra.Kossovsky@oktetlabs.ru> > CC: Konstantin Ushakov <Konstantin.Ushakov@oktetlabs.ru> > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com> > --- > net/core/dev_addr_lists.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c > index ec40a84..9974f48 100644 > --- a/net/core/dev_addr_lists.c > +++ b/net/core/dev_addr_lists.c > @@ -38,7 +38,7 @@ static int __hw_addr_create_ex(struct netdev_hw_addr_list *list, > ha->type = addr_type; > ha->refcount = 1; > ha->global_use = global; > - ha->synced = sync; > + ha->synced = sync ? 1 : 0; > ha->sync_cnt = 0; > list_add_tail_rcu(&ha->list, &list->list); > list->count++; > @@ -48,7 +48,8 @@ static int __hw_addr_create_ex(struct netdev_hw_addr_list *list, > > static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, > const unsigned char *addr, int addr_len, > - unsigned char addr_type, bool global, bool sync) > + unsigned char addr_type, bool global, bool sync, > + int sync_count) > { > struct netdev_hw_addr *ha; > > @@ -66,10 +67,10 @@ static int __hw_addr_add_ex(struct netdev_hw_addr_list *list, > ha->global_use = true; > } > if (sync) { > - if (ha->synced) > + if (ha->synced && sync_count) > return -EEXIST; > else > - ha->synced = true; > + ha->synced++; > } > ha->refcount++; > return 0; > @@ -84,7 +85,8 @@ static int __hw_addr_add(struct netdev_hw_addr_list *list, > const unsigned char *addr, int addr_len, > unsigned char addr_type) > { > - return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false); > + return __hw_addr_add_ex(list, addr, addr_len, addr_type, false, false, > + 0); > } > > static int __hw_addr_del_entry(struct netdev_hw_addr_list *list, > @@ -101,7 +103,7 @@ static int __hw_addr_del_entry(struct netdev_hw_addr_list *list, > ha->global_use = false; > > if (sync) > - ha->synced = false; > + ha->synced--; > > if (--ha->refcount) > return 0; > @@ -139,7 +141,7 @@ static int __hw_addr_sync_one(struct netdev_hw_addr_list *to_list, > int err; > > err = __hw_addr_add_ex(to_list, ha->addr, addr_len, ha->type, > - false, true); > + false, true, ha->sync_cnt); > if (err && err != -EEXIST) > return err; > > @@ -676,7 +678,7 @@ static int __dev_mc_add(struct net_device *dev, const unsigned char *addr, > > netif_addr_lock_bh(dev); > err = __hw_addr_add_ex(&dev->mc, addr, dev->addr_len, > - NETDEV_HW_ADDR_T_MULTICAST, global, false); > + NETDEV_HW_ADDR_T_MULTICAST, global, false, 0); > if (!err) > __dev_set_rx_mode(dev); > netif_addr_unlock_bh(dev); Everything works properly now. The patch has been tested with linux-3.12.6. Thanks, Andrey ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-01-24 11:33 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-01-17 19:52 [PATCH] net: Correctly sync addresses from multiple sources to single device Vlad Yasevich 2014-01-21 22:53 ` David Miller 2014-01-22 15:30 ` Vlad Yasevich 2014-01-22 14:18 ` Andrey Dmitrov 2014-01-22 15:06 ` Vlad Yasevich 2014-01-22 17:54 ` [PATCH v2] " Vlad Yasevich 2014-01-23 21:07 ` David Miller 2014-01-24 11:33 ` Andrey Dmitrov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).