From a2fd940f4cff74b932728bd6ca12848da21a0234 Mon Sep 17 00:00:00 2001 From: Andy Gospodarek Date: Thu, 25 Mar 2010 14:49:05 +0000 Subject: bonding: fix broken multicast with round-robin mode Round-robin (mode 0) does nothing to ensure that any multicast traffic originally destined for the host will continue to arrive at the host when the link that sent the IGMP join or membership report goes down. One of the benefits of absolute round-robin transmit. Keeping track of subscribed multicast groups for each slave did not seem like a good use of resources, so I decided to simply send on the curr_active slave of the bond (typically the first enslaved device that is up). This makes failover management simple as IGMP membership reports only need to be sent when the curr_active_slave changes. I tested this patch and it appears to work as expected. Originally reported by Lon Hohberger . Signed-off-by: Andy Gospodarek CC: Lon Hohberger CC: Jay Vosburgh Signed-off-by: Jay Vosburgh Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 40 ++++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 430c02267d7e..5b92fbff431d 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -1235,6 +1235,11 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active) write_lock_bh(&bond->curr_slave_lock); } } + + /* resend IGMP joins since all were sent on curr_active_slave */ + if (bond->params.mode == BOND_MODE_ROUNDROBIN) { + bond_resend_igmp_join_requests(bond); + } } /** @@ -4138,22 +4143,41 @@ static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev struct bonding *bond = netdev_priv(bond_dev); struct slave *slave, *start_at; int i, slave_no, res = 1; + struct iphdr *iph = ip_hdr(skb); read_lock(&bond->lock); if (!BOND_IS_OK(bond)) goto out; - /* - * Concurrent TX may collide on rr_tx_counter; we accept that - * as being rare enough not to justify using an atomic op here + * Start with the curr_active_slave that joined the bond as the + * default for sending IGMP traffic. For failover purposes one + * needs to maintain some consistency for the interface that will + * send the join/membership reports. The curr_active_slave found + * will send all of this type of traffic. */ - slave_no = bond->rr_tx_counter++ % bond->slave_cnt; + if ((iph->protocol == htons(IPPROTO_IGMP)) && + (skb->protocol == htons(ETH_P_IP))) { - bond_for_each_slave(bond, slave, i) { - slave_no--; - if (slave_no < 0) - break; + read_lock(&bond->curr_slave_lock); + slave = bond->curr_active_slave; + read_unlock(&bond->curr_slave_lock); + + if (!slave) + goto out; + } else { + /* + * Concurrent TX may collide on rr_tx_counter; we accept + * that as being rare enough not to justify using an + * atomic op here. + */ + slave_no = bond->rr_tx_counter++ % bond->slave_cnt; + + bond_for_each_slave(bond, slave, i) { + slave_no--; + if (slave_no < 0) + break; + } } start_at = slave; -- cgit v1.2.3-59-g8ed1b From 00ae702847df5566ce9182e9c895185e2ad1c181 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Tue, 30 Mar 2010 23:08:37 +0000 Subject: bonding: bond_xmit_roundrobin() fix Commit a2fd940f (bonding: fix broken multicast with round-robin mode) added a problem on litle endian machines. drivers/net/bonding/bond_main.c:4159: warning: comparison is always false due to limited range of data type Signed-off-by: Eric Dumazet Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 5b92fbff431d..5972a52a330c 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -4156,7 +4156,7 @@ static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev * send the join/membership reports. The curr_active_slave found * will send all of this type of traffic. */ - if ((iph->protocol == htons(IPPROTO_IGMP)) && + if ((iph->protocol == IPPROTO_IGMP) && (skb->protocol == htons(ETH_P_IP))) { read_lock(&bond->curr_slave_lock); -- cgit v1.2.3-59-g8ed1b From 9e2e61fbf8ad016d24e4af0afff13505f3dd2a2a Mon Sep 17 00:00:00 2001 From: Amerigo Wang Date: Wed, 31 Mar 2010 21:30:52 +0000 Subject: bonding: fix potential deadlock in bond_uninit() bond_uninit() is invoked with rtnl_lock held, when it does destroy_workqueue() which will potentially flush all works in this workqueue, if we hold rtnl_lock again in the work function, it will deadlock. So move destroy_workqueue() to destructor where rtnl_lock is not held any more, suggested by Eric. Signed-off-by: WANG Cong Cc: Jay Vosburgh Cc: "David S. Miller" Cc: Stephen Hemminger Cc: Jiri Pirko Cc: "Eric W. Biederman" Signed-off-by: David S. Miller --- drivers/net/bonding/bond_main.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) (limited to 'drivers/net/bonding') diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 5972a52a330c..0075514bf32f 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -4450,6 +4450,14 @@ static const struct net_device_ops bond_netdev_ops = { .ndo_vlan_rx_kill_vid = bond_vlan_rx_kill_vid, }; +static void bond_destructor(struct net_device *bond_dev) +{ + struct bonding *bond = netdev_priv(bond_dev); + if (bond->wq) + destroy_workqueue(bond->wq); + free_netdev(bond_dev); +} + static void bond_setup(struct net_device *bond_dev) { struct bonding *bond = netdev_priv(bond_dev); @@ -4470,7 +4478,7 @@ static void bond_setup(struct net_device *bond_dev) bond_dev->ethtool_ops = &bond_ethtool_ops; bond_set_mode_ops(bond, bond->params.mode); - bond_dev->destructor = free_netdev; + bond_dev->destructor = bond_destructor; /* Initialize the device options */ bond_dev->tx_queue_len = 0; @@ -4542,9 +4550,6 @@ static void bond_uninit(struct net_device *bond_dev) bond_remove_proc_entry(bond); - if (bond->wq) - destroy_workqueue(bond->wq); - netif_addr_lock_bh(bond_dev); bond_mc_list_destroy(bond); netif_addr_unlock_bh(bond_dev); @@ -4956,8 +4961,8 @@ int bond_create(struct net *net, const char *name) bond_setup); if (!bond_dev) { pr_err("%s: eek! can't alloc netdev!\n", name); - res = -ENOMEM; - goto out; + rtnl_unlock(); + return -ENOMEM; } dev_net_set(bond_dev, net); @@ -4966,19 +4971,16 @@ int bond_create(struct net *net, const char *name) if (!name) { res = dev_alloc_name(bond_dev, "bond%d"); if (res < 0) - goto out_netdev; + goto out; } res = register_netdevice(bond_dev); - if (res < 0) - goto out_netdev; out: rtnl_unlock(); + if (res < 0) + bond_destructor(bond_dev); return res; -out_netdev: - free_netdev(bond_dev); - goto out; } static int __net_init bond_net_init(struct net *net) -- cgit v1.2.3-59-g8ed1b