DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/mlx5: fix last completed built descriptor
@ 2020-02-20 21:17 Viacheslav Ovsiienko
  2020-02-21  8:25 ` Matan Azrad
  0 siblings, 1 reply; 3+ messages in thread
From: Viacheslav Ovsiienko @ 2020-02-20 21:17 UTC (permalink / raw)
  To: dev; +Cc: matan, rasland, thomas, ferruh.yigit, stable

The routine sending packets with Multi-Packet Write method assigns
the wqe_last variable with transmit descriptor (WQE - work queue entry)
being built. If send queue is close to full state, the WQE has no data
yet (trying to put the first packet) and there is no enough space
in descriptor for the next packet the WQE is discarded and the stored
wqe_last value becomes invalid - points to the discarded WQE.

The mlx5_tx_burst_request_completion() routine might set the completion
request flags in the WQE pointed by wqe_last, it is safe, but the next
mlx5_tx_burst call uses the WQE as the first free one and request
completion flags might be overwritten and completion request will be
lost causing the transmit  datapath malfunction.

Fixes: 8b581c690a54 ("net/mlx5: move Tx complete request routine")
Cc: stable@dpdk.org

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_rxtx.c | 51 +++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 0df811b..2b4fc2a 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -2262,6 +2262,7 @@ enum mlx5_txcmp_code {
 	     (uint16_t)(txq->wqe_ci - txq->wqe_comp) >= txq->wqe_thres)) {
 		volatile struct mlx5_wqe *last = loc->wqe_last;
 
+		MLX5_ASSERT(last);
 		txq->elts_comp = head;
 		if (MLX5_TXOFF_CONFIG(INLINE))
 			txq->wqe_comp = txq->wqe_ci;
@@ -3921,6 +3922,8 @@ enum mlx5_txcmp_code {
  *   Total size of descriptor/data in bytes.
  * @param slen
  *   Accumulated statistics, data bytes sent.
+ * @param wqem
+ *   The base WQE for the eMPW/MPW descriptor.
  * @param olx
  *   Configured Tx offloads mask. It is fully defined at
  *   compile time and may be used for optimization.
@@ -3934,9 +3937,10 @@ enum mlx5_txcmp_code {
 		   struct mlx5_txq_local *restrict loc,
 		   unsigned int len,
 		   unsigned int slen,
+		   struct mlx5_wqe *restrict wqem,
 		   unsigned int olx __rte_unused)
 {
-	struct mlx5_wqe_dseg *dseg = &loc->wqe_last->dseg[0];
+	struct mlx5_wqe_dseg *dseg = &wqem->dseg[0];
 
 	MLX5_ASSERT(MLX5_TXOFF_CONFIG(INLINE));
 #ifdef MLX5_PMD_SOFT_COUNTERS
@@ -3963,9 +3967,10 @@ enum mlx5_txcmp_code {
 		MLX5_ASSERT((len % MLX5_WSEG_SIZE) == 0);
 		len = len / MLX5_WSEG_SIZE + 2;
 	}
-	loc->wqe_last->cseg.sq_ds = rte_cpu_to_be_32(txq->qp_num_8s | len);
+	wqem->cseg.sq_ds = rte_cpu_to_be_32(txq->qp_num_8s | len);
 	txq->wqe_ci += (len + 3) / 4;
 	loc->wqe_free -= (len + 3) / 4;
+	loc->wqe_last = wqem;
 }
 
 /**
@@ -4202,7 +4207,7 @@ enum mlx5_txcmp_code {
 	pkts_n -= loc->pkts_sent;
 	for (;;) {
 		struct mlx5_wqe_dseg *restrict dseg;
-		struct mlx5_wqe_eseg *restrict eseg;
+		struct mlx5_wqe *restrict wqem;
 		enum mlx5_txcmp_code ret;
 		unsigned int room, part, nlim;
 		unsigned int slen = 0;
@@ -4221,22 +4226,21 @@ enum mlx5_txcmp_code {
 			return MLX5_TXCMP_CODE_EXIT;
 		if (likely(pkts_n > 1))
 			rte_prefetch0(*pkts);
-		loc->wqe_last = txq->wqes + (txq->wqe_ci & txq->wqe_m);
+		wqem = txq->wqes + (txq->wqe_ci & txq->wqe_m);
 		/*
 		 * Build eMPW title WQEBB:
 		 * - Control Segment, eMPW opcode, zero DS
 		 * - Ethernet Segment, no inline
 		 */
-		mlx5_tx_cseg_init(txq, loc, loc->wqe_last, 0,
+		mlx5_tx_cseg_init(txq, loc, wqem, 0,
 				  MLX5_OPCODE_ENHANCED_MPSW, olx);
-		mlx5_tx_eseg_none(txq, loc, loc->wqe_last,
+		mlx5_tx_eseg_none(txq, loc, wqem,
 				  olx & ~MLX5_TXOFF_CONFIG_VLAN);
-		eseg = &loc->wqe_last->eseg;
-		dseg = &loc->wqe_last->dseg[0];
+		dseg = &wqem->dseg[0];
 		/* Store the packet length for legacy MPW. */
 		if (MLX5_TXOFF_CONFIG(MPW))
-			eseg->mss = rte_cpu_to_be_16
-					(rte_pktmbuf_data_len(loc->mbuf));
+			wqem->eseg.mss = rte_cpu_to_be_16
+					 (rte_pktmbuf_data_len(loc->mbuf));
 		room = RTE_MIN(MLX5_WQE_SIZE_MAX / MLX5_WQE_SIZE,
 			       loc->wqe_free) * MLX5_WQE_SIZE -
 					MLX5_WQE_CSEG_SIZE -
@@ -4273,7 +4277,8 @@ enum mlx5_txcmp_code {
 				 * We have some successfully built
 				 * packet Data Segments to send.
 				 */
-				mlx5_tx_idone_empw(txq, loc, part, slen, olx);
+				mlx5_tx_idone_empw(txq, loc, part,
+						   slen, wqem, olx);
 				return MLX5_TXCMP_CODE_ERROR;
 			}
 			/* Inline or not inline - that's the Question. */
@@ -4295,7 +4300,7 @@ enum mlx5_txcmp_code {
 					 * No pointer and inline descriptor
 					 * intermix for legacy MPW sessions.
 					 */
-					if (loc->wqe_last->dseg[0].bcount)
+					if (wqem->dseg[0].bcount)
 						break;
 				}
 			} else {
@@ -4344,7 +4349,7 @@ enum mlx5_txcmp_code {
 			 */
 			if (MLX5_TXOFF_CONFIG(MPW) &&
 			    part != room &&
-			    loc->wqe_last->dseg[0].bcount == RTE_BE32(0))
+			    wqem->dseg[0].bcount == RTE_BE32(0))
 				break;
 			/*
 			 * Not inlinable VLAN packets are
@@ -4374,7 +4379,8 @@ enum mlx5_txcmp_code {
 				 * continue build descriptors.
 				 */
 				part -= room;
-				mlx5_tx_idone_empw(txq, loc, part, slen, olx);
+				mlx5_tx_idone_empw(txq, loc, part,
+						   slen, wqem, olx);
 				return MLX5_TXCMP_CODE_EXIT;
 			}
 			loc->mbuf = *pkts++;
@@ -4388,7 +4394,8 @@ enum mlx5_txcmp_code {
 			 */
 			if (ret == MLX5_TXCMP_CODE_MULTI) {
 				part -= room;
-				mlx5_tx_idone_empw(txq, loc, part, slen, olx);
+				mlx5_tx_idone_empw(txq, loc, part,
+						   slen, wqem, olx);
 				if (unlikely(!loc->elts_free ||
 					     !loc->wqe_free))
 					return MLX5_TXCMP_CODE_EXIT;
@@ -4397,7 +4404,8 @@ enum mlx5_txcmp_code {
 			MLX5_ASSERT(NB_SEGS(loc->mbuf) == 1);
 			if (ret == MLX5_TXCMP_CODE_TSO) {
 				part -= room;
-				mlx5_tx_idone_empw(txq, loc, part, slen, olx);
+				mlx5_tx_idone_empw(txq, loc, part,
+						   slen, wqem, olx);
 				if (unlikely(!loc->elts_free ||
 					     !loc->wqe_free))
 					return MLX5_TXCMP_CODE_EXIT;
@@ -4405,7 +4413,8 @@ enum mlx5_txcmp_code {
 			}
 			if (ret == MLX5_TXCMP_CODE_SINGLE) {
 				part -= room;
-				mlx5_tx_idone_empw(txq, loc, part, slen, olx);
+				mlx5_tx_idone_empw(txq, loc, part,
+						   slen, wqem, olx);
 				if (unlikely(!loc->elts_free ||
 					     !loc->wqe_free))
 					return MLX5_TXCMP_CODE_EXIT;
@@ -4414,7 +4423,8 @@ enum mlx5_txcmp_code {
 			if (ret != MLX5_TXCMP_CODE_EMPW) {
 				MLX5_ASSERT(false);
 				part -= room;
-				mlx5_tx_idone_empw(txq, loc, part, slen, olx);
+				mlx5_tx_idone_empw(txq, loc, part,
+						   slen, wqem, olx);
 				return MLX5_TXCMP_CODE_ERROR;
 			}
 			/* Check if we have minimal room left. */
@@ -4429,7 +4439,8 @@ enum mlx5_txcmp_code {
 			 * - software parser settings
 			 * - packets length (legacy MPW only)
 			 */
-			if (!mlx5_tx_match_empw(txq, eseg, loc, dlen, olx))
+			if (!mlx5_tx_match_empw(txq, &wqem->eseg,
+						loc, dlen, olx))
 				break;
 			/* Packet attributes match, continue the same eMPW. */
 			if ((uintptr_t)dseg >= (uintptr_t)txq->wqes_end)
@@ -4443,7 +4454,7 @@ enum mlx5_txcmp_code {
 		part -= room;
 		if (unlikely(!part))
 			return MLX5_TXCMP_CODE_EXIT;
-		mlx5_tx_idone_empw(txq, loc, part, slen, olx);
+		mlx5_tx_idone_empw(txq, loc, part, slen, wqem, olx);
 		if (unlikely(!loc->elts_free ||
 			     !loc->wqe_free))
 			return MLX5_TXCMP_CODE_EXIT;
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH] net/mlx5: fix last completed built descriptor
  2020-02-20 21:17 [dpdk-dev] [PATCH] net/mlx5: fix last completed built descriptor Viacheslav Ovsiienko
@ 2020-02-21  8:25 ` Matan Azrad
  2020-02-21  9:42   ` Ferruh Yigit
  0 siblings, 1 reply; 3+ messages in thread
From: Matan Azrad @ 2020-02-21  8:25 UTC (permalink / raw)
  To: Slava Ovsiienko, dev
  Cc: Raslan Darawsheh, Thomas Monjalon, ferruh.yigit, stable

From: Viacheslav Ovsiienko
> Sent: Thursday, February 20, 2020 11:18 PM
> To: dev@dpdk.org
> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
> ferruh.yigit@intel.com; stable@dpdk.org
> Subject: [PATCH] net/mlx5: fix last completed built descriptor
> 
> The routine sending packets with Multi-Packet Write method assigns the
> wqe_last variable with transmit descriptor (WQE - work queue entry) being
> built. If send queue is close to full state, the WQE has no data yet (trying to
> put the first packet) and there is no enough space in descriptor for the next
> packet the WQE is discarded and the stored wqe_last value becomes invalid -
> points to the discarded WQE.
> 
> The mlx5_tx_burst_request_completion() routine might set the completion
> request flags in the WQE pointed by wqe_last, it is safe, but the next
> mlx5_tx_burst call uses the WQE as the first free one and request
> completion flags might be overwritten and completion request will be lost
> causing the transmit  datapath malfunction.
> 
> Fixes: 8b581c690a54 ("net/mlx5: move Tx complete request routine")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH] net/mlx5: fix last completed built descriptor
  2020-02-21  8:25 ` Matan Azrad
