[dpdk-dev] [PATCH 1/5] net/bnx2x: fix packet drop

[dpdk-dev] [PATCH 1/5] net/bnx2x: fix packet drop

[Shahed Shaikh]

Patch "8bd31421c593 ("net/bnx2x: fix ramrod timeout")"
introduced a regression where sc->scan_fp flags is
set for unexpectedly long time. So the slow path completion
handler flow is run unnecessarily which walks over receive
descriptor ring of fast path and drops the data packets while looking
for slow path completion descriptor out of fast path ring.

This issue is seen under heavy traffic with link events happening
in background.

Fixes: 8bd31421c593 ("net/bnx2x: fix ramrod timeout")
Cc: stable@dpdk.org

Signed-off-by: Shahed Shaikh <shshaikh@marvell.com>
---
 drivers/net/bnx2x/bnx2x.c    | 2 ++
 drivers/net/bnx2x/ecore_sp.c | 4 ----
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 3e705c7a1..0698b1a8a 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -4572,6 +4572,8 @@ static void bnx2x_handle_fp_tq(struct bnx2x_fastpath *fp)
 		}
 	}
 
+	/* Assuming we have completed slow path completion, clear the flag */
+	rte_atomic32_set(&sc->scan_fp, 0);
 	bnx2x_ack_sb(sc, fp->igu_sb_id, USTORM_ID,
 		   le16toh(fp->fp_hc_idx), IGU_INT_ENABLE, 1);
 }
diff --git a/drivers/net/bnx2x/ecore_sp.c b/drivers/net/bnx2x/ecore_sp.c
index 43194095b..5ac22e725 100644
--- a/drivers/net/bnx2x/ecore_sp.c
+++ b/drivers/net/bnx2x/ecore_sp.c
@@ -291,10 +291,6 @@ static int ecore_state_wait(struct bnx2x_softc *sc, int state,
 		cnt *= 20;
 
 	ECORE_MSG(sc, "waiting for state to become %d", state);
-	/* being over protective to remind bnx2x_intr_legacy() to
-	 * process RAMROD
-	 */
-	rte_atomic32_set(&sc->scan_fp, 1);
 
 	ECORE_MIGHT_SLEEP();
 	while (cnt--) {
-- 
2.12.3

[dpdk-dev] [PATCH 2/5] net/bnx2x: fix interrupt flood

[Shahed Shaikh]

PMD sets up and clears the slow path interrupt status block in dev_start
and dev_stop flow and slow path interrupt status block DMA memory for
device is allocated in dev_configure flow.

This situation creates a state where, after dev_stop is called, and if
there is a slow path interrupt from device, PMD sees the old value of
status block consumer in dev_start flow, since DMA memory for status block
belongs to old configuration and dev_start will result in
new slow path interrupt status block configuration.
And since PMD fails to ack new slow path interrupt with correct status
block consumer value, device continues to trigger interrupt causing an
interrupt flood.

Fix is to create and destroy status block DMA memory in dev_start and
dev_stop flow instead of dev_configure and dev_close flow.

Fixes: 540a211084a7 ("bnx2x: driver core")
Cc: stable@dpdk.org

Signed-off-by: Shahed Shaikh <shshaikh@marvell.com>
---
 drivers/net/bnx2x/bnx2x.c        | 10 ++++++++++
 drivers/net/bnx2x/bnx2x_ethdev.c | 10 ----------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index 0698b1a8a..e9f05a73a 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -2401,6 +2401,9 @@ static void bnx2x_free_mem(struct bnx2x_softc *sc)
 	ecore_ilt_mem_op(sc, ILT_MEMOP_FREE);
 
 	bnx2x_free_ilt_lines_mem(sc);
+
+	/* free the host hardware/software hsi structures */
+	bnx2x_free_hsi_mem(sc);
 }
 
 static int bnx2x_alloc_mem(struct bnx2x_softc *sc)
@@ -2451,6 +2454,13 @@ static int bnx2x_alloc_mem(struct bnx2x_softc *sc)
 		return -1;
 	}
 
