netdev - Re: [RFC v1 05/22] net: add rx_buf

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]

Message-ID: <20250801161850.0ea0a5f1@kernel.org>
Date: Fri, 1 Aug 2025 16:18:50 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Mina Almasry <almasrymina@...gle.com>
Cc: Pavel Begunkov <asml.silence@...il.com>, netdev@...r.kernel.org,
 io-uring@...r.kernel.org, Eric Dumazet <edumazet@...gle.com>, Willem de
 Bruijn <willemb@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
 andrew+netdev@...n.ch, horms@...nel.org, davem@...emloft.net,
 sdf@...ichev.me, dw@...idwei.uk, michael.chan@...adcom.com,
 dtatulea@...dia.com, ap420073@...il.com
Subject: Re: [RFC v1 05/22] net: add rx_buf_len to netdev config

On Mon, 28 Jul 2025 14:50:12 -0700 Mina Almasry wrote:
> On Mon, Jul 28, 2025 at 4:03 AM Pavel Begunkov <asml.silence@...il.com> wrote:
> > Add rx_buf_len to configuration maintained by the core.
> > Use "three-state" semantics where 0 means "driver default".
> 
> What are three states in the semantics here?
> 
> - 0 = driver default.
> - non-zero means value set by userspace
> 
> What is the 3rd state here?

I just mean a value with an explicit default / unset state.
If you have a better name I'm all ears ..
> > diff --git a/net/ethtool/common.c b/net/ethtool/common.c
> > index a87298f659f5..8fdffc77e981 100644
> > --- a/net/ethtool/common.c
> > +++ b/net/ethtool/common.c
> > @@ -832,6 +832,7 @@ void ethtool_ringparam_get_cfg(struct net_device *dev,
> >
> >         /* Driver gives us current state, we want to return current config */
> >         kparam->tcp_data_split = dev->cfg->hds_config;
> > +       kparam->rx_buf_len = dev->cfg->rx_buf_len;  
> 
> I'm confused that struct netdev_config is defined in netdev_queues.h,
> and is documented to be a queue-related configuration, but doesn't
> seem to be actually per queue? This line is grabbing the current
> config for this queue from dev->cfg which looks like a shared value.
> 
> I don't think rx_buf_len should be a shared value between all the
> queues. I strongly think it should a per-queue value. The
> devmem/io_uring queues will probably want large rx_buf_len, but normal
> queues will want 0 buf len, me thinks.

I presume that question answered itself as you were reading the rest 
of the patches? :)