On Fri, 16 Jan 2026 18:16:15 +0100 Morten Brørup <[email protected]> wrote:
> > From: Stephen Hemminger [mailto:[email protected]] > > Sent: Friday, 16 January 2026 18.06 > > > > On Fri, 16 Jan 2026 11:16:21 +0000 > > Morten Brørup <[email protected]> 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 <[email protected]> > > > Acked-by: Konstantin Ananyev <[email protected]> > > > --- > > > 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?
