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: <SJ1PR11MB6180C920CED9ABECC9FD022EB8F3A@SJ1PR11MB6180.namprd11.prod.outlook.com>
Date: Sun, 10 Sep 2023 14:41:50 +0000
From: "Zulkifli, Muhammad Husaini" <muhammad.husaini.zulkifli@...el.com>
To: Simon Horman <horms@...nel.org>
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

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...

> 
> e.g. (untested!)
> 
>   # ethool -c <interface>
>   ...
>   rx-usecs: 3
>   ...
>   tx-usecs: 3
>   ...
> 
>   # ethool -C <interface> tx-usecs 3 rx-usecs 4
> 
>   # ethool -c <interface>
>   ...
>   rx-usecs: 4
>   ...
>   tx-usecs: 4
>   ...
> 
> ...