Re: [PATCH net v3] ip_tunnel: reserve FOU/GUE headroom before encapsulation

On Mon, Aug 24, 2026 at 07:19:44PM +0800, Chengfeng Ye wrote:
> ip_tunnel_encap() expects its callers to reserve headroom based on
> ip_encap_hlen(). Unlike the IPv6 tunnel transmit paths, the IPv4
> ip_tunnel_xmit() and ip_md_tunnel_xmit() push FOU and GUE headers
> before they grow the skb headroom.
> 
> That becomes visible when ipgre_changelink() publishes UDP
> encapsulation before it updates the device headroom. The transmit path
> does not serialize with RTNL, so it can interleave as follows:
> 
>   CPU 0 (ipgre_changelink)        CPU 1 (ipgre_xmit)
>   install GUE encapsulation
>                                   reserve the old needed_headroom
>   publish larger GRE flags
>   update tunnel->tun_hlen
>                                   push the larger GRE header
>                                   push the GUE and UDP headers
>   update dev->needed_headroom
> 
> With REMCSUM, the new layout can push 16 bytes of GRE and 20 bytes of
> GUE/UDP headers into an skb with only 32 bytes of actual headroom. The
> final UDP push writes four bytes before skb->head.
> 
> With the update window widened, the kernel reported:
> 
>   skbuff: skb_under_panic: ... len:128 put:8 ... dev:gre0poc
>   kernel BUG at net/core/skbuff.c:214!
>   Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
>   Call Trace:
>    skb_push
>    fou_build_udp
>    gue_build_header
>    ip_tunnel_xmit
>    __gre_xmit
>    ipgre_xmit
> 
> Use ip_encap_hlen() up front, route and perform PMTU handling first,
> then reserve the final headroom before ip_tunnel_encap() builds the
> UDP tunnel headers. This matches the existing IPv6 pattern and keeps
> ip_tunnel_encap() as a pure header builder.

Mention in the commit message that the IPv6 fix [1] is still WIP.
Otherwise, I'm pretty sure that Sashiko will complain about it.

[1] https://lore.kernel.org/netdev/b58876297f7d45de008f2e94b6ecab8b2ed84d21.1786088695.git.petalzu987@gmail.com/

