Subject: Re: [PATCH] skbuff: Fix a race between coalescing and releasing SKBs

From: Jakub Kicinski <[email-masked]>
Date: Tue, 4 Apr 2023 18:21:16 -0700

On Tue, 04 Apr 2023 08:51:18 -0700 Alexander H Duyck wrote:
> I'm not quite sure I agree with the fix. Couldn't we just modify the
> check further down that does:
> 
>         if (!skb_cloned(from))
>                 from_shinfo->nr_frags = 0;
> 
> And instead just make that:
> 	if (!skb->cloned || (!skb_cloned(from) && !from->pp_recycle))
>                 from_shinfo->nr_frags = 0;
> 
> With that we would retain the existing behavior and in the case of
> cloned from frames we would take the references and let the original
> from skb freed to take care of pulling the pages from the page pool.

Sounds like a better fix, indeed. But this sort of code will require
another fat comment above to explain why. This:

	if (to->pp_recycle == from->pp_recycle && !skb_cloned(from))

is much easier to understand, no?

We should at least include that in the explanatory comment, I reckon...