Re: [PATCH net-next v3 1/2] l2tp: account for IP version in SKB headroom

[Tom Parkin <tparkin@katalix.com>]

[-- Attachment #1: Type: text/plain, Size: 3616 bytes --]

On  Thu, Jul 30, 2026 at 02:21:04 +0200, David Bauer wrote:
> Account for the IP version of the tunnel when accounting skb headroom on
> xmit. This avoids having to potentially copy the skb a second time down
> the stack due to allocating not enough space for IPv6 headers in case
> the tunnel uses IPv6.
> 
> Signed-off-by: David Bauer <mail@david-bauer.net>
> ---
>  net/l2tp/l2tp_core.c | 3 ++-
>  net/l2tp/l2tp_core.h | 1 +
>  net/l2tp/l2tp_eth.c  | 9 ++-------
>  3 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
> index 4712cc41881a3..d6fa0f7436629 100644
> --- a/net/l2tp/l2tp_core.c
> +++ b/net/l2tp/l2tp_core.c
> @@ -1237,7 +1237,7 @@ static int l2tp_xmit_core(struct l2tp_session *session, struct sk_buff *skb, uns
>  	 * make room. Adjust truesize.
>  	 */
>  	uhlen = (tunnel->encap == L2TP_ENCAPTYPE_UDP) ? sizeof(*uh) : 0;
> -	headroom = NET_SKB_PAD + sizeof(struct iphdr) + uhlen + session->hdr_len;
> +	headroom = NET_SKB_PAD + tunnel->l3_overhead + uhlen + session->hdr_len;
>  	if (skb_cow_head(skb, headroom)) {
>  		kfree_skb(skb);
>  		return NET_XMIT_DROP;
> @@ -1688,6 +1688,7 @@ int l2tp_tunnel_register(struct l2tp_tunnel *tunnel, struct net *net,
>  	}
>  
>  	sk->sk_allocation = GFP_ATOMIC;
> +	tunnel->l3_overhead = kernel_sock_ip_overhead(sk);
>  	release_sock(sk);
>  
>  	sock_hold(sk);
> diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
> index ffd8ced3a51ff..aab574376d95f 100644
> --- a/net/l2tp/l2tp_core.h
> +++ b/net/l2tp/l2tp_core.h
> @@ -167,6 +167,7 @@ struct l2tp_tunnel {
>  	u32			tunnel_id;
>  	u32			peer_tunnel_id;
>  	int			version;	/* 2=>L2TPv2, 3=>L2TPv3 */
> +	int			l3_overhead;	/* IP header overhead */
>  
>  	char			name[L2TP_TUNNEL_NAME_MAX]; /* for logging */
>  	enum l2tp_encap_type	encap;
> diff --git a/net/l2tp/l2tp_eth.c b/net/l2tp/l2tp_eth.c
> index a4956ef9574cc..9e5f9deac08cf 100644
> --- a/net/l2tp/l2tp_eth.c
> +++ b/net/l2tp/l2tp_eth.c
> @@ -188,7 +188,6 @@ static void l2tp_eth_adjust_mtu(struct l2tp_tunnel *tunnel,
>  				struct net_device *dev)
>  {
>  	unsigned int overhead = 0;
> -	u32 l3_overhead = 0;
>  	u32 mtu;
>  
>  	/* if the encap is UDP, account for UDP header size */
> @@ -197,11 +196,7 @@ static void l2tp_eth_adjust_mtu(struct l2tp_tunnel *tunnel,
>  		dev->needed_headroom += sizeof(struct udphdr);
>  	}
>  
> -	lock_sock(tunnel->sock);
> -	l3_overhead = kernel_sock_ip_overhead(tunnel->sock);
> -	release_sock(tunnel->sock);
> -
> -	if (l3_overhead == 0) {
> +	if (tunnel->l3_overhead == 0) {
>  		/* L3 Overhead couldn't be identified, this could be
>  		 * because tunnel->sock was NULL or the socket's
>  		 * address family was not IPv4 or IPv6,

Minor nit: now that kernel_sock_ip_overhead() is called by
l2tp_tunnel_register() at a point where sk cannot be NULL, this
comment becomes misleading.

It'd be worth rewording to be accurate in the context of this
patch IMO.

> @@ -212,7 +207,7 @@ static void l2tp_eth_adjust_mtu(struct l2tp_tunnel *tunnel,
>  	/* Adjust MTU, factor overhead - underlay L3, overlay L2 hdr
>  	 * UDP overhead, if any, was already factored in above.
>  	 */
> -	overhead += session->hdr_len + ETH_HLEN + l3_overhead;
> +	overhead += session->hdr_len + ETH_HLEN + tunnel->l3_overhead;
>  
>  	mtu = l2tp_tunnel_dst_mtu(tunnel) - overhead;
>  	if (mtu < dev->min_mtu || mtu > dev->max_mtu)
> -- 
> 2.53.0
> 
> 

-- 
Tom Parkin
Katalix Systems Ltd
https://katalix.com
Catalysts for your Embedded Linux software development

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]