netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Taehee Yoo <ap420073@gmail.com>
Cc: davem@davemloft.net, pabeni@redhat.com, edumazet@google.com,
	almasrymina@google.com, donald.hunter@gmail.com, corbet@lwn.net,
	michael.chan@broadcom.com, andrew+netdev@lunn.ch,
	hawk@kernel.org, ilias.apalodimas@linaro.org, ast@kernel.org,
	daniel@iogearbox.net, john.fastabend@gmail.com, dw@davidwei.uk,
	sdf@fomichev.me, asml.silence@gmail.com, brett.creeley@amd.com,
	linux-doc@vger.kernel.org, netdev@vger.kernel.org,
	kory.maincent@bootlin.com, maxime.chevallier@bootlin.com,
	danieller@nvidia.com, hengqi@linux.alibaba.com,
	ecree.xilinx@gmail.com, przemyslaw.kitszel@intel.com,
	hkallweit1@gmail.com, ahmed.zaki@intel.com,
	rrameshbabu@nvidia.com, idosch@nvidia.com, jiri@resnulli.us,
	bigeasy@linutronix.de, lorenzo@kernel.org, jdamato@fastly.com,
	aleksander.lobakin@intel.com, kaiyuanz@google.com,
	willemb@google.com, daniel.zahka@gmail.com
Subject: Re: [PATCH net-next v7 02/10] net: ethtool: add support for configuring hds-thresh
Date: Mon, 6 Jan 2025 19:04:56 -0800	[thread overview]
Message-ID: <20250106190456.104e313e@kernel.org> (raw)
In-Reply-To: <20250106184854.4028c83d@kernel.org>

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.

  reply	other threads:[~2025-01-07  3:04 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-03 15:03 [PATCH net-next v7 0/10] bnxt_en: implement tcp-data-split and thresh option Taehee Yoo
2025-01-03 15:03 ` [PATCH net-next v7 01/10] net: ethtool: add hds_config member in ethtool_netdev_state Taehee Yoo
2025-01-03 15:03 ` [PATCH net-next v7 02/10] net: ethtool: add support for configuring hds-thresh Taehee Yoo
2025-01-07  2:48   ` Jakub Kicinski
2025-01-07  3:04     ` Jakub Kicinski [this message]
2025-01-07 14:52       ` Taehee Yoo
2025-01-03 15:03 ` [PATCH net-next v7 03/10] net: devmem: add ring parameter filtering Taehee Yoo
2025-01-03 15:03 ` [PATCH net-next v7 04/10] net: ethtool: " Taehee Yoo
2025-01-07  2:50   ` Jakub Kicinski
2025-01-03 15:03 ` [PATCH net-next v7 05/10] net: disallow setup single buffer XDP when tcp-data-split is enabled Taehee Yoo
2025-01-07  2:52   ` Jakub Kicinski
2025-01-03 15:03 ` [PATCH net-next v7 06/10] bnxt_en: add support for rx-copybreak ethtool command Taehee Yoo
2025-01-07  2:53   ` Jakub Kicinski
2025-01-10 19:12   ` Andy Gospodarek
2025-01-03 15:03 ` [PATCH net-next v7 07/10] bnxt_en: add support for tcp-data-split " Taehee Yoo
2025-01-07  2:54   ` Jakub Kicinski
2025-01-10 19:17   ` Andy Gospodarek
2025-01-03 15:03 ` [PATCH net-next v7 08/10] bnxt_en: add support for hds-thresh " Taehee Yoo
2025-01-08 15:38   ` Andy Gospodarek
2025-01-10 19:11   ` Andy Gospodarek
2025-01-11 13:45     ` Taehee Yoo
2025-01-03 15:03 ` [PATCH net-next v7 09/10] netdevsim: add HDS feature Taehee Yoo
2025-01-03 15:03 ` [PATCH net-next v7 10/10] selftest: net-drv: hds: add test for " Taehee Yoo
2025-01-07  3:01   ` Jakub Kicinski
2025-01-07 14:56     ` Taehee Yoo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250106190456.104e313e@kernel.org \
    --to=kuba@kernel.org \
    --cc=ahmed.zaki@intel.com \
    --cc=aleksander.lobakin@intel.com \
    --cc=almasrymina@google.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=ap420073@gmail.com \
    --cc=asml.silence@gmail.com \
    --cc=ast@kernel.org \
    --cc=bigeasy@linutronix.de \
    --cc=brett.creeley@amd.com \
    --cc=corbet@lwn.net \
    --cc=daniel.zahka@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=danieller@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=donald.hunter@gmail.com \
    --cc=dw@davidwei.uk \
    --cc=ecree.xilinx@gmail.com \
    --cc=edumazet@google.com \
    --cc=hawk@kernel.org \
    --cc=hengqi@linux.alibaba.com \
    --cc=hkallweit1@gmail.com \
    --cc=idosch@nvidia.com \
    --cc=ilias.apalodimas@linaro.org \
    --cc=jdamato@fastly.com \
    --cc=jiri@resnulli.us \
    --cc=john.fastabend@gmail.com \
    --cc=kaiyuanz@google.com \
    --cc=kory.maincent@bootlin.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=lorenzo@kernel.org \
    --cc=maxime.chevallier@bootlin.com \
    --cc=michael.chan@broadcom.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=rrameshbabu@nvidia.com \
    --cc=sdf@fomichev.me \
    --cc=willemb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).