* [PATCH net-next v1] bonding: Fix another case of LACPDU not sent on slave
@ 2015-03-30 21:31 Mahesh Bandewar
2015-03-31 15:59 ` Andy Gospodarek
0 siblings, 1 reply; 3+ messages in thread
From: Mahesh Bandewar @ 2015-03-30 21:31 UTC (permalink / raw)
To: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
Nikolay Aleksandrov, David Miller
Cc: Mahesh Bandewar, Maciej Zenczykowski, netdev, Eric Dumazet
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@google.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
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net-next v1] bonding: Fix another case of LACPDU not sent on slave
2015-03-30 21:31 [PATCH net-next v1] bonding: Fix another case of LACPDU not sent on slave Mahesh Bandewar
@ 2015-03-31 15:59 ` Andy Gospodarek
2015-03-31 17:13 ` Mahesh Bandewar
0 siblings, 1 reply; 3+ messages in thread
From: Andy Gospodarek @ 2015-03-31 15:59 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
Nikolay Aleksandrov, David Miller, Maciej Zenczykowski, netdev,
Eric Dumazet
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@google.com>
This won't make the state machine start-up more quickly will it? Either
way...
Signed-off-by: Andy Gospodarek <gospo@cumulusnetworks.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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net-next v1] bonding: Fix another case of LACPDU not sent on slave
2015-03-31 15:59 ` Andy Gospodarek
@ 2015-03-31 17:13 ` Mahesh Bandewar
0 siblings, 0 replies; 3+ messages in thread
From: Mahesh Bandewar @ 2015-03-31 17:13 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico,
Nikolay Aleksandrov, David Miller, Maciej Zenczykowski, netdev,
Eric Dumazet
On Tue, Mar 31, 2015 at 8:59 AM, Andy Gospodarek
<gospo@cumulusnetworks.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@google.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@cumulusnetworks.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@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-03-31 17:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-30 21:31 [PATCH net-next v1] bonding: Fix another case of LACPDU not sent on slave Mahesh Bandewar
2015-03-31 15:59 ` Andy Gospodarek
2015-03-31 17:13 ` Mahesh Bandewar
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).