netdev - Re: [PATCH iwl-net v5] igc: Expose tx-usecs coalesce setting to user

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: <20230911055520.GM775887@kernel.org>
Date: Mon, 11 Sep 2023 07:55:20 +0200
From: Simon Horman <horms@...nel.org>
To: "Zulkifli, Muhammad Husaini" <muhammad.husaini.zulkifli@...el.com>
Cc: "intel-wired-lan@...osl.org" <intel-wired-lan@...osl.org>,
	"Neftin, Sasha" <sasha.neftin@...el.com>,
	"bcreeley@....com" <bcreeley@....com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>,
	"edumazet@...gle.com" <edumazet@...gle.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"naamax.meir@...ux.intel.com" <naamax.meir@...ux.intel.com>,
	"Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
	"husainizulkifli@...il.com" <husainizulkifli@...il.com>
Subject: Re: [PATCH iwl-net v5] igc: Expose tx-usecs coalesce setting to user

On Sun, Sep 10, 2023 at 02:41:50PM +0000, Zulkifli, Muhammad Husaini wrote:
> Dear Simon,
> 
> Thanks for reviewing. Replied inline
> 
> > -----Original Message-----
> > From: Simon Horman <horms@...nel.org>
> > Sent: Sunday, 10 September, 2023 10:24 PM
> > To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@...el.com>
> > Cc: intel-wired-lan@...osl.org; Neftin, Sasha <sasha.neftin@...el.com>;
> > bcreeley@....com; davem@...emloft.net; kuba@...nel.org;
> > pabeni@...hat.com; edumazet@...gle.com; netdev@...r.kernel.org;
> > naamax.meir@...ux.intel.com; Nguyen, Anthony L
> > <anthony.l.nguyen@...el.com>; husainizulkifli@...il.com
> > Subject: Re: [PATCH iwl-net v5] igc: Expose tx-usecs coalesce setting to user
> > 
> > On Fri, Sep 08, 2023 at 04:17:34PM +0800, Muhammad Husaini Zulkifli wrote:
> > > When users attempt to obtain the coalesce setting using the ethtool
> > > command, current code always returns 0 for tx-usecs.
> > > This is because I225/6 always uses a queue pair setting, hence
> > > tx_coalesce_usecs does not return a value during the
> > > igc_ethtool_get_coalesce() callback process. The pair queue condition
> > > checking in igc_ethtool_get_coalesce() is removed by this patch so
> > > that the user gets information of the value of tx-usecs.
> > >
> > > Even if i225/6 is using queue pair setting, there is no harm in
> > > notifying the user of the tx-usecs. The implementation of the current
> > > code may have previously been a copy of the legacy code i210.
> > > Since I225 has the queue pair setting enabled, tx-usecs will always
> > > adhere to the user-set rx-usecs value. An error message will appear
> > > when the user attempts to set the tx-usecs value for the input
> > > parameters because, by default, they should only set the rx-usecs value.
> > 
> > Hi Muhammad,
> > 
> > Most likely I'm misunderstanding things. And even if that is not the case
> > perhaps this is as good as it gets. But my reading is that an error will not be
> > raised if a user provides an input value for tx-usecs that matches the current
> > value of tx-usecs, even if a different value is provided for rx-usecs (which will
> > also be applied to tx-usecs).
> 
> Yes you are right. This is what I mentioned in previous version discussion.
> https://lore.kernel.org/netdev/20230905101504.4a9da6b8@kernel.org/
> But at least IMHO, better than current or my previous design submission.
> 
> Previously, I had considered changing the ".supported_coalesce_params"
> during ethtool set ops to only set ETHTOOL_COALESCE_RX_USECS with a new
> define of ETHTOOL_QUEUE_PAIR_COALESCE_USECS. But, if we change the
> queue/cpu during runtime setting, I believe this
> ".supported_coalesce_params" need to change as well...

Thanks Muhammad, and sorry for missing that thread.

With that discussion in mind, I think that what this patch does is as good
as it gets with the current uAPI, and changes to the uAPI is a follow-up
topic.

So, FWIIW, I am happy with this patch as it is.

Reviewed-by: Simon Horman <horms@...nel.org>