Re: [PATCH net] rxrpc: Also unshare DATA/RESPONSE packets when paged frags are present

[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
---
rxrpc: Also unshare DATA/RESPONSE packets when paged frags are present

This commit updates the DATA and RESPONSE packet handlers in rxrpc to copy
the skb into a linear one if there are paged fragments present. This ensures
the security handler operates on a fully linear skb during decryption.

> diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
> index fdd683261226..6c924ef55208 100644
> --- a/net/rxrpc/call_event.c
> +++ b/net/rxrpc/call_event.c
> @@ -334,7 +334,7 @@ bool rxrpc_input_call_event(struct rxrpc_call *call)
>  
>  			if (sp->hdr.type == RXRPC_PACKET_TYPE_DATA &&
>  			    sp->hdr.securityIndex != 0 &&
> -			    skb_cloned(skb)) {
> +			    (skb_cloned(skb) || skb->data_len)) {
>  				/* Unshare the packet so that it can be
>  				 * modified by in-place decryption.
>  				 */

Since modern NIC drivers commonly use page pools where packet payloads are
placed in page fragments, skb->data_len will be greater than zero for almost
all standard incoming packets.

By taking this path, the code calls skb_copy() right after this check.
Will this force a full packet copy for nearly all incoming RxRPC DATA
packets, bypassing zero-copy receive paths and causing a large performance
regression?

Additionally, skb_copy() fully linearizes the packet by allocating a single
contiguous buffer for the entire packet size. For RxRPC Jumbo packets or UDP
GRO-aggregated packets (which can reach up to ~64KB), this requires an
order-4 page allocation.

Could relying on an order-4 GFP_ATOMIC allocation cause reliable OOM
failures and packet drops on fragmented systems?

If the goal is to ensure the skb is writable for in-place decryption, would
it be safer to use skb_cow_data() instead of forcing a full contiguous
linearization?

Simon says:

I note that this patch included
Fixes: d0d5c0cd1e71 ("rxrpc: Use skb_unshare() rather than skb_cow_data()")
And that the suggestion in the last paragraph above would
reintroduce back to the problem addressed by d0d5c0cd1e71.