Re: [dpdk-dev] [PATCH] net/mlx5: fix eagain on admin down

[Mike Manning <mmanning@vyatta.att-mail.com>]

Hi Slava,
Thanks, you are right - I retract my patch, as this was fixed in v19.08
by 6fd05da9efbd ("net/mlx5: fix link speed info when link is down") as
per the line you indicate.

I should explain that we are using Debian 10 which uses v18.11, here we
had a painful issue with the combination of this -EAGAIN and use of
netlink to obtain the ifindex resulting in concurrent use of the netlink
socket causing the calling thread to block indefinitely.

Regards
Mike

On 13/12/2019 14:43, Slava Ovsiienko wrote:
> Hi, Mike     
>
> In the mlx5_link_update_unlocked_gs() the dev_link.link_speed is set like this:
>
> dev_link.link_speed = (ecmd->speed == UINT32_MAX) ? ETH_SPEED_NUM_NONE :  ecmd->speed;
>
> So, dev_link.link_speed can't be assigned with -1. Do I misunderstand you commit message? 
>
> With best regards, Slava
>
>> -----Original Message-----
>> From: Thomas Monjalon <thomas@monjalon.net>
>> Sent: Friday, December 13, 2019 0:31
>> To: dev@dpdk.org
>> Cc: Mike Manning <mmanning@vyatta.att-mail.com>; Nélio Laranjeiro
>> <nelio.laranjeiro@6wind.com>; Matan Azrad <matan@mellanox.com>; Slava
>> Ovsiienko <viacheslavo@mellanox.com>; Shahaf Shuler
>> <shahafs@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>
>> Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix eagain on admin down
>>
>> +Cc maintainers
>>
>> 09/12/2019 19:23, Mike Manning:
>>> The check in mlx5_link_update_unlocked_gs() returns -EAGAIN if link
>>> status does not correspond to link speed. If status is DOWN, the speed
>>> is expected to be ETH_SPEED_NUM_NONE (0). But as the link speed is -1
>>> on admin down, modify the check to account for this.
>>>
>>> Fixes: cfee94752b8f ("net/mlx5: fix link status to use wait to
>>> complete")
>>> Cc: Nélio Laranjeiro <nelio.laranjeiro@6wind.com>
>>>
>>> Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
>>> ---
>>>  drivers/net/mlx5/mlx5_ethdev.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/mlx5/mlx5_ethdev.c
>>> b/drivers/net/mlx5/mlx5_ethdev.c index d80ae458b..6ef2dfd74 100644
>>> --- a/drivers/net/mlx5/mlx5_ethdev.c
>>> +++ b/drivers/net/mlx5/mlx5_ethdev.c
>>> @@ -1031,8 +1031,8 @@ mlx5_link_update_unlocked_gs(struct
>> rte_eth_dev *dev,
>>>  				ETH_LINK_HALF_DUPLEX :
>> ETH_LINK_FULL_DUPLEX);
>>>  	dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
>>>  				  ETH_LINK_SPEED_FIXED);
>>> -	if (((dev_link.link_speed && !dev_link.link_status) ||
>>> -	     (!dev_link.link_speed && dev_link.link_status))) {
>>> +	if ((((int)dev_link.link_speed > 0 && !dev_link.link_status) ||
>>> +	     ((int)dev_link.link_speed <= 0 && dev_link.link_status))) {
>>>  		rte_errno = EAGAIN;
>>>  		return -rte_errno;
>>>  	}
>>>
>>
>>
>>