Message ID | 1366295697-31037-2-git-send-email-nikolay@redhat.com |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
Nikolay Aleksandrov <nikolay@redhat.com> wrote: >Add dev_mc_del after err_detach as that's the first error path >after they're added. The main issue is the mc addresses' refcount >which only gets bumped up. > >Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com> All 5 of these patches look good to me. The only minor nits are that the above description says "dev_mc_del," but the actual function call added is bond_mc_list_flush (which in turn does call dev_mc_dev), and that this patch (#1) modifies the lacpdu_multicast variable initialization, which isn't really necessary for the bug fix. -J >--- > drivers/net/bonding/bond_main.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >index a61a760..dc0153b 100644 >--- a/drivers/net/bonding/bond_main.c >+++ b/drivers/net/bonding/bond_main.c >@@ -1523,6 +1523,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) > struct sockaddr addr; > int link_reporting; > int res = 0; >+ u8 lacpdu_multicast[ETH_ALEN] = MULTICAST_LACPDU_ADDR; > > if (!bond->params.use_carrier && > slave_dev->ethtool_ops->get_link == NULL && >@@ -1726,12 +1727,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) > netif_addr_unlock_bh(bond_dev); > } > >- if (bond->params.mode == BOND_MODE_8023AD) { >+ if (bond->params.mode == BOND_MODE_8023AD) > /* add lacpdu mc addr to mc list */ >- u8 lacpdu_multicast[ETH_ALEN] = MULTICAST_LACPDU_ADDR; >- > dev_mc_add(slave_dev, lacpdu_multicast); >- } > > bond_add_vlans_on_slave(bond, slave_dev); > >@@ -1901,6 +1899,11 @@ err_dest_symlinks: > bond_destroy_slave_symlinks(bond_dev, slave_dev); > > err_detach: >+ if (!USES_PRIMARY(bond->params.mode)) { >+ netif_addr_lock_bh(bond_dev); >+ bond_mc_list_flush(bond_dev, slave_dev); >+ netif_addr_unlock_bh(bond_dev); >+ } > write_lock_bh(&bond->lock); > bond_detach_slave(bond, new_slave); > write_unlock_bh(&bond->lock); >-- >1.8.1.4 --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com -- 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
On 18/04/13 18:58, Jay Vosburgh wrote: > Nikolay Aleksandrov <nikolay@redhat.com> wrote: > >> Add dev_mc_del after err_detach as that's the first error path >> after they're added. The main issue is the mc addresses' refcount >> which only gets bumped up. >> >> Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com> > > All 5 of these patches look good to me. The only minor nits are > that the above description says "dev_mc_del," but the actual function > call added is bond_mc_list_flush (which in turn does call dev_mc_dev), > and that this patch (#1) modifies the lacpdu_multicast variable > initialization, which isn't really necessary for the bug fix. > > -J > > Yes, that all is because initially I wrote it directly with dev_mc_del where I needed the variable and then I changed it to use bond_mc_list_flush. I'll re-post v2 with updated log message and without that initialization, sorry about this. Nik -- 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 --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index a61a760..dc0153b 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1523,6 +1523,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) struct sockaddr addr; int link_reporting; int res = 0; + u8 lacpdu_multicast[ETH_ALEN] = MULTICAST_LACPDU_ADDR; if (!bond->params.use_carrier && slave_dev->ethtool_ops->get_link == NULL && @@ -1726,12 +1727,9 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev) netif_addr_unlock_bh(bond_dev); } - if (bond->params.mode == BOND_MODE_8023AD) { + if (bond->params.mode == BOND_MODE_8023AD) /* add lacpdu mc addr to mc list */ - u8 lacpdu_multicast[ETH_ALEN] = MULTICAST_LACPDU_ADDR; - dev_mc_add(slave_dev, lacpdu_multicast); - } bond_add_vlans_on_slave(bond, slave_dev); @@ -1901,6 +1899,11 @@ err_dest_symlinks: bond_destroy_slave_symlinks(bond_dev, slave_dev); err_detach: + if (!USES_PRIMARY(bond->params.mode)) { + netif_addr_lock_bh(bond_dev); + bond_mc_list_flush(bond_dev, slave_dev); + netif_addr_unlock_bh(bond_dev); + } write_lock_bh(&bond->lock); bond_detach_slave(bond, new_slave); write_unlock_bh(&bond->lock);
Add dev_mc_del after err_detach as that's the first error path after they're added. The main issue is the mc addresses' refcount which only gets bumped up. Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com> --- drivers/net/bonding/bond_main.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)