This addresses two bugs in mbuf operations history recording.
1. With mbuf operations history recording enabled, when allocating mbufs
from a mempool failed, the array of fetched mbuf pointers was not set, but
it was dereferenced for mbuf operations history recording anyway, which
would trigger a segmentation fault or cause undefined behavior.
This was fixed by changing how the return value from the mempool
allocation is checked, so the function returns early on failure, and only
proceeds on success.
2. When allocating a bulk of mbufs using rte_pktmbuf_alloc_bulk(), two
mbuf library allocation operations were recorded on the mbuf, because the
function calls rte_mbuf_raw_alloc_bulk() for allocation, and both
functions record a mbuf library allocation operation.
This was fixed by not recording a mbuf library allocation operation in
rte_pktmbuf_alloc_bulk().
3. When freeing a bulk of segmented mbufs, the free operations were only
recorded on the first segments.
This was fixed by freeing the pending bulks of segments using
rte_mbuf_raw_free_bulk(), which records the free operation on the mbufs,
instead of calling rte_mempool_put_bulk() directly.
The bulk operation recording at the start of the function, which only
affected the first segments of segmented packets, was removed.
Fixes: d265a24a32a4 ("mbuf: record mbuf operations history")
Cc: [email protected]
Signed-off-by: Morten Brørup <[email protected]>
Acked-by: Thomas Monjalon <[email protected]>
Acked-by: Konstantin Ananyev <[email protected]>
---
v2:
* Added fix for rte_pktmbuf_free_bulk.
---
lib/mbuf/rte_mbuf.c | 8 +++-----
lib/mbuf/rte_mbuf.h | 12 +++++-------
2 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
index c2476e7704..005bfaa573 100644
--- a/lib/mbuf/rte_mbuf.c
+++ b/lib/mbuf/rte_mbuf.c
@@ -540,8 +540,8 @@ __rte_pktmbuf_free_seg_via_array(struct rte_mbuf *m,
if (likely(m != NULL)) {
if (*nb_pending == pending_sz ||
(*nb_pending > 0 && m->pool != pending[0]->pool)) {
- rte_mempool_put_bulk(pending[0]->pool,
- (void **)pending, *nb_pending);
+ rte_mbuf_raw_free_bulk(pending[0]->pool,
+ pending, *nb_pending);
*nb_pending = 0;
}
@@ -562,8 +562,6 @@ void rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs,
unsigned int count)
struct rte_mbuf *m, *m_next, *pending[RTE_PKTMBUF_FREE_PENDING_SZ];
unsigned int idx, nb_pending = 0;
- rte_mbuf_history_mark_bulk(mbufs, count, RTE_MBUF_HISTORY_OP_LIB_FREE);
-
for (idx = 0; idx < count; idx++) {
m = mbufs[idx];
if (unlikely(m == NULL))
@@ -581,7 +579,7 @@ void rte_pktmbuf_free_bulk(struct rte_mbuf **mbufs,
unsigned int count)
}
if (nb_pending > 0)
- rte_mempool_put_bulk(pending[0]->pool, (void **)pending,
nb_pending);
+ rte_mbuf_raw_free_bulk(pending[0]->pool, pending, nb_pending);
}
/* Creates a shallow copy of mbuf */
diff --git a/lib/mbuf/rte_mbuf.h b/lib/mbuf/rte_mbuf.h
index e7c3bbadd4..60ec8158cd 100644
--- a/lib/mbuf/rte_mbuf.h
+++ b/lib/mbuf/rte_mbuf.h
@@ -663,14 +663,14 @@ static __rte_always_inline int
rte_mbuf_raw_alloc_bulk(struct rte_mempool *mp, struct rte_mbuf **mbufs,
unsigned int count)
{
int rc = rte_mempool_get_bulk(mp, (void **)mbufs, count);
- if (likely(rc == 0)) {
- for (unsigned int idx = 0; idx < count; idx++)
- __rte_mbuf_raw_sanity_check_mp(mbufs[idx], mp);
- }
+ if (unlikely(rc))
+ return rc;
+ for (unsigned int idx = 0; idx < count; idx++)
+ __rte_mbuf_raw_sanity_check_mp(mbufs[idx], mp);
rte_mbuf_history_mark_bulk(mbufs, count, RTE_MBUF_HISTORY_OP_LIB_ALLOC);
- return rc;
+ return 0;
}
/**
@@ -1068,8 +1068,6 @@ static inline int rte_pktmbuf_alloc_bulk(struct
rte_mempool *pool,
if (unlikely(rc))
return rc;
- rte_mbuf_history_mark_bulk(mbufs, count, RTE_MBUF_HISTORY_OP_LIB_ALLOC);
-
rte_mbuf_raw_reset_bulk(pool, mbufs, count);
return 0;
--
2.43.0
