bonding: various fixes

A few collected fixes to bonding. Patches are against net-next, but should apply fine to 38-rc.

Phil

 bond_main.c  |   15 +++++++++++----
 bond_sysfs.c |    5 ++++-
 2 files changed, 15 insertions(+), 5 deletions(-)

When packets come in from a device with >= 16 receive queues headed out a bonding interface, syslog gets filled with this:

kernel: bond0 selects TX queue 16, but real number of TX queues is 16

because queue_mapping is offset by 1. Adjust return value to account for the offset.

This is a revision of my earlier patch (which did not use the skb_rx_queue_* helpers - thanks to Ben for the suggestion). Andy submitted a similar patch which emits a pr_warning on invalid queue selection, but I believe the log spew is not useful. We can revisit that question in the future, but in the interim I believe fixing the core problem is worthwhile.

---
 drivers/net/bonding/bond_main.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

index 3ad4f50..a93d941 100644
@@ -4341,11 +4341,18 @@ static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb)
 {
 	/*
 	 * This helper function exists to help dev_pick_tx get the correct
-	 * destination queue.  Using a helper function skips the a call to
+	 * destination queue.  Using a helper function skips a call to
 	 * skb_tx_hash and will put the skbs in the queue we expect on their
 	 * way down to the bonding driver.
 	 */
-	return skb->queue_mapping;
+	u16 txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
+
+	if (unlikely(txq >= dev->real_num_tx_queues)) {
+		do
+			txq -= dev->real_num_tx_queues;
+		while (txq >= dev->real_num_tx_queues);
+	}
+	return txq;
 }
 
 static netdev_tx_t bond_start_xmit(struct sk_buff *skb, struct net_device *dev)

1.7.4

Bringing up a bond interface with all network cables disconnected does not properly set the interface as DOWN because the call to netif_carrier_off occurs too early in bond_init. The call needs to occur after register_netdevice has set dev->reg_state to NETREG_REGISTERED, so that netif_carrier_off will trigger the call to linkwatch_fire_event.

---
 drivers/net/bonding/bond_main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

index a93d941..66c98e6 100644
@@ -4980,8 +4980,6 @@ static int bond_init(struct net_device *bond_dev)
 
 	bond_set_lockdep_class(bond_dev);
 
-	netif_carrier_off(bond_dev);
-
 	bond_create_proc_entry(bond);
 	list_add_tail(&bond->bond_list, &bn->dev_list);
 

@@ -5051,6 +5049,8 @@ int bond_create(struct net *net, const char *name)
 
 	res = register_netdevice(bond_dev);
 
+	netif_carrier_off(bond_dev);
+
 out:
 	rtnl_unlock();
 	if (res < 0)

1.7.4

When the bonding module is loaded, it creates bond0 by default. Then, when attempting to create bond0, the following messages are printed to syslog:

kernel: bonding: bond0 is being created...
kernel: bonding: Bond creation failed.

Which seems to indicate a problem, when in reality there is no problem. Since the actual error code is passed down from bond_create, make use of it to print a bit less ominous message:

kernel: bonding: bond0 is being created...
kernel: bond0 already exists.

---
 drivers/net/bonding/bond_sysfs.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

index 72bb0f6..e718144 100644
@@ -118,7 +118,10 @@ static ssize_t bonding_store_bonds(struct class *cls,
 		pr_info("%s is being created...\n", ifname);
 		rv = bond_create(net, ifname);
 		if (rv) {
-			pr_info("Bond creation failed.\n");
+			if (rv == -EEXIST)
+				pr_info("%s already exists.\n", ifname);
+			else
+				pr_info("%s creation failed.\n", ifname);
 			res = rv;
 		}
 	} else if (command[0] == '-') {

1.7.4

All in the series look good. Thanks for posting the queue cleanup, Phil.

From: Andy Gospodarek <[email protected]>
Date: Mon, 14 Mar 2011 17:04:35 -0400

All applied, thanks.