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
| ||
|
Message-ID: <SJ1PR11MB61801A9E39A77D8D89349FF4B818A@SJ1PR11MB6180.namprd11.prod.outlook.com> Date: Sat, 19 Aug 2023 13:21:58 +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 2/2] igc: Modify the tx-usecs coalesce setting Dear Brett, Sorry for the late reply. I truly miss these feedbacks. My bad ☹ > -----Original Message----- > From: Brett Creeley <bcreeley@....com> > Sent: Wednesday, 2 August, 2023 2:18 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 2/2] igc: Modify the tx-usecs coalesce setting > > 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> > > > > This patch enables users to modify the tx-usecs parameter. > > The rx-usecs value will adhere to the same value as tx-usecs if the > > queue pair setting is enabled. > > > > How to test: > > User can set the coalesce value using ethtool command. > > > > Example command: > > Set: ethtool -C <interface> > > > > Previous output: > > > > root@...DYHUSAINI:~# ethtool -C enp170s0 tx-usecs 10 netlink error: > > Invalid argument > > > > New output: > > > > root@...DYHUSAINI:~# ethtool -C enp170s0 tx-usecs 10 > > rx-usecs: 10 > > rx-frames: n/a > > rx-usecs-irq: n/a > > rx-frames-irq: n/a > > > > tx-usecs: 10 > > 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 | 49 +++++++++++++++----- > > 1 file changed, 37 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c > > b/drivers/net/ethernet/intel/igc/igc_ethtool.c > > index 62d925b26f2c..ed67d9061452 100644 > > --- a/drivers/net/ethernet/intel/igc/igc_ethtool.c > > +++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c > > @@ -914,19 +914,44 @@ static int igc_ethtool_set_coalesce(struct > net_device *netdev, > > adapter->flags &= ~IGC_FLAG_DMAC; > > } > > > > - /* 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; > > + > > + if (old_tx_itr != ec->tx_coalesce_usecs) { > > + if (ec->tx_coalesce_usecs && ec->tx_coalesce_usecs <= 3) > > + adapter->tx_itr_setting = ec->tx_coalesce_usecs; > > + else > > + adapter->tx_itr_setting = > > + ec->tx_coalesce_usecs << 2; > > It seems like this if/else flow is duplicated multiple times throughout this > patch. Maybe it would be useful to introduce a helper function so you can > just do the following: Yeah that is a great suggestion. Will do that. > > adapter->tx_itr_setting = > igc_ethtool_coalesce_to_itr_setting(ec->tx_coalesce); > > > + > > + adapter->rx_itr_setting = adapter->tx_itr_setting; > > + } else if (old_rx_itr != ec->rx_coalesce_usecs) { > > + 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; > > Seems like the helper function could work for both tx/rx: Yup. Both are using same condition. > > adapter->rx_itr_setting = > igc_ethtool_coalsece_to_itr_setting(ec->rx_coalesce); > > + > > + adapter->tx_itr_setting = adapter->rx_itr_setting; > > + } > > + } else { > > + /* 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; > > adapter->rx_itr_setting = > igc_ethtool_coalsece_to_itr_setting(ec->rx_coalesce); Noted. > > > > > - /* convert to rate of irq's per second */ > > - if (adapter->flags & IGC_FLAG_QUEUE_PAIRS) > > - adapter->tx_itr_setting = adapter->rx_itr_setting; > > - else if (ec->tx_coalesce_usecs && ec->tx_coalesce_usecs <= 3) > > - adapter->tx_itr_setting = ec->tx_coalesce_usecs; > > - else > > - adapter->tx_itr_setting = ec->tx_coalesce_usecs << 2; > > + /* convert to rate of irq's per second */ > > + if (ec->tx_coalesce_usecs && ec->tx_coalesce_usecs <= 3) > > + adapter->tx_itr_setting = ec->tx_coalesce_usecs; > > + else > > + adapter->tx_itr_setting = > > + ec->tx_coalesce_usecs << 2; > > adapter->tx_itr_setting = > igc_ethtool_coalesce_to_itr_setting(ec->tx_coalesce); Sure. 😊 > > > > + } > > > > for (i = 0; i < adapter->num_q_vectors; i++) { > > struct igc_q_vector *q_vector = adapter->q_vector[i]; > > -- > > 2.38.1 > > > >
Powered by blists - more mailing lists