Re: [RFC PATCH v2] net: esp: fix bad handling of pages from page_pool

[Jakub Kicinski]

On Wed, 6 Mar 2024 19:09:58 +0000 Dragos Tatulea wrote:
> > Changes in v2:
> > - Added napi_page_unref api based on discussion in v1 [0].
> > 
> > [0]https://lore.kernel.org/netdev/CAHS8izOoO-EovwMwAm9tLYetwikNPxC0FKyVGu1TPJWSz4bGoA@xxxxxxxxxxxxxx/T/#t
> >   
> Jakub, are you sure that it is ok to have all this in a single patch?

Yup.

> > ---
> >  include/linux/skbuff.h | 10 +++++++---
> >  net/ipv4/esp4.c        | 16 ++++++++++------
> >  net/ipv6/esp6.c        | 16 ++++++++++------
> >  3 files changed, 27 insertions(+), 15 deletions(-)
> > 
> > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> > index 696e7680656f..009603db2a43 100644
> > --- a/include/linux/skbuff.h
> > +++ b/include/linux/skbuff.h
> > @@ -3453,10 +3453,8 @@ int skb_cow_data_for_xdp(struct page_pool *pool, struct sk_buff **pskb,
> >  bool napi_pp_put_page(struct page *page, bool napi_safe);
> >  
> >  static inline void
> > -napi_frag_unref(skb_frag_t *frag, bool recycle, bool napi_safe)
> > +napi_page_unref(struct page *page, bool recycle, bool napi_safe)

I'd call it skb_page_unref()

The napi_ prefix will just confuse people, because we don't have to be
in NAPI context at all when calling this. As long as "napi_safe" is
correctly set to false.  So let's use skb_ as the prefix.

And I'd pass skb to this, not "bool recycle" so that the caller doesn't
have to know the weird details..