Skip to contentSkip to conversation

net/mlx5: return RSS hash result in mbuf

PATCH New Nelio Laranjeiro 4 participants 12 messages Sep 14, 2016 – Oct 12, 2016 Network drivers

lore
Share & tools
lore

Nelio Laranjeiro

author
#0 lore raw

PATCH 1/1 net/mlx5: return RSS hash result in mbuf

Fill RSS hash result in mbuf.

To be applied on top of
"[PATCH 0/6] net/mlx5: performance improvement"

Nelio Laranjeiro (1):
net/mlx5: return RSS hash result in mbuf

 drivers/net/mlx5/mlx5_rxq.c  | 1 +
 drivers/net/mlx5/mlx5_rxtx.c | 6 +++++-
 drivers/net/mlx5/mlx5_rxtx.h | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)
Signature · 1 line
2.1.4
author
#1 lore raw

PATCH 1/1 net/mlx5: return RSS hash result in mbuf

Signed-off-by Nelio Laranjeiro <[email protected]>
---
 drivers/net/mlx5/mlx5_rxq.c  | 1 +
 drivers/net/mlx5/mlx5_rxtx.c | 6 +++++-
 drivers/net/mlx5/mlx5_rxtx.h | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)
drivers/net/mlx5/mlx5_rxq.c +1 -0
index f6f4315..65c264b 100644
@@ -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;
drivers/net/mlx5/mlx5_rxtx.c +5 -1
index b91b644..17ae5e4 100644
@@ -1338,12 +1338,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 = ntohl(cqe->rx_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 &
drivers/net/mlx5/mlx5_rxtx.h +2 -0
index 9828aef..e813f38 100644
@@ -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;
Signature · 1 line
2.1.4
Acked-by on v1 1/1
#2 lore raw
Quoted 1 line · Nelio Laranjeiro
Signed-off-by Nelio Laranjeiro <[email protected]>
Acked-by Adrien Mazarguil <[email protected]>
Quoted 56 lines
---
 drivers/net/mlx5/mlx5_rxq.c  | 1 +
 drivers/net/mlx5/mlx5_rxtx.c | 6 +++++-
 drivers/net/mlx5/mlx5_rxtx.h | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)
drivers/net/mlx5/mlx5_rxq.c +1 -0
index f6f4315..65c264b 100644
@@ -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;
drivers/net/mlx5/mlx5_rxtx.c +5 -1
index b91b644..17ae5e4 100644
@@ -1338,12 +1338,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 = ntohl(cqe->rx_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 &
drivers/net/mlx5/mlx5_rxtx.h +2 -0
index 9828aef..e813f38 100644
@@ -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;
Signature · 1 line
2.1.4
Signature · 2 lines
Adrien Mazarguil
6WIND
#3 lore raw
In reply to Nelio Laranjeiro
Quoted 12 lines · Nelio Laranjeiro
Signed-off-by Nelio Laranjeiro <[email protected]>
---
 drivers/net/mlx5/mlx5_rxq.c  | 1 +
 drivers/net/mlx5/mlx5_rxtx.c | 6 +++++-
 drivers/net/mlx5/mlx5_rxtx.h | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)
drivers/net/mlx5/mlx5_rxq.c +0 -0
index f6f4315..65c264b 100644

drivers/net/mlx5/mlx5_rxq.c rxq_ctrl_setup(struct rte_eth_dev *dev, struct rxq_ctrl *rxq_ctrl, · line 930

Quoted 5 lines
drivers/net/mlx5/mlx5_rxq.c +1 -0
@@ -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),
 		},

Two comments on this:
1. I don't think the !! is needed, since this is a comparison which will result in 0 or 1, rather than an int value that needs to be clamped to those values.
2. Why limit the use of RSS to when there is more than one RX queues? Sometimes it might be useful to have a precomputed hash for a packet even when not spreading packets among RX queues. For example, when using a single RX queue, but doing a hash table lookup for each packet, the RSS value could be used as the lookup hash.

/Bruce

author
#4 lore raw

Hi Bruce,

Quoted 8 lines · Bruce Richardson
Quoted 7 lines · Nelio Laranjeiro
Signed-off-by Nelio Laranjeiro <[email protected]>
---
 drivers/net/mlx5/mlx5_rxq.c  | 1 +
 drivers/net/mlx5/mlx5_rxtx.c | 6 +++++-
 drivers/net/mlx5/mlx5_rxtx.h | 2 ++
 3 files changed, 8 insertions(+), 1 deletion(-)
Quoted 14 lines

drivers/net/mlx5/mlx5_rxq.c rxq_ctrl_setup(struct rte_eth_dev *dev, struct rxq_ctrl *rxq_ctrl, · line 930

Quoted 10 lines
drivers/net/mlx5/mlx5_rxq.c +1 -0
index f6f4315..65c264b 100644
@@ -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),
 		},

Two comments on this:
1. I don't think the !! is needed, since this is a comparison which will result in 0 or 1, rather than an int value that needs to be clamped to those values.

Ok for this one.

Quoted 7 lines

2. Why limit the use of RSS to when there is more than one RX queues? Sometimes it might be useful to have a precomputed hash for a packet even when not spreading packets among RX queues. For example, when using a single RX queue, but doing a hash table lookup for each packet, the RSS value could be used as the lookup hash.

/Bruce

ConnectX-4 NICs does not return a hash value for a single queue as software does not request RSS in this situation, that is why it is not filled.

I will send a v2 to also fix a bug discovered since concerning the hash result which is not retrieve from the right place when Completion Queue Entry is compressed.

Regards,

