From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <adrien.mazarguil@6wind.com>
Received: from mail-wm0-f52.google.com (mail-wm0-f52.google.com [74.125.82.52])
 by dpdk.org (Postfix) with ESMTP id 572612BA1
 for <dev@dpdk.org>; Wed, 28 Sep 2016 10:59:14 +0200 (CEST)
Received: by mail-wm0-f52.google.com with SMTP id l132so223367204wmf.0
 for <dev@dpdk.org>; Wed, 28 Sep 2016 01:59:14 -0700 (PDT)
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:in-reply-to;
 bh=fzsmBOUont8CIDr7ETr3mOADi0S9s3tDdgQvOaxho2g=;
 b=mUG/mNP1TCeZy+N1dQNbwpg5O3lv9kB02/FrvGTDJjWCDNNsqP+go8c5wzN6lTa1k0
 04+rKlJk5c2ScRFqnTdv6PIf7QAQpOOUC+mVXEfoBd5tfmyey7GI8vKvMQ6NZf0uVc/r
 SPIwLAoJqYtN6It54q1s8YmN1lYlPSTR845WWdNeVoVmPOSI+2F5xog0BdEYFKEvVm/r
 Me478v9tsM4hqTZcOe1JWahATdxaLPXA7m4lIOdoxuPqiBAq/Z+By3QkCE9smIu1Pbms
 guDEdNospiqubPfvBERWRu5AHQ9JzLqdkzjTKi9bwx4PAA96KY0gXNoQnHnJ40H9mc/N
 kgAA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20130820;
 h=x-gm-message-state:date:from:to:cc:subject:message-id:references
 :mime-version:content-disposition:in-reply-to;
 bh=fzsmBOUont8CIDr7ETr3mOADi0S9s3tDdgQvOaxho2g=;
 b=YHuaV5/KeubhS8UWFHTH/A0ZNsQYkgvmWQY2DYEZNaObaH9YGP8nPsHivtsS1kcnDE
 GfsbWXuGZUqWM/9dBVv97aCsqULtET4XrzSrureUmW1AnKOESRTyeG7SGiPxrYVrUorM
 08hvArR7DaqTuJ0pt3ZlRk+VPAmAGoteVC/6NQsDSlVm2EHPXSlYkdXcbJWmBQcDmQGX
 Itbu8riyM77UfMgZqNnxzzGY61QC3VfPuaDyqG/mC6tjEpDq+CCJtGSlLWcEYBCtINSo
 IERLSfbOb1pewdUyc703AA3up0i6weWdv08/U1ZjKYsb6cSMkPE7C2B1O6BGnCyVU5go
 owQg==
X-Gm-Message-State: AA6/9RlhHV+707Rdu05tOvp71er2lbhDw60yUamsFsEeVKhJfnVDlhQaQUwvMMM0PWfV9c9l
X-Received: by 10.28.109.156 with SMTP id b28mr7155986wmi.68.1475053154031;
 Wed, 28 Sep 2016 01:59:14 -0700 (PDT)
Received: from 6wind.com (guy78-3-82-239-227-177.fbx.proxad.net.
 [82.239.227.177])
 by smtp.gmail.com with ESMTPSA id vx7sm2835014wjc.1.2016.09.28.01.59.12
 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Wed, 28 Sep 2016 01:59:13 -0700 (PDT)
Date: Wed, 28 Sep 2016 10:59:06 +0200
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Cc: dev@dpdk.org, Bruce Richardson <bruce.richardson@intel.com>,
 Ferruh Yigit <ferruh.yigit@intel.com>
Message-ID: <20160928085906.GD17252@6wind.com>
References: <cover.1473840791.git.nelio.laranjeiro@6wind.com>
 <f8c87a7a5f3b860880e041223c6fa596fcf41b2f.1475045833.git.nelio.laranjeiro@6wind.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f8c87a7a5f3b860880e041223c6fa596fcf41b2f.1475045833.git.nelio.laranjeiro@6wind.com>
Subject: Re: [dpdk-dev] [PATCH v3] net/mlx5: return RSS hash result in mbuf
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 28 Sep 2016 08:59:14 -0000

Hi all,

Actually while this patch may silence clang, it still contains a bug. len
must remain in the outer scope as its value must be preserved across
iterations. I forgot this while reading the original patch and wrongly
suggested Nelio to move it to in the inner loop, sorry about that.

In short clang properly detected the issue, this was not a false
positive. We should use it more often.

