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

[Hyunwoo Kim <imv4bel@gmail.com>]

On Fri, May 01, 2026 at 04:58:08PM +0100, Simon Horman wrote:
> 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.

Yes. applying the ai's suggestion would reintroduce the bug fixed
by d0d5c0cd1e71.

The performance concern regarding skb_copy() is, however, valid.
That said, I cannot think of a way to keep the zero-copy path while
still handling in-place decryption properly. Suggestions welcome.


Best regards,
Hyunwoo Kim