> From: Stephen Hemminger [mailto:[email protected]] > Sent: Friday, 16 January 2026 06.55 > > On Thu, 15 Jan 2026 11:53:19 +0100 > Morten Brørup <[email protected]> wrote: > > > Stephen, > > > > As the author of rte_pktmbuf_copy(), can you please review this > patch? > > > > You might find my answers to Konstantin's review informative: > > https://patchwork.dpdk.org/project/dpdk/patch/20251119120403.907511- > [email protected]/#181914 > > > > > > Venlig hilsen / Kind regards, > > -Morten Brørup > > > > > > 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; > > It makes more sense to return NULL (as error) rather than creating a 0 > length mbuf in this corner case.
As replied to Kontantin, 0 length buffers are perfectly valid, so a library should not optimize them away, on an assumption that they are useless. E.g. consider TCP, which carries many empty packets, adding feedback information (ACK, SACK) to the TCP header. I know it's a very theoretical example stripping all headers and then adding headers again; but I can creatively imagine something like that. If a normal application don't want to deal with 0 length buffers, it can optimize them away. But if an exotic application does want to deal with 0 length buffers, it would be a bug if the library optimized them away. > > > 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); > > Should have space in expression. Yes. Copy-paste bug. :-) > At that point it is a new mbuf (the copy) so offload flags should be > clear, not sure > what the issue is here. > But hadn't expected usage of this function with an external mbuf pool. I consider pinned external buffers exotic too, but we need to support them throughout DPDK. That's the downside of exotic features in core libraries. > > > > > > prev = &mc->next; > > m_last = mc;
