linux-kernel - Re: [PATCH net-next v4 06/24] net: clarify the meaning of netdev

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: <85a85b35-3862-4260-833c-267720125817@gmail.com>
Date: Tue, 14 Oct 2025 13:53:14 +0100
From: Pavel Begunkov <asml.silence@...il.com>
To: Randy Dunlap <rdunlap@...radead.org>, netdev@...r.kernel.org
Cc: Andrew Lunn <andrew@...n.ch>, Jakub Kicinski <kuba@...nel.org>,
 davem@...emloft.net, Eric Dumazet <edumazet@...gle.com>,
 Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
 Donald Hunter <donald.hunter@...il.com>,
 Michael Chan <michael.chan@...adcom.com>,
 Pavan Chebbi <pavan.chebbi@...adcom.com>,
 Jesper Dangaard Brouer <hawk@...nel.org>,
 John Fastabend <john.fastabend@...il.com>,
 Stanislav Fomichev <sdf@...ichev.me>, Joshua Washington
 <joshwash@...gle.com>, Harshitha Ramamurthy <hramamurthy@...gle.com>,
 Jian Shen <shenjian15@...wei.com>, Salil Mehta <salil.mehta@...wei.com>,
 Jijie Shao <shaojijie@...wei.com>, Sunil Goutham <sgoutham@...vell.com>,
 Geetha sowjanya <gakula@...vell.com>, Subbaraya Sundeep
 <sbhatta@...vell.com>, hariprasad <hkelam@...vell.com>,
 Bharat Bhushan <bbhushan2@...vell.com>, Saeed Mahameed <saeedm@...dia.com>,
 Tariq Toukan <tariqt@...dia.com>, Mark Bloch <mbloch@...dia.com>,
 Leon Romanovsky <leon@...nel.org>, Alexander Duyck <alexanderduyck@...com>,
 kernel-team@...a.com, Ilias Apalodimas <ilias.apalodimas@...aro.org>,
 Joe Damato <joe@...a.to>, David Wei <dw@...idwei.uk>,
 Willem de Bruijn <willemb@...gle.com>, Mina Almasry
 <almasrymina@...gle.com>, Breno Leitao <leitao@...ian.org>,
 Dragos Tatulea <dtatulea@...dia.com>, linux-kernel@...r.kernel.org,
 linux-doc@...r.kernel.org, linux-rdma@...r.kernel.org,
 Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH net-next v4 06/24] net: clarify the meaning of
 netdev_config members

On 10/13/25 18:12, Randy Dunlap wrote:
> Hi,
> 
> On 10/13/25 7:54 AM, Pavel Begunkov wrote:
>> From: Jakub Kicinski <kuba@...nel.org>
>>
>> hds_thresh and hds_config are both inside struct netdev_config
>> but have quite different semantics. hds_config is the user config
>> with ternary semantics (on/off/unset). hds_thresh is a straight
>> up value, populated by the driver at init and only modified by
>> user space. We don't expect the drivers to have to pick a special
>> hds_thresh value based on other configuration.
>>
>> The two approaches have different advantages and downsides.
>> hds_thresh ("direct value") gives core easy access to current
>> device settings, but there's no way to express whether the value
>> comes from the user. It also requires the initialization by
>> the driver.
>>
>> hds_config ("user config values") tells us what user wanted, but
>> doesn't give us the current value in the core.
>>
>> Try to explain this a bit in the comments, so at we make a conscious
>> choice for new values which semantics we expect.
>>
>> Move the init inside ethtool_ringparam_get_cfg() to reflect the semantics.
>> Commit 216a61d33c07 ("net: ethtool: fix ethtool_ringparam_get_cfg()
>> returns a hds_thresh value always as 0.") added the setting for the
>> benefit of netdevsim which doesn't touch the value at all on get.
>> Again, this is just to clarify the intention, shouldn't cause any
>> functional change.
>>
>> Signed-off-by: Jakub Kicinski <kuba@...nel.org>
>> [pavel: applied clarification on relationship b/w HDS thresh and config]
>> Signed-off-by: Pavel Begunkov <asml.silence@...il.com>
>> ---
>>   include/net/netdev_queues.h | 20 ++++++++++++++++++--
>>   net/ethtool/common.c        |  3 ++-
>>   2 files changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/net/netdev_queues.h b/include/net/netdev_queues.h
>> index cd00e0406cf4..9d5dde36c2e5 100644
>> --- a/include/net/netdev_queues.h
>> +++ b/include/net/netdev_queues.h
>> @@ -6,11 +6,27 @@
>>   
>>   /**
>>    * struct netdev_config - queue-related configuration for a netdev
>> - * @hds_thresh:		HDS Threshold value.
>> - * @hds_config:		HDS value from userspace.
>>    */
>>   struct netdev_config {
>> +	/* Direct value
>> +	 *
>> +	 * Driver default is expected to be fixed, and set in this struct
>> +	 * at init. From that point on user may change the value. There is
>> +	 * no explicit way to "unset" / restore driver default. Used only
>> +	 * when @hds_config is set.
>> +	 */
>> +	/** @hds_thresh: HDS Threshold value (ETHTOOL_A_RINGS_HDS_THRESH).
>> +	 */
>>   	u32	hds_thresh;
>> +
>> +	/* User config values
>> +	 *
>> +	 * Contain user configuration. If "set" driver must obey.
>> +	 * If "unset" driver is free to decide, and may change its choice
>> +	 * as other parameters change.
>> +	 */
>> +	/** @hds_config: HDS enabled (ETHTOOL_A_RINGS_TCP_DATA_SPLIT).
>> +	 */
>>   	u8	hds_config;
>>   };
> 
> kernel-doc comments should being with
> /**
> on a separate line. This will prevent warnings like these new ones:
> 
> Warning: include/net/netdev_queues.h:36 struct member 'hds_thresh' not described in 'netdev_config'
> Warning: include/net/netdev_queues.h:36 struct member 'hds_config' not described in 'netdev_config'
> Warning: include/net/netdev_queues.h:36 struct member 'rx_buf_len' not described in 'netdev_config'
> 
> (but there are 4 variables that this applies to. I don't know why kernel-doc.py
> only reported 3 of them.)

Thanks for taking a look! I was a bit reluctant to change it
too much since it's not authored by me. I'll be moving in direction
of removing the patch completely.

-- 
Pavel Begunkov