From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f68.google.com (mail-wm0-f68.google.com [74.125.82.68]) by dpdk.org (Postfix) with ESMTP id 5D1101E35 for ; Wed, 24 Jan 2018 09:01:32 +0100 (CET) Received: by mail-wm0-f68.google.com with SMTP id f71so6585455wmf.0 for ; Wed, 24 Jan 2018 00:01:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=OBjeWH7TSoo+0ycTb32iFMu7cAbrdgv/JzAHR3GUJo4=; b=cWhLmu/N/QV3sZbY/7Yua+Ke0GBJByHBycqmdsgtjqFyGqw1gdumHCx+lGnluyZeat asOA5zWcObAtT1v7rPd4aJKkqeY60Sw0w+XGkOt2tPfMH9WfkkMm6pJbYNBc6zm8K/tX BG5CaVVmBytdGm24miDanl7irEzjbU1t3tsqXHk1SY611BtmBre5nQBZwuDPxiyBRlQ8 FRnpJJqz6sW00EzZQMib6xZvwpICU63WJd8doGnyuNfOETNv4gJUQ7HnJHnRxxLn9aJA GWI9Vmy/Hu2Y5dm/9bbn94YayVxyY0/VuuA+wX/VpVjEfGZ2oM7Gb/4uPYwNPFcFG2ht mfAA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=OBjeWH7TSoo+0ycTb32iFMu7cAbrdgv/JzAHR3GUJo4=; b=V0y3Ioq5pb4DdTL6s8tbL7MFAuQi/QwimUkysq9MM3SRCpZsRnEhKtcd6dJn06J7vW EIzlqC6ywzpAPU/nCzjEwKILyMNRKP9OWKFy3eShBreYEfwmPTyP//7TsKqbGKBAoPmp Apgtf828Su7BikS2oXPpJve7Rn7ZFsAh2dvENCqTO5GXd98EMOBmprIb02XY220hOCkD Zg7zkgMwQE5W9IvkIbgqSepJ9Q3A/JMTmv68GG0I1CIFN7SfzXDROxwrvXXdHBwnNpgF so1rApq7JYHS6+wDQ9AWr1ArBvCAnb+DxDFPY1azxZkYuu+nj0yMai08dFSTtdWEDGQo xRow== X-Gm-Message-State: AKwxyte1sB7dY7VZqq2IW6blOk2xdmE7Iqc24utZL7UPM4Ldp0L4mMc+ bOZWjWtFdJc7Kt8wZ6rTRiu/ X-Google-Smtp-Source: AH8x227zWKJ33YnM3dMpdr/ynn/1j/MY/n+jrn/XdabrhXx+h8M6LGoXwjmVA5ZCHsqzu1fcd5Z2fg== X-Received: by 10.28.11.70 with SMTP id 67mr3645649wml.158.1516780891877; Wed, 24 Jan 2018 00:01:31 -0800 (PST) Received: from laranjeiro-vm.dev.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id k20sm4350473wrk.37.2018.01.24.00.01.31 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 24 Jan 2018 00:01:31 -0800 (PST) Date: Wed, 24 Jan 2018 09:01:31 +0100 From: =?iso-8859-1?Q?N=E9lio?= Laranjeiro To: Shahaf Shuler Cc: yskoh@mellanox.com, adrien.mazarguil@6wind.com, dev@dpdk.org Message-ID: <20180124080131.eenv3vdw6fe6f6aj@laranjeiro-vm.dev.6wind.com> References: <20180123170106.9047-1-shahafs@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180123170106.9047-1-shahafs@mellanox.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix link state on device start X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Jan 2018 08:01:32 -0000 Hi Shahaf, On Tue, Jan 23, 2018 at 07:01:06PM +0200, Shahaf Shuler wrote: > Following commit c7bf62255edf ("net/mlx5: fix handling link status event") > the link state must be up in order for the burst function to be set on > the device ops. > > As the link may take time to move between down and up state it is > possible the rte_eth_dev_start call will return with wrong burst > function (either null or the empty burst function). > > Fixing it by forcing the link to be up before returning from device > start. In case the link is still not up after 5 seconds fail the function. > > Fixes: c7bf62255edf ("net/mlx5: fix handling link status event") > Cc: yskoh@mellanox.com > > Signed-off-by: Shahaf Shuler > --- > drivers/net/mlx5/mlx5.h | 1 + > drivers/net/mlx5/mlx5_defs.h | 3 +++ > drivers/net/mlx5/mlx5_ethdev.c | 27 +++++++++++++++++++++++++++ > drivers/net/mlx5/mlx5_trigger.c | 8 +++++++- > 4 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h > index a7ec607c3..30b737f76 100644 > --- a/drivers/net/mlx5/mlx5.h > +++ b/drivers/net/mlx5/mlx5.h > @@ -246,6 +246,7 @@ int mlx5_dev_configure(struct rte_eth_dev *); > void mlx5_dev_infos_get(struct rte_eth_dev *, struct rte_eth_dev_info *); > const uint32_t *mlx5_dev_supported_ptypes_get(struct rte_eth_dev *dev); > int priv_link_update(struct priv *, int); > +int priv_force_link_status_change(struct priv *, int); > int mlx5_link_update(struct rte_eth_dev *, int); > int mlx5_dev_set_mtu(struct rte_eth_dev *, uint16_t); > int mlx5_dev_get_flow_ctrl(struct rte_eth_dev *, struct rte_eth_fc_conf *); > diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h > index a71db281d..57f295c58 100644 > --- a/drivers/net/mlx5/mlx5_defs.h > +++ b/drivers/net/mlx5/mlx5_defs.h > @@ -110,4 +110,7 @@ > /* Supported RSS */ > #define MLX5_RSS_HF_MASK (~(ETH_RSS_IP | ETH_RSS_UDP | ETH_RSS_TCP)) > > +/* Maximum number of attempts to query link status before giving up. */ > +#define MLX5_MAX_LINK_QUERY_ATTEMPTS 5 > + > #endif /* RTE_PMD_MLX5_DEFS_H_ */ > diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c > index 6624888c9..523865d15 100644 > --- a/drivers/net/mlx5/mlx5_ethdev.c > +++ b/drivers/net/mlx5/mlx5_ethdev.c > @@ -966,6 +966,33 @@ priv_link_update(struct priv *priv, int wait_to_complete) > } > > /** > + * Querying the link status till it changes to the desired state. > + * Number of query attempts is bounded by MLX5_MAX_LINK_QUERY_ATTEMPTS. > + * > + * @param priv > + * Pointer to private structure. > + * @param status > + * Link desired status. > + * > + * @return > + * 0 on success, -1 on error. > + */ > +int > +priv_force_link_status_change(struct priv *priv, int status) > +{ > + int try = 0; > + > + while (try < MLX5_MAX_LINK_QUERY_ATTEMPTS) { > + priv_link_update(priv, 0); > + if (priv->dev->data->dev_link.link_status == status) > + return 0; > + try++; > + sleep(1); > + } > + return -1; > +} > + > +/** > * DPDK callback to retrieve physical link information. > * > * @param dev > diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c > index 827db2e7e..c5429e182 100644 > --- a/drivers/net/mlx5/mlx5_trigger.c > +++ b/drivers/net/mlx5/mlx5_trigger.c > @@ -166,7 +166,13 @@ mlx5_dev_start(struct rte_eth_dev *dev) > priv_xstats_init(priv); > /* Update link status and Tx/Rx callbacks for the first time. */ > memset(&dev->data->dev_link, 0, sizeof(struct rte_eth_link)); > - priv_link_update(priv, 1); > + INFO("Forcing port %u link to be up", dev->data->port_id); > + err = priv_force_link_status_change(priv, ETH_LINK_UP); > + if (err) { > + DEBUG("Failed to set port %u link to be up", > + dev->data->port_id); > + goto error; > + } > priv_dev_interrupt_handler_install(priv, dev); > priv_unlock(priv); > return 0; > -- > 2.12.0 According to mlx5_dev_start() documentation function: * @return * 0 on success, negative errno value on failure. This code is returning -1 in case of error, which means: EPERM 1 /* Operation not permitted */ which is a wrong value. Why not returning an errno in your priv function with an EBUSY or EAGAIN which is more accurate? Regards, -- Nélio Laranjeiro 6WIND