@ 2020-02-21  9:42   ` Ferruh Yigit
  0 siblings, 0 replies; 3+ messages in thread
From: Ferruh Yigit @ 2020-02-21  9:42 UTC (permalink / raw)
  To: Matan Azrad, Slava Ovsiienko, dev
  Cc: Raslan Darawsheh, Thomas Monjalon, stable

On 2/21/2020 8:25 AM, Matan Azrad wrote:
> From: Viacheslav Ovsiienko
>> Sent: Thursday, February 20, 2020 11:18 PM
>> To: dev@dpdk.org
>> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh
>> <rasland@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
>> ferruh.yigit@intel.com; stable@dpdk.org
>> Subject: [PATCH] net/mlx5: fix last completed built descriptor
>>
>> The routine sending packets with Multi-Packet Write method assigns the
>> wqe_last variable with transmit descriptor (WQE - work queue entry) being
>> built. If send queue is close to full state, the WQE has no data yet (trying to
>> put the first packet) and there is no enough space in descriptor for the next
>> packet the WQE is discarded and the stored wqe_last value becomes invalid -
>> points to the discarded WQE.
>>
>> The mlx5_tx_burst_request_completion() routine might set the completion
>> request flags in the WQE pointed by wqe_last, it is safe, but the next
>> mlx5_tx_burst call uses the WQE as the first free one and request
>> completion flags might be overwritten and completion request will be lost
>> causing the transmit  datapath malfunction.
>>
>> Fixes: 8b581c690a54 ("net/mlx5: move Tx complete request routine")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> Acked-by: Matan Azrad <matan@mellanox.com>
> 

Applied to dpdk-next-net/master, thanks.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-02-21  9:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-20 21:17 [dpdk-dev] [PATCH] net/mlx5: fix last completed built descriptor Viacheslav Ovsiienko
2020-02-21  8:25 ` Matan Azrad
2020-02-21  9:42   ` Ferruh Yigit

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git