Signature · 2 lines
Nélio Laranjeiro
6WIND
author
#5 lore raw

PATCH v2 1/1 net/mlx5: return RSS hash result in mbuf

In reply to Nelio Laranjeiro
Signed-off-by Nelio Laranjeiro <[email protected]>
---
 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(-)
doc/guides/nics/mlx5.rst +1 -0
index 8923173..0d1fabb 100644
@@ -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
 -----------
drivers/net/mlx5/mlx5_rxq.c +1 -0
index f6f4315..d32ad68 100644
@@ -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;
drivers/net/mlx5/mlx5_rxtx.c +16 -5
index e0ff47f..d3a427c 100644
@@ -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;
+		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 &
drivers/net/mlx5/mlx5_rxtx.h +2 -0
index 9828aef..e813f38 100644
@@ -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;
Signature · 1 line
2.1.4
#6 lore raw
Quoted 1 line · Nelio Laranjeiro
Signed-off-by Nelio Laranjeiro <[email protected]>

<...>

drivers/net/mlx5/mlx5_rxtx.c mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) · line 1299

Quoted 11 lines
drivers/net/mlx5/mlx5_rxtx.c +1 -1
@@ -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;

clang generates following warning, -it is quite detailed compiler message:

.../drivers/net/mlx5/mlx5_rxtx.c:1327:7: error: variable 'len' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]

                if (!pkt) {
                    ^~~~
.../drivers/net/mlx5/mlx5_rxtx.c:1384:7: note: uninitialized use occurs here
                if (len > DATA_LEN(seg)) {
                    ^~~
.../drivers/net/mlx5/mlx5_rxtx.c:1327:3: note: remove the 'if' if its
condition is always true
                if (!pkt) {
                ^~~~~~~~~~
.../drivers/net/mlx5/mlx5_rxtx.c:1299:10: note: initialize the variable
'len' to silence this warning
                int len;
                       ^
                        = 0
author
#7 lore raw
Quoted 22 lines · Ferruh Yigit
Quoted 1 line · Nelio Laranjeiro
Signed-off-by Nelio Laranjeiro <[email protected]>

<...>

drivers/net/mlx5/mlx5_rxtx.c mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n) · line 1299

Quoted 11 lines
drivers/net/mlx5/mlx5_rxtx.c +1 -1
@@ -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;

clang generates following warning, -it is quite detailed compiler message:

.../drivers/net/mlx5/mlx5_rxtx.c:1327:7: error: variable 'len' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]

Quoted 14 lines
                if (!pkt) {
                    ^~~~
.../drivers/net/mlx5/mlx5_rxtx.c:1384:7: note: uninitialized use occurs here
                if (len > DATA_LEN(seg)) {
                    ^~~
.../drivers/net/mlx5/mlx5_rxtx.c:1327:3: note: remove the 'if' if its
condition is always true
                if (!pkt) {
                ^~~~~~~~~~
.../drivers/net/mlx5/mlx5_rxtx.c:1299:10: note: initialize the variable
'len' to silence this warning
                int len;
                       ^
                        = 0

Right, my version of clang don't report this error, the one from ubuntu does.

I will fix it in a v3.

Thanks Ferruh,

Signature · 2 lines
Nélio Laranjeiro
6WIND
author
#8 lore raw

PATCH v3 1/1 net/mlx5: return RSS hash result in mbuf

In reply to Nelio Laranjeiro
Signed-off-by Nelio Laranjeiro <[email protected]>
---
 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(-)
doc/guides/nics/mlx5.rst +1 -0
index 8923173..0d1fabb 100644
@@ -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
 -----------
drivers/net/mlx5/mlx5_rxq.c +1 -0
index f6f4315..d32ad68 100644
@@ -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;
drivers/net/mlx5/mlx5_rxtx.c +16 -5
index e0ff47f..4491ae8 100644
@@ -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 &
drivers/net/mlx5/mlx5_rxtx.h +2 -0
index 9828aef..e813f38 100644
@@ -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;
Signature · 1 line
2.1.4
#9 lore raw

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.

Quoted 135 lines · Nelio Laranjeiro
Signed-off-by Nelio Laranjeiro <[email protected]>
---
 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(-)
doc/guides/nics/mlx5.rst +1 -0
index 8923173..0d1fabb 100644
@@ -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
 -----------
drivers/net/mlx5/mlx5_rxq.c +1 -0
index f6f4315..d32ad68 100644
@@ -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;
drivers/net/mlx5/mlx5_rxtx.c +16 -5
index e0ff47f..4491ae8 100644
@@ -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 &
drivers/net/mlx5/mlx5_rxtx.h +2 -0
index 9828aef..e813f38 100644
@@ -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;
Signature · 1 line
2.1.4
Signature · 2 lines
Adrien Mazarguil
6WIND
author
#10 lore raw

PATCH v4 1/1 net/mlx5: return RSS hash result in mbuf

In reply to Nelio Laranjeiro
Signed-off-by Nelio Laranjeiro <[email protected]>
---
 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(-)
doc/guides/nics/mlx5.rst +1 -0
index 8923173..0d1fabb 100644
@@ -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
 -----------
drivers/net/mlx5/mlx5_rxq.c +1 -0
index f6f4315..d32ad68 100644
@@ -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;
drivers/net/mlx5/mlx5_rxtx.c +16 -5
index e0ff47f..729a89b 100644
@@ -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;
+	int len; /* keep its value across iterations. */
 
 	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];
+		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 &
drivers/net/mlx5/mlx5_rxtx.h +2 -0
index 9828aef..e813f38 100644
@@ -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;
Signature · 1 line
2.1.4