* Re: [PATCH net-next v2] seg6: reallocate the skb head on L2 encapsulation only when needed
2026-09-03 8:23 [PATCH net-next v2] seg6: reallocate the skb head on L2 encapsulation only when needed Yuya Kusakabe
@ 2026-09-03 8:47 ` Eric Dumazet
2026-09-08 8:24 ` netdev-bot+sashiko
1 sibling, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2026-09-03 8:47 UTC (permalink / raw)
To: Yuya Kusakabe
Cc: Andrea Mayer, David S. Miller, Jakub Kicinski, Paolo Abeni,
Simon Horman, netdev, linux-kernel
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!
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net-next v2] seg6: reallocate the skb head on L2 encapsulation only when needed
2026-09-03 8:23 [PATCH net-next v2] seg6: reallocate the skb head on L2 encapsulation only when needed Yuya Kusakabe
2026-09-03 8:47 ` Eric Dumazet
@ 2026-09-08 8:24 ` netdev-bot+sashiko
2026-09-09 23:43 ` Yuya Kusakabe
1 sibling, 1 reply; 4+ messages in thread
From: netdev-bot+sashiko @ 2026-09-08 8:24 UTC (permalink / raw)
To: yuya.kusakabe
Cc: andrea.mayer, davem, edumazet, kuba, pabeni, horms, netdev, linux-kernel
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
^ permalink raw reply [flat|nested] 4+ messages in thread