login
Header Space

 
 

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

Previous thread: [PATCH] net/bonding: Send more than one gratuitous ARP when slave takes over by Moni Shoua on Thursday, April 10, 2008 - 11:07 am. (7 messages)

Next thread: [PATCH 0/14 (3 subsets)] Make tuns and vlans devices work per-net. by Pavel Emelyanov on Thursday, April 10, 2008 - 10:57 am. (35 messages)
To: Jay Vosburgh <fubar@...>
Cc: netdev <netdev@...>, Olga Stern <olgas@...>, Or Gerlitz <ogerlitz@...>
Date: Thursday, April 10, 2008 - 11:09 am

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 &lt;monis@voltaire.com&gt;
---
 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-&gt;dev-&gt;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", &amp;new_value) != 1) {
+		printk(KERN_ERR DRV_NAME
+		       ": %s: no device type value specified.\n",
+		       bond-&gt;dev-&gt;name);
+		ret = -EINVAL;
+		goto out;
+	}
+	if (bond-&gt;slave_cnt &gt; ...
To: <monis@...>
Cc: netdev <netdev@...>, Olga Stern <olgas@...>, Or Gerlitz <ogerlitz@...>
Date: Thursday, April 10, 2008 - 4:48 pm

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
--
To: Jay Vosburgh <fubar@...>
Cc: netdev <netdev@...>, Olga Stern <olgas@...>, Or Gerlitz <ogerlitz@...>
Date: Sunday, April 13, 2008 - 10:09 am

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.



--
To: Moni Shoua <monis@...>
Cc: netdev <netdev@...>, Olga Stern <olgas@...>, Or Gerlitz <ogerlitz@...>
Date: Wednesday, April 16, 2008 - 3:27 pm

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: Jay Vosburgh <fubar@...>
Cc: netdev <netdev@...>, Olga Stern <olgas@...>, Or Gerlitz <ogerlitz@...>
Date: Thursday, April 17, 2008 - 2:29 am

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.

--
Previous thread: [PATCH] net/bonding: Send more than one gratuitous ARP when slave takes over by Moni Shoua on Thursday, April 10, 2008 - 11:07 am. (7 messages)

Next thread: [PATCH 0/14 (3 subsets)] Make tuns and vlans devices work per-net. by Pavel Emelyanov on Thursday, April 10, 2008 - 10:57 am. (35 messages)
speck-geostationary