net/mana: fix wrong indexing on CQE error when coalescing is used

PATCH Applied [email protected] 3 participants 5 messages Jul 7, 2023 – Jul 10, 2023 Network drivers

lore

Share & tools

lore

[email protected]

author

2023-07-07 00:18 UTC #0 lore raw

PATCH 1/1 net/mana: fix wrong indexing on CQE error when coalescing is used

From: Long Li <[email protected]>

On a fatal CQE error when coalescing is used, update the correct index and allow proceeding to the next CQE.

Fixes 3409e0f172f6 ("net/mana: implement Rx CQE coalescing")

Signed-off-by Long Li <[email protected]>

---
 drivers/net/mana/rx.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

drivers/net/mana/rx.c +8 ~~-10~~

index cacfd9ae1b..220b372b15 100644
@@ -416,23 +416,21 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
 
 		switch (oob->cqe_hdr.cqe_type) {
 		case CQE_RX_OKAY:
+		case CQE_RX_COALESCED_4:
 			/* Proceed to process mbuf */
 			break;
 
 		case CQE_RX_TRUNCATED:
-			DP_LOG(DEBUG, "Drop a truncated packet");
+		default:
+			DP_LOG(ERR, "RX CQE type %d client %d vendor %d",
+			       oob->cqe_hdr.cqe_type, oob->cqe_hdr.client_type,
+			       oob->cqe_hdr.vendor_err);
+
 			rxq->stats.errors++;
 			rte_pktmbuf_free(mbuf);
-			goto drop;
-
-		case CQE_RX_COALESCED_4:
-			/* Proceed to process mbuf */
-			break;
 
-		default:
-			DP_LOG(ERR, "Unknown RX CQE type %d",
-			       oob->cqe_hdr.cqe_type);
-			continue;
+			i++;
+			goto drop;
 		}
 
 		DP_LOG(DEBUG, "mana_rx_comp_oob type %d rxq %p",

Signature · 1 line

2.34.1

Ferruh Yigit

2023-07-07 09:01 UTC #1 lore raw

Quoted 7 lines · On 7/7/2023 1:17 AM, [email protected] wrote:

From: Long Li <[email protected]>
On a fatal CQE error when coalescing is used, update the correct index and allow proceeding to the next CQE.
Fixes 3409e0f172f6 ("net/mana: implement Rx CQE coalescing")

Is above fixes commit correct?
Logic for 'CQE_RX_COALESCED_4' is not changed with this commit, in above commit and in this commit both does breaks the loop.

This commit changes logic for 'CQE_RX_TRUNCATED' and 'default' cases, which are added with different commits, not the one in fixes line.

"fatal CQE error when coalescing" mentioned in the commit log, to which switch case does this corresponds to?

Quoted 41 lines

Signed-off-by Long Li <[email protected]>

---
 drivers/net/mana/rx.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

drivers/net/mana/rx.c +8 ~~-10~~

index cacfd9ae1b..220b372b15 100644
@@ -416,23 +416,21 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
 
 		switch (oob->cqe_hdr.cqe_type) {
 		case CQE_RX_OKAY:
+		case CQE_RX_COALESCED_4:
 			/* Proceed to process mbuf */
 			break;
 
 		case CQE_RX_TRUNCATED:
-			DP_LOG(DEBUG, "Drop a truncated packet");
+		default:
+			DP_LOG(ERR, "RX CQE type %d client %d vendor %d",
+			       oob->cqe_hdr.cqe_type, oob->cqe_hdr.client_type,
+			       oob->cqe_hdr.vendor_err);
+
 			rxq->stats.errors++;
 			rte_pktmbuf_free(mbuf);
-			goto drop;
-
-		case CQE_RX_COALESCED_4:
-			/* Proceed to process mbuf */
-			break;
 
-		default:
-			DP_LOG(ERR, "Unknown RX CQE type %d",
-			       oob->cqe_hdr.cqe_type);
-			continue;
+			i++;
+			goto drop;
 		}
 
 		DP_LOG(DEBUG, "mana_rx_comp_oob type %d rxq %p",

Long Li

2023-07-07 18:01 UTC #2 lore raw

Quoted 9 lines

Subject: Re: [PATCH] net/mana: fix wrong indexing on CQE error when coalescing is used
Quoted 5 lines · On 7/7/2023 1:17 AM, [email protected] wrote:
From: Long Li <[email protected]>
On a fatal CQE error when coalescing is used, update the correct index and allow proceeding to the next CQE.

Quoted 6 lines

Quoted 2 lines
Fixes 3409e0f172f6 ("net/mana: implement Rx CQE coalescing")
Is above fixes commit correct?
Logic for 'CQE_RX_COALESCED_4' is not changed with this commit, in above commit and in this commit both does breaks the loop.

Yes, the "Fixes" tag is correct. Here CQE_RX_COALESCED_4 is rearranged to make it easier to read, but it doesn't change the behavior.

Quoted 6 lines

This commit changes logic for 'CQE_RX_TRUNCATED' and 'default' cases, which are added with different commits, not the one in fixes line.
"fatal CQE error when coalescing" mentioned in the commit log, to which switch case does this corresponds to?

The previous patch (3409e0f172f6 ) introduced variable "i", an index to completion CQEs. But both 'default' and 'CQE_RX_TRUNCATED' cases don't advance "i", hence not advance to next CQE on error.

The fatal CQE error means the "default" case. On 'CQE_RX_TRUNCATED' the code can recover when all CQEs are read. But on "default", it's a dead loop.

Quoted 44 lines

Signed-off-by Long Li <[email protected]>

---
 drivers/net/mana/rx.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

drivers/net/mana/rx.c index renamed from drivers/net/mana/rx.c +0 -0

cacfd9ae1b..220b372b15 100644

drivers/net/mana/rx.c +8 ~~-10~~

@@ -416,23 +416,21 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf
 **pkts, uint16_t pkts_n)
 
 		switch (oob->cqe_hdr.cqe_type) {
 		case CQE_RX_OKAY:
+		case CQE_RX_COALESCED_4:
 			/* Proceed to process mbuf */
 			break;
 
 		case CQE_RX_TRUNCATED:
-			DP_LOG(DEBUG, "Drop a truncated packet");
+		default:
+			DP_LOG(ERR, "RX CQE type %d client %d vendor %d",
+			       oob->cqe_hdr.cqe_type, oob->cqe_hdr.client_type,
+			       oob->cqe_hdr.vendor_err);
+
 			rxq->stats.errors++;
 			rte_pktmbuf_free(mbuf);
-			goto drop;
-
-		case CQE_RX_COALESCED_4:
-			/* Proceed to process mbuf */
-			break;
 
-		default:
-			DP_LOG(ERR, "Unknown RX CQE type %d",
-			       oob->cqe_hdr.cqe_type);
-			continue;
+			i++;
+			goto drop;
 		}

DP_LOG(DEBUG, "mana_rx_comp_oob type %d rxq %p",

Ferruh Yigit

2023-07-10 10:42 UTC #3 lore raw

Quoted 19 lines · Long Li

Quoted 9 lines
Subject: Re: [PATCH] net/mana: fix wrong indexing on CQE error when coalescing is used
Quoted 5 lines · On 7/7/2023 1:17 AM, [email protected] wrote:
From: Long Li <[email protected]>
On a fatal CQE error when coalescing is used, update the correct index and allow proceeding to the next CQE.
Quoted 6 lines
Quoted 2 lines
Fixes 3409e0f172f6 ("net/mana: implement Rx CQE coalescing")
Is above fixes commit correct?
Logic for 'CQE_RX_COALESCED_4' is not changed with this commit, in above commit and in this commit both does breaks the loop.
Yes, the "Fixes" tag is correct. Here CQE_RX_COALESCED_4 is rearranged to make it easier to read, but it doesn't change the behavior.
Quoted 1 line

Quoted 8 lines

Quoted 5 lines
This commit changes logic for 'CQE_RX_TRUNCATED' and 'default' cases, which are added with different commits, not the one in fixes line.
"fatal CQE error when coalescing" mentioned in the commit log, to which switch case does this corresponds to?
The previous patch (3409e0f172f6 ) introduced variable "i", an index to completion CQEs. But both 'default' and 'CQE_RX_TRUNCATED' cases don't advance "i", hence not advance to next CQE on error.

Right, thanks for clarification, I was checking from CQE_RX_COALESCED_4 perspective.

Quoted 2 lines

The fatal CQE error means the "default" case. On 'CQE_RX_TRUNCATED' the code can recover when all CQEs are read. But on "default", it's a dead loop.

ack.

Quoted 45 lines

Quoted 44 lines

Signed-off-by Long Li <[email protected]>

---
 drivers/net/mana/rx.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

drivers/net/mana/rx.c index renamed from drivers/net/mana/rx.c +0 -0

cacfd9ae1b..220b372b15 100644

drivers/net/mana/rx.c +8 ~~-10~~

@@ -416,23 +416,21 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf
 **pkts, uint16_t pkts_n)
 
 		switch (oob->cqe_hdr.cqe_type) {
 		case CQE_RX_OKAY:
+		case CQE_RX_COALESCED_4:
 			/* Proceed to process mbuf */
 			break;
 
 		case CQE_RX_TRUNCATED:
-			DP_LOG(DEBUG, "Drop a truncated packet");
+		default:
+			DP_LOG(ERR, "RX CQE type %d client %d vendor %d",
+			       oob->cqe_hdr.cqe_type, oob->cqe_hdr.client_type,
+			       oob->cqe_hdr.vendor_err);
+
 			rxq->stats.errors++;
 			rte_pktmbuf_free(mbuf);
-			goto drop;
-
-		case CQE_RX_COALESCED_4:
-			/* Proceed to process mbuf */
-			break;
 
-		default:
-			DP_LOG(ERR, "Unknown RX CQE type %d",
-			       oob->cqe_hdr.cqe_type);
-			continue;
+			i++;
+			goto drop;
 		}

DP_LOG(DEBUG, "mana_rx_comp_oob type %d rxq %p",

Ferruh Yigit

Applied

2023-07-10 11:00 UTC #4 lore raw

In reply to [email protected]

Quoted 8 lines · On 7/7/2023 1:17 AM, [email protected] wrote:

From: Long Li <[email protected]>
On a fatal CQE error when coalescing is used, update the correct index and allow proceeding to the next CQE.
Fixes 3409e0f172f6 ("net/mana: implement Rx CQE coalescing">
Signed-off-by Long Li <[email protected]>

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