[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <595d754a-c8c2-4ef0-bdbd-3ff25330224a@amd.com>
Date: Fri, 15 Aug 2025 15:08:20 +0530
From: "Badole, Vishal" <vishal.badole@....com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Shyam-sundar.S-k@....com, andrew+netdev@...n.ch, davem@...emloft.net,
edumazet@...gle.com, pabeni@...hat.com, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 net-next] amd-xgbe: Configure and retrieve 'tx-usecs'
for Tx coalescing
On 8/14/2025 5:30 AM, Jakub Kicinski wrote:
> On Tue, 12 Aug 2025 10:20:35 +0530 Vishal Badole wrote:
>> Ethtool has advanced with additional configurable options, but the
>> current driver does not support tx-usecs configuration.
>
> Not sure what you mean by this, perhaps:
>
> current driver does not even support tx-usecs configuration.
>
> ? tx-usecs is a very old tunable.
>
>> Add support to configure and retrieve 'tx-usecs' using ethtool, which
>> specifies the wait time before servicing an interrupt for Tx coalescing.
>>
Thanks for pointing that out. My intent was to highlight that while
ethtool has gained more advanced and configurable options over time, the
driver in its current form does not have support for tx-usecs
configuration using Ethool.
I’ll reword the sentence to make this clearer in next patch version.>
>> + /* Check if both tx_usecs and tx_frames are set to 0 simultaneously */
>> + if (!tx_usecs && !tx_frames) {
>> + NL_SET_ERR_MSG_FMT_MOD(extack,
>> + "tx_usecs and tx_frames must not be 0 together");
>> + return -EINVAL;
>> + }
>> +
>> /* Check the bounds of values for Tx */
>> + if (tx_usecs > XGMAC_MAX_COAL_TX_TICK) {
>> + NL_SET_ERR_MSG_FMT_MOD(extack, "tx-usecs is limited to %d usec",
>> + XGMAC_MAX_COAL_TX_TICK);
>> + return -EINVAL;
>> + }
>
> Normal configuration granularity for this parameter is in 10s of usecs.
> You seem to be using a timer, so I think you should either round the
> value up / down to what the jiffy resolution will give you or
> reject configuration that's not expressible in jiffies (not a multiple
> of jiffies_to_usecs(1)). Otherwise users may waste time turning this
> knob by 100usec which will have zero effect.
Good point — the hardware uses a timer, so sub-jiffy resolution won’t
have any real effect. I’ll update the code in next patch version to
round the tx-usecs value to the nearest multiple.
Powered by blists - more mailing lists