[dpdk-dev] [PATCH 8/8] net/hns3: fix segfault when Tx multiple buffer packets

Wei Hu (Xavier) huwei013 at chinasoftinc.com
Mon Sep 7 11:08:25 CEST 2020


From: Chengchang Tang <tangchengchang at huawei.com>

Currently, there is a possibility that segment faults occur when sending
packets whose payloads are stored in multiple buffers based on hns3 network
engine. The related core dump information as follows:

Program terminated with signal 11, Segmentation fault.
0  hns3_reassemble_tx_pkts
2512                            temp = temp->next;
Missing separate debuginfos, use:
(gdb) bt
0  hns3_reassemble_tx_pkts
1  0x0000000000969c60 in hns3_check_non_tso_pkt
2  0x000000000096adbc in hns3_xmit_pkts
3  0x000000000050d4d0 in rte_eth_tx_burst
4  0x000000000050fca4 in pkt_burst_transmit
5  0x00000000004ca6b8 in run_pkt_fwd_on_lcore
6  0x00000000004ca7fc in start_pkt_forward_on_core
7  0x00000000006975a4 in eal_thread_loop
8  0x0000ffffa6f7fc48 in start_thread
9  0x0000ffffa6ed1600 in thread_start

The root cause is that hns3 PMD driver invokes the rte_pktmbuf_free_seg API
function to release the same rte_mbuf multiple times. The rte_mbuf pointer
is not set to NULL in the internal function hns3_rx_queue_release_mbufs
which is invoked during queue setup, stop and close. As a result the
rte_mbuf in Rx queues will be repeatedly released when the user application
setup queues or stop/start the dev for multiple times. Probably for
performance reasons, DPDK mempool lib does not check for the repeated
rte_mbuf releases. The Address of released rte_mbuf are directly stored
into the per lcore cache of the mempool. This makes the rte_mbufs obtained
from mempool by calling rte_mempool_get_bulk API function repetitively.
ultimately, it causes to access to a NULL pointer in PMD driver.

This patch fixes this problem by setting released mbuf pointer to NULL in
the internal function named hns3_rx_queue_release_mbuf. And the other
internal function named hns3_reassemble_tx_pkts is optimized to avoid a
similar problem.

Fixes: bba636698316 ("net/hns3: support Rx/Tx and related operations")
Cc: stable at dpdk.org

Signed-off-by: Chengchang Tang <tangchengchang at huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei at huawei.com>
Signed-off-by: Chengwen Feng <fengchengwen at huawei.com>
---
 drivers/net/hns3/hns3_rxtx.c | 61 +++++++++++++++++---------------------------
 1 file changed, 23 insertions(+), 38 deletions(-)

diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index 03d69b1..1a1f828 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -43,15 +43,19 @@ hns3_rx_queue_release_mbufs(struct hns3_rx_queue *rxq)
 
 	if (rxq->rx_rearm_nb == 0) {
 		for (i = 0; i < rxq->nb_rx_desc; i++) {
-			if (rxq->sw_ring[i].mbuf != NULL)
+			if (rxq->sw_ring[i].mbuf != NULL) {
 				rte_pktmbuf_free_seg(rxq->sw_ring[i].mbuf);
+				rxq->sw_ring[i].mbuf = NULL;
+			}
 		}
 	} else {
 		for (i = rxq->next_to_use;
 		     i != rxq->rx_rearm_start;
 		     i = (i + 1) % rxq->nb_rx_desc) {
-			if (rxq->sw_ring[i].mbuf != NULL)
+			if (rxq->sw_ring[i].mbuf != NULL) {
 				rte_pktmbuf_free_seg(rxq->sw_ring[i].mbuf);
+				rxq->sw_ring[i].mbuf = NULL;
+			}
 		}
 	}
 
@@ -2368,37 +2372,24 @@ hns3_fill_first_desc(struct hns3_tx_queue *txq, struct hns3_desc *desc,
 	}
 }
 
