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]

Message-ID: <SJ1PR11MB6180D8DC61608FAFAC535A9EB80AA@SJ1PR11MB6180.namprd11.prod.outlook.com>
Date: Tue, 1 Aug 2023 23:46:32 +0000
From: "Zulkifli, Muhammad Husaini" <muhammad.husaini.zulkifli@...el.com>
To: Brett Creeley <bcreeley@....com>, "Nguyen, Anthony L"
	<anthony.l.nguyen@...el.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>
CC: "Neftin, Sasha" <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

Hi Brett,

Thanks for the review.

> -----Original Message-----
> From: Brett Creeley <bcreeley@....com>
> Sent: Wednesday, 2 August, 2023 2:20 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@...el.com>; davem@...emloft.net;
> kuba@...nel.org; pabeni@...hat.com; edumazet@...gle.com;
> netdev@...r.kernel.org
> Cc: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@...el.com>;
> Neftin, Sasha <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?

It should be part of 1/2. 
You won't be able to set the rx-usecs if we don't remove this line since the tx-usec will 
begin to have value.

Thanks,
Husaini

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