[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CALzJLG9JNYP41UqagGoCsjB5JZb-TaDHVnSEHHtNL8WJmA5P0w@mail.gmail.com>
Date: Thu, 27 Jul 2017 16:33:23 +0300
From: Saeed Mahameed <saeedm@....mellanox.co.il>
To: Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
Cc: Saeed Mahameed <saeedm@...lanox.com>,
"David S. Miller" <davem@...emloft.net>,
Linux Netdev List <netdev@...r.kernel.org>,
Aviv Heller <avivh@...lanox.com>
Subject: Re: [net 02/14] net/mlx5: Consider tx_enabled in all modes on remap
On Thu, Jul 27, 2017 at 2:55 PM, Sergei Shtylyov
<sergei.shtylyov@...entembedded.com> wrote:
> Hello!
>
>
> On 07/27/2017 01:25 PM, Saeed Mahameed wrote:
>
>> From: Aviv Heller <avivh@...lanox.com>
>>
>> The tx_enabled lag event field is used to determine whether a slave is
>> active.
>> Current logic uses this value only if the mode is active-backup.
>>
>> However, LACP mode, although considered a load balancing mode, can mark
>> a slave as inactive in certain situations (e.g., LACP timeout).
>>
>> This fix takes the tx_enabled value into account when remapping, with
>> no respect to the LAG mode (this should not affect the behavior in XOR
>> mode, since in this mode both slaves are marked as active).
>>
>> Fixes: 7907f23adc18 (net/mlx5: Implement RoCE LAG feature)
>> Signed-off-by: Aviv Heller <avivh@...lanox.com>
>> Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>
>> ---
>> drivers/net/ethernet/mellanox/mlx5/core/lag.c | 26
>> +++++++++++---------------
>> 1 file changed, 11 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag.c
>> b/drivers/net/ethernet/mellanox/mlx5/core/lag.c
>> index a3a836bdcfd2..654b02e7d1aa 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/lag.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag.c
>> @@ -162,21 +162,17 @@ static bool mlx5_lag_is_bonded(struct mlx5_lag
>> *ldev)
>> static void mlx5_infer_tx_affinity_mapping(struct lag_tracker *tracker,
>> u8 *port1, u8 *port2)
>> {
>> - if (tracker->tx_type == NETDEV_LAG_TX_TYPE_ACTIVEBACKUP) {
>> - if (tracker->netdev_state[0].tx_enabled) {
>> - *port1 = 1;
>> - *port2 = 1;
>> - } else {
>> - *port1 = 2;
>> - *port2 = 2;
>> - }
>> - } else {
>> - *port1 = 1;
>> - *port2 = 2;
>> - if (!tracker->netdev_state[0].link_up)
>> - *port1 = 2;
>> - else if (!tracker->netdev_state[1].link_up)
>> - *port2 = 1;
>> + *port1 = 1;
>> + *port2 = 2;
>> + if (!tracker->netdev_state[0].tx_enabled ||
>> + !tracker->netdev_state[0].link_up) {
>> + *port1 = 2;
>> + return;
>> + }
>> +
>> + if (!tracker->netdev_state[1].tx_enabled ||
>> + !tracker->netdev_state[1].link_up) {
>> + *port2 = 1;
>> }
>
>
> {} not needed here.
Right, will fix and resubmit, Thanks Sergei.
>
> [...]
>
> MBR, Sergei
>
Powered by blists - more mailing lists