Re: [PATCH net-next1/2] net: core: use listified Rx for GRO_NORMAL in napi_gro_receive()

[Alexander Lobakin]

Edward Cree wrote 10.10.2019 21:23:
> On 10/10/2019 15:42, Alexander Lobakin wrote:
> > Commit 323ebb61e32b4 ("net: use listified RX for handling GRO_NORMAL
> > skbs") made use of listified skb processing for the users of
> > napi_gro_frags().
> > The same technique can be used in a way more common napi_gro_receive()
> > to speed up non-merged (GRO_NORMAL) skbs for a wide range of drivers,
> > including gro_cells and mac80211 users.
> >
> > Signed-off-by: Alexander Lobakin <alobakin@xxxxxxxx>
> > ---
> >  net/core/dev.c | 49 +++++++++++++++++++++++++------------------------
> >  1 file changed, 25 insertions(+), 24 deletions(-)
> >
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 8bc3dce71fc0..a33f56b439ce 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -5884,6 +5884,26 @@ struct packet_offload 
> > *gro_find_complete_by_type(__be16 type)
> >  }
> >  EXPORT_SYMBOL(gro_find_complete_by_type);
> >
> > +/* Pass the currently batched GRO_NORMAL SKBs up to the stack. */
> > +static void gro_normal_list(struct napi_struct *napi)
> > +{
> > +	if (!napi->rx_count)
> > +		return;
> > +	netif_receive_skb_list_internal(&napi->rx_list);
> > +	INIT_LIST_HEAD(&napi->rx_list);
> > +	napi->rx_count = 0;
> > +}
> > +
> > +/* Queue one GRO_NORMAL SKB up for list processing.  If batch size 
> > exceeded,
> > + * pass the whole batch up to the stack.
> > + */
> > +static void gro_normal_one(struct napi_struct *napi, struct sk_buff 
> > *skb)
> > +{
> > +	list_add_tail(&skb->list, &napi->rx_list);
> > +	if (++napi->rx_count >= gro_normal_batch)
> > +		gro_normal_list(napi);
> > +}
> > +
> >  static void napi_skb_free_stolen_head(struct sk_buff *skb)
> >  {
> >  	skb_dst_drop(skb);
> > @@ -5891,12 +5911,13 @@ static void napi_skb_free_stolen_head(struct 
> > sk_buff *skb)
> >  	kmem_cache_free(skbuff_head_cache, skb);
> >  }
> >
> > -static gro_result_t napi_skb_finish(gro_result_t ret, struct sk_buff 
> > *skb)
> > +static gro_result_t napi_skb_finish(struct napi_struct *napi,
> > +				    struct sk_buff *skb,
> > +				    gro_result_t ret)
> Any reason why the argument order here is changed around?

Actually yes: to match napi_skb_finish() and napi_frags_finish()
prototypes, as gro_normal one() required an addition of napi
argument anyway.

> -Ed
> >  {
> >  	switch (ret) {
> >  	case GRO_NORMAL:
> > -		if (netif_receive_skb_internal(skb))
> > -			ret = GRO_DROP;
> > +		gro_normal_one(napi, skb);
> >  		break;
> >
> >  	case GRO_DROP:
> > @@ -5928,7 +5949,7 @@ gro_result_t napi_gro_receive(struct napi_struct 
> > *napi, struct sk_buff *skb)
> >
> >  	skb_gro_reset_offset(skb);
> >
> > -	ret = napi_skb_finish(dev_gro_receive(napi, skb), skb);
> > +	ret = napi_skb_finish(napi, skb, dev_gro_receive(napi, skb));
> >  	trace_napi_gro_receive_exit(ret);
> >
> >  	return ret;
> > @@ -5974,26 +5995,6 @@ struct sk_buff *napi_get_frags(struct 
> > napi_struct *napi)
> >  }
> >  EXPORT_SYMBOL(napi_get_frags);
> >
> > -/* Pass the currently batched GRO_NORMAL SKBs up to the stack. */
> > -static void gro_normal_list(struct napi_struct *napi)
> > -{
> > -	if (!napi->rx_count)
> > -		return;
> > -	netif_receive_skb_list_internal(&napi->rx_list);
> > -	INIT_LIST_HEAD(&napi->rx_list);
> > -	napi->rx_count = 0;
> > -}
> > -
> > -/* Queue one GRO_NORMAL SKB up for list processing.  If batch size 
> > exceeded,
> > - * pass the whole batch up to the stack.
> > - */
> > -static void gro_normal_one(struct napi_struct *napi, struct sk_buff 
> > *skb)
> > -{
> > -	list_add_tail(&skb->list, &napi->rx_list);
> > -	if (++napi->rx_count >= gro_normal_batch)
> > -		gro_normal_list(napi);
> > -}
> > -
> >  static gro_result_t napi_frags_finish(struct napi_struct *napi,
> >  				      struct sk_buff *skb,
> >  				      gro_result_t ret)

Regards,
á á á á á á