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

[Slava Ovsiienko <viacheslavo@mellanox.com>]

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;
> >  	}
> >
> 
> 
> 
>