Re: [PATCH ipsec] esp: Force skb_cow_data() on RX when the skb is non-linear

[Simon Horman <horms@kernel.org>]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
esp: Force skb_cow_data() on RX when the skb is non-linear

This commit modifies esp_input() and esp6_input() to unconditionally call
skb_cow_data() for all non-linear skbs. This removes the fast path that
previously allowed in-place AEAD decryption for skbs carrying paged
fragments without a frag_list.

> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> index 6dfc0bcdef65..a6fbdec139dc 100644
> --- a/net/ipv4/esp4.c
> +++ b/net/ipv4/esp4.c
> @@ -868,17 +868,10 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
>  		assoclen += seqhilen;
>  	}
>  
> -	if (!skb_cloned(skb)) {
> -		if (!skb_is_nonlinear(skb)) {
> -			nfrags = 1;
> -
> -			goto skip_cow;
> -		} else if (!skb_has_frag_list(skb)) {
> -			nfrags = skb_shinfo(skb)->nr_frags;
> -			nfrags++;
> +	if (!skb_cloned(skb) && !skb_is_nonlinear(skb)) {
> +		nfrags = 1;
>  
> -			goto skip_cow;
> -		}
> +		goto skip_cow;
>  	}

By removing this fast path and forcing all non-linear skbs to fall through
to skb_cow_data(), does this break the IPsec GRO/GSO fast path?

Looking at skb_cow_data() in net/core/skbuff.c:

int skb_cow_data(struct sk_buff *skb, int tailbits, struct sk_buff **trailer)
{
	...
	if ((skb_cloned(skb) || skb_shinfo(skb)->nr_frags) &&
	    !__pskb_pull_tail(skb, __skb_pagelen(skb)))
		return -ENOMEM;
	...

For a large GRO or GSO packet (which can be up to 64KB) with paged fragments,
this attempts to pull the entire paged payload into the linear data area
via __pskb_pull_tail().

Since this runs in the RX softirq path, pulling the entire payload requires
a high-order contiguous memory allocation using GFP_ATOMIC. 

Could this lead to widespread packet drops when memory is fragmented and
these high-order atomic allocations fail?

Additionally, even when the allocations succeed, does unconditionally copying
up to 64KB of data per packet introduce a severe performance regression by
entirely negating the software crypto fast path?

[ ... ]