[PATCH] mbuf: fix packet copy

[PATCH] mbuf: fix packet copy

[Morten Brørup]

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 <mb@smartsharesystems.com>
---
 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;
 
 	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);
 
 	prev = &mc->next;
 	m_last = mc;
-- 
2.43.0

RE: [PATCH] mbuf: fix packet copy

[Morten Brørup]

Recheck-request: iol-compile-amd64-testing

RE: [PATCH] mbuf: fix packet copy

[Morten Brørup]

Recheck-request: iol-compile-amd64-testing

RE: [PATCH] mbuf: fix packet copy

[Morten Brørup]

Recheck-request: iol-compile-amd64-testing

RE: [PATCH] mbuf: fix packet copy

[Morten Brørup]

PING for review.

> From: Morten Brørup [mailto:mb@smartsharesystems.com]
> 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 <mb@smartsharesystems.com>
> ---
>  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;
> 
>  	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);
> 
>  	prev = &mc->next;
>  	m_last = mc;
> --
> 2.43.0

RE: [PATCH] mbuf: fix packet copy

[Konstantin Ananyev]

> 
> PING for review.
> 
> > From: Morten Brørup [mailto:mb@smartsharesystems.com]
> > 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 <mb@smartsharesystems.com>
> > ---
> >  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? 

> >  	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.
BTW, if there are some flags that we would like to preserve,
wouldn't that be a  change in public API behavior?

> >
> >  	prev = &mc->next;
> >  	m_last = mc;
> > --
> > 2.43.0

RE: [PATCH] mbuf: fix packet copy

[Morten Brørup]

> From: Konstantin Ananyev [mailto:konstantin.ananyev@huawei.com]
> 
> >
> > PING for review.
> >
> > > From: Morten Brørup [mailto:mb@smartsharesystems.com]
> > > 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 <mb@smartsharesystems.com>
> > > ---
> > >  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.

> 
> > >
> > >  	prev = &mc->next;
> > >  	m_last = mc;
> > > --
> > > 2.43.0

RE: [PATCH] mbuf: fix packet copy

[Konstantin Ananyev]

> > > > From: Morten Brørup [mailto:mb@smartsharesystems.com]
> > > > 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 <mb@smartsharesystems.com>
> > > > ---
> > > >  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.

RE: [PATCH] mbuf: fix packet copy

[Konstantin Ananyev]

> PING for review.
> 
> > From: Morten Brørup [mailto:mb@smartsharesystems.com]
> > 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 <mb@smartsharesystems.com>
> > ---
> >  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;
> >
> >  	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);
> >
> >  	prev = &mc->next;
> >  	m_last = mc;
> > --

Acked-by: Konstantin Ananyev <konstantin.ananyev@huawei.com>

> > 2.43.0

[PATCH v2] mbuf: fix packet copy

[Morten Brørup]

mbuf: 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;
 
 	mc = rte_pktmbuf_alloc(mp);
@@ -688,8 +688,12 @@ 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,
+	 * while preserving flags of newly allocated mbuf
+	 * (specifically RTE_MBUF_F_EXTERNAL if using pinned external buffer)
+	 */
+	mc->ol_flags |= m->ol_flags & ~(RTE_MBUF_F_INDIRECT | RTE_MBUF_F_EXTERNAL);
 
 	prev = &mc->next;
 	m_last = mc;
-- 
2.43.0

Re: [PATCH v2] mbuf: fix packet copy

[Stephen Hemminger]

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.

RE: [PATCH v2] mbuf: fix packet copy

[Morten Brørup]

> 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 ">".

Re: [PATCH v2] mbuf: fix packet copy

[Stephen Hemminger]

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?

[PATCH v3] mbuf: fix packet copy

[Morten Brørup]

mbuf: 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>
---
v3:
* Improved comment about flags being copied from newly allocated mbuf.
* Added test cases for copying at and past end of packet. (Stephen)
v2:
* Improved comment about preserving flags for newly allocated mbuf
  potentially using pinned external buffer.
