[PATCH net-next v2] seg6: reallocate the skb head on L2 encapsulation only when needed

Yuya Kusakabe posted 1 patch 3 weeks, 1 day ago
There is a newer version of this series
net/ipv6/seg6_iptunnel.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
[PATCH net-next v2] seg6: reallocate the skb head on L2 encapsulation only when needed
Posted by Yuya Kusakabe 3 weeks, 1 day ago
The L2 encapsulation modes of the seg6 lwtunnel reallocate the skb head
on every packet, where the IPv6 encapsulation modes reallocate only when
they have to.  Ask for the whole encapsulation up front instead, so that
the reallocation happens at most once and only when the headroom really
is too small:

  skb->mac_len + sizeof(struct ipv6hdr) + ipv6_optlen(tinfo->srh)
  + dst_dev_overhead(cache_dst, skb)

__seg6_do_srh_encap() then finds the room it needs and its own
skb_cow_head() becomes a no-op.

Drivers reserve more than that on the forwarding path, so the
reallocation usually disappears altogether.  A single-segment policy
on ixgbe needs

  14 (mac_len) + 40 (ipv6hdr) + 24 (SRH) + 16 (LL_RESERVED_SPACE) = 94

against the 206 bytes the driver leaves.  Where the headroom is
smaller, as on a veth pair, pskb_expand_head() is called once per
forwarded packet instead of twice.  Asking only for skb->mac_len would
still take two whenever the skb is header-cloned, because the cow that
unclones it does not also make room for the outer header.

The cost is amplified by CONFIG_INIT_ON_ALLOC_DEFAULT_ON, which many
distributions enable: every new head is zeroed in full, and that memset
alone accounts for 16% of the datapath profile.

Throughput at 0.5% packet loss, 64-byte frames forwarded through one
2.30 GHz core (Xeon E5-2650 v3, ixgbe 82599ES), offered by TRex and
binary-searched over 10 runs of 10 s:

  Before: 654.6 kpps
  After:  965.7 kpps

Assisted-by: Claude:claude-opus-5
Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
---
Changes in v2:
- Ask for the whole encapsulation headroom at once, so that a cloned
  skb no longer takes a second reallocation inside
  __seg6_do_srh_encap() [Eric]
- Re-measure against unpatched net-next rather than an older base
- Link to v1: https://lore.kernel.org/r/20260902-seg6-l2cow-v1-1-e823ce216454@gmail.com
---
 net/ipv6/seg6_iptunnel.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
index 61c6a27bf202..ecd8146089ee 100644
--- a/net/ipv6/seg6_iptunnel.c
+++ b/net/ipv6/seg6_iptunnel.c
@@ -400,6 +400,7 @@ static int seg6_do_srh(struct sk_buff *skb, struct dst_entry *cache_dst)
 	struct dst_entry *dst = skb_dst(skb);
 	struct seg6_iptunnel_encap *tinfo;
 	struct seg6_lwt *slwt;
+	unsigned int headroom;
 	int proto, err = 0;
 
 	slwt = seg6_lwt_lwtunnel(dst->lwtstate);
@@ -446,8 +447,13 @@ static int seg6_do_srh(struct sk_buff *skb, struct dst_entry *cache_dst)
 		if (!skb_mac_header_was_set(skb))
 			return -EINVAL;
 
-		if (pskb_expand_head(skb, skb->mac_len, 0, GFP_ATOMIC) < 0)
-			return -ENOMEM;
+		headroom = skb->mac_len + sizeof(struct ipv6hdr) +
+			   ipv6_optlen(tinfo->srh) +
+			   dst_dev_overhead(cache_dst, skb);
+
+		err = skb_cow_head(skb, headroom);
+		if (unlikely(err))
+			return err;
 
 		skb_mac_header_rebuild(skb);
 		skb_push(skb, skb->mac_len);

---
base-commit: c8ea08ba34f2a2e9bfb18ff3d69eb2d69b324f49
change-id: 20260902-seg6-l2cow-77dc3ba41232

Best regards,
--  
Yuya Kusakabe <yuya.kusakabe@gmail.com>
Re: [PATCH net-next v2] seg6: reallocate the skb head on L2 encapsulation only when needed
Posted by netdev-bot+sashiko@kernel.org 2 weeks, 3 days ago
Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 0 · Low: 1

- [High] The computed pre-encapsulation headroom in seg6_do_srh()
  (net/ipv6/seg6_iptunnel.c:452-454, L2ENCAP/L2ENCAP_RED case) accounts…
- [Low] The commit uses the AI-assistance trailer `Assisted-by:
  Claude:claude-opus-5`, which does not match the only format documented…

