[RFC] [PATCH V2 0/3] bonding support for operation over IPoIB

[RFC] [PATCH V2 0/3] bonding support for operation over IPoIB

[Or Gerlitz]

This patch series is a second version (see below link to V1) of the suggested
changes to the bonding driver such that it would be able to support non ARPHRD_ETHER
netdevices for its High-Availability (active-backup) mode.

The motivation is to enable the bonding driver on its HA mode to work with the
IP over Infiniband (IPoIB) driver. With these patches I was able to enslave
IPoIB netdevices and run TCP, UDP, IP (UDP) Multicast and ICMP traffic with
fail-over and fail-back working fine. My working env was the net-2.6.20 git.

More over, as IPoIB is also the IB ARP provider for the RDMA CM driver which
is used by native IB ULPs whose addressing scheme is based on IP (eg iSER, SDP,
Lustre, NFSoRDMA, RDS), bonding support for IPoIB devices **enables** HA for
these ULPs. This holds as when the ULP is informed by the IB HW on the failure
of the current IB connection, it just need to reconnect, where the bonding
device will now issue the IB ARP over the active IPoIB slave.

The first patch changes some of the bond netdevice attributes and functions
to be that of the active slave for the case of the enslaved device not being
of ARPHRD_ETHER type. Basically it overrides those setting done by ether_setup(),
which are netdevice **type** dependent and hence might be not appropriate for
devices of other types. It also enforces mutual exclusion on bonding slaves
from dissimilar ether types, as was concluded over the v1 discussion.

IPoIB (see Documentation/infiniband/ipoib.txt) MAC address is made of a 3 bytes
IB QP (Queue Pair) number and 16 bytes IB port GID (Global ID) of the port this
IPoIB device is bounded to. The QP is a resource created by the IB HW and the
GID is an identifier burned into the HCA (i have omitted here some details which
are not important for the bonding RFC).

Basically the IPoIB spec and impl. do not allow for setting the MAC address of
an IPoIB device and this work was made under this assumption.

Hence, the second patch allows for enslaving netdevices which do not support
the set_mac_address() function. In that case the bond mac address is the one
of the active slave, where remote peers are notified on the mac address
(neighbour) change by Gratuitous ARP sent by bonding when fail-over occurs
(this is already done by the bonding code).

Normally, the bonding driver is UP before any enslavement takes place.
Once a netdevice is UP, the network stack acts to have it join some multicast groups
(eg the all-hosts 224.0.0.1). Now, since ether_setup() have set the bonding device
type to be ARPHRD_ETHER and address len to be ETHER_ALEN, the net core code
computes a wrong multicast link address. This is b/c ip_eth_mc_map() is called
where for mcast joins taking place **after** the enslavement another ip_xxx_mc_map()
is called (eg ip_ib_mc_map() when the bond type is ARPHRD_INFINIBAND)

The third patch handles this problem by allowing to enslave devices when the
bonding device is not up. Over the discussion held at the previous post this
seemed to be the most clean way to go, where it is not expected to cause
instabilities.

These patches are not enough for configuration of IPoIB bonding through tools
(eg /sbin/ifenslave and /sbin/ifup) provided by packages such as sysconfig and
initscripts, specifically since these tools sets the bonding device to be UP
before enslaving anything. Once this patchset gets positive/feedback the next step
would be to look how to enhance the tools/packages so it would be possible to
bond/enslave with the modified code. As suggested by the bonding maintainer, this
step can potentially involve converting ifenslave to be a script based on the bonding
sysfs infrastructure rather on the somehow obsoleted Documentation/networking/ifenslave.c

For the ease of potential testers, I will post an example bonding sysfs script which
can be used to set bonding to work with patches 1-3 (let me know!)

Or.

changes from V1 (the links point to V1 0-3/3)

http://marc.theaimsgroup.com/?l=linux-netdev&m=115926582209736&w=2
http://marc.theaimsgroup.com/?l=linux-netdev&m=115926599515568&w=2
http://marc.theaimsgroup.com/?l=linux-netdev&m=115926599430055&w=2
http://marc.theaimsgroup.com/?l=linux-netdev&m=115926599415729&w=2

+ enforce mutual exclusion on the slaves ether types
+ don't attempt to set the bond mtu when enslaving a non ARPHRD_ETHER device
+ rather than hack the bond device ether type through mod params allow enslavement
  when the bond device is not up

[RFC][PATCH V2 1/3] enable bonding to enslave non ARPHRD_ETHER netdevices

[Or Gerlitz]

Signed-off-by: Or Gerlitz <ogerlitz@voltaire.com>

Index: net-2.6.20/drivers/net/bonding/bond_main.c
===================================================================
--- net-2.6.20.orig/drivers/net/bonding/bond_main.c	2006-11-30 10:54:23.000000000 +0200
+++ net-2.6.20/drivers/net/bonding/bond_main.c	2006-11-30 11:53:06.000000000 +0200
@@ -1252,6 +1252,24 @@ static int bond_compute_features(struct
 	return 0;
 }

+
+static void bond_setup_by_slave(struct net_device *bond_dev,
+				struct net_device *slave_dev)
+{
+	bond_dev->hard_header	        = slave_dev->hard_header;
+	bond_dev->rebuild_header        = slave_dev->rebuild_header;
+	bond_dev->hard_header_cache	= slave_dev->hard_header_cache;
+	bond_dev->header_cache_update   = slave_dev->header_cache_update;
+	bond_dev->hard_header_parse	= slave_dev->hard_header_parse;
+
+	bond_dev->type		    = slave_dev->type;
+	bond_dev->hard_header_len   = slave_dev->hard_header_len;
+	bond_dev->addr_len	    = slave_dev->addr_len;
+
+	memcpy(bond_dev->broadcast, slave_dev->broadcast,
+		slave_dev->addr_len);
+}
+
 /* enslave device <slave> to bond device <master> */
 int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 {
@@ -1326,6 +1344,24 @@ int bond_enslave(struct net_device *bond
 		goto err_undo_flags;
 	}

+	/* set bonding device ether type by slave - bonding netdevices are
+	 * created with ether_setup, so when the slave type is not ARPHRD_ETHER
+	 * there is a need to override some of the type dependent attribs/funcs.
+	 *
+	 * bond ether type mutual exclusion - don't allow slaves of dissimilar
+	 * ether type (eg ARPHRD_ETHER and ARPHRD_INFINIBAND) share the same bond
+	 */
+	if (bond->slave_cnt == 0) {
+		if (slave_dev->type != ARPHRD_ETHER)
+			bond_setup_by_slave(bond_dev, slave_dev);
+	} else if (bond_dev->type != slave_dev->type) {
+		printk(KERN_ERR DRV_NAME ": %s ether type (%d) is different from "
+			"other slaves (%d), can not enslave it.\n", slave_dev->name,
+			slave_dev->type, bond_dev->type);
+			res = -EINVAL;
+			goto err_undo_flags;
+	}
+
 	if (slave_dev->set_mac_address == NULL) {
 		printk(KERN_ERR DRV_NAME
 			": %s: Error: The slave device you specified does "
Index: net-2.6.20/drivers/net/bonding/bonding.h
===================================================================
--- net-2.6.20.orig/drivers/net/bonding/bonding.h	2006-11-30 10:54:23.000000000 +0200
+++ net-2.6.20/drivers/net/bonding/bonding.h	2006-11-30 10:58:10.000000000 +0200
@@ -201,6 +201,7 @@ struct bonding {
 	struct   list_head vlan_list;
 	struct   vlan_group *vlgrp;
 	struct   packet_type arp_mon_pt;
+	s8       do_set_mac_addr;
 };

 /**

[RFC][PATCH V2 2/3] enable bonding to enslave netdevices not supporting set_mac_address()

[Or Gerlitz]

Signed-off-by: Or Gerlitz <ogerlitz@voltaire.com>

Index: net-2.6.20/drivers/net/bonding/bond_main.c
===================================================================
--- net-2.6.20.orig/drivers/net/bonding/bond_main.c	2006-11-30 11:53:06.000000000 +0200
+++ net-2.6.20/drivers/net/bonding/bond_main.c	2006-11-30 11:53:16.000000000 +0200
@@ -1103,6 +1103,14 @@ void bond_change_active_slave(struct bon
 		if (new_active) {
 			bond_set_slave_active_flags(new_active);
 		}
+
+		/* when bonding does not set the slave MAC address, the bond MAC
+		 * address is the one of the active slave.
+		 */
+		if (new_active && !bond->do_set_mac_addr)
+			memcpy(bond->dev->dev_addr,  new_active->dev->dev_addr,
+				new_active->dev->addr_len);
+
 		bond_send_gratuitous_arp(bond);
 	}
 }
@@ -1363,14 +1371,23 @@ int bond_enslave(struct net_device *bond
 	}

 	if (slave_dev->set_mac_address == NULL) {
-		printk(KERN_ERR DRV_NAME
-			": %s: Error: The slave device you specified does "
-			"not support setting the MAC address. "
-			"Your kernel likely does not support slave "
-			"devices.\n", bond_dev->name);
-  		res = -EOPNOTSUPP;
-		goto err_undo_flags;
-	}
+		if (bond->slave_cnt == 0) {
+			printk(KERN_WARNING DRV_NAME
+				": %s: Warning: The first slave device you "
+				"specified does not support setting the MAC "
+				"address. This bond MAC address would be that "
+				"of the active slave.\n", bond_dev->name);
+			bond->do_set_mac_addr = 0;
+		} else if (bond->do_set_mac_addr) {
+			printk(KERN_ERR DRV_NAME
+				": %s: Error: The slave device you specified "
+				"does not support setting the MAC addres,."
+				"but this bond uses this practice. \n"
+				, bond_dev->name);
+			res = -EOPNOTSUPP;
+			goto err_undo_flags;
+		}
+	}

 	new_slave = kmalloc(sizeof(struct slave), GFP_KERNEL);
 	if (!new_slave) {
@@ -1392,16 +1409,18 @@ int bond_enslave(struct net_device *bond
 	 */
 	memcpy(new_slave->perm_hwaddr, slave_dev->dev_addr, ETH_ALEN);

-	/*
-	 * Set slave to master's mac address.  The application already
-	 * set the master's mac address to that of the first slave
-	 */
-	memcpy(addr.sa_data, bond_dev->dev_addr, bond_dev->addr_len);
-	addr.sa_family = slave_dev->type;
-	res = dev_set_mac_address(slave_dev, &addr);
-	if (res) {
-		dprintk("Error %d calling set_mac_address\n", res);
-		goto err_free;
+	if (bond->do_set_mac_addr) {
+		/*
+		 * Set slave to master's mac address.  The application already
+		 * set the master's mac address to that of the first slave
+		 */
+		memcpy(addr.sa_data, bond_dev->dev_addr, bond_dev->addr_len);
+		addr.sa_family = slave_dev->type;
+		res = dev_set_mac_address(slave_dev, &addr);
+		if (res) {
+			dprintk("Error %d calling set_mac_address\n", res);
+			goto err_free;
+		}
 	}

 	/* open the slave since the application closed it */
@@ -1627,9 +1646,11 @@ err_close:
 	dev_close(slave_dev);

 err_restore_mac:
-	memcpy(addr.sa_data, new_slave->perm_hwaddr, ETH_ALEN);
-	addr.sa_family = slave_dev->type;
-	dev_set_mac_address(slave_dev, &addr);
+	if (bond->do_set_mac_addr) {
+		memcpy(addr.sa_data, new_slave->perm_hwaddr, ETH_ALEN);
+		addr.sa_family = slave_dev->type;
+		dev_set_mac_address(slave_dev, &addr);
+	}

 err_free:
 	kfree(new_slave);
@@ -1807,10 +1828,12 @@ int bond_release(struct net_device *bond
 	/* close slave before restoring its mac address */
 	dev_close(slave_dev);

-	/* restore original ("permanent") mac address */
-	memcpy(addr.sa_data, slave->perm_hwaddr, ETH_ALEN);
-	addr.sa_family = slave_dev->type;
-	dev_set_mac_address(slave_dev, &addr);
+	if (bond->do_set_mac_addr) {
+		/* restore original ("permanent") mac address */
+		memcpy(addr.sa_data, slave->perm_hwaddr, ETH_ALEN);
+		addr.sa_family = slave_dev->type;
+		dev_set_mac_address(slave_dev, &addr);
+	}

 	slave_dev->priv_flags &= ~(IFF_MASTER_8023AD | IFF_MASTER_ALB |
 				   IFF_SLAVE_INACTIVE | IFF_BONDING |
@@ -1897,10 +1920,12 @@ static int bond_release_all(struct net_d
 		/* close slave before restoring its mac address */
 		dev_close(slave_dev);

-		/* restore original ("permanent") mac address*/
-		memcpy(addr.sa_data, slave->perm_hwaddr, ETH_ALEN);
-		addr.sa_family = slave_dev->type;
-		dev_set_mac_address(slave_dev, &addr);
+		if (bond->do_set_mac_addr) {
+			/* restore original ("permanent") mac address*/
+			memcpy(addr.sa_data, slave->perm_hwaddr, ETH_ALEN);
+			addr.sa_family = slave_dev->type;
+			dev_set_mac_address(slave_dev, &addr);
+		}

 		slave_dev->priv_flags &= ~(IFF_MASTER_8023AD | IFF_MASTER_ALB |
 					   IFF_SLAVE_INACTIVE);
@@ -3932,6 +3957,10 @@ static int bond_set_mac_address(struct n

 	dprintk("bond=%p, name=%s\n", bond, (bond_dev ? bond_dev->name : "None"));

+	if (!bond->do_set_mac_addr) {
+		return -EOPNOTSUPP;
+	}
+
 	if (!is_valid_ether_addr(sa->sa_data)) {
 		return -EADDRNOTAVAIL;
 	}
@@ -4373,6 +4402,9 @@ static int bond_init(struct net_device *
 	bond_create_proc_entry(bond);
 #endif

+	/* set do_set_mac_addr to true on startup */
+	bond->do_set_mac_addr = 1;
+
 	list_add_tail(&bond->bond_list, &bond_dev_list);

 	return 0;

[RFC] [PATCH V2 3/3] enable IP multicast for bonding IPoIB devices - allow not UP enslavement

[Or Gerlitz]

Signed-off-by: Or Gerlitz <ogerlitz@voltaire.com>

Index: net-2.6.20/drivers/net/bonding/bond_sysfs.c
===================================================================
--- net-2.6.20.orig/drivers/net/bonding/bond_sysfs.c	2006-11-30 10:45:53.000000000 +0200
+++ net-2.6.20/drivers/net/bonding/bond_sysfs.c	2006-11-30 10:48:13.000000000 +0200
@@ -265,11 +265,9 @@ static ssize_t bonding_store_slaves(stru

 	/* Quick sanity check -- is the bond interface up? */
 	if (!(bond->dev->flags & IFF_UP)) {
-		printk(KERN_ERR DRV_NAME
-		       ": %s: Unable to update slaves because interface is down.\n",
+		printk(KERN_WARNING DRV_NAME
+		       ": %s: doing slave updates when interface is down.\n",
 		       bond->dev->name);
-		ret = -EPERM;
-		goto out;
 	}

 	/* Note:  We can't hold bond->lock here, as bond_create grabs it. */
Index: net-2.6.20/drivers/net/bonding/bond_main.c
===================================================================
--- net-2.6.20.orig/drivers/net/bonding/bond_main.c	2006-11-30 10:46:57.000000000 +0200
+++ net-2.6.20/drivers/net/bonding/bond_main.c	2006-11-30 10:48:13.000000000 +0200
@@ -1298,8 +1298,8 @@ int bond_enslave(struct net_device *bond

 	/* bond must be initialized by bond_open() before enslaving */
 	if (!(bond_dev->flags & IFF_UP)) {
-		dprintk("Error, master_dev is not up\n");
-		return -EPERM;
+		printk(KERN_WARNING DRV_NAME
+			" %s: master_dev is not up in bond_enslave\n", bond_dev->name);
 	}

 	/* already enslaved */

re: [RFC] [PATCH V2 0/3] bonding support for operation over IPoIB - example config script

[Or Gerlitz]

Below is an example script i was using to configure bonding in my testing

Or.

--- /dev/null	2006-10-30 17:30:04.465997856 +0200
+++ bond_init_sysfs.bash	2006-11-30 12:51:05.109565889 +0200
@@ -0,0 +1,25 @@
+#!/bin/bash
+
+BOND_IP=192.168.10.118
+BOND_NETMASK=255.255.255.0
+
+SLAVE_A=ib0
+SLAVE_B=ib1
+
+modprobe bonding
+
+echo active-backup > /sys/class/net/bond0/bonding/mode
+echo 100           > /sys/class/net/bond0/bonding/miimon
+
+modprobe ib_ipoib
+
+## this is some debug info that can enable seeing below the scenes...
+## to learn more see Documentation/infiniband/ipoib.txt
+
+#echo 1 > /sys/module/ib_ipoib/parameters/mcast_debug_level
+#echo 1 > /sys/module/ib_ipoib/parameters/debug_level
+
+echo +$SLAVE_A > /sys/class/net/bond0/bonding/slaves
+echo +$SLAVE_B > /sys/class/net/bond0/bonding/slaves
+
+ifconfig bond0 $BOND_IP netmask $BOND_NETMASK up

Re: [RFC][PATCH V2 1/3] enable bonding to enslave non ARPHRD_ETHER netdevices

[Or Gerlitz]

Or Gerlitz wrote:
> Index: net-2.6.20/drivers/net/bonding/bonding.h
> ===================================================================
> --- net-2.6.20.orig/drivers/net/bonding/bonding.h	2006-11-30 10:54:23.000000000 +0200
> +++ net-2.6.20/drivers/net/bonding/bonding.h	2006-11-30 10:58:10.000000000 +0200
> @@ -201,6 +201,7 @@ struct bonding {
>  	struct   list_head vlan_list;
>  	struct   vlan_group *vlgrp;
>  	struct   packet_type arp_mon_pt;
> +	s8       do_set_mac_addr;
>  };
> 
>  /**

oops - this piece belongs to the second patch, which actually uses the 
added field, sorry for that. I will fix it for the next version.

Or.

Re: [RFC] [PATCH V2 0/3] bonding support for operation over IPoIB

[Or Gerlitz]

Or Gerlitz wrote:
> This patch series is a second version (see below link to V1) of the suggested
> changes to the bonding driver such that it would be able to support non ARPHRD_ETHER
> netdevices for its High-Availability (active-backup) mode.
> 
> The motivation is to enable the bonding driver on its HA mode to work with the
> IP over Infiniband (IPoIB) driver. With these patches I was able to enslave
> IPoIB netdevices and run TCP, UDP, IP (UDP) Multicast and ICMP traffic with
> fail-over and fail-back working fine. My working env was the net-2.6.20 git.


> These patches are not enough for configuration of IPoIB bonding through tools
> (eg /sbin/ifenslave and /sbin/ifup) provided by packages such as sysconfig and
> initscripts, specifically since these tools sets the bonding device to be UP
> before enslaving anything. Once this patchset gets positive/feedback the next step
> would be to look how to enhance the tools/packages so it would be possible to
> bond/enslave with the modified code. As suggested by the bonding maintainer, this
> step can potentially involve converting ifenslave to be a script based on the bonding
> sysfs infrastructure rather on the somehow obsoleted Documentation/networking/ifenslave.c

Jay,

I would like to move forward and push the V2 patch series upstream 
through netdev and then start working on the configuration tools etc 
changes needed to support bonding IPoIB devices through non direct 
bonding sysfs scripts... are you OK with that?

If you agree to the push, who is doing this nowadays, is it Jeff Garzik 
or David Miller?

Roland - any other comments/concerns that you might have are very much 
appreciated.

Or.