* [PATCH] bonding: check for assigned mac before adopting the slaves mac address
@ 2010-11-24 23:12 David Strand
2010-11-24 23:33 ` Jay Vosburgh
0 siblings, 1 reply; 10+ messages in thread
From: David Strand @ 2010-11-24 23:12 UTC (permalink / raw)
To: netdev; +Cc: Jay Vosburgh, linux-kernel
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 dpstrand@gmail.com
---
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);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bonding: check for assigned mac before adopting the slaves mac address
2010-11-24 23:12 [PATCH] bonding: check for assigned mac before adopting the slaves mac address David Strand
@ 2010-11-24 23:33 ` Jay Vosburgh
2010-11-25 0:44 ` Laurent Chavey
2010-11-25 1:45 ` David Strand
0 siblings, 2 replies; 10+ messages in thread
From: Jay Vosburgh @ 2010-11-24 23:33 UTC (permalink / raw)
To: David Strand; +Cc: netdev, linux-kernel
David Strand <dpstrand@gmail.com> 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 dpstrand@gmail.com
>---
>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, fubar@us.ibm.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bonding: check for assigned mac before adopting the slaves mac address
2010-11-24 23:33 ` Jay Vosburgh
@ 2010-11-25 0:44 ` Laurent Chavey
2010-11-25 1:45 ` David Strand
1 sibling, 0 replies; 10+ messages in thread
From: Laurent Chavey @ 2010-11-25 0:44 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: David Strand, netdev, linux-kernel
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 <fubar@us.ibm.com> wrote:
> David Strand <dpstrand@gmail.com> 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 dpstrand@gmail.com
>>---
>>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, 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
>
--
--------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bonding: check for assigned mac before adopting the slaves mac address
2010-11-24 23:33 ` Jay Vosburgh
2010-11-25 0:44 ` Laurent Chavey
@ 2010-11-25 1:45 ` David Strand
2010-11-26 3:26 ` Jay Vosburgh
1 sibling, 1 reply; 10+ messages in thread
From: David Strand @ 2010-11-25 1:45 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev, linux-kernel
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 <fubar@us.ibm.com> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bonding: check for assigned mac before adopting the slaves mac address
2010-11-25 1:45 ` David Strand
@ 2010-11-26 3:26 ` Jay Vosburgh
2010-12-01 18:25 ` David Strand
0 siblings, 1 reply; 10+ messages in thread
From: Jay Vosburgh @ 2010-11-26 3:26 UTC (permalink / raw)
To: David Strand; +Cc: netdev, linux-kernel
David Strand <dpstrand@gmail.com> 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, fubar@us.ibm.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bonding: check for assigned mac before adopting the slaves mac address
2010-11-26 3:26 ` Jay Vosburgh
@ 2010-12-01 18:25 ` David Strand
2010-12-01 18:45 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: David Strand @ 2010-12-01 18:25 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev, linux-kernel
On Thu, Nov 25, 2010 at 7:26 PM, Jay Vosburgh <fubar@us.ibm.com> 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, fubar@us.ibm.com
>
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);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bonding: check for assigned mac before adopting the slaves mac address
2010-12-01 18:25 ` David Strand
@ 2010-12-01 18:45 ` David Miller
0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2010-12-01 18:45 UTC (permalink / raw)
To: dpstrand; +Cc: fubar, netdev, linux-kernel
From: David Strand <dpstrand@gmail.com>
Date: Wed, 1 Dec 2010 10:25:19 -0800
> On Thu, Nov 25, 2010 at 7:26 PM, Jay Vosburgh <fubar@us.ibm.com> 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, fubar@us.ibm.com
>>
>
> 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] bonding: check for assigned mac before adopting the slaves mac address
@ 2010-12-01 19:15 David Strand
2010-12-01 19:21 ` Jay Vosburgh
0 siblings, 1 reply; 10+ messages in thread
From: David Strand @ 2010-12-01 19:15 UTC (permalink / raw)
To: netdev; +Cc: Jay Vosburgh, linux-kernel
Restore the check for an unassigned mac address before adopting the
first slaves as it's own. The change in behavior was introduced with
the following patch:
http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.36.y.git;a=commit;h=c20811a79e671a6a1fe86a8c1afe04aca8a7f085
Signed-off-by: David Strand <dpstrand@gmail.com>
---
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);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bonding: check for assigned mac before adopting the slaves mac address
2010-12-01 19:15 David Strand
@ 2010-12-01 19:21 ` Jay Vosburgh
2010-12-01 19:43 ` David Miller
0 siblings, 1 reply; 10+ messages in thread
From: Jay Vosburgh @ 2010-12-01 19:21 UTC (permalink / raw)
To: David Strand; +Cc: netdev, linux-kernel, David S. Miller
From: David Strand <dpstrand@gmail.com>
Restore the check for an unassigned mac address before adopting the
first slaves as it's own. The change in behavior was introduced by:
commit c20811a79e671a6a1fe86a8c1afe04aca8a7f085
Author: Jiri Pirko <jpirko@redhat.com>
bonding: move dev_addr cpy to bond_enslave
Signed-off-by: David Strand <dpstrand@gmail.com>
Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
---
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);
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] bonding: check for assigned mac before adopting the slaves mac address
2010-12-01 19:21 ` Jay Vosburgh
@ 2010-12-01 19:43 ` David Miller
0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2010-12-01 19:43 UTC (permalink / raw)
To: fubar; +Cc: dpstrand, netdev, linux-kernel
From: Jay Vosburgh <fubar@us.ibm.com>
Date: Wed, 01 Dec 2010 11:21:08 -0800
>
> From: David Strand <dpstrand@gmail.com>
>
> Restore the check for an unassigned mac address before adopting the
> first slaves as it's own. The change in behavior was introduced by:
>
> commit c20811a79e671a6a1fe86a8c1afe04aca8a7f085
> Author: Jiri Pirko <jpirko@redhat.com>
>
> bonding: move dev_addr cpy to bond_enslave
>
>
> Signed-off-by: David Strand <dpstrand@gmail.com>
> Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-12-01 19:42 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-24 23:12 [PATCH] bonding: check for assigned mac before adopting the slaves mac address David Strand
2010-11-24 23:33 ` Jay Vosburgh
2010-11-25 0:44 ` Laurent Chavey
2010-11-25 1:45 ` David Strand
2010-11-26 3:26 ` Jay Vosburgh
2010-12-01 18:25 ` David Strand
2010-12-01 18:45 ` David Miller
-- strict thread matches above, loose matches on Subject: below --
2010-12-01 19:15 David Strand
2010-12-01 19:21 ` Jay Vosburgh
2010-12-01 19:43 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).