netdev - Re: [PATCH net-next v1] bonding: Fix another case of LACPDU not sent on slave

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 31 Mar 2015 10:13:09 -0700
From:	Mahesh Bandewar <maheshb@...gle.com>
To:	Andy Gospodarek <gospo@...ulusnetworks.com>
Cc:	Jay Vosburgh <j.vosburgh@...il.com>,
	Andy Gospodarek <andy@...yhouse.net>,
	Veaceslav Falico <vfalico@...il.com>,
	Nikolay Aleksandrov <nikolay@...hat.com>,
	David Miller <davem@...emloft.net>,
	Maciej Zenczykowski <maze@...gle.com>,
	netdev <netdev@...r.kernel.org>,
	Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH net-next v1] bonding: Fix another case of LACPDU not sent
 on slave

On Tue, Mar 31, 2015 at 8:59 AM, Andy Gospodarek
<gospo@...ulusnetworks.com> wrote:
> On Mon, Mar 30, 2015 at 02:31:16PM -0700, Mahesh Bandewar wrote:
>> When mii-mon discovers that the link is up, it will call
>> bond_3ad_handle_link_change() but we forget to add the LACP_ENABLED
>> flag when we discover the speed and duplex for the slave link are
>> normal.
>>
>> Change-Id: Ie8b268ecfeea0f99bf9fdcd72706c0653f9d9e49
>> Signed-off-by: Mahesh Bandewar <maheshb@...gle.com>
>
> This won't make the state machine start-up more quickly will it?  Either
> way...
>
I'm seeing some occasions where LACP state for a port is in weird
state. The port has correct speed and duplex but there are no LACPDUs
are exchanged with partner. Seems like there is some sort of race
condition in the code and trying to hunt that down.

This definitely looks like a place where it would happen since miimon
will detect and update the speed and duplex for the port but lack of
ENABLE_LACP flag will prevent state-machine from sending LACPDU. This
is especially a big problem when the partner is in passive mode! This
takes port out of the active aggregation group so unless there is
another link-event on this port, it will not join the aggregator
again.

So the idea behind this patch is not to expedite the state-machine run
but to make it correctly.

> Signed-off-by: Andy Gospodarek <gospo@...ulusnetworks.com>
>
>
>> ---
>>  drivers/net/bonding/bond_3ad.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>> index 4309b5a76708..30e8d118664b 100644
>> --- a/drivers/net/bonding/bond_3ad.c
>> +++ b/drivers/net/bonding/bond_3ad.c
>> @@ -2436,12 +2436,15 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
>>               port->actor_admin_port_key &= ~AD_SPEED_KEY_MASKS;
>>               port->actor_oper_port_key = port->actor_admin_port_key |=
>>                       (__get_link_speed(port) << 1);
>> +             if (port->actor_oper_port_key & AD_DUPLEX_KEY_MASKS)
>> +                     port->sm_vars |= AD_PORT_LACP_ENABLED;
>>       } else {
>>               /* link has failed */
>>               port->is_enabled = false;
>>               port->actor_admin_port_key &= ~AD_DUPLEX_KEY_MASKS;
>>               port->actor_oper_port_key = (port->actor_admin_port_key &=
>>                                            ~AD_SPEED_KEY_MASKS);
>> +             port->sm_vars &= ~AD_PORT_LACP_ENABLED;
>>       }
>>       netdev_dbg(slave->bond->dev, "Port %d changed link status to %s\n",
>>                  port->actor_port_number,
>> --
>> 2.2.0.rc0.207.ga3a616c
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@...r.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html