On Wed, Sep 28, 2016 at 09:02:15AM +0200, Nelio Laranjeiro wrote:
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> ---
>  doc/guides/nics/mlx5.rst     |  1 +
>  drivers/net/mlx5/mlx5_rxq.c  |  1 +
>  drivers/net/mlx5/mlx5_rxtx.c | 21 ++++++++++++++++-----
>  drivers/net/mlx5/mlx5_rxtx.h |  2 ++
>  4 files changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
> index 8923173..0d1fabb 100644
> --- a/doc/guides/nics/mlx5.rst
> +++ b/doc/guides/nics/mlx5.rst
> @@ -88,6 +88,7 @@ Features
>    RTE_ETH_FDIR_REJECT).
>  - Secondary process TX is supported.
>  - KVM and VMware ESX SR-IOV modes are supported.
> +- RSS hash result is supported.
>  
>  Limitations
>  -----------
> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> index f6f4315..d32ad68 100644
> --- a/drivers/net/mlx5/mlx5_rxq.c
> +++ b/drivers/net/mlx5/mlx5_rxq.c
> @@ -926,6 +926,7 @@ rxq_ctrl_setup(struct rte_eth_dev *dev, struct rxq_ctrl *rxq_ctrl,
>  		.rxq = {
>  			.elts_n = log2above(desc),
>  			.mp = mp,
> +			.rss_hash = priv->rxqs_n > 1,
>  		},
>  	};
>  	struct ibv_exp_wq_attr mod;
> diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
> index e0ff47f..4491ae8 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.c
> +++ b/drivers/net/mlx5/mlx5_rxtx.c
> @@ -1128,6 +1128,8 @@ rxq_cq_to_pkt_type(volatile struct mlx5_cqe64 *cqe)
>   *   Pointer to RX queue.
>   * @param cqe
>   *   CQE to process.
> + * @param[out] rss_hash
> + *   Packet RSS Hash result.
>   *
>   * @return
>   *   Packet size in bytes (0 if there is none), -1 in case of completion
> @@ -1135,7 +1137,7 @@ rxq_cq_to_pkt_type(volatile struct mlx5_cqe64 *cqe)
>   */
>  static inline int
>  mlx5_rx_poll_len(struct rxq *rxq, volatile struct mlx5_cqe64 *cqe,
> -		 uint16_t cqe_cnt)
> +		 uint16_t cqe_cnt, uint32_t *rss_hash)
>  {
>  	struct rxq_zip *zip = &rxq->zip;
>  	uint16_t cqe_n = cqe_cnt + 1;
> @@ -1148,6 +1150,7 @@ mlx5_rx_poll_len(struct rxq *rxq, volatile struct mlx5_cqe64 *cqe,
>  			(uintptr_t)(&(*rxq->cqes)[zip->ca & cqe_cnt].cqe64);
>  
>  		len = ntohl((*mc)[zip->ai & 7].byte_cnt);
> +		*rss_hash = ntohl((*mc)[zip->ai & 7].rx_hash_result);
>  		if ((++zip->ai & 7) == 0) {
>  			/*
>  			 * Increment consumer index to skip the number of
> @@ -1202,9 +1205,11 @@ mlx5_rx_poll_len(struct rxq *rxq, volatile struct mlx5_cqe64 *cqe,
>  			zip->cq_ci = rxq->cq_ci + zip->cqe_cnt;
>  			/* Get packet size to return. */
>  			len = ntohl((*mc)[0].byte_cnt);
> +			*rss_hash = ntohl((*mc)[0].rx_hash_result);
>  			zip->ai = 1;
>  		} else {
>  			len = ntohl(cqe->byte_cnt);
> +			*rss_hash = ntohl(cqe->rx_hash_res);
>  		}
>  		/* Error while receiving packet. */
>  		if (unlikely(MLX5_CQE_OPCODE(op_own) == MLX5_CQE_RESP_ERR))
> @@ -1286,12 +1291,13 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
>  		&(*rxq->cqes)[rxq->cq_ci & cqe_cnt].cqe64;
>  	unsigned int i = 0;
>  	unsigned int rq_ci = rxq->rq_ci << sges_n;
> -	int len;
>  
>  	while (pkts_n) {
>  		unsigned int idx = rq_ci & wqe_cnt;
>  		volatile struct mlx5_wqe_data_seg *wqe = &(*rxq->wqes)[idx];
>  		struct rte_mbuf *rep = (*rxq->elts)[idx];
> +		int len = 0;
> +		uint32_t rss_hash_res = 0;
>  
>  		if (pkt)
>  			NEXT(seg) = rep;
> @@ -1320,8 +1326,9 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
>  		}
>  		if (!pkt) {
>  			cqe = &(*rxq->cqes)[rxq->cq_ci & cqe_cnt].cqe64;
> -			len = mlx5_rx_poll_len(rxq, cqe, cqe_cnt);
> -			if (len == 0) {
> +			len = mlx5_rx_poll_len(rxq, cqe, cqe_cnt,
> +					       &rss_hash_res);
> +			if (!len) {
>  				rte_mbuf_refcnt_set(rep, 0);
>  				__rte_mbuf_raw_free(rep);
>  				break;
> @@ -1338,12 +1345,16 @@ mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
>  			/* Update packet information. */
>  			pkt->packet_type = 0;
>  			pkt->ol_flags = 0;
> +			if (rxq->rss_hash) {
> +				pkt->hash.rss = rss_hash_res;
> +				pkt->ol_flags = PKT_RX_RSS_HASH;
> +			}
>  			if (rxq->csum | rxq->csum_l2tun | rxq->vlan_strip |
>  			    rxq->crc_present) {
>  				if (rxq->csum) {
>  					pkt->packet_type =
>  						rxq_cq_to_pkt_type(cqe);
> -					pkt->ol_flags =
> +					pkt->ol_flags |=
>  						rxq_cq_to_ol_flags(rxq, cqe);
>  				}
>  				if (cqe->l4_hdr_type_etc &
> diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
> index 9828aef..e813f38 100644
> --- a/drivers/net/mlx5/mlx5_rxtx.h
> +++ b/drivers/net/mlx5/mlx5_rxtx.h
> @@ -113,6 +113,8 @@ struct rxq {
>  	unsigned int cqe_n:4; /* Log 2 of CQ elements. */
>  	unsigned int elts_n:4; /* Log 2 of Mbufs. */
>  	unsigned int port_id:8;
> +	unsigned int rss_hash:1; /* RSS hash result is enabled. */
> +	unsigned int :9; /* Remaining bits. */
>  	volatile uint32_t *rq_db;
>  	volatile uint32_t *cq_db;
>  	uint16_t rq_ci;
> -- 
> 2.1.4
> 

-- 
Adrien Mazarguil
6WIND