[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAF=yD-JcWUhi3o==cQj5ByYosH0bQBxsyCk7fUAzMDXRX_fa9w@mail.gmail.com>
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.
Powered by blists - more mailing lists