Re: [net PATCH] skb: Do mix page pool and page referenced frags in GRO

From: Paolo Abeni
Date: Mon Jan 30 2023 - 03:51:49 EST


On Sat, 2023-01-28 at 08:08 +0100, Eric Dumazet wrote:
> On Sat, Jan 28, 2023 at 6:26 AM Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
> >
> > On Sat, 28 Jan 2023 10:37:47 +0800 Yunsheng Lin wrote:
> > > If we are not allowing gro for the above case, setting NAPI_GRO_CB(p)->flush
> > > to 1 in gro_list_prepare() seems to be making more sense so that the above
> > > case has the same handling as skb_has_frag_list() handling?
> > > https://elixir.bootlin.com/linux/v6.2-rc4/source/net/core/gro.c#L503
> > >
> > > As it seems to avoid some unnecessary operation according to comment
> > > in tcp4_gro_receive():
> > > https://elixir.bootlin.com/linux/v6.2-rc4/source/net/ipv4/tcp_offload.c#L322
> >
> > The frag_list case can be determined with just the input skb.
> > For pp_recycle we need to compare input skb's pp_recycle with
> > the pp_recycle of the skb already held by GRO.
> >
> > I'll hold off with applying a bit longer tho, in case Eric
> > wants to chime in with an ack or opinion.
>
> We can say that we are adding in the fast path an expensive check
> about an unlikely condition.
>
> GRO is by far the most expensive component in our stack.

Slightly related to the above: currently the GRO engine performs the
skb metadata check for every packet. My understanding is that even with
XDP enabled and ebpf running on the given packet, the skb should
usually have meta_len == 0. 

What about setting 'skb->slow_gro' together with meta_len and moving
the skb_metadata_differs() check under the slow_gro guard?

Cheers,

Paolo