app/testpmd: fix testpmd crash when quit with mlx5 avail_thresh enabled

PATCH In review Spike Du 4 participants 5 messages Oct 24, 2022 – Nov 6, 2022 Network drivers

Share & tools

Spike Du

author

2022-10-24 03:14 UTC #0 lore raw

PATCH 1/1 app/testpmd: fix testpmd crash when quit with mlx5 avail_thresh enabled

When testpmd quit with mlx5 avail_thresh enabled, a rte timer handler delays to reconfigure rx queue to re-arm this event. However at the same time, testpmd is destroying rx queues.
It's never a valid use case for mlx5 avail_thresh. Before testpmd quit, user should disable avail_thresh configuration to not handle the events. This is documented in mlx5 driver guide.

To avoid the crash in such use case, check port status, if it is not RTE_PORT_STARTED, don't process the avail_thresh event.

Fixes 0edfc9b08316 ("app/testpmd: add Host Shaper command")

Signed-off-by Spike Du <[email protected]>

---
 drivers/net/mlx5/mlx5_testpmd.c | 9 +++++++++
 1 file changed, 9 insertions(+)

drivers/net/mlx5/mlx5_testpmd.c +9 -0

index ed84583..1a9ec78 100644
@@ -25,6 +25,7 @@
 
 static uint8_t host_shaper_avail_thresh_triggered[RTE_MAX_ETHPORTS];
 #define SHAPER_DISABLE_DELAY_US 100000 /* 100ms */
+extern struct rte_port *ports;
 
 /**
  * Disable the host shaper and re-arm available descriptor threshold event.

@@ -39,7 +40,15 @@
 	uint16_t port_id = port_rxq_id & 0xffff;
 	uint16_t qid = (port_rxq_id >> 16) & 0xffff;
 	struct rte_eth_rxq_info qinfo;
+	struct rte_port *port;
 
+	port = &ports[port_id];
+	if (port->port_status != RTE_PORT_STARTED) {
+		printf("%s port_status(%d) is incorrect, stop avail_thresh "
+		       "event processing.\n",
+		       __func__, port->port_status);
+		return;
+	}
 	printf("%s disable shaper\n", __func__);
 	if (rte_eth_rx_queue_info_get(port_id, qid, &qinfo)) {
 		printf("rx_queue_info_get returns error\n");

Signature · 1 line

1.8.3.1

Singh, Aman Deep

Acked-by on v1 1/1

2022-10-27 09:05 UTC #1 lore raw

Quoted 7 lines · Spike Du

When testpmd quit with mlx5 avail_thresh enabled, a rte timer handler delays to reconfigure rx queue to re-arm this event. However at the same time, testpmd is destroying rx queues.
It's never a valid use case for mlx5 avail_thresh. Before testpmd quit, user should disable avail_thresh configuration to not handle the events. This is documented in mlx5 driver guide.

Quoted 6 lines

To avoid the crash in such use case, check port status, if it is not RTE_PORT_STARTED, don't process the avail_thresh event.
Fixes 0edfc9b08316 ("app/testpmd: add Host Shaper command")
Signed-off-by Spike Du <[email protected]>

Acked-by Aman Singh<[email protected]>

drivers/net/mlx5/mlx5_testpmd.c line 28

Quoted 13 lines

---
  drivers/net/mlx5/mlx5_testpmd.c | 9 +++++++++
  1 file changed, 9 insertions(+)

drivers/net/mlx5/mlx5_testpmd.c +1 -0

index ed84583..1a9ec78 100644
@@ -25,6 +25,7 @@
 
  static uint8_t host_shaper_avail_thresh_triggered[RTE_MAX_ETHPORTS];
  #define SHAPER_DISABLE_DELAY_US 100000 /* 100ms */
+extern struct rte_port *ports;

This might not be needed, as we have testpmd.h included.

Quoted 19 lines

