linux-kernel - Re: [RFC net-next 1/9] skb: introduce gro

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 21 Jun 2024 10:34:08 -0500
From: Yan Zhai <yan@...udflare.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Jesper Dangaard Brouer <hawk@...nel.org>, John Fastabend <john.fastabend@...il.com>, 
	Willem de Bruijn <willemb@...gle.com>, Simon Horman <horms@...nel.org>, Florian Westphal <fw@...len.de>, 
	Mina Almasry <almasrymina@...gle.com>, Abhishek Chauhan <quic_abchauha@...cinc.com>, 
	David Howells <dhowells@...hat.com>, Alexander Lobakin <aleksander.lobakin@...el.com>, 
	David Ahern <dsahern@...nel.org>, Richard Gobert <richardbgobert@...il.com>, 
	Antoine Tenart <atenart@...nel.org>, Felix Fietkau <nbd@....name>, 
	Soheil Hassas Yeganeh <soheil@...gle.com>, Pavel Begunkov <asml.silence@...il.com>, 
	Lorenzo Bianconi <lorenzo@...nel.org>, Thomas Weißschuh <linux@...ssschuh.net>, 
	linux-kernel@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [RFC net-next 1/9] skb: introduce gro_disabled bit

> > -static inline bool netif_elide_gro(const struct net_device *dev)
> > +static inline bool netif_elide_gro(const struct sk_buff *skb)
> >  {
> > -     if (!(dev->features & NETIF_F_GRO) || dev->xdp_prog)
> > +     if (!(skb->dev->features & NETIF_F_GRO) || skb->dev->xdp_prog)
> >               return true;
> > +
> > +#ifdef CONFIG_SKB_GRO_CONTROL
> > +     return skb->gro_disabled;
> > +#else
> >       return false;
> > +#endif
>
> Yet more branches in the hot path.
>
> Compile time configurability does not help, as that will be
> enabled by distros.
>
> For a fairly niche use case. Where functionality of GRO already
> works. So just a performance for a very rare case at the cost of a
> regression in the common case. A small regression perhaps, but death
> by a thousand cuts.
>

I share your concern on operating on this hotpath. Will a
static_branch + sysctl make it less aggressive? Speaking of
performance, I'd hope this can give us more control so we can achieve
the best of two worlds: for TCP and some UDP traffic, we can enable
GRO, while for some other classes that we know GRO does no good or
even harm, let's disable GRO to save more cycles. The key observation
is that developers may already know which traffic is blessed by GRO,
but lack a way to realize it.

best
Yan