From: Yi Yang <yangy...@inspur.com> In GSO case, segmented mbufs are attached to original mbuf which can't be freed when it is external. The issue is free_cb doesn't know original mbuf and doesn't free it when refcnt of shinfo is 0.
Original mbuf can be freed by rte_pktmbuf_free segmented mbufs or by rte_pktmbuf_free original mbuf. Two kind of cases should have different behaviors. free_cb won't explicitly call rte_pktmbuf_free to free original mbuf if it is freed by rte_pktmbuf_free original mbuf, but it has to call rte_pktmbuf_free to free original mbuf if it is freed by rte_pktmbuf_free segmented mbufs. In order to fix this issue, free_cb interface has to been changed, __rte_pktmbuf_free_extbuf must deliver called mbuf pointer to free_cb, argument opaque can be defined as a custom struct by user, it can includes original mbuf pointer, user-defined free_cb can compare caller mbuf with mbuf in opaque struct, free_cb should free original mbuf if they are not same, this corresponds to rte_pktmbuf_free segmented mbufs case, otherwise, free_cb won't free original mbuf because the caller explicitly called rte_pktmbuf_free to free it. Here is pseduo code to show two kind of cases. case 1. rte_pktmbuf_free segmented mbufs nb_tx = rte_gso_segment(original_mbuf, /* original mbuf */ &gso_ctx, /* segmented mbuf */ (struct rte_mbuf **)&gso_mbufs, MAX_GSO_MBUFS); rte_eth_tx_burst(dev->port_id, qid, gso_mbufs, nb_tx); case 2. rte_pktmbuf_free original mbuf rte_eth_tx_burst(dev->port_id, qid, original_mbuf, 1); Here are user defined free_cb and opaque. struct shinfo_arg { void *buf; struct rte_mbuf *mbuf; }; custom_free_cb(struct rte_mbuf *caller_m, void *opaque) { struct shinfo_arg *arg = (struct shinfo_arg *)opaque; rte_free(arg->buf); if (caller_m != arg->mbuf) rte_pktmbuf_free(arg->mbuf); rte_free(arg); } struct shinfo_arg *arg = (struct shinfo_arg *) rte_malloc(NULL, sizeof(struct shinfo_arg), RTE_CACHE_LINE_SIZE); arg->buf = buf; arg->mbuf = pkt; shinfo->free_cb = custom_free_cb; shinfo->fcb_opaque = arg; Signed-off-by: Yi Yang <yangy...@inspur.com> --- app/test-compress-perf/comp_perf_test_common.c | 2 +- app/test/test_compressdev.c | 2 +- app/test/test_mbuf.c | 2 +- drivers/net/mlx5/mlx5_rxtx.c | 2 +- drivers/net/mlx5/mlx5_rxtx.h | 2 +- drivers/net/netvsc/hn_rxtx.c | 3 ++- lib/librte_mbuf/rte_mbuf.c | 4 ++-- lib/librte_mbuf/rte_mbuf.h | 2 +- lib/librte_mbuf/rte_mbuf_core.h | 2 +- lib/librte_vhost/virtio_net.c | 2 +- 10 files changed, 12 insertions(+), 11 deletions(-) diff --git a/app/test-compress-perf/comp_perf_test_common.c b/app/test-compress-perf/comp_perf_test_common.c index b402a0d..b1eb733 100644 --- a/app/test-compress-perf/comp_perf_test_common.c +++ b/app/test-compress-perf/comp_perf_test_common.c @@ -115,7 +115,7 @@ struct cperf_buffer_info { } static void -comp_perf_extbuf_free_cb(void *addr __rte_unused, void *opaque __rte_unused) +comp_perf_extbuf_free_cb(struct rte_mbuf *m __rte_unused, void *opaque __rte_unused) { } diff --git a/app/test/test_compressdev.c b/app/test/test_compressdev.c index 0571c17..a4a5d7c 100644 --- a/app/test/test_compressdev.c +++ b/app/test/test_compressdev.c @@ -778,7 +778,7 @@ struct test_private_arrays { } static void -extbuf_free_callback(void *addr __rte_unused, void *opaque __rte_unused) +extbuf_free_callback(struct rte_mbuf *m __rte_unused, void *opaque __rte_unused) { } diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c index 06e44f0..f9e5ca5 100644 --- a/app/test/test_mbuf.c +++ b/app/test/test_mbuf.c @@ -2300,7 +2300,7 @@ struct test_case { /* Define a free call back function to be used for external buffer */ static void -ext_buf_free_callback_fn(void *addr __rte_unused, void *opaque) +ext_buf_free_callback_fn(struct rte_mbuf *caller_m __rte_unused, void *opaque) { void *ext_buf_addr = opaque; diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c index 3eb0243..f084488 100644 --- a/drivers/net/mlx5/mlx5_rxtx.c +++ b/drivers/net/mlx5/mlx5_rxtx.c @@ -1622,7 +1622,7 @@ enum mlx5_txcmp_code { } void -mlx5_mprq_buf_free_cb(void *addr __rte_unused, void *opaque) +mlx5_mprq_buf_free_cb(struct rte_mbuf *caller_m __rte_unused, void *opaque) { struct mlx5_mprq_buf *buf = opaque; diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h index c02a007..8c230c6 100644 --- a/drivers/net/mlx5/mlx5_rxtx.h +++ b/drivers/net/mlx5/mlx5_rxtx.h @@ -499,7 +499,7 @@ struct mlx5_txq_ctrl *mlx5_txq_hairpin_new uint16_t mlx5_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n); void mlx5_rxq_initialize(struct mlx5_rxq_data *rxq); __rte_noinline int mlx5_rx_err_handle(struct mlx5_rxq_data *rxq, uint8_t vec); -void mlx5_mprq_buf_free_cb(void *addr, void *opaque); +void mlx5_mprq_buf_free_cb(struct rte_mbuf *caller_m, void *opaque); void mlx5_mprq_buf_free(struct mlx5_mprq_buf *buf); uint16_t mlx5_rx_burst_mprq(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n); diff --git a/drivers/net/netvsc/hn_rxtx.c b/drivers/net/netvsc/hn_rxtx.c index 86a4c0d..30af404 100644 --- a/drivers/net/netvsc/hn_rxtx.c +++ b/drivers/net/netvsc/hn_rxtx.c @@ -519,7 +519,8 @@ int hn_dev_tx_descriptor_status(void *arg, uint16_t offset) return 0; } -static void hn_rx_buf_free_cb(void *buf __rte_unused, void *opaque) +static void hn_rx_buf_free_cb(struct rte_mbuf *caller_m __rte_unused, + void *opaque) { struct hn_rx_bufinfo *rxb = opaque; struct hn_data *hv = rxb->hv; diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c index 8a456e5..52445b3 100644 --- a/lib/librte_mbuf/rte_mbuf.c +++ b/lib/librte_mbuf/rte_mbuf.c @@ -118,11 +118,11 @@ * buffer. */ static void -rte_pktmbuf_free_pinned_extmem(void *addr, void *opaque) +rte_pktmbuf_free_pinned_extmem(struct rte_mbuf *caller_m, void *opaque) { struct rte_mbuf *m = opaque; - RTE_SET_USED(addr); + RTE_SET_USED(caller_m); RTE_ASSERT(RTE_MBUF_HAS_EXTBUF(m)); RTE_ASSERT(RTE_MBUF_HAS_PINNED_EXTBUF(m)); RTE_ASSERT(m->shinfo->fcb_opaque == m); diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index 7259575..5f8626f 100644 --- a/lib/librte_mbuf/rte_mbuf.h +++ b/lib/librte_mbuf/rte_mbuf.h @@ -1193,7 +1193,7 @@ static inline void rte_pktmbuf_attach(struct rte_mbuf *mi, struct rte_mbuf *m) RTE_ASSERT(m->shinfo != NULL); if (rte_mbuf_ext_refcnt_update(m->shinfo, -1) == 0) - m->shinfo->free_cb(m->buf_addr, m->shinfo->fcb_opaque); + m->shinfo->free_cb(m, m->shinfo->fcb_opaque); } /** diff --git a/lib/librte_mbuf/rte_mbuf_core.h b/lib/librte_mbuf/rte_mbuf_core.h index 8cd7137..d194429 100644 --- a/lib/librte_mbuf/rte_mbuf_core.h +++ b/lib/librte_mbuf/rte_mbuf_core.h @@ -671,7 +671,7 @@ struct rte_mbuf { /** * Function typedef of callback to free externally attached buffer. */ -typedef void (*rte_mbuf_extbuf_free_callback_t)(void *addr, void *opaque); +typedef void (*rte_mbuf_extbuf_free_callback_t)(struct rte_mbuf *, void *); /** * Shared data at the end of an external buffer. diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c index 95a0bc1..e663fd4 100644 --- a/lib/librte_vhost/virtio_net.c +++ b/lib/librte_vhost/virtio_net.c @@ -2137,7 +2137,7 @@ uint16_t rte_vhost_poll_enqueue_completed(int vid, uint16_t queue_id, } static void -virtio_dev_extbuf_free(void *addr __rte_unused, void *opaque) +virtio_dev_extbuf_free(struct rte_mbuf * caller_m __rte_unused, void *opaque) { rte_free(opaque); } -- 1.8.3.1