netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Tony Nguyen <anthony.l.nguyen@intel.com>
Cc: davem@davemloft.net, pabeni@redhat.com, edumazet@google.com,
	netdev@vger.kernel.org,
	Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>,
	sasha.neftin@intel.com, horms@kernel.org, bcreeley@amd.com,
	Naama Meir <naamax.meir@linux.intel.com>
Subject: Re: [PATCH net v3 2/2] igc: Modify the tx-usecs coalesce setting
Date: Wed, 23 Aug 2023 19:19:28 -0700	[thread overview]
Message-ID: <20230823191928.1a32aed7@kernel.org> (raw)
In-Reply-To: <20230822221620.2988753-3-anthony.l.nguyen@intel.com>

On Tue, 22 Aug 2023 15:16:20 -0700 Tony Nguyen wrote:
> root@P12DYHUSAINI:~# ethtool -C enp170s0 tx-usecs 10
> netlink error: Invalid argument

Why was it returning an error previously? It's not clear from just 
this patch.

> -	/* convert to rate of irq's per second */
> -	if (ec->rx_coalesce_usecs && ec->rx_coalesce_usecs <= 3)
> -		adapter->rx_itr_setting = ec->rx_coalesce_usecs;
> -	else
> -		adapter->rx_itr_setting = ec->rx_coalesce_usecs << 2;
> +	if (adapter->flags & IGC_FLAG_QUEUE_PAIRS) {
> +		u32 old_tx_itr, old_rx_itr;
> +
> +		/* This is to get back the original value before byte shifting */
> +		old_tx_itr = (adapter->tx_itr_setting <= 3) ?
> +			      adapter->tx_itr_setting : adapter->tx_itr_setting >> 2;
> +
> +		old_rx_itr = (adapter->rx_itr_setting <= 3) ?
> +			      adapter->rx_itr_setting : adapter->rx_itr_setting >> 2;
> +
> +		/* convert to rate of irq's per second */
> +		if (old_tx_itr != ec->tx_coalesce_usecs) {
> +			adapter->tx_itr_setting =
> +				igc_ethtool_coalesce_to_itr_setting(ec->tx_coalesce_usecs);
> +			adapter->rx_itr_setting = adapter->tx_itr_setting;
> +		} else if (old_rx_itr != ec->rx_coalesce_usecs) {
> +			adapter->rx_itr_setting =
> +				igc_ethtool_coalesce_to_itr_setting(ec->rx_coalesce_usecs);
> +			adapter->tx_itr_setting = adapter->rx_itr_setting;
> +		}

I'm not sure about this fix. Systems which try to converge
configuration like chef will keep issuing: 

ethtool -C enp170s0 tx-usecs 20 rx-usecs 10

and AFAICT the values will flip back and froth between 10 and 20, 
and never stabilize. Returning an error for unsupported config
sounds right to me. This function takes extack, you can tell 
the user what the problem is.
-- 
pw-bot: cr

  reply	other threads:[~2023-08-24  2:19 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-22 22:16 [PATCH net v3 0/2][pull request] igc: Enhance the tx-usecs coalesce setting implementation Tony Nguyen
2023-08-22 22:16 ` [PATCH net v3 1/2] igc: Expose tx-usecs coalesce setting to user Tony Nguyen
2023-08-22 22:16 ` [PATCH net v3 2/2] igc: Modify the tx-usecs coalesce setting Tony Nguyen
2023-08-24  2:19   ` Jakub Kicinski [this message]
2023-08-24 22:50     ` Zulkifli, Muhammad Husaini
2023-08-25  0:00       ` Jakub Kicinski
2023-08-25  3:44         ` Zulkifli, Muhammad Husaini
2023-08-26  0:34           ` Jakub Kicinski
2023-09-04  0:59             ` Zulkifli, Muhammad Husaini
2023-09-05 17:15               ` Jakub Kicinski
2023-09-06  2:52                 ` Zulkifli, Muhammad Husaini
2023-09-06 14:46                   ` Jakub Kicinski
2023-09-06 15:02                     ` Zulkifli, Muhammad Husaini

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=20230823191928.1a32aed7@kernel.org \
    --to=kuba@kernel.org \
    --cc=anthony.l.nguyen@intel.com \
    --cc=bcreeley@amd.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=muhammad.husaini.zulkifli@intel.com \
    --cc=naamax.meir@linux.intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sasha.neftin@intel.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).