Restore the check for a missing mac address before adopting the first
slaves as it's own. This regression was introduced in:
http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.36.y.git;a=commit;h=c20811a79e671a6a1fe86a8c1afe04aca8a7f085
Signed-off-by: David Strand [email protected]
---
diff -uprN a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
--- a/drivers/net/bonding/bond_main.c 2010-11-24 11:36:58.125640000 -0800
+++ b/drivers/net/bonding/bond_main.c 2010-11-24 11:40:58.175640000 -0800
@@ -1577,8 +1577,9 @@ int bond_enslave(struct net_device *bond
/* If this is the first slave, then we need to set the master's hardware
* address to be the same as the slave's. */
if (bond->slave_cnt == 0)
- memcpy(bond->dev->dev_addr, slave_dev->dev_addr,
- slave_dev->addr_len);
+ if (is_zero_ether_addr(bond->dev->dev_addr))
+ memcpy(bond->dev->dev_addr, slave_dev->dev_addr,
+ slave_dev->addr_len);
new_slave = kzalloc(sizeof(struct slave), GFP_KERNEL);
David Strand <[email protected]> wrote:
>Restore the check for a missing mac address before adopting the first
>slaves as it's own. This regression was introduced in:
>http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.36.y.git;a=commit;h=c20811a79e671a6a1fe86a8c1afe04aca8a7f085
How exactly is this a regression? The above referenced patch
changes the method used to decide if the bonding master needs to have
it's MAC address set. The original way was "bonding master's MAC is
zero," after the above, it's "adding first slave."
Do you have some use case that manually sets the master's MAC
address prior to adding any slaves?
-J
>Signed-off-by: David Strand [email protected]
>---
>diff -uprN a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>--- a/drivers/net/bonding/bond_main.c 2010-11-24 11:36:58.125640000 -0800
>+++ b/drivers/net/bonding/bond_main.c 2010-11-24 11:40:58.175640000 -0800
>@@ -1577,8 +1577,9 @@ int bond_enslave(struct net_device *bond
> /* If this is the first slave, then we need to set the master's hardware
> * address to be the same as the slave's. */
> if (bond->slave_cnt == 0)
>- memcpy(bond->dev->dev_addr, slave_dev->dev_addr,
>- slave_dev->addr_len);
>+ if (is_zero_ether_addr(bond->dev->dev_addr))
>+ memcpy(bond->dev->dev_addr, slave_dev->dev_addr,
>+ slave_dev->addr_len);
>
>
> new_slave = kzalloc(sizeof(struct slave), GFP_KERNEL);
---
-Jay Vosburgh, IBM Linux Technology Center, [email protected]
based on the new code, the behavior of ifenslave works the same.
>- if (bond->slave_cnt == 0)
>+ if (is_zero_ether_addr(bond->dev->dev_addr))
>- memcpy(bond->dev->dev_addr, slave_dev->dev_addr,
>- slave_dev->addr_len);
> + bond_sethwaddr(bond, slave_dev);
On Wed, Nov 24, 2010 at 3:33 PM, Jay Vosburgh <[email protected]> wrote:
> David Strand <[email protected]> wrote:
>
>>Restore the check for a missing mac address before adopting the first
>>slaves as it's own. This regression was introduced in:
>>http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.36.y.git;a=commit;h=c20811a79e671a6a1fe86a8c1afe04aca8a7f085
>
> ? ? ? ?How exactly is this a regression? ?The above referenced patch
> changes the method used to decide if the bonding master needs to have
> it's MAC address set. ?The original way was "bonding master's MAC is
> zero," after the above, it's "adding first slave."
>
> ? ? ? ?Do you have some use case that manually sets the master's MAC
> address prior to adding any slaves?
>
> ? ? ? ?-J
>
>>Signed-off-by: David Strand [email protected]
>>---
>>diff -uprN a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>>--- a/drivers/net/bonding/bond_main.c ?2010-11-24 11:36:58.125640000 -0800
>>+++ b/drivers/net/bonding/bond_main.c ?2010-11-24 11:40:58.175640000 -0800
>>@@ -1577,8 +1577,9 @@ int bond_enslave(struct net_device *bond
>> ? ? ? /* If this is the first slave, then we need to set the master's hardware
>> ? ? ? ?* address to be the same as the slave's. */
>> ? ? ? if (bond->slave_cnt == 0)
>>- ? ? ? ? ? ? ?memcpy(bond->dev->dev_addr, slave_dev->dev_addr,
>>- ? ? ? ? ? ? ? ? ? ? slave_dev->addr_len);
>>+ ? ? ? ? ? ? ?if (is_zero_ether_addr(bond->dev->dev_addr))
>>+ ? ? ? ? ? ? ? ? ? ? ?memcpy(bond->dev->dev_addr, slave_dev->dev_addr,
>>+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? slave_dev->addr_len);
>>
>>
>> ? ? ? new_slave = kzalloc(sizeof(struct slave), GFP_KERNEL);
>
> ---
> ? ? ? ?-Jay Vosburgh, IBM Linux Technology Center, [email protected]
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
--
--------------------------------------------------------------------------------
We have a use case where we assign a mac to the bond device, because
the slave device configuration may change periodically. With older
kernels, it honored the assigned mac and everything was fine, with
2.6.36 it now uses the mac of whatever slave device is first instead
of our assigned one.
ifenslave code and documentation appears to still support the old way,
where a bond assigned mac will reign supreme, so this patch restores
that behavior.
On Wed, Nov 24, 2010 at 3:33 PM, Jay Vosburgh <[email protected]> wrote:
> How exactly is this a regression? The above referenced patch
> changes the method used to decide if the bonding master needs to have
> it's MAC address set. The original way was "bonding master's MAC is
> zero," after the above, it's "adding first slave."
>
> Do you have some use case that manually sets the master's MAC
> address prior to adding any slaves?
>
> -J
David Strand <[email protected]> wrote:
>We have a use case where we assign a mac to the bond device, because
>the slave device configuration may change periodically. With older
>kernels, it honored the assigned mac and everything was fine, with
>2.6.36 it now uses the mac of whatever slave device is first instead
>of our assigned one.
>
>ifenslave code and documentation appears to still support the old way,
>where a bond assigned mac will reign supreme, so this patch restores
>that behavior.
Ok, fair enough. If we want to get back to the original
behavior, however, your patch should only test for zero MAC address
instead of testing for zero MAC address in addition to first slave.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, [email protected]
On Thu, Nov 25, 2010 at 7:26 PM, Jay Vosburgh <[email protected]> wrote:
>
> Ok, fair enough. If we want to get back to the original
> behavior, however, your patch should only test for zero MAC address
> instead of testing for zero MAC address in addition to first slave.
>
> -J
>
> ---
> -Jay Vosburgh, IBM Linux Technology Center, [email protected]
>
Understood, that makes sense. The updated patch is below.
---
diff -uprN a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
--- a/drivers/net/bonding/bond_main.c 2010-11-24 11:36:58.125640000 -0800
+++ b/drivers/net/bonding/bond_main.c 2010-12-01 10:12:33.728640001 -0800
@@ -1576,7 +1576,7 @@ int bond_enslave(struct net_device *bond
/* If this is the first slave, then we need to set the master's hardware
* address to be the same as the slave's. */
- if (bond->slave_cnt == 0)
+ if (is_zero_ether_addr(bond->dev->dev_addr))
memcpy(bond->dev->dev_addr, slave_dev->dev_addr,
slave_dev->addr_len);
From: David Strand <[email protected]>
Date: Wed, 1 Dec 2010 10:25:19 -0800
> On Thu, Nov 25, 2010 at 7:26 PM, Jay Vosburgh <[email protected]> wrote:
>>
>> ? ? ? ?Ok, fair enough. ?If we want to get back to the original
>> behavior, however, your patch should only test for zero MAC address
>> instead of testing for zero MAC address in addition to first slave.
>>
>> ? ? ? ?-J
>>
>> ---
>> ? ? ? ?-Jay Vosburgh, IBM Linux Technology Center, [email protected]
>>
>
> Understood, that makes sense. The updated patch is below.
You need to provide a fresh posting of your patch, with a full
commit messag and proper Subject: line, as well as a proper
"Signed-off-by: " tag.
See Documentation/SubmittingPatches