> > > > From: Morten Brørup [mailto:[email protected]] > > > > Sent: Wednesday, 19 November 2025 13.04 > > > > > > > > Requests for copying the at the end of a packet incorrectly > > returned > > > > NULL, > > > > as if copying past the end of a packet. > > > > > > > > When allocating copies from a mempool using pinned external > > buffers, > > > > the > > > > external flag was not preserved in these mbufs. > > > > > > > > Fixes: c3a90c381daa ("mbuf: add a copy routine") > > > > > > > > Signed-off-by: Morten Brørup <[email protected]> > > > > --- > > > > lib/mbuf/rte_mbuf.c | 6 +++--- > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c > > > > index 0d931c7a15..e639aff03e 100644 > > > > --- a/lib/mbuf/rte_mbuf.c > > > > +++ b/lib/mbuf/rte_mbuf.c > > > > @@ -675,7 +675,7 @@ rte_pktmbuf_copy(const struct rte_mbuf *m, > > struct > > > > rte_mempool *mp, > > > > __rte_mbuf_sanity_check(m, 1); > > > > > > > > /* check for request to copy at offset past end of mbuf */ > > > > - if (unlikely(off >= m->pkt_len)) > > > > + if (unlikely(off > m->pkt_len)) > > > > return NULL; > > > > So, when off= m->pkt_len, what do we want it to return? > > New mbuf with pkt_len == 0? > > Any point of such copying then? > > Empty packets are perfectly valid. > A library should not optimize them away; it is the application's choice how > to handle > empty packets. > > > > > > > mc = rte_pktmbuf_alloc(mp); > > > > @@ -688,8 +688,8 @@ rte_pktmbuf_copy(const struct rte_mbuf *m, > > struct > > > > rte_mempool *mp, > > > > > > > > __rte_pktmbuf_copy_hdr(mc, m); > > > > > > > > - /* copied mbuf is not indirect or external */ > > > > - mc->ol_flags = m->ol_flags & > > > > ~(RTE_MBUF_F_INDIRECT|RTE_MBUF_F_EXTERNAL); > > > > + /* copy flags except indirect and external, and preserve flags > > > > of > > > > newly allocated mbuf */ > > > > + mc->ol_flags |= m->ol_flags & > > > > ~(RTE_MBUF_F_INDIRECT|RTE_MBUF_F_EXTERNAL); > > > > Which flags in the new mbuf we want to preserve? > > AFAIK mbuf_alloc() doesn't set any flags. > > Correct: the newly allocated mbuf returned by mbuf_alloc() normally has no > flags > set. > However, if it is allocated from an mbuf pool holding mbufs using pinned > external > buffers, the RTE_MBUF_F_EXTERNAL is already set in the mbufs held in that > pool, > and will remain set when returning the mbuf by mbuf_alloc(). We want to > preserve > that flag. > > > BTW, if there are some flags that we would like to preserve, > > wouldn't that be a change in public API behavior? > > This patch does not change which flags to preserve from the source mbuf. > The patch only adds that the flags of the newly allocated mbuf are not > cleared when > copying the flags from the source mbuf.
Makes sense, thanks for clarifying.