* Added missing spaces in expression. (Stephen)
---
 app/test/test_mbuf.c | 20 ++++++++++++++++++++
 lib/mbuf/rte_mbuf.c  | 10 +++++++---
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c
index 17be977f31..19b49cec12 100644
--- a/app/test/test_mbuf.c
+++ b/app/test/test_mbuf.c
@@ -541,6 +541,26 @@ test_pktmbuf_copy(struct rte_mempool *pktmbuf_pool,
 
 	rte_pktmbuf_free(copy2);
 
+	/* test offset copy at the end */
+	copy2 = rte_pktmbuf_copy(copy, pktmbuf_pool,
+				 2 * sizeof(uint32_t), UINT32_MAX);
+	if (copy2 == NULL)
+		GOTO_FAIL("cannot copy at the end of the copy\n");
+
+	if (rte_pktmbuf_pkt_len(copy2) != 0)
+		GOTO_FAIL("copy at the end, length incorrect\n");
+
+	if (rte_pktmbuf_data_len(copy2) != 0)
+		GOTO_FAIL("copy at the end, data length incorrect\n");
+
+	rte_pktmbuf_free(copy2);
+
+	/* test offset copy past the end */
+	copy2 = rte_pktmbuf_copy(copy, pktmbuf_pool,
+				 2 * sizeof(uint32_t) + 1, UINT32_MAX);
+	if (copy2 != NULL)
+		GOTO_FAIL("can copy past the end of the copy\n");
+
 	/* test truncation copy */
 	copy2 = rte_pktmbuf_copy(copy, pktmbuf_pool,
 				 0, sizeof(uint32_t));
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;
 
 	mc = rte_pktmbuf_alloc(mp);
@@ -688,8 +688,12 @@ 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,
+	 * while preserving flags of newly allocated mbuf
+	 * (specifically RTE_MBUF_F_EXTERNAL if using pinned external buffer)
+	 */
+	mc->ol_flags |= m->ol_flags & ~(RTE_MBUF_F_INDIRECT | RTE_MBUF_F_EXTERNAL);
 
 	prev = &mc->next;
 	m_last = mc;
-- 
2.43.0

Re: [PATCH v3] mbuf: fix packet copy

[Thomas Monjalon]

20/01/2026 08:24, Morten Brørup:
> mbuf: 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>

Applied, thanks.

Re: [PATCH v3] mbuf: fix packet copy

[Kevin Traynor]

On 10/02/2026 17:21, Thomas Monjalon wrote:
> 20/01/2026 08:24, Morten Brørup:
>> mbuf: 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>
> 
> Applied, thanks.
> 
> 
> 
Hi All. I see this is not marked for stable release. Is it deliberate
because of the flags changes ?

RE: [PATCH v3] mbuf: fix packet copy

[Morten Brørup]

> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Wednesday, 25 February 2026 16.59
> 
> On 10/02/2026 17:21, Thomas Monjalon wrote:
> > 20/01/2026 08:24, Morten Brørup:
> >> mbuf: 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>
> >
> > Applied, thanks.
> >
> >
> >
> Hi All. I see this is not marked for stable release. Is it deliberate
> because of the flags changes ?

IIRC, the consensus was that using pinned external buffers is considered exotic, so the bug is unlikely to occur in real applications. And if it did happen, we would have heard about it.

But, since you ask, we really should have left that decision to the stable tree maintainers.

-Morten

Re: [PATCH v3] mbuf: fix packet copy

[Kevin Traynor]

On 25/02/2026 17:17, Morten Brørup wrote:
>> From: Kevin Traynor [mailto:ktraynor@redhat.com]
>> Sent: Wednesday, 25 February 2026 16.59
>>
>> On 10/02/2026 17:21, Thomas Monjalon wrote:
>>> 20/01/2026 08:24, Morten Brørup:
>>>> mbuf: 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>
>>>
>>> Applied, thanks.
>>>
>>>
>>>
>> Hi All. I see this is not marked for stable release. Is it deliberate
>> because of the flags changes ?
> 
> IIRC, the consensus was that using pinned external buffers is considered exotic, so the bug is unlikely to occur in real applications. And if it did happen, we would have heard about it.
> 
> But, since you ask, we really should have left that decision to the stable tree maintainers.
> 

Thanks Morten. Background knowledge and context is important, so it's
fine you made a decision on it, I just wanted to check.

> -Morten
>