Re: [RFC net-next v2 1/3] net: skb: plumb napi state thru skb freeing paths

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 7 Apr 2023 17:15:15 +0800 Yunsheng Lin wrote:
> On 2023/4/6 7:20, Jakub Kicinski wrote:
> > We maintain a NAPI-local cache of skbs which is fed by napi_consume_skb().
> > Going forward we will also try to cache head and data pages.
> > Plumb the "are we in a normal NAPI context" information thru
> > deeper into the freeing path, up to skb_release_data() and
> > skb_free_head()/skb_pp_recycle().
> > 
> > Use "bool in_normal_napi" rather than bare "int budget",
> > the further we get from NAPI the more confusing the budget
> > argument may seem (particularly whether 0 or MAX is the
> > correct value to pass in when not in NAPI).

> > @@ -839,7 +839,7 @@ static void skb_clone_fraglist(struct sk_buff *skb)
> >  		skb_get(list);
> >  }
> >  
> > -static bool skb_pp_recycle(struct sk_buff *skb, void *data)
> > +static bool skb_pp_recycle(struct sk_buff *skb, void *data, bool in_normal_napi)  
> 
> What does *normal* means in 'in_normal_napi'?
> can we just use in_napi?

Technically netpoll also calls NAPI, that's why I threw in the
"normal". If folks prefer in_napi or some other name I'm more 
than happy to change. Naming is hard.

> > @@ -1226,7 +1228,7 @@ static void napi_skb_cache_put(struct sk_buff *skb)
> >  
> >  void __kfree_skb_defer(struct sk_buff *skb)
> >  {
> > -	skb_release_all(skb, SKB_DROP_REASON_NOT_SPECIFIED);
> > +	skb_release_all(skb, SKB_DROP_REASON_NOT_SPECIFIED, false);
> >  	napi_skb_cache_put(skb);  
> 
> Is there any reson not to call skb_release_all() with in_normal_napi
> being true while napi_skb_cache_put() is called here?

True, __kfree_skb_defer() needs more work also. I'll set in to true 
in the PATCH posting and clean up the function in a follow up.