> 
> Fixes: dd9d598c6657 ("ip_gre: add the support for i/o_flags update via netlink")
> Cc: [email protected]
> Signed-off-by: Chengfeng Ye <[email protected]>
> ---
> Changes in v3:
> - Move the headroom reservation into ip_tunnel_xmit() and
>   ip_md_tunnel_xmit() instead of growing the skb inside the FOU/GUE
>   builders.
> - Use ip_encap_hlen() to reserve the final caller-side headroom before
>   ip_tunnel_encap(), matching the existing IPv6 transmit pattern.
> - Drop the IPv4 raw-pointer refreshes that were only needed when
>   skb_cow_head() could run inside the encapsulation builders.
> 
> Link: https://lore.kernel.org/netdev/[email protected]/ [v2]
> Link: https://lore.kernel.org/netdev/[email protected]/ [v1]
> ---
>  net/ipv4/ip_tunnel.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> index 9d114bd575f9..5d5e7db11b3d 100644
> --- a/net/ipv4/ip_tunnel.c
> +++ b/net/ipv4/ip_tunnel.c
> @@ -578,6 +578,7 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>  	const struct iphdr *inner_iph;
>  	struct rtable *rt = NULL;
>  	struct flowi4 fl4;
> +	int encap_hlen;
>  	__be16 df = 0;
>  	u8 tos, ttl;
>  	bool use_cache;
> @@ -601,11 +602,11 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>  			    tos & INET_DSCP_MASK, tunnel->net, 0, skb->mark,
>  			    skb_get_hash(skb), key->flow_flags);
>  
> -	if (!tunnel_hlen)
> -		tunnel_hlen = ip_encap_hlen(&tun_info->encap);
> -
> -	if (ip_tunnel_encap(skb, &tun_info->encap, &proto, &fl4) < 0)
> +	encap_hlen = ip_encap_hlen(&tun_info->encap);
> +	if (encap_hlen < 0)
>  		goto tx_error;
> +	if (!tunnel_hlen)
> +		tunnel_hlen = encap_hlen;
>  
>  	use_cache = ip_tunnel_dst_cache_usable(skb, tun_info);
>  	if (use_cache)
> @@ -645,7 +646,8 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>  			ttl = ip4_dst_hoplimit(&rt->dst);
>  	}
>  
> -	headroom += LL_RESERVED_SPACE(rt->dst.dev) + rt->dst.header_len;
> +	headroom += encap_hlen + LL_RESERVED_SPACE(rt->dst.dev) +
> +		    rt->dst.header_len;
>  	if (skb_cow_head(skb, headroom)) {
>  		ip_rt_put(rt);
>  		goto tx_dropped;
> @@ -653,6 +655,11 @@ void ip_md_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>  
>  	ip_tunnel_adj_headroom(dev, headroom);
>  
> +	if (ip_tunnel_encap(skb, &tun_info->encap, &proto, &fl4) < 0) {
> +		ip_rt_put(rt);
> +		goto tx_error;
> +	}
> +
>  	iptunnel_xmit(NULL, rt, skb, fl4.saddr, fl4.daddr, proto, tos, ttl,
>  		      df, !net_eq(tunnel->net, dev_net(dev)), 0);
>  	return;

I don't think we need to change ip_md_tunnel_xmit(). It's not deriving
the FOU/GUE encapsulation info from the device's configuration, so it's
not exposed to the race described in the commit message. Instead, the
encapsulation information is set by the bpf_skb_set_fou_encap() kfunc on
the metadata dst attached to the skb.

Mention this in the commit message, so that it's clear why only
ip_tunnel_xmit() is patched.

> @@ -677,6 +684,7 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>  	__be16 payload_protocol;
>  	bool use_cache = false;
>  	struct flowi4 fl4;
> +	int encap_hlen;

Move this further down to maintain reverse xmas tree [2].

[2] https://docs.kernel.org/next/process/maintainer-netdev.html#local-variable-ordering-reverse-xmas-tree-rcs

>  	bool md = false;
>  	bool connected;
>  	int err_count;
> @@ -765,7 +773,8 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>  			    tunnel->net, READ_ONCE(tunnel->parms.link),
>  			    tunnel->fwmark, skb_get_hash(skb), 0);
>  
> -	if (ip_tunnel_encap(skb, &tunnel->encap, &protocol, &fl4) < 0)
> +	encap_hlen = ip_encap_hlen(&tunnel->encap);

This doesn't completely close the race. 'tunnel->encap' is still read
twice. Here and in the call to ip_tunnel_encap() below. That's why the
IPv6 fix takes a snapshot:

ipencap = data_race(t->encap);

And then continues to use 'ipencap' instead of 't->encap'.

> +	if (encap_hlen < 0)
>  		goto tx_error;
>  
>  	if (connected && md) {
> @@ -834,7 +843,7 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>  	}
>  
>  	max_headroom = LL_RESERVED_SPACE(rt->dst.dev) + sizeof(struct iphdr)
> -			+ rt->dst.header_len + ip_encap_hlen(&tunnel->encap);
> +			+ rt->dst.header_len + encap_hlen;
>  
>  	if (skb_cow_head(skb, max_headroom)) {
>  		ip_rt_put(rt);
> @@ -845,6 +854,11 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
>  
>  	ip_tunnel_adj_headroom(dev, max_headroom);
>  
> +	if (ip_tunnel_encap(skb, &tunnel->encap, &protocol, &fl4) < 0) {
> +		ip_rt_put(rt);
> +		goto tx_error;
> +	}

tnl_update_pmtu() is calculating the size of the inner packet:

pkt_size = skb->len - tunnel_hlen;

And 'tunnel_hlen' includes the length of the encapsulation header which
is no longer reflected in 'skb->len'. You need to pass the length of the
encapsulation header as an argument to tnl_update_pmtu() and do:

pkt_size = skb->len + encap_hlen - tunnel_hlen;

Pass 0 from ip_md_tunnel_xmit() as there tnl_update_pmtu() is still
called after ip_tunnel_encap().

> +
>  	iptunnel_xmit(NULL, rt, skb, fl4.saddr, fl4.daddr, protocol, tos, ttl,
>  		      df, !net_eq(tunnel->net, dev_net(dev)), 0);
>  	return;
> -- 
> 2.43.0
>