[PATCH 0/3] bonding: various fixes

[PATCH 0/3] bonding: various fixes

[Phil Oester]

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(-)

[PATCH 1/3] bonding: Incorrect TX queue offset

[Phil Oester]

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.

Signed-off-by: Phil Oester <kernel@linuxace.com>
---
 drivers/net/bonding/bond_main.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 3ad4f50..a93d941 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -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

[PATCH 2/3] bonding: Call netif_carrier_off after register_netdevice

[Phil Oester]

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.

Signed-off-by: Phil Oester <kernel@linuxace.com>
---
 drivers/net/bonding/bond_main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index a93d941..66c98e6 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -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

[PATCH 3/3] bonding: Improve syslog message at device creation time

[Phil Oester]

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.

Signed-off-by: Phil Oester <kernel@linuxace.com>
---
 drivers/net/bonding/bond_sysfs.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 72bb0f6..e718144 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -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

Re: [PATCH 0/3] bonding: various fixes

[Andy Gospodarek]

On Mon, Mar 14, 2011 at 09:22:03AM -0700, Phil Oester wrote:
> 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(-)

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

Signed-off-by: Andy Gospodarek <andy@greyhouse.net>

Re: [PATCH 0/3] bonding: various fixes

[David Miller]

From: Andy Gospodarek <andy@greyhouse.net>
Date: Mon, 14 Mar 2011 17:04:35 -0400

> On Mon, Mar 14, 2011 at 09:22:03AM -0700, Phil Oester wrote:
>> 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(-)
> 
> All in the series look good.  Thanks for posting the queue cleanup,
> Phil.
> 
> Signed-off-by: Andy Gospodarek <andy@greyhouse.net>

All applied, thanks.