[v2,22/25] net/bnxt: fix bugs in representor data path

Message ID 20200916042851.32914-23-ajit.khaparde@broadcom.com (mailing list archive)
State Accepted, archived
Delegated to: Ajit Khaparde
Headers
Series patchset for bnxt |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Ajit Khaparde Sept. 16, 2020, 4:28 a.m. UTC
  From: Somnath Kotur <somnath.kotur@broadcom.com>

1.Representor Rx ring producer index was not getting reset in
the ring full case. Fix it by incrementing only in
success case.
2.Instead of calling the mbuf specific routine to free the mbuf when
representor ring is full rte_free was being called leading to
'invalid memory' errors being logged.
3. Do not account the pkt meant for the representor in the parent
Rx ring's array that is returned to the application.

Fixes: 6dc83230b43b ("net/bnxt: support port representor data path")

Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/bnxt_reps.c |  6 ++++--
 drivers/net/bnxt/bnxt_rxr.c  | 27 +++++++++++++--------------
 2 files changed, 17 insertions(+), 16 deletions(-)
  

Patch

diff --git a/drivers/net/bnxt/bnxt_reps.c b/drivers/net/bnxt/bnxt_reps.c
index 7350c0967..17010f1ee 100644
--- a/drivers/net/bnxt/bnxt_reps.c
+++ b/drivers/net/bnxt/bnxt_reps.c
@@ -55,15 +55,17 @@  bnxt_vfr_recv(uint16_t port_id, uint16_t queue_id, struct rte_mbuf *mbuf)
 	mask = rep_rxr->rx_ring_struct->ring_mask;
 
 	/* Put this mbuf on the RxQ of the Representor */
-	prod_rx_buf = &rep_rxr->rx_buf_ring[rep_rxr->rx_prod++ & mask];
+	prod_rx_buf = &rep_rxr->rx_buf_ring[rep_rxr->rx_prod & mask];
 	if (!*prod_rx_buf) {
 		*prod_rx_buf = mbuf;
 		vfr_bp->rx_bytes[que] += mbuf->pkt_len;
 		vfr_bp->rx_pkts[que]++;
+		rep_rxr->rx_prod++;
 	} else {
+		/* Representor Rx ring full, drop pkt */
 		vfr_bp->rx_drop_bytes[que] += mbuf->pkt_len;
 		vfr_bp->rx_drop_pkts[que]++;
-		rte_pktmbuf_free(mbuf); /* Representor Rx ring full, drop pkt */
+		rte_pktmbuf_free(mbuf);
 	}
 
 	return 0;
diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
index ca3e0a521..039217fa6 100644
--- a/drivers/net/bnxt/bnxt_rxr.c
+++ b/drivers/net/bnxt/bnxt_rxr.c
@@ -795,6 +795,19 @@  static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
 		goto rx;
 	}
 	rxr->rx_prod = prod;
+
+	if (BNXT_TRUFLOW_EN(bp) && (BNXT_VF_IS_TRUSTED(bp) || BNXT_PF(bp)) &&
+	    vfr_flag) {
+		bnxt_vfr_recv(mark_id, rxq->queue_id, mbuf);
+		/* Now return an error so that nb_rx_pkts is not
+		 * incremented.
+		 * This packet was meant to be given to the representor.
+		 * So no need to account the packet and give it to
+		 * parent Rx burst function.
+		 */
+		rc = -ENODEV;
+		goto next_rx;
+	}
 	/*
 	 * All MBUFs are allocated with the same size under DPDK,
 	 * no optimization for rx_copy_thresh
@@ -802,20 +815,6 @@  static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
 rx:
 	*rx_pkt = mbuf;
 
-	if (BNXT_TRUFLOW_EN(bp) &&
-	    (BNXT_VF_IS_TRUSTED(bp) || BNXT_PF(bp)) &&
-	    vfr_flag) {
-		if (!bnxt_vfr_recv(mark_id, rxq->queue_id, mbuf)) {
-			/* Now return an error so that nb_rx_pkts is not
-			 * incremented.
-			 * This packet was meant to be given to the representor.
-			 * So no need to account the packet and give it to
-			 * parent Rx burst function.
-			 */
-			rc = -ENODEV;
-		}
-	}
-
 next_rx:
 
 	*raw_cons = tmp_raw_cons;