netdev - Re: [PATCH net-next v7 02/10] net: ethtool: add support for configuring hds-thresh

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: <20250106190456.104e313e@kernel.org>
Date: Mon, 6 Jan 2025 19:04:56 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Taehee Yoo <ap420073@...il.com>
Cc: davem@...emloft.net, pabeni@...hat.com, edumazet@...gle.com,
 almasrymina@...gle.com, donald.hunter@...il.com, corbet@....net,
 michael.chan@...adcom.com, andrew+netdev@...n.ch, hawk@...nel.org,
 ilias.apalodimas@...aro.org, ast@...nel.org, daniel@...earbox.net,
 john.fastabend@...il.com, dw@...idwei.uk, sdf@...ichev.me,
 asml.silence@...il.com, brett.creeley@....com, linux-doc@...r.kernel.org,
 netdev@...r.kernel.org, kory.maincent@...tlin.com,
 maxime.chevallier@...tlin.com, danieller@...dia.com,
 hengqi@...ux.alibaba.com, ecree.xilinx@...il.com,
 przemyslaw.kitszel@...el.com, hkallweit1@...il.com, ahmed.zaki@...el.com,
 rrameshbabu@...dia.com, idosch@...dia.com, jiri@...nulli.us,
 bigeasy@...utronix.de, lorenzo@...nel.org, jdamato@...tly.com,
 aleksander.lobakin@...el.com, kaiyuanz@...gle.com, willemb@...gle.com,
 daniel.zahka@...il.com
Subject: Re: [PATCH net-next v7 02/10] net: ethtool: add support for
 configuring hds-thresh

On Mon, 6 Jan 2025 18:48:54 -0800 Jakub Kicinski wrote:
> >   * @module_fw_flash_in_progress: Module firmware flashing is in progress.
> > @@ -1141,6 +1148,7 @@ int ethtool_virtdev_set_link_ksettings(struct net_device *dev,
> >  struct ethtool_netdev_state {
> >  	struct xarray		rss_ctx;
> >  	struct mutex		rss_lock;
> > +	u32			hds_thresh;  
> 
> this value is checked in devmem.c but nothing ever sets it.
> net/ethtool/rings.c needs to handle it like it handles
> dev->ethtool->hds_config

Oh, I see you set it in the driver in patch 8.
That should work, my only concern is that this is not how
any of the other ethtool config options work today.
And there isn't any big warning in the code here telling
driver authors that they are responsible for the state update.

So even tho your patches are correct I still think it's better 
to handle it like hds_config, just for consistency.