--

seg6: reallocate the skb head on L2 encapsulation only when needed

The L2ENCAP and L2ENCAP_RED modes of the seg6 lwtunnel previously called
pskb_expand_head() unconditionally for skb->mac_len.  This patch replaces
that with a skb_cow_head() for the whole encapsulation (mac_len, outer
IPv6 header, SRH and dst_dev_overhead()) so the head is reallocated at
most once and only when the existing headroom is too small.

Link to v1:
https://lore.kernel.org/r/20260902-seg6-l2cow-v1-1-e823ce216454@gmail.com

> Assisted-by: Claude:claude-opus-5

[Severity: Low]
This isn't a bug, but does this trailer match the form documented in
Documentation/process/coding-assistants.rst?  That file describes only:

  Assisted-by: LLM [TOOL1] [TOOL2]

where the optional trailing entries are specialized analysis tools
(coccinelle, sparse, smatch, clang-tidy), with the example
"Assisted-by: LLM coccinelle sparse", rather than a vendor or model name.

> diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c
> index 61c6a27bf202..ecd8146089ee 100644
> --- a/net/ipv6/seg6_iptunnel.c
> +++ b/net/ipv6/seg6_iptunnel.c

[ ... ]

> @@ -446,8 +447,13 @@ static int seg6_do_srh(struct sk_buff *skb, struct dst_entry *cache_dst)
>  		if (!skb_mac_header_was_set(skb))
>  			return -EINVAL;
>  
> -		if (pskb_expand_head(skb, skb->mac_len, 0, GFP_ATOMIC) < 0)
> -			return -ENOMEM;
> +		headroom = skb->mac_len + sizeof(struct ipv6hdr) +
> +			   ipv6_optlen(tinfo->srh) +
> +			   dst_dev_overhead(cache_dst, skb);
> +
> +		err = skb_cow_head(skb, headroom);
> +		if (unlikely(err))
> +			return err;
>  
>  		skb_mac_header_rebuild(skb);
>  		skb_push(skb, skb->mac_len);

[Severity: High]
Does this account for skb->mac_len often enough?  The L2 path rebuilds the
inner MAC header twice, and mac_len is counted only once here.

First rebuild, in seg6_do_srh():

	skb_mac_header_rebuild(skb);
	skb_push(skb, skb->mac_len);

Second rebuild, in __seg6_do_srh_encap() (and identically in
seg6_do_srh_encap_red()), after the outer header has been pushed:

	skb_push(skb, tot_len);
	skb_reset_network_header(skb);
	skb_mac_header_rebuild(skb);

The commit message says that "__seg6_do_srh_encap() then finds the room it
needs and its own skb_cow_head() becomes a no-op".  With that cow turned
into a no-op, is anything left to supply headroom for the second rebuild?

After the push of tot_len = sizeof(struct ipv6hdr) + ipv6_optlen(srh), the
headroom guaranteed by the new request is only dst_dev_overhead().  If
skb->mac_len is larger than that, skb_mac_header_rebuild() calls
skb_set_mac_header(skb, -skb->mac_len) with a negative resulting offset,
which is stored unchecked into the __u16 field:

include/linux/skbuff.h:
static inline void skb_set_mac_header(struct sk_buff *skb, const int offset)
{
	skb_reset_mac_header(skb);
	skb->mac_header += offset;
}

Can that wrap to roughly 65534 and make the following

	memmove(skb_mac_header(skb), old_mac, skb->mac_len);

write skb->mac_len bytes about 64 KB past skb->head?

Can skb->mac_len exceed dst_dev_overhead()?  dst_dev_overhead() returns
LL_RESERVED_SPACE(dst->dev), which is 16 for a plain Ethernet egress
device, while on ingress vlan_do_receive() re-inserts the tag when
VLAN_FLAG_REORDER_HDR is off:

net/8021q/vlan_core.c:vlan_do_receive() {
	...
		skb_pull(skb, offset + VLAN_HLEN);
		skb_reset_mac_len(skb);
	...
}

giving skb->mac_len = 18, or 22 for stacked VLAN devices, and
__netif_receive_skb_core() does not reset mac_len again on the forwarding
path.

Note also that skb_cow_head()/__skb_cow() only guarantee headroom greater
than or equal to the request, so the skb need not be reallocated for the
underflow to happen.  With mac_len = 18, overhead = 16 and a one-segment
SRH (ipv6_optlen = 24, tot_len = 64) the request is 98, and an skb whose
resulting headroom is 98 or 99 gives mac_header = 16 - 18 = -2.

Would

	skb->mac_len + sizeof(struct ipv6hdr) + ipv6_optlen(tinfo->srh) +
	max(skb->mac_len, dst_dev_overhead(cache_dst, skb))

