All of lore.kernel.org
 help / color / mirror / Atom feed
From: "zheng.li" <zheng.x.li@oracle.com>
To: Jay Vosburgh <fubar@us.ibm.com>
Cc: netdev@vger.kernel.org, andy@greyhouse.net,
	linux-kernel@vger.kernel.org, davem@davemloft.net,
	joe.jin@oracle.com
Subject: Re: [PATCH] bonding: Inactive slaves should keep inactive flag's value to 1 in tlb and alb mode.
Date: Fri, 28 Mar 2014 16:39:24 +0800	[thread overview]
Message-ID: <5335353C.1050409@oracle.com> (raw)
In-Reply-To: <9725.1395940983@death.nxdomain>

于 2014年03月28日 01:23, Jay Vosburgh 写道:
> Zheng Li <zheng.x.li@oracle.com> wrote:
> 
>> In bond mode tlb and alb, inactive slaves should keep inactive flag to
>> 1 to refuse to receive broadcast packets. Now, active slave send broadcast packets
>> (for example ARP requests) which will arrive inactive slaves on same host from switch,
>> but inactive slave's inactive flag is zero that cause bridge receive the broadcast
>> packets to produce a wrong entry in forward table. Typical situation is domu send some
>> ARP request which go out from dom0 bond's active slave, then the ARP broadcast request
>> packets go back to inactive slave from switch, because the inactive slave's inactive
>> flag is zero, kernel will receive the packets and pass them to bridge, that cause dom0's
>> bridge map domu's MAC address to port of bond, bridge should map domu's MAC to port of vif.
> 
> 	It's probably worth noting that this effect is something that
> happens after the bonding master device is opened with slaves, i.e.,
> it's got a bunch of slaves, and is then set administratively down for
> whatever reason, and is now being set back up, and needs to set the
> active or inactive state of all the slaves.

I think bond_miimon_commit() funciotn will process this situation,
case BOND_LINK_UP:
   if (!bond->curr_active_slave ||
			    (slave == bond->primary_slave))
				goto do_failover;

do_failover:
		bond_select_active_slave(bond);

> 
> 	It'd also be a little easier to read if it was formatted for 80
> columns.
> 
>> Signed-off-by: Zheng Li <zheng.x.li@oracle.com>
>> ---
>> drivers/net/bonding/bond_main.c |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index e5628fc..8761df6 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -3062,7 +3062,7 @@ static int bond_open(struct net_device *bond_dev)
>> 				&& (slave != bond->curr_active_slave)) {
>> 				bond_set_slave_inactive_flags(slave,
>> 							      BOND_SLAVE_NOTIFY_NOW);
>> -			} else {
>> +			} else if (!bond_is_lb(bond)) {
>> 				bond_set_slave_active_flags(slave,
>> 							    BOND_SLAVE_NOTIFY_NOW);
> 
> 	This patch doesn't do anything for the modes that are
> bond_is_lb, i.e., the balance-tlb and -alb modes.  I believe those two
> should be set similarly to active-backup: the curr_active_slave is
> active, other slaves are inactive.  The "inactive" setting for alb is
> special, and means to not pass broadcast or multicast, but let unicast
> through.

>From dmesg and the source code in kernel, every slave of bond default been set to inactive 1 when slave be added in the bond, and then "Link UP" will comes that let bond choose one as cur_active_salve and change it's inactive value to 0 and other slaves will keep their value of inactive to 1, so it doesn't need set inactive again in bong_open by default all salves inactive is 1 when bond add salves.

You can add printk in kernel, will find the flow, my print show the flow:
1 step ) bonding: bond1 is being created...

2 step ) bonding: bond1: Adding slave eth2. // in this stage, every slave will be set to inactive 1;
          zheng debug set inactive eth2 bond bond1 
          zheng debug set inactive flag eth2 

   bonding: bond1: Adding slave eth3.
            zheng2 set slave inactive eth3
            zheng debug set inactive eth3 bond bond1 
   
3 step ) zheng bonding open 
   //in bond_open function, we just need keep the inactive to 1 for alb and tlb, .i.e don't 
   clear the inactive is ok, my patch did that.

4 setp)  "LINK UP" will make one slave to cur_active_slave and change it's inactive to 0, other slaves will keep their original inactive value to 1;

e1000e: eth2 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: None

bonding: bond1: link status up for interface eth2, enabling it in 0 ms.

bonding: bond1: link status definitely up for interface eth2, 1000 Mbps full duplex.

bonding: bond1: making interface eth2 the new active one.

device eth2 entered promiscuous mode

zheng1 set new slave active eth2

zheng debug set active flag eth2 

bonding: bond1: first active interface up!

Thanks,
Zheng Li


> 	-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
> 
> 



WARNING: multiple messages have this Message-ID (diff)
From: "zheng.li" <zheng.x.li@oracle.com>
To: Jay Vosburgh <fubar@us.ibm.com>
Cc: netdev@vger.kernel.org, andy@greyhouse.net,
	linux-kernel@vger.kernel.org, davem@davemloft.net,
	joe.jin@oracle.com