+	/* allocate the host hardware/software hsi structures */
+	if (bnx2x_alloc_hsi_mem(sc) != 0) {
+		PMD_DRV_LOG(ERR, sc, "bnx2x_alloc_hsi_mem was failed");
+		bnx2x_free_mem(sc);
+		return -ENXIO;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 3063aea64..5dfd708ef 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -207,13 +207,6 @@ bnx2x_dev_configure(struct rte_eth_dev *dev)
 		return -ENXIO;
 	}
 
-	/* allocate the host hardware/software hsi structures */
-	if (bnx2x_alloc_hsi_mem(sc) != 0) {
-		PMD_DRV_LOG(ERR, sc, "bnx2x_alloc_hsi_mem was failed");
-		bnx2x_free_ilt_mem(sc);
-		return -ENXIO;
-	}
-
 	bnx2x_dev_rxtx_init_dummy(dev);
 	return 0;
 }
@@ -294,9 +287,6 @@ bnx2x_dev_close(struct rte_eth_dev *dev)
 	bnx2x_dev_clear_queues(dev);
 	memset(&(dev->data->dev_link), 0 , sizeof(struct rte_eth_link));
 
-	/* free the host hardware/software hsi structures */
-	bnx2x_free_hsi_mem(sc);
-
 	/* free ilt */
 	bnx2x_free_ilt_mem(sc);
 }
-- 
2.12.3

[dpdk-dev] [PATCH 3/5] net/bnx2x: fix memory leak

[Shahed Shaikh]

bnx2x_free_hsi_mem() does not free DMA memory.
Fix it here.

Fixes: 540a211084a7 ("bnx2x: driver core")
Cc: stable@dpdk.org

Signed-off-by: Shahed Shaikh <shshaikh@marvell.com>
---
 drivers/net/bnx2x/bnx2x.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
index e9f05a73a..d3b260830 100644
--- a/drivers/net/bnx2x/bnx2x.c
+++ b/drivers/net/bnx2x/bnx2x.c
@@ -9018,36 +9018,42 @@ void bnx2x_free_hsi_mem(struct bnx2x_softc *sc)
 /*******************/
 
 		memset(&fp->status_block, 0, sizeof(fp->status_block));
+		bnx2x_dma_free(&fp->sb_dma);
 	}
 
 	/***************************/
 	/* FW DECOMPRESSION BUFFER */
 	/***************************/
 
+	bnx2x_dma_free(&sc->gz_buf_dma);
 	sc->gz_buf = NULL;
 
 	/*******************/
 	/* SLOW PATH QUEUE */
 	/*******************/
 
+	bnx2x_dma_free(&sc->spq_dma);
 	sc->spq = NULL;
 
 	/*************/
 	/* SLOW PATH */
 	/*************/
 
+	bnx2x_dma_free(&sc->sp_dma);
 	sc->sp = NULL;
 
 	/***************/
 	/* EVENT QUEUE */
 	/***************/
 
+	bnx2x_dma_free(&sc->eq_dma);
 	sc->eq = NULL;
 
 	/************************/
 	/* DEFAULT STATUS BLOCK */
 	/************************/
 
+	bnx2x_dma_free(&sc->def_sb_dma);
 	sc->def_sb = NULL;
 
 }
-- 
2.12.3

[dpdk-dev] [PATCH 4/5] net/bnx2x: fix link inconsistent state

[Shahed Shaikh]

Don't call bnx2x_link_status_update() from bnx2x_link_update().
Actual use case of bnx2x_link_status_update() is to update the link
status in shared memory between driver and MFW, and not to get the
link status from HW.

So ideally, bnx2x_link_status_update() should be called when there
is an actual link event or change in link status.

Calling bnx2x_link_status_update() from bnx2x_link_update() may
corrupt the data of link status in shared memory and result
in inconsistent state of link.

Fixes: 540a211084a7 ("bnx2x: driver core")
Cc: stable@dpdk.org

