2014-04-01 07:48:35

by zheng.li

Subject: [PATCH v2] bonding: Inactive slaves should keep inactive flag's value

bond_open is not setting the inactive flag correctly for some modes (alb and
tlb), resulting in error behavior if the bond has been administratively set
down and then back up. This effect should not occur when slaves are added while
the bond is up; it's something that only happens after a down/up bounce of the
bond.

For example, in bond tlb or alb mode, 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.

Signed-off-by: Zheng Li <[email protected]>
Signed-off-by: Jay Vosburgh <[email protected]>
---
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..f97d72e 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3058,7 +3058,7 @@ static int bond_open(struct net_device *bond_dev)
if (bond_has_slaves(bond)) {
read_lock(&bond->curr_slave_lock);
bond_for_each_slave(bond, slave, iter) {
- if ((bond->params.mode == BOND_MODE_ACTIVEBACKUP)
+ if ((bond->params.mode == BOND_MODE_ACTIVEBACKUP || bond_is_lb(bond))
&& (slave != bond->curr_active_slave)) {
bond_set_slave_inactive_flags(slave,
BOND_SLAVE_NOTIFY_NOW);
--
1.7.6.5

2014-04-01 11:35:14

by Nikolay Aleksandrov

Subject: Re: [PATCH v2] bonding: Inactive slaves should keep inactive flag's value

On 04/01/2014 10:02 AM, Zheng Li wrote:
> bond_open is not setting the inactive flag correctly for some modes (alb and
> tlb), resulting in error behavior if the bond has been administratively set
> down and then back up. This effect should not occur when slaves are added while
> the bond is up; it's something that only happens after a down/up bounce of the
> bond.
>
> For example, in bond tlb or alb mode, 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.
>
> Signed-off-by: Zheng Li <[email protected]>
> Signed-off-by: Jay Vosburgh <[email protected]>
> ---
> 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..f97d72e 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -3058,7 +3058,7 @@ static int bond_open(struct net_device *bond_dev)
> if (bond_has_slaves(bond)) {
> read_lock(&bond->curr_slave_lock);
> bond_for_each_slave(bond, slave, iter) {
> - if ((bond->params.mode == BOND_MODE_ACTIVEBACKUP)
> + if ((bond->params.mode == BOND_MODE_ACTIVEBACKUP || bond_is_lb(bond))
IIRC, this check is identical to USES_PRIMARY().

> && (slave != bond->curr_active_slave)) {
> bond_set_slave_inactive_flags(slave,
> BOND_SLAVE_NOTIFY_NOW);
>

2014-04-01 20:12:24

by David Miller

Subject: Re: [PATCH v2] bonding: Inactive slaves should keep inactive flag's value

From: Zheng Li <[email protected]>
Date: Tue, 1 Apr 2014 16:02:07 +0800

> - if ((bond->params.mode == BOND_MODE_ACTIVEBACKUP)
> + if ((bond->params.mode == BOND_MODE_ACTIVEBACKUP || bond_is_lb(bond))

As has been suggested, please use USES_PRIMARY() for this entire
logical test.