Re: [dpdk-dev] [PATCH] net/mlx5: fix link status initialization

[Shahaf Shuler <shahafs@mellanox.com>]

Wednesday, April 4, 2018 10:30 AM, Nélio Laranjeiro:
> Subject: Re: [PATCH] net/mlx5: fix link status initialization
> 
> On Tue, Apr 03, 2018 at 07:48:17AM +0300, Shahaf Shuler wrote:
> > Following commit 7ba5320baa32 ("net/mlx5: fix link status behavior")
> >
> > The initial link status is no longer set as part of the port start.
> > This may cause application to query the link as down while in fact it
> > was already up before the DPDK application start.
> 
> There is something wrong in this explanation, the application should query
> the link using this same callback, why the PMD should call it?

It is how ethdev is implemented. The application is doing nothing wrong, it queries the link status using rte_eth_link_get_nowait

When the application works with LSC interrupts the ethdev layer skips the PMD callback and just update according to the link status exists on device data.
It is because it assumes the link status on the device data is the correct one since any link change is processed by the application.

The issue is with the initial state of the link. If the link is already up when the PMD starts there will be no callback for the application. 

I think this logic is OK, and it is also a good practice to initialize the link status to the actual state of the link as part of the port probing. 

> 
> > Fixes: 7ba5320baa32 ("net/mlx5: fix link status behavior")
> > Cc: nelio.laranjeiro@6wind.com
> >
> > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> > Acked-by: Yongseok Koh <yskoh@mellanox.com>
> > ---
> >  drivers/net/mlx5/mlx5.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
> > 7d58d66bb9..f954ea2862 100644
> > --- a/drivers/net/mlx5/mlx5.c
> > +++ b/drivers/net/mlx5/mlx5.c
> > @@ -961,6 +961,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv
> __rte_unused,
> >  		DRV_LOG(DEBUG, "port %u forcing Ethernet interface up",
> >  			eth_dev->data->port_id);
> >  		mlx5_set_link_up(eth_dev);
> > +		mlx5_link_update(eth_dev, 1);
> >  		/* Store device configuration on private structure. */
> >  		priv->config = config;
> >  		continue;
> > --
> > 2.12.0
> 
> 
> --
> Nélio Laranjeiro
> 6WIND