netdev - Re: [PATCH net] bonding: fix 802.3ad aggregator reselection

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:	Thu, 18 Aug 2016 12:28:07 +0300
From:	Veli-Matti Lintu <veli-matti.lintu@...nsys.fi>
To:	Jay Vosburgh <jay.vosburgh@...onical.com>
Cc:	netdev <netdev@...r.kernel.org>,
	Veaceslav Falico <vfalico@...il.com>,
	Andy Gospodarek <gospo@...ulusnetworks.com>,
	zhuyj <zyjzyj2000@...il.com>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net] bonding: fix 802.3ad aggregator reselection

2016-07-08 15:22 GMT+03:00 Veli-Matti Lintu <veli-matti.lintu@...nsys.fi>:
> 2016-07-08 4:48 GMT+03:00 Jay Vosburgh <jay.vosburgh@...onical.com>:

>> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>> index edc70ffad660..2da5be91226e 100644
>> --- a/drivers/net/bonding/bond_3ad.c
>> +++ b/drivers/net/bonding/bond_3ad.c
>> @@ -1713,18 +1713,12 @@ static void ad_agg_selection_logic(struct aggregator *agg,
>>                 *update_slave_arr = true;
>>         }
>>
>> -       /* if the selected aggregator is of join individuals
>> -        * (partner_system is NULL), enable their ports
>> -        */
>>         active = __get_active_agg(origin);
>>
>>         if (active) {
>> -               if (!__agg_has_partner(active)) {
>> -                       for (port = active->lag_ports; port;
>> -                            port = port->next_port_in_aggregator) {
>> -                               __enable_port(port);
>> -                       }
>> -               }
>> +               for (port = active->lag_ports; port;
>> +                    port = port->next_port_in_aggregator)
>> +                       __enable_port(port);
>>         }
>>
>>         rcu_read_unlock();
>>
>>
>>         Rather than adding a new loop as your original patch did, this
>> one repurposes the existing loop for individual links and does the
>> enable for all cases.
>>
>>         I think this may still need a check for the port state in there,
>> as ports in the aggregator may be link up but still not suitable for
>> TX/RX if the LACP negotiation doesn't put the port into COLL_DIST state.
>>
>>         Enabling a port that is already enabled is harmless, and if the
>> port is link down, the enable will do nothing, so I think this should
>> resolve things.
>
> I can test any patches with additional checks, but this change seems
> to fix it for us.

Do you think something else is needed is addition to this? This has
been working on my servers now for some time without problems. I
tested this also against mainline 4.8-rc1 that needed it for bonding
to work properly with the previous test case.

Veli-Matti