Signed-off-by: Shahed Shaikh <shshaikh@marvell.com>
---
 drivers/net/bnx2x/bnx2x_ethdev.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 5dfd708ef..df5634fda 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -88,7 +88,6 @@ bnx2x_link_update(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE(sc);
 
-	bnx2x_link_status_update(sc);
 	memset(&link, 0, sizeof(link));
 	mb();
 	link.link_speed = sc->link_vars.line_speed;
-- 
2.12.3

[dpdk-dev] [PATCH 5/5] net/bnx2x: fix supported max Rx and Tx descriptor count

[Shahed Shaikh]

Driver does not provide limit on number Rx and Tx descriptors per queue,
this may result in application configuring 64k descriptors (default set
by rte_eth_dev_info_get()) and further result in issues in PMD and HW
flows due to unsupported number.

Fixes: 540a211084a7 ("bnx2x: driver core")
Cc: stable@dpdk.org

Signed-off-by: Shahed Shaikh <shshaikh@marvell.com>
---
 drivers/net/bnx2x/bnx2x.h        | 10 ++++++++--
 drivers/net/bnx2x/bnx2x_ethdev.c |  5 +++++
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
index ef1688ff3..e4b4ecf1e 100644
--- a/drivers/net/bnx2x/bnx2x.h
+++ b/drivers/net/bnx2x/bnx2x.h
@@ -155,13 +155,14 @@ struct bnx2x_device_type {
  * Transmit Buffer Descriptor (tx_bd) definitions*
  */
 /* NUM_TX_PAGES must be a power of 2. */
+#define NUM_TX_PAGES		 16
 #define TOTAL_TX_BD_PER_PAGE     (BNX2X_PAGE_SIZE / sizeof(union eth_tx_bd_types)) /*  256 */
 #define USABLE_TX_BD_PER_PAGE    (TOTAL_TX_BD_PER_PAGE - 1)                      /*  255 */
 
 #define TOTAL_TX_BD(q)           (TOTAL_TX_BD_PER_PAGE * q->nb_tx_pages)         /*  512 */
 #define USABLE_TX_BD(q)          (USABLE_TX_BD_PER_PAGE * q->nb_tx_pages)        /*  510 */
 #define MAX_TX_BD(q)             (TOTAL_TX_BD(q) - 1)                            /*  511 */
-
+#define MAX_TX_AVAIL		 (USABLE_TX_BD_PER_PAGE * NUM_TX_PAGES - 2)
 #define NEXT_TX_BD(x)                                                   \
 	((((x) & USABLE_TX_BD_PER_PAGE) ==                              \
 	  (USABLE_TX_BD_PER_PAGE - 1)) ? (x) + 2 : (x) + 1)
@@ -182,13 +183,14 @@ struct bnx2x_device_type {
 /*
  * Receive Buffer Descriptor (rx_bd) definitions*
  */
-//#define NUM_RX_PAGES            1
+#define MAX_RX_PAGES            8
 #define TOTAL_RX_BD_PER_PAGE    (BNX2X_PAGE_SIZE / sizeof(struct eth_rx_bd))      /*  512 */
 #define USABLE_RX_BD_PER_PAGE   (TOTAL_RX_BD_PER_PAGE - 2)                      /*  510 */
 #define RX_BD_PER_PAGE_MASK     (TOTAL_RX_BD_PER_PAGE - 1)                      /*  511 */
 #define TOTAL_RX_BD(q)          (TOTAL_RX_BD_PER_PAGE * q->nb_rx_pages)         /*  512 */
 #define USABLE_RX_BD(q)         (USABLE_RX_BD_PER_PAGE * q->nb_rx_pages)        /*  510 */
 #define MAX_RX_BD(q)            (TOTAL_RX_BD(q) - 1)                            /*  511 */
+#define MAX_RX_AVAIL		(USABLE_RX_BD_PER_PAGE * MAX_RX_PAGES - 2)
 #define RX_BD_NEXT_PAGE_DESC_CNT 2
 
 #define NEXT_RX_BD(x)                                                   \
@@ -244,6 +246,10 @@ struct bnx2x_device_type {
 #define MIN_RX_AVAIL(sc)				\
 	((sc)->dropless_fc ? BD_TH_HI(sc) + 128 : 128)
 
+#define MIN_RX_SIZE_NONTPA_HW	ETH_MIN_RX_CQES_WITHOUT_TPA
+#define MIN_RX_SIZE_NONTPA	(RTE_MAX((uint32_t)MIN_RX_SIZE_NONTPA_HW,\
+					(uint32_t)MIN_RX_AVAIL(sc)))
+
 /*
  * dropless fc calculations for RCQs
  * Number of RCQs should be as number of buffers in BRB:
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index df5634fda..5be487765 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -480,6 +480,7 @@ static void
 bnx2x_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 {
 	struct bnx2x_softc *sc = dev->data->dev_private;
+
 	dev_info->max_rx_queues  = sc->max_rx_queues;
 	dev_info->max_tx_queues  = sc->max_tx_queues;
 	dev_info->min_rx_bufsize = BNX2X_MIN_RX_BUF_SIZE;
@@ -487,6 +488,10 @@ bnx2x_dev_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	dev_info->max_mac_addrs  = BNX2X_MAX_MAC_ADDRS;
 	dev_info->speed_capa = ETH_LINK_SPEED_10G | ETH_LINK_SPEED_20G;
 	dev_info->rx_offload_capa = DEV_RX_OFFLOAD_JUMBO_FRAME;
+
+	dev_info->rx_desc_lim.nb_max = MAX_RX_AVAIL;
+	dev_info->rx_desc_lim.nb_min = MIN_RX_SIZE_NONTPA;
+	dev_info->tx_desc_lim.nb_max = MAX_TX_AVAIL;
 }
 
 static int
-- 
2.12.3

Re: [dpdk-dev] [PATCH 1/5] net/bnx2x: fix packet drop

[Rasesh Mody]

>From: Shahed Shaikh <shshaikh@marvell.com>
>Sent: Tuesday, June 04, 2019 11:54 AM
>
>Patch "8bd31421c593 ("net/bnx2x: fix ramrod timeout")"
>introduced a regression where sc->scan_fp flags is set for unexpectedly long
>time. So the slow path completion handler flow is run unnecessarily which
>walks over receive descriptor ring of fast path and drops the data packets
>while looking for slow path completion descriptor out of fast path ring.
>
>This issue is seen under heavy traffic with link events happening in
>background.
>
>Fixes: 8bd31421c593 ("net/bnx2x: fix ramrod timeout")
>Cc: stable@dpdk.org
>
>Signed-off-by: Shahed Shaikh <shshaikh@marvell.com>
>---

For the whole series.

Acked-by: Rasesh Mody <rmody@marvell.com>

Re: [dpdk-dev] [PATCH 1/5] net/bnx2x: fix packet drop

[Ferruh Yigit]

On 6/5/2019 6:17 AM, Rasesh Mody wrote:
>> From: Shahed Shaikh <shshaikh@marvell.com>
>> Sent: Tuesday, June 04, 2019 11:54 AM
>>
>> Patch "8bd31421c593 ("net/bnx2x: fix ramrod timeout")"
>> introduced a regression where sc->scan_fp flags is set for unexpectedly long
>> time. So the slow path completion handler flow is run unnecessarily which
>> walks over receive descriptor ring of fast path and drops the data packets
>> while looking for slow path completion descriptor out of fast path ring.
>>
>> This issue is seen under heavy traffic with link events happening in
>> background.
>>
>> Fixes: 8bd31421c593 ("net/bnx2x: fix ramrod timeout")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Shahed Shaikh <shshaikh@marvell.com>
>> ---
> 
> For the whole series.
> 
> Acked-by: Rasesh Mody <rmody@marvell.com>
> 

Series applied to dpdk-next-net/master, thanks.