[PATCH net-next-2.6 0/5] bonding: Fixes and updates

[PATCH net-next-2.6 0/5] bonding: Fixes and updates

[Jay Vosburgh]

	Five patches for bonding; these apply to net-next-2.6.

	Patches 1 - 3 comprise support for NETDEV_BONDING_FAILOVER
notifiers; the first patch adds the define, the second cleans up part of
bonding in preparation for the notifier call, and the third adds the
notifier call itself.

	Patch 4 cleans up and improves previously added support for
specifying the number of gratuitous ARPs to be issued at failover time for
the active-backup mode.

	Patch 5 permits specifying max_bonds as zero (creating no bond0
device).  Also provides an API from net/core to create and destroy files
in net_class so that /sys/class/net/bonding_masters will be created.  This
was previously done by backtracking from the first bonding device's sysfs
information (dev->dev.class).

	Please apply.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

[PATCH 1/5] net/core: add NETDEV_BONDING_FAILOVER event

[Jay Vosburgh]

From: Or Gerlitz <ogerlitz@voltaire.com>

Add NETDEV_BONDING_FAILOVER event to be used in a successive patch
by bonding to announce fail-over for the active-backup mode through the
netdev events notifier chain mechanism. Such an event can be of use for the
RDMA CM (communication manager) to let native RDMA ULPs (eg NFS-RDMA, iSER)
always be aligned with the IP stack, in the sense that they use the same
ports/links as the stack does. More usages can be done to allow monitoring
tools based on netlink events being aware to bonding fail-over.

Signed-off-by: Or Gerlitz <ogerlitz@voltaire.com>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
---
 include/linux/netdevice.h |    1 +
 include/linux/notifier.h  |    1 +
 net/core/dev.c            |    6 ++++++
 3 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f27fd20..e92fc83 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1479,6 +1479,7 @@ extern void		__dev_addr_unsync(struct dev_addr_list **to, int *to_count, struct
 extern void		dev_set_promiscuity(struct net_device *dev, int inc);
 extern void		dev_set_allmulti(struct net_device *dev, int inc);
 extern void		netdev_state_change(struct net_device *dev);
+extern void		netdev_bonding_change(struct net_device *dev);
 extern void		netdev_features_change(struct net_device *dev);
 /* Load a device via the kmod */
 extern void		dev_load(struct net *net, const char *name);
diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index 0ff6224..bd3d72d 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -197,6 +197,7 @@ static inline int notifier_to_errno(int ret)
 #define NETDEV_GOING_DOWN	0x0009
 #define NETDEV_CHANGENAME	0x000A
 #define NETDEV_FEAT_CHANGE	0x000B
+#define NETDEV_BONDING_FAILOVER 0x000C
 
 #define SYS_DOWN	0x0001	/* Notify of system down */
 #define SYS_RESTART	SYS_DOWN
diff --git a/net/core/dev.c b/net/core/dev.c
index 5829630..da8367c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -960,6 +960,12 @@ void netdev_state_change(struct net_device *dev)
 	}
 }
 
