Re: [PATCH net-next v3 05/23] net: clarify the meaning of netdev_config members
From: Mina Almasry <hidden>
Date: 2025-08-19 01:46:39
Also in:
io-uring, lkml
On Mon, Aug 18, 2025 at 6:56 AM Pavel Begunkov [off-list ref] wrote:
From: Jakub Kicinski <kuba@kernel.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.TBH I can't say that moving the init to before dev->ethtool_ops->get_ringparam(dev, param, kparam, extack) made me understand semantics better. If you do a respin, maybe a comment above the kparam->hds_thresh to say what you mean would help the next reader understand.
Signed-off-by: Jakub Kicinski <kuba@kernel.org> [pavel: applied clarification on relationship b/w HDS thresh and config] Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
Reviewed-by: Mina Almasry <redacted> -- Thanks, Mina