[PATCH v2 net] net: skbuff: don't copy alloc_cpu in skb header copies

[PATCH v2 net] net: skbuff: don't copy alloc_cpu in skb header copies

[Carolina Jubran]

Commit 68822bdf76f1 ("net: generalize skb freeing deferral to
per-cpu lists") added alloc_cpu to struct sk_buff and placed it in the
headers group copied by __copy_skb_header().

This is wrong for newly allocated skbs. For example, skb_segment()
allocates a new skb and records the current CPU in alloc_cpu, but
__copy_skb_header() overwrites it with the value from the original skb.

If the original skb was allocated on another CPU, the segment may later
be treated as remotely allocated and unnecessarily deferred there when
it is freed.

Move alloc_cpu out of the headers group so it is preserved across
header copies. Initialize it explicitly in skb_clone(), which allocates
the skb object directly instead of going through __alloc_skb().

This was discovered while investigating a throughput regression with
software IPsec. The load used 512 IPsec tunnels carrying traffic over
VXLAN, with 512 parallel iperf3 flows and an MTU of 4500.

Before the fix:

  16.09%  queued_spin_lock_slowpath
             |
             --15.90%--_raw_spin_lock_irqsave
                       |
                       --9.37%--__slab_free
                                 kfree
                                 skb_release_data
                                 napi_consume_skb
                                 mlx5e_poll_tx_cq

After the fix:

   6.84%  queued_spin_lock_slowpath
             |
             --6.57%--_raw_spin_lock_irqsave
                      |
                      --5.00%--__slab_free
                                kfree
                                skb_release_data
                                napi_consume_skb
                                mlx5e_poll_tx_cq

Fixes: 68822bdf76f1 ("net: generalize skb freeing deferral to per-cpu lists")
Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
Reviewed-by: Björn Töpel <bjorn@kernel.org>
---
 include/linux/skbuff.h | 2 +-
 net/core/skbuff.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
---
V2 log:
- Move alloc_cpu after the headers group and fix the indentation. (Eric Dumazet)
---
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index a3ff380d43f01..b3272d379409e 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1036,7 +1036,6 @@ struct sk_buff {
 	__u16			tc_index;	/* traffic control index */
 #endif
 
-	u16			alloc_cpu;
 
 	union {
 		__wsum		csum;
@@ -1088,6 +1087,7 @@ struct sk_buff {
 
 	); /* end headers group */
 
+	u16			alloc_cpu;
 	/* These elements must be at the end, see alloc_skb() for details.  */
 	sk_buff_data_t		tail;
 	sk_buff_data_t		end;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 41beaf625421a..3d82eb87f4582 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1593,7 +1593,6 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
 #ifdef CONFIG_NET_RX_BUSY_POLL
 	CHECK_SKB_FIELD(napi_id);
 #endif
-	CHECK_SKB_FIELD(alloc_cpu);
 #ifdef CONFIG_XPS
 	CHECK_SKB_FIELD(sender_cpu);
 #endif
@@ -1618,6 +1617,7 @@ static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb)
 	C(len);
 	C(data_len);
 	C(mac_len);
+	C(alloc_cpu);
 	n->hdr_len = skb->nohdr ? skb_headroom(skb) : skb->hdr_len;
 	n->cloned = 1;
 	n->nohdr = 0;
-- 
2.38.1

Re: [PATCH v2 net] net: skbuff: don't copy alloc_cpu in skb header copies

[Jason Xing]

On Tue, Oct 13, 2026 at 11:59 PM Carolina Jubran <cjubran@nvidia.com> wrote:
>
> Commit 68822bdf76f1 ("net: generalize skb freeing deferral to
> per-cpu lists") added alloc_cpu to struct sk_buff and placed it in the
> headers group copied by __copy_skb_header().
>
> This is wrong for newly allocated skbs. For example, skb_segment()
> allocates a new skb and records the current CPU in alloc_cpu, but
> __copy_skb_header() overwrites it with the value from the original skb.
>
> If the original skb was allocated on another CPU, the segment may later
> be treated as remotely allocated and unnecessarily deferred there when
> it is freed.
>
> Move alloc_cpu out of the headers group so it is preserved across
> header copies. Initialize it explicitly in skb_clone(), which allocates

nit: s/Initialize/Copying

> the skb object directly instead of going through __alloc_skb().
>
> This was discovered while investigating a throughput regression with
> software IPsec. The load used 512 IPsec tunnels carrying traffic over
> VXLAN, with 512 parallel iperf3 flows and an MTU of 4500.
>
> Before the fix:
>
>   16.09%  queued_spin_lock_slowpath
>              |
>              --15.90%--_raw_spin_lock_irqsave
>                        |
>                        --9.37%--__slab_free
>                                  kfree
>                                  skb_release_data
>                                  napi_consume_skb
>                                  mlx5e_poll_tx_cq
>
> After the fix:
>
>    6.84%  queued_spin_lock_slowpath
>              |
>              --6.57%--_raw_spin_lock_irqsave
>                       |
>                       --5.00%--__slab_free
>                                 kfree
>                                 skb_release_data
>                                 napi_consume_skb
>                                 mlx5e_poll_tx_cq
>
> Fixes: 68822bdf76f1 ("net: generalize skb freeing deferral to per-cpu lists")
> Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
> Reviewed-by: Björn Töpel <bjorn@kernel.org>

Thanks. Overall it looks good to me:
Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>

> ---
>  include/linux/skbuff.h | 2 +-
>  net/core/skbuff.c      | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> ---
> V2 log:
> - Move alloc_cpu after the headers group and fix the indentation. (Eric Dumazet)
> ---
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index a3ff380d43f01..b3272d379409e 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -1036,7 +1036,6 @@ struct sk_buff {
>         __u16                   tc_index;       /* traffic control index */
>  #endif
>
> -       u16                     alloc_cpu;
>

nit: no blank line here after you remove it

>         union {
>                 __wsum          csum;
> @@ -1088,6 +1087,7 @@ struct sk_buff {
>
>         ); /* end headers group */
>
> +       u16                     alloc_cpu;
>         /* These elements must be at the end, see alloc_skb() for details.  */
>         sk_buff_data_t          tail;
>         sk_buff_data_t          end;
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 41beaf625421a..3d82eb87f4582 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -1593,7 +1593,6 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
>  #ifdef CONFIG_NET_RX_BUSY_POLL
>         CHECK_SKB_FIELD(napi_id);
>  #endif
> -       CHECK_SKB_FIELD(alloc_cpu);
>  #ifdef CONFIG_XPS
>         CHECK_SKB_FIELD(sender_cpu);
>  #endif
> @@ -1618,6 +1617,7 @@ static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb)
>         C(len);
>         C(data_len);
>         C(mac_len);
> +       C(alloc_cpu);

One minor question: does it affect the following two cases:
1) skb_morph()
2) else branch in skb_clone()

Looking at the above logic, I think as to #2 point it seems the impact
doesn't matter...

And I'm a bit confused reading the allocation process because there
are two possible split allocations for the skb structure itself and
the head/data area. They might run on different cpus, which means it's
hard for 'alloc_cpu" to choose appropriate one to free under this
condition.

Thanks,
Jason

>         n->hdr_len = skb->nohdr ? skb_headroom(skb) : skb->hdr_len;
>         n->cloned = 1;
>         n->nohdr = 0;
> --
> 2.38.1
>

Re: [PATCH v2 net] net: skbuff: don't copy alloc_cpu in skb header copies

[Carolina Jubran]

On 16/10/2026 11:11, Jason Xing wrote:
> On Tue, Oct 13, 2026 at 11:59 PM Carolina Jubran <cjubran@nvidia.com> wrote:
>> Commit 68822bdf76f1 ("net: generalize skb freeing deferral to
>> per-cpu lists") added alloc_cpu to struct sk_buff and placed it in the
>> headers group copied by __copy_skb_header().
>>
>> This is wrong for newly allocated skbs. For example, skb_segment()
>> allocates a new skb and records the current CPU in alloc_cpu, but
>> __copy_skb_header() overwrites it with the value from the original skb.
>>
>> If the original skb was allocated on another CPU, the segment may later
>> be treated as remotely allocated and unnecessarily deferred there when
>> it is freed.
>>
>> Move alloc_cpu out of the headers group so it is preserved across
>> header copies. Initialize it explicitly in skb_clone(), which allocates
> nit: s/Initialize/Copying
>
>> the skb object directly instead of going through __alloc_skb().
>>
>> This was discovered while investigating a throughput regression with
>> software IPsec. The load used 512 IPsec tunnels carrying traffic over
>> VXLAN, with 512 parallel iperf3 flows and an MTU of 4500.
>>
>> Before the fix:
>>
>>    16.09%  queued_spin_lock_slowpath
>>               |
>>               --15.90%--_raw_spin_lock_irqsave
>>                         |
>>                         --9.37%--__slab_free
>>                                   kfree
>>                                   skb_release_data
>>                                   napi_consume_skb
>>                                   mlx5e_poll_tx_cq
>>
>> After the fix:
>>
>>     6.84%  queued_spin_lock_slowpath
>>               |
>>               --6.57%--_raw_spin_lock_irqsave
>>                        |
>>                        --5.00%--__slab_free
>>                                  kfree
>>                                  skb_release_data
>>                                  napi_consume_skb
>>                                  mlx5e_poll_tx_cq
>>
>> Fixes: 68822bdf76f1 ("net: generalize skb freeing deferral to per-cpu lists")
>> Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
>> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
>> Reviewed-by: Björn Töpel <bjorn@kernel.org>
> Thanks. Overall it looks good to me:
> Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>
Thanks for the review!
>> ---
>>   include/linux/skbuff.h | 2 +-
>>   net/core/skbuff.c      | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>> ---
>> V2 log:
>> - Move alloc_cpu after the headers group and fix the indentation. (Eric Dumazet)
>> ---
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index a3ff380d43f01..b3272d379409e 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -1036,7 +1036,6 @@ struct sk_buff {
>>          __u16                   tc_index;       /* traffic control index */
>>   #endif
>>
>> -       u16                     alloc_cpu;
>>
> nit: no blank line here after you remove it


Yeah, I will drop one line. Thanks

>
>>          union {
>>                  __wsum          csum;
>> @@ -1088,6 +1087,7 @@ struct sk_buff {
>>
>>          ); /* end headers group */
>>
>> +       u16                     alloc_cpu;
>>          /* These elements must be at the end, see alloc_skb() for details.  */
>>          sk_buff_data_t          tail;
>>          sk_buff_data_t          end;
>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>> index 41beaf625421a..3d82eb87f4582 100644
>> --- a/net/core/skbuff.c
>> +++ b/net/core/skbuff.c
>> @@ -1593,7 +1593,6 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
>>   #ifdef CONFIG_NET_RX_BUSY_POLL
>>          CHECK_SKB_FIELD(napi_id);
>>   #endif
>> -       CHECK_SKB_FIELD(alloc_cpu);
>>   #ifdef CONFIG_XPS
>>          CHECK_SKB_FIELD(sender_cpu);
>>   #endif
>> @@ -1618,6 +1617,7 @@ static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb)
>>          C(len);
>>          C(data_len);
>>          C(mac_len);
>> +       C(alloc_cpu);
> One minor question: does it affect the following two cases:
> 1) skb_morph()
> 2) else branch in skb_clone()
>
> Looking at the above logic, I think as to #2 point it seems the impact
> doesn't matter...

For skb_morph() this patch does not change the existing behavior.
skb_morph() reuses an existing destination skb and turns it into a clone of
src by calling __skb_clone(dst, src).
Before this patch, alloc_cpu was part of the headers group, so
__copy_skb_header() inside __skb_clone() copied src->alloc_cpu to
dst->alloc_cpu implicitly.
After moving alloc_cpu out of the headers group, the explicit
C(alloc_cpu);
in __skb_clone() preserves exactly the same behavior.


>
> And I'm a bit confused reading the allocation process because there
> are two possible split allocations for the skb structure itself and
> the head/data area. They might run on different cpus, which means it's
> hard for 'alloc_cpu" to choose appropriate one to free under this
> condition.

Good point. The current documentation describes alloc_cpu as the CPU
that allocated the skb, while skb_attempt_defer_free() refers to the
CPU that allocated the skb/pages.
So it seems that alloc_cpu is best effort for deferred freeing?


>
> Thanks,
> Jason
>
>>          n->hdr_len = skb->nohdr ? skb_headroom(skb) : skb->hdr_len;
>>          n->cloned = 1;
>>          n->nohdr = 0;
>> --
>> 2.38.1
>>

Re: [PATCH v2 net] net: skbuff: don't copy alloc_cpu in skb header copies

[Jason Xing]

On Sun, Oct 18, 2026 at 11:58 PM Carolina Jubran <cjubran@nvidia.com> wrote:
>
>
> On 16/10/2026 11:11, Jason Xing wrote:
> > On Tue, Oct 13, 2026 at 11:59 PM Carolina Jubran <cjubran@nvidia.com> wrote:
> >> Commit 68822bdf76f1 ("net: generalize skb freeing deferral to
> >> per-cpu lists") added alloc_cpu to struct sk_buff and placed it in the
> >> headers group copied by __copy_skb_header().
> >>
> >> This is wrong for newly allocated skbs. For example, skb_segment()
> >> allocates a new skb and records the current CPU in alloc_cpu, but
> >> __copy_skb_header() overwrites it with the value from the original skb.
> >>
> >> If the original skb was allocated on another CPU, the segment may later
> >> be treated as remotely allocated and unnecessarily deferred there when
> >> it is freed.
> >>
> >> Move alloc_cpu out of the headers group so it is preserved across
> >> header copies. Initialize it explicitly in skb_clone(), which allocates
> > nit: s/Initialize/Copying
> >
> >> the skb object directly instead of going through __alloc_skb().
> >>
> >> This was discovered while investigating a throughput regression with
> >> software IPsec. The load used 512 IPsec tunnels carrying traffic over
> >> VXLAN, with 512 parallel iperf3 flows and an MTU of 4500.
> >>
> >> Before the fix:
> >>
> >>    16.09%  queued_spin_lock_slowpath
> >>               |
> >>               --15.90%--_raw_spin_lock_irqsave
> >>                         |
> >>                         --9.37%--__slab_free
> >>                                   kfree
> >>                                   skb_release_data
> >>                                   napi_consume_skb
> >>                                   mlx5e_poll_tx_cq
> >>
> >> After the fix:
> >>
> >>     6.84%  queued_spin_lock_slowpath
> >>               |
> >>               --6.57%--_raw_spin_lock_irqsave
> >>                        |
> >>                        --5.00%--__slab_free
> >>                                  kfree
> >>                                  skb_release_data
> >>                                  napi_consume_skb
> >>                                  mlx5e_poll_tx_cq
> >>
> >> Fixes: 68822bdf76f1 ("net: generalize skb freeing deferral to per-cpu lists")
> >> Signed-off-by: Carolina Jubran <cjubran@nvidia.com>
> >> Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
> >> Reviewed-by: Björn Töpel <bjorn@kernel.org>
> > Thanks. Overall it looks good to me:
> > Reviewed-by: Jason Xing <kerneljasonxing@gmail.com>
> Thanks for the review!
> >> ---
> >>   include/linux/skbuff.h | 2 +-
> >>   net/core/skbuff.c      | 2 +-
> >>   2 files changed, 2 insertions(+), 2 deletions(-)
> >> ---
> >> V2 log:
> >> - Move alloc_cpu after the headers group and fix the indentation. (Eric Dumazet)
> >> ---
> >> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> >> index a3ff380d43f01..b3272d379409e 100644
> >> --- a/include/linux/skbuff.h
> >> +++ b/include/linux/skbuff.h
> >> @@ -1036,7 +1036,6 @@ struct sk_buff {
> >>          __u16                   tc_index;       /* traffic control index */
> >>   #endif
> >>
> >> -       u16                     alloc_cpu;
> >>
> > nit: no blank line here after you remove it
>
>
> Yeah, I will drop one line. Thanks
>
> >
> >>          union {
> >>                  __wsum          csum;
> >> @@ -1088,6 +1087,7 @@ struct sk_buff {
> >>
> >>          ); /* end headers group */
> >>
> >> +       u16                     alloc_cpu;
> >>          /* These elements must be at the end, see alloc_skb() for details.  */
> >>          sk_buff_data_t          tail;
> >>          sk_buff_data_t          end;
> >> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> >> index 41beaf625421a..3d82eb87f4582 100644
> >> --- a/net/core/skbuff.c
> >> +++ b/net/core/skbuff.c
> >> @@ -1593,7 +1593,6 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
> >>   #ifdef CONFIG_NET_RX_BUSY_POLL
> >>          CHECK_SKB_FIELD(napi_id);
> >>   #endif
> >> -       CHECK_SKB_FIELD(alloc_cpu);
> >>   #ifdef CONFIG_XPS
> >>          CHECK_SKB_FIELD(sender_cpu);
> >>   #endif
> >> @@ -1618,6 +1617,7 @@ static struct sk_buff *__skb_clone(struct sk_buff *n, struct sk_buff *skb)
> >>          C(len);
> >>          C(data_len);
> >>          C(mac_len);
> >> +       C(alloc_cpu);
> > One minor question: does it affect the following two cases:
> > 1) skb_morph()
> > 2) else branch in skb_clone()
> >
> > Looking at the above logic, I think as to #2 point it seems the impact
> > doesn't matter...
>
> For skb_morph() this patch does not change the existing behavior.
> skb_morph() reuses an existing destination skb and turns it into a clone of
> src by calling __skb_clone(dst, src).
> Before this patch, alloc_cpu was part of the headers group, so
> __copy_skb_header() inside __skb_clone() copied src->alloc_cpu to
> dst->alloc_cpu implicitly.
> After moving alloc_cpu out of the headers group, the explicit
> C(alloc_cpu);
> in __skb_clone() preserves exactly the same behavior.

Makes sense.

>
>
> >
> > And I'm a bit confused reading the allocation process because there
> > are two possible split allocations for the skb structure itself and
> > the head/data area. They might run on different cpus, which means it's
> > hard for 'alloc_cpu" to choose appropriate one to free under this
> > condition.
>
> Good point. The current documentation describes alloc_cpu as the CPU
> that allocated the skb, while skb_attempt_defer_free() refers to the
> CPU that allocated the skb/pages.
> So it seems that alloc_cpu is best effort for deferred freeing?

I suppose so.

Thanks,
Jason