-static int
-hns3_tx_alloc_mbufs(struct hns3_tx_queue *txq, struct rte_mempool *mb_pool,
-		    uint16_t nb_new_buf, struct rte_mbuf **alloc_mbuf)
+static inline int
+hns3_tx_alloc_mbufs(struct rte_mempool *mb_pool, uint16_t nb_new_buf,
+			struct rte_mbuf **alloc_mbuf)
 {
-	struct rte_mbuf *new_mbuf = NULL;
-	struct rte_eth_dev *dev;
-	struct rte_mbuf *temp;
-	struct hns3_hw *hw;
+#define MAX_NON_TSO_BD_PER_PKT 18
+	struct rte_mbuf *pkt_segs[MAX_NON_TSO_BD_PER_PKT];
 	uint16_t i;
 
 	/* Allocate enough mbufs */
-	for (i = 0; i < nb_new_buf; i++) {
-		temp = rte_pktmbuf_alloc(mb_pool);
-		if (unlikely(temp == NULL)) {
-			dev = &rte_eth_devices[txq->port_id];
-			hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-			hns3_err(hw, "Failed to alloc TX mbuf port_id=%d,"
-				     "queue_id=%d in reassemble tx pkts.",
-				     txq->port_id, txq->queue_id);
-			rte_pktmbuf_free(new_mbuf);
-			return -ENOMEM;
-		}
-		temp->next = new_mbuf;
-		new_mbuf = temp;
-	}
-
-	if (new_mbuf == NULL)
+	if (rte_mempool_get_bulk(mb_pool, (void **)pkt_segs, nb_new_buf))
 		return -ENOMEM;
 
-	new_mbuf->nb_segs = nb_new_buf;
-	*alloc_mbuf = new_mbuf;
+	for (i = 0; i < nb_new_buf - 1; i++)
+		pkt_segs[i]->next = pkt_segs[i + 1];
+
+	pkt_segs[nb_new_buf - 1]->next = NULL;
+	pkt_segs[0]->nb_segs = nb_new_buf;
+	*alloc_mbuf = pkt_segs[0];
 
 	return 0;
 }
@@ -2418,10 +2409,8 @@ hns3_pktmbuf_copy_hdr(struct rte_mbuf *new_pkt, struct rte_mbuf *old_pkt)
 }
 
 static int
-hns3_reassemble_tx_pkts(void *tx_queue, struct rte_mbuf *tx_pkt,
-			struct rte_mbuf **new_pkt)
+hns3_reassemble_tx_pkts(struct rte_mbuf *tx_pkt, struct rte_mbuf **new_pkt)
 {
-	struct hns3_tx_queue *txq = tx_queue;
 	struct rte_mempool *mb_pool;
 	struct rte_mbuf *new_mbuf;
 	struct rte_mbuf *temp_new;
@@ -2433,7 +2422,6 @@ hns3_reassemble_tx_pkts(void *tx_queue, struct rte_mbuf *tx_pkt,
 	uint16_t len_s;
 	uint16_t len_d;
 	uint16_t len;
-	uint16_t i;
 	int ret;
 	char *s;
 	char *d;
@@ -2449,7 +2437,7 @@ hns3_reassemble_tx_pkts(void *tx_queue, struct rte_mbuf *tx_pkt,
 		last_buf_len = buf_size;
 
 	/* Allocate enough mbufs */
-	ret = hns3_tx_alloc_mbufs(txq, mb_pool, nb_new_buf, &new_mbuf);
+	ret = hns3_tx_alloc_mbufs(mb_pool, nb_new_buf, &new_mbuf);
 	if (ret)
 		return ret;
 
@@ -2458,12 +2446,9 @@ hns3_reassemble_tx_pkts(void *tx_queue, struct rte_mbuf *tx_pkt,
 	s = rte_pktmbuf_mtod(temp, char *);
 	len_s = rte_pktmbuf_data_len(temp);
 	temp_new = new_mbuf;
-	for (i = 0; i < nb_new_buf; i++) {
+	while (temp != NULL && temp_new != NULL) {
 		d = rte_pktmbuf_mtod(temp_new, char *);
-		if (i < nb_new_buf - 1)
-			buf_len = buf_size;
-		else
-			buf_len = last_buf_len;
+		buf_len = temp_new->next == NULL ? last_buf_len : buf_size;
 		len_d = buf_len;
 
 		while (len_d) {
@@ -2924,7 +2909,7 @@ hns3_check_non_tso_pkt(uint16_t nb_buf, struct rte_mbuf **m_seg,
 
 	if (unlikely(nb_buf > HNS3_MAX_NON_TSO_BD_PER_PKT)) {
 		txq->exceed_limit_bd_pkt_cnt++;
-		ret = hns3_reassemble_tx_pkts(txq, tx_pkt, &new_pkt);
+		ret = hns3_reassemble_tx_pkts(tx_pkt, &new_pkt);
 		if (ret) {
 			txq->exceed_limit_bd_reassem_fail++;
 			return ret;
-- 
2.9.5



More information about the dev mailing list