+void netdev_bonding_change(struct net_device *dev)
+{
+	call_netdevice_notifiers(NETDEV_BONDING_FAILOVER, dev);
+}
+EXPORT_SYMBOL(netdev_bonding_change);
+
 /**
  *	dev_load 	- load a network module
  *	@net: the applicable net namespace
-- 
1.5.2.4

[PATCH 2/5] bonding: bond_change_active_slave() cleanup under active-backup

[Jay Vosburgh]

From: Or Gerlitz <ogerlitz@voltaire.com>

simplified the code of bond_change_active_slave() such that under
active-backup mode there's one "if (new_active)" test and the rest
of the code only does extra checks on top of it. This removed an
unneeded "if (bond->send_grat_arp > 0)" check and avoid calling
bond_send_gratuitous_arp when there's no active slave.

Jay Vosburgh made minor coding style changes to the orignal patch.

Signed-off-by: Or Gerlitz <ogerlitz@voltaire.com>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
---
 drivers/net/bonding/bond_main.c |   18 ++++++++----------
 1 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5b4af3c..2db2d05 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1189,21 +1189,19 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
 
 		if (new_active) {
 			bond_set_slave_active_flags(new_active);
-		}
 
-		if (new_active && bond->params.fail_over_mac)
-			bond_do_fail_over_mac(bond, new_active, old_active);
+			if (bond->params.fail_over_mac)
+				bond_do_fail_over_mac(bond, new_active,
+						      old_active);
 
-		bond->send_grat_arp = bond->params.num_grat_arp;
-		if (bond->curr_active_slave &&
-			test_bit(__LINK_STATE_LINKWATCH_PENDING,
+			bond->send_grat_arp = bond->params.num_grat_arp;
+			if (!test_bit(__LINK_STATE_LINKWATCH_PENDING,
 					&bond->curr_active_slave->dev->state)) {
-			dprintk("delaying gratuitous arp on %s\n",
-				bond->curr_active_slave->dev->name);
-		} else {
-			if (bond->send_grat_arp > 0) {
 				bond_send_gratuitous_arp(bond);
 				bond->send_grat_arp--;
+			} else {
+				dprintk("delaying gratuitous arp on %s\n",
+					bond->curr_active_slave->dev->name);
 			}
 		}
 	}
-- 
1.5.2.4

[PATCH 3/5] bonding: deliver netdev event for fail-over under the active-backup mode

[Jay Vosburgh]

From: Or Gerlitz <ogerlitz@voltaire.com>

under active-backup mode and when there's actual new_active slave,
have bond_change_active_slave() call the networking core to deliver
NETDEV_BONDING_FAILOVER event such that the fail-over can be notable
by code outside of the bonding driver such as the RDMA stack and
monitoring tools.

As the correct context of locking appropriate for notifier calls is RTNL
and nothing else, bond->curr_slave_lock and bond->lock are unlocked and
later locked again. This is ensured by the rest of the code to be safe
under backup-mode AND when new_active is not NULL.

Jay Vosburgh modified the original patch for formatting and fixed a
compiler error.

Signed-off-by: Or Gerlitz <ogerlitz@voltaire.com>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
---
 drivers/net/bonding/bond_main.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 2db2d05..925402b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1203,6 +1203,14 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
 				dprintk("delaying gratuitous arp on %s\n",
 					bond->curr_active_slave->dev->name);
 			}
+
+			write_unlock_bh(&bond->curr_slave_lock);
+			read_unlock(&bond->lock);
+
+			netdev_bonding_change(bond->dev);
+
+			read_lock(&bond->lock);
+			write_lock_bh(&bond->curr_slave_lock);
 		}
 	}
 }
-- 
1.5.2.4

[PATCH 4/5] bonding: Rework / fix multiple gratuitous ARP support

[Jay Vosburgh]

	Support for sending multiple gratuitous ARPs during failovers
was added by commit:

commit 7893b2491a2d5f716540ac5643d78d37a7f6628b
Author: Moni Shoua <monis@voltaire.com>
Date:   Sat May 17 21:10:12 2008 -0700

    bonding: Send more than one gratuitous ARP when slave takes over

	This change modifies that support to remove duplicated code,
add support for ARP monitor (the original only supported miimon), clear
the grat ARP counter in bond_close (lest a later "ifconfig up" immediately
start spewing ARPs), and add documentation for the module parameter.

	Also updated driver version to 3.3.0.

Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
---
 Documentation/networking/bonding.txt |   11 +++++++++
 drivers/net/bonding/bond_main.c      |   42 +++++++++++++++++----------------
 drivers/net/bonding/bonding.h        |    4 +-
 3 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 8e6b8d3..370b7da 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -539,6 +539,17 @@ mode
 		swapped with the new curr_active_slave that was
 		chosen.
 
+num_grat_arp
+
+	Specifies the number of gratuitous ARPs to be issued after a
+	failover event.  One gratuitous ARP is issued immediately after
+	the failover, subsequent ARPs are sent at a rate of one per link
+	monitor interval (arp_interval or miimon, whichever is active).
+
+	The valid range is 0 - 255; the default value is 1.  This option
+	affects only the active-backup mode.  This option was added for
+	bonding version 3.3.0.
+
 primary
 
 	A string (eth0, eth2, etc) specifying which slave is the
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 925402b..3b6d66a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1195,14 +1195,7 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
 						      old_active);
 
 			bond->send_grat_arp = bond->params.num_grat_arp;
-			if (!test_bit(__LINK_STATE_LINKWATCH_PENDING,
-					&bond->curr_active_slave->dev->state)) {
-				bond_send_gratuitous_arp(bond);
-				bond->send_grat_arp--;
-			} else {
-				dprintk("delaying gratuitous arp on %s\n",
-					bond->curr_active_slave->dev->name);
-			}
+			bond_send_gratuitous_arp(bond);
 
 			write_unlock_bh(&bond->curr_slave_lock);
 			read_unlock(&bond->lock);
@@ -2241,17 +2234,6 @@ static int __bond_mii_monitor(struct bonding *bond, int have_locks)
 	 * program could monitor the link itself if needed.
 	 */
 
-	if (bond->send_grat_arp) {
-		if (bond->curr_active_slave && test_bit(__LINK_STATE_LINKWATCH_PENDING,
-				&bond->curr_active_slave->dev->state))
-			dprintk("Needs to send gratuitous arp but not yet\n");
-		else {
-			dprintk("sending delayed gratuitous arp on on %s\n",
-				bond->curr_active_slave->dev->name);
-			bond_send_gratuitous_arp(bond);
-			bond->send_grat_arp--;
-		}
-	}
 	read_lock(&bond->curr_slave_lock);
 	oldcurrent = bond->curr_active_slave;
 	read_unlock(&bond->curr_slave_lock);
@@ -2492,6 +2474,13 @@ void bond_mii_monitor(struct work_struct *work)
 		read_unlock(&bond->lock);
 		return;
 	}
+
+	if (bond->send_grat_arp) {
+		read_lock(&bond->curr_slave_lock);
+		bond_send_gratuitous_arp(bond);
+		read_unlock(&bond->curr_slave_lock);
+	}
+
 	if (__bond_mii_monitor(bond, 0)) {
 		read_unlock(&bond->lock);
 		rtnl_lock();
@@ -2657,6 +2646,8 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
 /*
  * Kick out a gratuitous ARP for an IP on the bonding master plus one
  * for each VLAN above us.
+ *
+ * Caller must hold curr_slave_lock for read or better
  */
 static void bond_send_gratuitous_arp(struct bonding *bond)
 {
@@ -2666,9 +2657,13 @@ static void bond_send_gratuitous_arp(struct bonding *bond)
 
 	dprintk("bond_send_grat_arp: bond %s slave %s\n", bond->dev->name,
 				slave ? slave->dev->name : "NULL");
-	if (!slave)
+
+	if (!slave || !bond->send_grat_arp ||
+	    test_bit(__LINK_STATE_LINKWATCH_PENDING, &slave->dev->state))
 		return;
 
+	bond->send_grat_arp--;
+
 	if (bond->master_ip) {
 		bond_arp_send(slave->dev, ARPOP_REPLY, bond->master_ip,
 				bond->master_ip, 0);
@@ -3172,6 +3167,12 @@ void bond_activebackup_arp_mon(struct work_struct *work)
 	if (bond->slave_cnt == 0)
 		goto re_arm;
 
+	if (bond->send_grat_arp) {
+		read_lock(&bond->curr_slave_lock);
+		bond_send_gratuitous_arp(bond);
+		read_unlock(&bond->curr_slave_lock);
+	}
+
 	if (bond_ab_arp_inspect(bond, delta_in_ticks)) {
 		read_unlock(&bond->lock);
 		rtnl_lock();
@@ -3846,6 +3847,7 @@ static int bond_close(struct net_device *bond_dev)
 
 	write_lock_bh(&bond->lock);
 
+	bond->send_grat_arp = 0;
 
 	/* signal timers not to re-arm */
 	bond->kill_timers = 1;
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 89fd996..fb730ec 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -22,8 +22,8 @@
 #include "bond_3ad.h"
 #include "bond_alb.h"
 
-#define DRV_VERSION	"3.2.5"
-#define DRV_RELDATE	"March 21, 2008"
+#define DRV_VERSION	"3.3.0"
+#define DRV_RELDATE	"June 10, 2008"
 #define DRV_NAME	"bonding"
 #define DRV_DESCRIPTION	"Ethernet Channel Bonding Driver"
 
-- 
1.5.2.4

[PATCH 5/5] bonding: Allow setting max_bonds to zero

[Jay Vosburgh]

	Permit bonding to function rationally if max_bonds is set to
zero.  This will load the module, but create no master devices (which can
be created via sysfs).

	Requires some change to bond_create_sysfs; currently, the
netdev sysfs directory is determined from the first bonding device created,
but this is no longer possible.  Instead, an interface from net/core is
created to create and destroy files in net_class.

	Based on a patch submitted by Phil Oester <kernel@linuxaces.com>.
Modified by Jay Vosburgh to fix the sysfs issue mentioned above and to
update the documentation.

Signed-off-by: Phil Oester <kernel@linuxace.com>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
---
 Documentation/networking/bonding.txt |    3 ++-
 drivers/net/bonding/bond_main.c      |    6 +++---
 drivers/net/bonding/bond_sysfs.c     |   22 +++-------------------
 include/linux/netdevice.h            |    3 +++
 net/core/net-sysfs.c                 |   13 +++++++++++++
 5 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 370b7da..7fa7fe7 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -376,7 +376,8 @@ max_bonds
 	Specifies the number of bonding devices to create for this
 	instance of the bonding driver.  E.g., if max_bonds is 3, and
 	the bonding driver is not already loaded, then bond0, bond1
-	and bond2 will be created.  The default value is 1.
+	and bond2 will be created.  The default value is 1.  Specifying
+	a value of 0 will load bonding, but will not create any devices.
 
 miimon
 
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 3b6d66a..d57b65d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4750,11 +4750,11 @@ static int bond_check_params(struct bond_params *params)
 		}
 	}
 
-	if (max_bonds < 1 || max_bonds > INT_MAX) {
+	if (max_bonds < 0 || max_bonds > INT_MAX) {
 		printk(KERN_WARNING DRV_NAME
 		       ": Warning: max_bonds (%d) not in range %d-%d, so it "
 		       "was reset to BOND_DEFAULT_MAX_BONDS (%d)\n",
-		       max_bonds, 1, INT_MAX, BOND_DEFAULT_MAX_BONDS);
+		       max_bonds, 0, INT_MAX, BOND_DEFAULT_MAX_BONDS);
 		max_bonds = BOND_DEFAULT_MAX_BONDS;
 	}
 
@@ -4953,7 +4953,7 @@ static int bond_check_params(struct bond_params *params)
 
 		printk("\n");
 
-	} else {
+	} else if (max_bonds) {
 		/* miimon and arp_interval not set, we need one so things
 		 * work as expected, see bonding.txt for details
 		 */
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index dd265c6..6caac0f 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -53,7 +53,6 @@ extern struct bond_parm_tbl arp_validate_tbl[];
 extern struct bond_parm_tbl fail_over_mac_tbl[];
 
 static int expected_refcount = -1;
-static struct class *netdev_class;
 /*--------------------------- Data Structures -----------------------------*/
 
 /* Bonding sysfs lock.  Why can't we just use the subsystem lock?
@@ -1447,19 +1446,9 @@ static struct attribute_group bonding_group = {
  */
 int bond_create_sysfs(void)
 {
-	int ret = 0;
-	struct bonding *firstbond;
-
-	/* get the netdev class pointer */
-	firstbond = container_of(bond_dev_list.next, struct bonding, bond_list);
-	if (!firstbond)
-		return -ENODEV;
-
-	netdev_class = firstbond->dev->dev.class;
-	if (!netdev_class)
-		return -ENODEV;
+	int ret;
 
-	ret = class_create_file(netdev_class, &class_attr_bonding_masters);
+	ret = netdev_class_create_file(&class_attr_bonding_masters);
 	/*
 	 * Permit multiple loads of the module by ignoring failures to
 	 * create the bonding_masters sysfs file.  Bonding devices
@@ -1478,10 +1467,6 @@ int bond_create_sysfs(void)
 			printk(KERN_ERR
 			       "network device named %s already exists in sysfs",
 			       class_attr_bonding_masters.attr.name);
-		else {
-			netdev_class = NULL;
-			return 0;
-		}
 	}
 
 	return ret;
@@ -1493,8 +1478,7 @@ int bond_create_sysfs(void)
  */
 void bond_destroy_sysfs(void)
 {
-	if (netdev_class)
-		class_remove_file(netdev_class, &class_attr_bonding_masters);
+	netdev_class_remove_file(&class_attr_bonding_masters);
 }
 
 /*
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e92fc83..9ccbfac 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1506,6 +1506,9 @@ extern void *dev_seq_next(struct seq_file *seq, void *v, loff_t *pos);
 extern void dev_seq_stop(struct seq_file *seq, void *v);
 #endif
 
+extern int netdev_class_create_file(struct class_attribute *class_attr);
+extern void netdev_class_remove_file(struct class_attribute *class_attr);
+
 extern void linkwatch_run_queue(void);
 
 extern int netdev_compute_features(unsigned long all, unsigned long one);
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index dccd737..3f79413 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -468,6 +468,19 @@ int netdev_register_kobject(struct net_device *net)
 	return device_add(dev);
 }
 
+int netdev_class_create_file(struct class_attribute *class_attr)
+{
+	return class_create_file(&net_class, class_attr);
+}
+
+void netdev_class_remove_file(struct class_attribute *class_attr)
+{
+	class_remove_file(&net_class, class_attr);
+}
+
+EXPORT_SYMBOL(netdev_class_create_file);
+EXPORT_SYMBOL(netdev_class_remove_file);
+
 void netdev_initialize_kobject(struct net_device *net)
 {
 	struct device *device = &(net->dev);
-- 
1.5.2.4

Re: [PATCH 5/5] bonding: Allow setting max_bonds to zero

[Jeff Garzik]

Jay Vosburgh wrote:
> 	Permit bonding to function rationally if max_bonds is set to
> zero.  This will load the module, but create no master devices (which can
> be created via sysfs).
> 
> 	Requires some change to bond_create_sysfs; currently, the
> netdev sysfs directory is determined from the first bonding device created,
> but this is no longer possible.  Instead, an interface from net/core is
> created to create and destroy files in net_class.
> 
> 	Based on a patch submitted by Phil Oester <kernel@linuxaces.com>.
> Modified by Jay Vosburgh to fix the sysfs issue mentioned above and to
> update the documentation.
> 
> Signed-off-by: Phil Oester <kernel@linuxace.com>
> Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>

applied 1-5