Subject: Re: [PATCH] bonding: Inactive slaves should keep inactive flag's value to 1 in tlb and alb mode.
Date: Fri, 28 Mar 2014 16:39:24 +0800	[thread overview]
Message-ID: <5335353C.1050409@oracle.com> (raw)
In-Reply-To: <9725.1395940983@death.nxdomain>

于 2014年03月28日 01:23, Jay Vosburgh 写道:
> Zheng Li <zheng.x.li@oracle.com> wrote:
> 
>> In bond mode tlb and alb, inactive slaves should keep inactive flag to
>> 1 to refuse to receive broadcast packets. Now, active slave send broadcast packets
>> (for example ARP requests) which will arrive inactive slaves on same host from switch,
>> but inactive slave's inactive flag is zero that cause bridge receive the broadcast
>> packets to produce a wrong entry in forward table. Typical situation is domu send some
>> ARP request which go out from dom0 bond's active slave, then the ARP broadcast request
>> packets go back to inactive slave from switch, because the inactive slave's inactive
>> flag is zero, kernel will receive the packets and pass them to bridge, that cause dom0's
>> bridge map domu's MAC address to port of bond, bridge should map domu's MAC to port of vif.
> 
> 	It's probably worth noting that this effect is something that
> happens after the bonding master device is opened with slaves, i.e.,
> it's got a bunch of slaves, and is then set administratively down for
> whatever reason, and is now being set back up, and needs to set the
> active or inactive state of all the slaves.

I think bond_miimon_commit() funciotn will process this situation,
case BOND_LINK_UP:
   if (!bond->curr_active_slave ||
			    (slave == bond->primary_slave))
				goto do_failover;

do_failover:
		bond_select_active_slave(bond);

> 
> 	It'd also be a little easier to read if it was formatted for 80
> columns.
> 
>> Signed-off-by: Zheng Li <zheng.x.li@oracle.com>
>> ---
>> drivers/net/bonding/bond_main.c |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index e5628fc..8761df6 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -3062,7 +3062,7 @@ static int bond_open(struct net_device *bond_dev)
>> 				&& (slave != bond->curr_active_slave)) {
>> 				bond_set_slave_inactive_flags(slave,
>> 							      BOND_SLAVE_NOTIFY_NOW);
>> -			} else {
>> +			} else if (!bond_is_lb(bond)) {
>> 				bond_set_slave_active_flags(slave,
>> 							    BOND_SLAVE_NOTIFY_NOW);
> 
> 	This patch doesn't do anything for the modes that are
> bond_is_lb, i.e., the balance-tlb and -alb modes.  I believe those two
> should be set similarly to active-backup: the curr_active_slave is
> active, other slaves are inactive.  The "inactive" setting for alb is
> special, and means to not pass broadcast or multicast, but let unicast
> through.

From dmesg and the source code in kernel, every slave of bond default been set to inactive 1 when slave be added in the bond, and then "Link UP" will comes that let bond choose one as cur_active_salve and change it's inactive value to 0 and other slaves will keep their value of inactive to 1, so it doesn't need set inactive again in bong_open by default all salves inactive is 1 when bond add salves.

You can add printk in kernel, will find the flow, my print show the flow:
1 step ) bonding: bond1 is being created...

2 step ) bonding: bond1: Adding slave eth2. // in this stage, every slave will be set to inactive 1;
          zheng debug set inactive eth2 bond bond1 
          zheng debug set inactive flag eth2 

   bonding: bond1: Adding slave eth3.
            zheng2 set slave inactive eth3
            zheng debug set inactive eth3 bond bond1 
   
3 step ) zheng bonding open 
   //in bond_open function, we just need keep the inactive to 1 for alb and tlb, .i.e don't 
   clear the inactive is ok, my patch did that.

4 setp)  "LINK UP" will make one slave to cur_active_slave and change it's inactive to 0, other slaves will keep their original inactive value to 1;

e1000e: eth2 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: None

bonding: bond1: link status up for interface eth2, enabling it in 0 ms.

bonding: bond1: link status definitely up for interface eth2, 1000 Mbps full duplex.

bonding: bond1: making interface eth2 the new active one.

device eth2 entered promiscuous mode

zheng1 set new slave active eth2

zheng debug set active flag eth2 

bonding: bond1: first active interface up!

Thanks,
Zheng Li


> 	-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
> 
> 

  reply	other threads:[~2014-03-28  8:25 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-25  3:00 [PATCH] bonding: Inactive slaves should keep inactive flag's value to 1 in tlb and alb mode Zheng Li
2014-03-25  3:42 ` Ding Tianhong
2014-03-25  8:36   ` zheng.li
2014-03-26  0:53     ` Ding Tianhong
2014-03-27  2:26       ` zheng.li
2014-03-27 17:23 ` Jay Vosburgh
2014-03-28  8:39   ` zheng.li [this message]
2014-03-28  8:39     ` zheng.li
2014-04-02  7:35 ` Pavel Machek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5335353C.1050409@oracle.com \
    --to=zheng.x.li@oracle.com \
    --cc=andy@greyhouse.net \
    --cc=davem@davemloft.net \
    --cc=fubar@us.ibm.com \
    --cc=joe.jin@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.