netdev - Re: [PATCH net v2 1/2] 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]

Date: Tue, 1 Aug 2023 11:20:22 -0700
From: Brett Creeley <bcreeley@....com>
To: Tony Nguyen <anthony.l.nguyen@...el.com>, davem@...emloft.net,
 kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com,
 netdev@...r.kernel.org
Cc: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@...el.com>,
 sasha.neftin@...el.com, Naama Meir <naamax.meir@...ux.intel.com>,
 Simon Horman <horms@...nel.org>
Subject: Re: [PATCH net v2 1/2] igc: Expose tx-usecs coalesce setting to user

On 8/1/2023 10:27 AM, Tony Nguyen wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@...el.com>
> 
> 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.
> 
> How to test:
> User can get the coalesce value using ethtool command.
> 
> Example command:
> Get: ethtool -c <interface>
> 
> Previous output:
> 
> rx-usecs: 3
> rx-frames: n/a
> rx-usecs-irq: n/a
> rx-frames-irq: n/a
> 
> tx-usecs: 0
> tx-frames: n/a
> tx-usecs-irq: n/a
> tx-frames-irq: n/a
> 
> New output:
> 
> rx-usecs: 3
> rx-frames: n/a
> rx-usecs-irq: n/a
> rx-frames-irq: n/a
> 
> tx-usecs: 3
> tx-frames: n/a
> tx-usecs-irq: n/a
> tx-frames-irq: n/a
> 
> Fixes: 8c5ad0dae93c ("igc: Add ethtool support")
> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@...el.com>
> Tested-by: Naama Meir <naamax.meir@...ux.intel.com>
> Reviewed-by: Simon Horman <horms@...nel.org>
> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
> ---
>   drivers/net/ethernet/intel/igc/igc_ethtool.c | 13 ++++---------
>   1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
> index 93bce729be76..62d925b26f2c 100644
> --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
> +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
> @@ -880,12 +880,10 @@ static int igc_ethtool_get_coalesce(struct net_device *netdev,
>          else
>                  ec->rx_coalesce_usecs = adapter->rx_itr_setting >> 2;
> 
> -       if (!(adapter->flags & IGC_FLAG_QUEUE_PAIRS)) {
> -               if (adapter->tx_itr_setting <= 3)
> -                       ec->tx_coalesce_usecs = adapter->tx_itr_setting;
> -               else
> -                       ec->tx_coalesce_usecs = adapter->tx_itr_setting >> 2;
> -       }
> +       if (adapter->tx_itr_setting <= 3)
> +               ec->tx_coalesce_usecs = adapter->tx_itr_setting;
> +       else
> +               ec->tx_coalesce_usecs = adapter->tx_itr_setting >> 2;
> 
>          return 0;
>   }
> @@ -910,9 +908,6 @@ static int igc_ethtool_set_coalesce(struct net_device *netdev,
>              ec->tx_coalesce_usecs == 2)
>                  return -EINVAL;
> 
> -       if ((adapter->flags & IGC_FLAG_QUEUE_PAIRS) && ec->tx_coalesce_usecs)
> -               return -EINVAL;
> -

Should this be part of patch 2/2 or is it necessary to remove this for 
the get flow?

>          /* If ITR is disabled, disable DMAC */
>          if (ec->rx_coalesce_usecs == 0) {
>                  if (adapter->flags & IGC_FLAG_DMAC)
> --
> 2.38.1
> 
>