be the amount to ask for, since the rebuilt inner MAC copy and the egress
device's own link header both live below the outer IPv6 header?

For seg6_do_srh_encap_red() the same sequence applies; it is partly
shielded because the caller here asks for the full ipv6_optlen() while
only red_hdrlen is pushed, but the first_segment == 0 && sr_has_hmac()
sub-case sets red_hdrlen = hdrlen and is exposed the same way as plain
L2ENCAP.

A similarly shaped window existed before this change, since the
skb_cow_head() inside __seg6_do_srh_encap() never accounted for mac_len
either.  Given that this patch takes over sizing the whole encapsulation
up front, would it make sense to fix the amount here?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260903-seg6-l2cow-v2-1-f37b3b35416f%40gmail.com
Re: [PATCH net-next v2] seg6: reallocate the skb head on L2 encapsulation only when needed
Posted by Yuya Kusakabe 2 weeks, 2 days ago
On Tue, Sep 8, 2026 at 5:24 PM netdev-bot+sashiko@kernel.org wrote:
> Can that wrap to roughly 65534 and make the following
>
> 	memmove(skb_mac_header(skb), old_mac, skb->mac_len);
>
> write skb->mac_len bytes about 64 KB past skb->head?

Yes.  Forwarding plain ping6 traffic through a VLAN device with
reorder_hdr off on ingress reproduces it: skb->mac_header comes back as
65534 on a 704-byte head, and skb_mac_header_rebuild() writes 26 bytes
there.

> A similarly shaped window existed before this change, since the
> skb_cow_head() inside __seg6_do_srh_encap() never accounted for mac_len
> either.  Given that this patch takes over sizing the whole encapsulation
> up front, would it make sense to fix the amount here?

The window is not specific to the L2 modes, so I would rather fix it in
__seg6_do_srh_encap() and seg6_do_srh_encap_red() themselves.  Mode
encap reproduces it too, on unpatched net-next.  40475b63761a ("net:
ipv6: seg6_iptunnel: mitigate 2-realloc issue") replaced skb->mac_len
with dst_dev_overhead() in those two skb_cow_head() requests, and
dst_dev_overhead() is the smaller of the two whenever mac_len exceeds
LL_RESERVED_SPACE() of the egress device.  Asking for the larger of them
restores the guarantee without giving up the one 40475b63761a added.

I will send that against net, separately from this patch.

> This isn't a bug, but does this trailer match the form documented in
> Documentation/process/coding-assistants.rst?

No, it does not.  I will use "Assisted-by: LLM" from now on.
Re: [PATCH net-next v2] seg6: reallocate the skb head on L2 encapsulation only when needed
Posted by Eric Dumazet 3 weeks, 1 day ago
On Thu, Sep 3, 2026 at 10:23 AM Yuya Kusakabe <yuya.kusakabe@gmail.com> wrote:
>
> The L2 encapsulation modes of the seg6 lwtunnel reallocate the skb head
> on every packet, where the IPv6 encapsulation modes reallocate only when
> they have to.  Ask for the whole encapsulation up front instead, so that
> the reallocation happens at most once and only when the headroom really
> is too small:
>
>   skb->mac_len + sizeof(struct ipv6hdr) + ipv6_optlen(tinfo->srh)
>   + dst_dev_overhead(cache_dst, skb)
>
> __seg6_do_srh_encap() then finds the room it needs and its own
> skb_cow_head() becomes a no-op.
>
> Drivers reserve more than that on the forwarding path, so the
> reallocation usually disappears altogether.  A single-segment policy
> on ixgbe needs
>
>   14 (mac_len) + 40 (ipv6hdr) + 24 (SRH) + 16 (LL_RESERVED_SPACE) = 94
>
> against the 206 bytes the driver leaves.  Where the headroom is
> smaller, as on a veth pair, pskb_expand_head() is called once per
> forwarded packet instead of twice.  Asking only for skb->mac_len would
> still take two whenever the skb is header-cloned, because the cow that
> unclones it does not also make room for the outer header.
>
> The cost is amplified by CONFIG_INIT_ON_ALLOC_DEFAULT_ON, which many
> distributions enable: every new head is zeroed in full, and that memset
> alone accounts for 16% of the datapath profile.
>
> Throughput at 0.5% packet loss, 64-byte frames forwarded through one
> 2.30 GHz core (Xeon E5-2650 v3, ixgbe 82599ES), offered by TRex and
> binary-searched over 10 runs of 10 s:
>
>   Before: 654.6 kpps
>   After:  965.7 kpps
>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>

Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks!