netdev - Re: [PATCH net-next] gve: add header split support

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] [day] [month] [year] [list]

Date: Tue, 15 Aug 2023 13:06:29 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Ziwei Xiao <ziweixiao@...gle.com>, netdev@...r.kernel.org, davem@...emloft.net, 
	Jeroen de Borst <jeroendb@...gle.com>, Praveen Kaligineedi <pkaligineedi@...gle.com>, 
	Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net-next] gve: add header split support

On Mon, Aug 14, 2023 at 10:22 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Fri, 11 Aug 2023 15:39:38 -0700 Ziwei Xiao wrote:
> > - Add header-split and strict-header-split ethtool priv flags. These
> >   flags control header-split behavior. It can be turned on/off and it
> >   can be set to 'strict' which will cause the driver to drop all the
> >   packets that don't have a proper header split.
> > - Add max-rx-buffer-size priv flag to allow user to switch the packet
> >   buffer size between max and default(e.g. 4K <-> 2K).
> > - Add reconfigure rx rings to support the header split and
> >   max-rx-buffer-size enable/disable switch.

The bulleted list is an indication that maybe this patch is combining
too much in a single commit.

> Someone on your team needs to participate upstream or you need
> to get your patches reviewed from someone upstream-savvy before
> posting.
>
> Anyone participating in netdev reviews would have told you that
> private flags are unlikely to fly upstream.
>
> One part of an organization participating upstream while another
> has no upstream understanding and throws code over the wall is
> a common anti-pattern, and I intend to stop it.

The one point I want to raise about private flags is that ethtool
currently lacks an alternative for header-split.

That probably requires a non-driver patch to add as a new ethtool feature.

I had not given much thought on the two modes of header-split seen
here until now: strict or not. We'll have to figure out whether both
are really needed, especially if turning into ethtool ABI.