Re: [PATCH v2] mbuf: fix packet copy

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 16 Jan 2026 18:16:15 +0100
Morten Brørup <mb@smartsharesystems.com> wrote:

> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Friday, 16 January 2026 18.06
> > 
> > On Fri, 16 Jan 2026 11:16:21 +0000
> > Morten Brørup <mb@smartsharesystems.com> wrote:
> >   
> > > buf: fix packet copy
> > >
> > > Requests for copying the at the end of a packet incorrectly returned  
> > NULL,  
> > > as if copying past the end of a packet.
> > >
> > > When allocating the mbuf for the copy from a mempool using pinned  
> > external  
> > > buffers, the external flag in this mbuf was not preserved.
> > >
> > > Fixes: c3a90c381daa ("mbuf: add a copy routine")
> > >
> > > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > > Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>
> > > ---
> > > v2:
> > > * Improved comment about preserving flags for newly allocated mbuf
> > >   potentially using pinned external buffer.
> > > * Added missing spaces in expression. (Stephen)
> > > ---
> > >  lib/mbuf/rte_mbuf.c | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/lib/mbuf/rte_mbuf.c b/lib/mbuf/rte_mbuf.c
> > > index 0d931c7a15..a5d16e4c97 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;
> > >  
> > 
> > I still think asking for a copy of data that isn't there should return
> > NULL
> > not a zero length mbuf.  Kind of academic since I dont think any code
> > uses
> > non-zero offset now.  
> 
> Yes, I totally agree it's kind of academic.
> But I insist that it is an off-by-one bug, so I fixed it.
> 
> Consider the function documentation:
> 
> * @param offset
>  *   The number of bytes to skip before copying.
>  *   If the mbuf does not have that many bytes, it is an error
>  *   and NULL is returned.
> 
> An offset resulting in copying zero bytes is not an error according to this.
> 
> Also consider the comment at the comparison in the source code:
> /* check for request to copy at offset past end of mbuf */
> 
> It says "past the end", not "at the end"... although I'm not confident enough in my English skills to determine if this means ">=" or ">".
> 

OK, the documentation does match your change. Maybe there should be a test for that?