net/hns3: fix crash when Tx multiple buffer packets

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@dpdk.org

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
This commit is contained in:
Chengchang Tang 2020-09-09 17:23:39 +08:00 committed by Ferruh Yigit
parent db433d5f75
commit 0134a5c7b4

View File

@ -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;