The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH net v3] ip_tunnel: reserve FOU/GUE headroom before encapsulation
@ 2026-08-24 11:19 Chengfeng Ye
  2026-08-25  7:41 ` Ido Schimmel
  0 siblings, 1 reply; 2+ messages in thread
From: Chengfeng Ye @ 2026-08-24 11:19 UTC (permalink / raw)
  To: David Ahern, Ido Schimmel, netdev
  Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Xin Long, William Tu, linux-kernel, Chengfeng Ye,
	stable

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.

Fixes: dd9d598c6657 ("ip_gre: add the support for i/o_flags update via netlink")
Cc: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
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/20260808005956.3761487-1-nicoyip.dev@gmail.com/ [v2]
Link: https://lore.kernel.org/netdev/20260801060115.3538849-1-nicoyip.dev@gmail.com/ [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;
@@ -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;
 	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);
+	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;
+	}
+
 	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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net v3] ip_tunnel: reserve FOU/GUE headroom before encapsulation
  2026-08-24 11:19 [PATCH net v3] ip_tunnel: reserve FOU/GUE headroom before encapsulation Chengfeng Ye
@ 2026-08-25  7:41 ` Ido Schimmel
  0 siblings, 0 replies; 2+ messages in thread
From: Ido Schimmel @ 2026-08-25  7:41 UTC (permalink / raw)
  To: Chengfeng Ye
  Cc: David Ahern, netdev, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Xin Long, William Tu,
	linux-kernel, stable

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: stable@vger.kernel.org
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
> ---
> 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/20260808005956.3761487-1-nicoyip.dev@gmail.com/ [v2]
> Link: https://lore.kernel.org/netdev/20260801060115.3538849-1-nicoyip.dev@gmail.com/ [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
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-25  7:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-24 11:19 [PATCH net v3] ip_tunnel: reserve FOU/GUE headroom before encapsulation Chengfeng Ye
2026-08-25  7:41 ` Ido Schimmel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox