From: Chengchang Tang <tangchengch...@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: sta...@dpdk.org
Signed-off-by: Chengchang Tang <tangchengch...@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.hu...@huawei.com>
Signed-off-by: Chengwen Feng <fengcheng...@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 ada02de..68d7a6a 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
