On Tue, Jan 7, 2025 at 12:04 PM Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
>
Hi Jakub,
Thanks a lot for the review!
> 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.
Thanks, I will set hds_thresh in the ethnl_set_rings() like the hds_config.
So, I will remove code setting hds_thresh in the netdevsim and bnxt driver.
Also, I will change the comments of hds_thresh and hds_thresh_max to
your suggestion!
Thank you so much!
Taehee Yoo