drivers/net/mlx5/mlx5_testpmd.c +8 -0

  /**
   * Disable the host shaper and re-arm available descriptor threshold event.

@@ -39,7 +40,15 @@
  	uint16_t port_id = port_rxq_id & 0xffff;
  	uint16_t qid = (port_rxq_id >> 16) & 0xffff;
  	struct rte_eth_rxq_info qinfo;
+	struct rte_port *port;
 
+	port = &ports[port_id];
+	if (port->port_status != RTE_PORT_STARTED) {
+		printf("%s port_status(%d) is incorrect, stop avail_thresh "
+		       "event processing.\n",
+		       __func__, port->port_status);
+		return;
+	}
  	printf("%s disable shaper\n", __func__);
  	if (rte_eth_rx_queue_info_get(port_id, qid, &qinfo)) {
  		printf("rx_queue_info_get returns error\n");

Spike Du

author

2022-11-02 11:45 UTC #2 lore raw

PATCH v2 1/1 mlx5/testpmd: fix crash on quit with avail thresh enabled

In reply to Spike Du

To avoid the crash in such use case, check port status, if it is not RTE_PORT_STARTED, don't process the avail_thresh event.

Fixes f41a5092e6ae ("app/testpmd: add host shaper command")

Cc [email protected]

Signed-off-by Spike Du <[email protected]>

---
 drivers/net/mlx5/mlx5_testpmd.c | 8 ++++++++
 1 file changed, 8 insertions(+)

drivers/net/mlx5/mlx5_testpmd.c +8 -0

index ed84583..879ea28 100644
@@ -39,7 +39,15 @@
 	uint16_t port_id = port_rxq_id & 0xffff;
 	uint16_t qid = (port_rxq_id >> 16) & 0xffff;
 	struct rte_eth_rxq_info qinfo;
+	struct rte_port *port;
 
+	port = &ports[port_id];
+	if (port->port_status != RTE_PORT_STARTED) {
+		printf("%s port_status(%d) is incorrect, stop avail_thresh "
+		       "event processing.\n",
+		       __func__, port->port_status);
+		return;
+	}
 	printf("%s disable shaper\n", __func__);
 	if (rte_eth_rx_queue_info_get(port_id, qid, &qinfo)) {
 		printf("rx_queue_info_get returns error\n");

Signature · 1 line

1.8.3.1

Matan Azrad

Acked-by on v2 1/1

2022-11-06 13:26 UTC #3 lore raw

RE: [PATCH v2] mlx5/testpmd: fix crash on quit with avail thresh enabled

From: Spike Du <[email protected]>

Quoted 7 lines

When testpmd quit with mlx5 avail_thresh enabled, a rte timer handler delays to reconfigure rx queue to re-arm this event. However at the same time, testpmd is destroying rx queues.
It's never a valid use case for mlx5 avail_thresh. Before testpmd quit, user should disable avail_thresh configuration to not handle the events. This is documented in mlx5 driver guide.

Quoted 7 lines

To avoid the crash in such use case, check port status, if it is not RTE_PORT_STARTED, don't process the avail_thresh event.
Fixes f41a5092e6ae ("app/testpmd: add host shaper command")
Cc [email protected]
Signed-off-by Spike Du <[email protected]>

Acked-by Matan Azrad <[email protected]>

Raslan Darawsheh

2022-11-06 15:42 UTC #4 lore raw

RE: [PATCH v2] mlx5/testpmd: fix crash on quit with avail thresh enabled

In reply to Spike Du

Hi,

Quoted 18 lines

-----Original Message-----
From: Spike Du <[email protected]>
Sent: Wednesday, November 2, 2022 1:44 PM
To: Matan Azrad <[email protected]>; Slava Ovsiienko <[email protected]>; Ori Kam <[email protected]>; NBU-Contact- Thomas Monjalon (EXTERNAL) <[email protected]>; Shahaf Shuler <[email protected]>
Cc: [email protected]; Raslan Darawsheh <[email protected]>; [email protected]
Subject: [PATCH v2] mlx5/testpmd: fix crash on quit with avail thresh enabled
When testpmd quit with mlx5 avail_thresh enabled, a rte timer handler delays to reconfigure rx queue to re-arm this event. However at the same time, testpmd is destroying rx queues.
It's never a valid use case for mlx5 avail_thresh. Before testpmd quit, user should disable avail_thresh configuration to not handle the events. This is documented in mlx5 driver guide.

Quoted 7 lines

To avoid the crash in such use case, check port status, if it is not RTE_PORT_STARTED, don't process the avail_thresh event.
Fixes f41a5092e6ae ("app/testpmd: add host shaper command")
Cc [email protected]
Signed-off-by Spike Du <[email protected]>

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh