[PATCH]: net/bonding: Enable to change device type before enslaving

[PATCH]: net/bonding: Enable to change device type before enslaving

[Moni Shoua]

The bonding network device is being created  with device type ARPHDR_ETHER.
Although the device type changes with first slave we want to be able to change
it when it has zero slaves. The reason is to make the kernel choose the right
function for multicast address translation (ib_xxx_mc_map) which is determined by
device type even when no slaves are enslaved. If not so, the kernel picks a wrong
translation function and wrong HW addresses will be passed to slaves when the
bonding device tries to set their multicast lists.

Signed-off-by: Moni Shoua <monis@voltaire.com>
---
 drivers/net/bonding/bond_sysfs.c |   48 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 48 insertions(+)

diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 90a1f31..86fec7e 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -950,6 +950,7 @@ out:
 }
 static DEVICE_ATTR(lacp_rate, S_IRUGO | S_IWUSR, bonding_show_lacp, bonding_store_lacp);
 
+
 /*
  * Show and set the MII monitor interval.  There are two tricky bits
  * here.  First, if MII monitoring is activated, then we must disable
@@ -1039,6 +1040,52 @@ out:
 static DEVICE_ATTR(miimon, S_IRUGO | S_IWUSR, bonding_show_miimon, bonding_store_miimon);
 
 /*
+ * Show and set the device type of the bonding device
+ */
+static ssize_t bonding_show_devtype(struct device *d,
+				   struct device_attribute *attr,
+				   char *buf)
+{
+	struct bonding *bond = to_bond(d);
+
+	return sprintf(buf, "%d\n", bond->dev->type);
+}
+
+static ssize_t bonding_store_devtype(struct device *d,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	int new_value, ret = count;
+	struct bonding *bond = to_bond(d);
+
+	if (sscanf(buf, "%d", &new_value) != 1) {
+		printk(KERN_ERR DRV_NAME
+		       ": %s: no device type value specified.\n",
+		       bond->dev->name);
+		ret = -EINVAL;
+		goto out;
+	}
+	if (bond->slave_cnt > 0) {
+		printk(KERN_ERR DRV_NAME
+		       ": %s: Can't change device type when slaves are present; rejected.\n",
+		       bond->dev->name);
+		ret = -EINVAL;
+		goto out;
+	}
+	if (new_value <= 0) {
+		printk(KERN_ERR DRV_NAME
+		       ": %s: Invalid  device type %d not in range %d-%d; rejected.\n",
+		       bond->dev->name, new_value, 1, INT_MAX);
+		ret = -EINVAL;
+		goto out;
+	}
+	bond->dev->type = new_value;
+out:
+	return ret;
+}
+static DEVICE_ATTR(devtype, S_IRUGO | S_IWUSR, bonding_show_devtype, bonding_store_devtype);
+
+/*
  * Show and set the primary slave.  The store function is much
  * simpler than bonding_store_slaves function because it only needs to
  * handle one interface name.
@@ -1387,6 +1434,7 @@ static struct attribute *per_bond_attrs[
 	&dev_attr_updelay.attr,
 	&dev_attr_lacp_rate.attr,
 	&dev_attr_xmit_hash_policy.attr,
+	&dev_attr_devtype.attr,
 	&dev_attr_miimon.attr,
 	&dev_attr_primary.attr,
 	&dev_attr_use_carrier.attr,

Re: [PATCH]: net/bonding: Enable to change device type before enslaving

[Jay Vosburgh]

Moni Shoua <monis@voltaire.com> wrote:

>
>The bonding network device is being created  with device type ARPHDR_ETHER.
>Although the device type changes with first slave we want to be able to change
>it when it has zero slaves. The reason is to make the kernel choose the right
>function for multicast address translation (ib_xxx_mc_map) which is determined by
>device type even when no slaves are enslaved. If not so, the kernel picks a wrong
>translation function and wrong HW addresses will be passed to slaves when the
>bonding device tries to set their multicast lists.

	Does this mean that the automatic selection on first enslavement
is no longer needed, and all setting of the type for IB devices must
occur prior to first enslavement?

	Or is this more of a special case for some devices, and the
automatic selection is still correct for most cases?

	-J

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

Re: [PATCH]: net/bonding: Enable to change device type before enslaving

[Moni Shoua]

> 	Does this mean that the automatic selection on first enslavement
> is no longer needed, and all setting of the type for IB devices must
> occur prior to first enslavement?
> 
> 	Or is this more of a special case for some devices, and the
> automatic selection is still correct for most cases?
> 

I think that the later is closer to the truth. 
The goal is to modify the bonding module to work with IPoIB slaves but
with as small as possible changes to what already exists.

In details, the bonding net device device gets its type by ether_setup(),
which runs for all types of slaves and I don't see a way to change that.
However, bonding master for IPoIB slaves requires a different device type setting 
before it comes up for some OSs (redhat 4 for instance). On other OSs I don't see the problem 
and I guess that this is because master becomes up only after it has slaves.

Re: [PATCH]: net/bonding: Enable to change device type before enslaving

[Jay Vosburgh]

Moni Shoua <monis@voltaire.com> wrote:

>> 	Does this mean that the automatic selection on first enslavement
>> is no longer needed, and all setting of the type for IB devices must
>> occur prior to first enslavement?
>> 
>> 	Or is this more of a special case for some devices, and the
>> automatic selection is still correct for most cases?
>> 
>
>I think that the later is closer to the truth. 
>The goal is to modify the bonding module to work with IPoIB slaves but
>with as small as possible changes to what already exists.
>
>In details, the bonding net device device gets its type by
>ether_setup(), which runs for all types of slaves and I don't see a way
>to change that.  However, bonding master for IPoIB slaves requires a
>different device type setting before it comes up for some OSs (redhat 4
>for instance). On other OSs I don't see the problem and I guess that
>this is because master becomes up only after it has slaves.

	If I'm reading the above correctly, then this type selection is
only needed for Red Hat 4 (or, really, versions of bonding prior to when
the bonding master started to set its carrier state based upon the state
of the slaves), correct?

	If that's the case, then is this patch is fixing a problem that
doesn't exist in the mainline?  If this isn't a problem with the current
driver (where "current" here seems to be bonding 3.0.3 and later, which
is about two years old), I don't see why it should go into the mainline.

	Am I misunderstanding something?

	-J

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

Re: [PATCH]: net/bonding: Enable to change device type before enslaving

[Moni Shoua]

Jay Vosburgh wrote:
> Moni Shoua <monis@voltaire.com> wrote:
>
>   
>>> 	Does this mean that the automatic selection on first enslavement
>>> is no longer needed, and all setting of the type for IB devices must
>>> occur prior to first enslavement?
>>>
>>> 	Or is this more of a special case for some devices, and the
>>> automatic selection is still correct for most cases?
>>>
>>>       
>> I think that the later is closer to the truth. 
>> The goal is to modify the bonding module to work with IPoIB slaves but
>> with as small as possible changes to what already exists.
>>
>> In details, the bonding net device device gets its type by
>> ether_setup(), which runs for all types of slaves and I don't see a way
>> to change that.  However, bonding master for IPoIB slaves requires a
>> different device type setting before it comes up for some OSs (redhat 4
>> for instance). On other OSs I don't see the problem and I guess that
>> this is because master becomes up only after it has slaves.
>>     
>
> 	If I'm reading the above correctly, then this type selection is
> only needed for Red Hat 4 (or, really, versions of bonding prior to when
> the bonding master started to set its carrier state based upon the state
> of the slaves), correct?
>
> 	If that's the case, then is this patch is fixing a problem that
> doesn't exist in the mainline?  If this isn't a problem with the current
> driver (where "current" here seems to be bonding 3.0.3 and later, which
> is about two years old), I don't see why it should go into the mainline.
>
> 	Am I misunderstanding something?
>
> 	-J
>
> ---
> 	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   
The problem was seen under  RedHat 4 with a bonding driver from 2.6.24 
with backports.
We deliver this module to customers who use IPoIB.
However, the bug is  not only for RedHat necessarily. Anywhere that will do
1. ifup bond0
2. Enslave ib0,ib1 to bond0
will end up the same problem described above.