| 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
| ||
|
Date: Fri, 19 Feb 2016 13:54:24 -0800
From: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
To: Kan Liang <kan.liang@...el.com>, netdev@...r.kernel.org,
davem@...emloft.net, bwh@...nel.org, ben@...adent.org.uk
Cc: andi@...stfloor.org, jesse.brandeburg@...el.com,
shannon.nelson@...el.com, f.fainelli@...il.com,
alexander.duyck@...il.com, carolyn.wyborny@...el.com,
donald.c.skidmore@...el.com, mitch.a.williams@...el.com,
ogerlitz@...lanox.com, edumazet@...gle.com, jiri@...lanox.com,
sfeldma@...il.com, gospo@...ulusnetworks.com,
sasha.levin@...cle.com, dsahern@...il.com, tj@...nel.org,
cascardo@...hat.com, corbet@....net, decot@...glers.com
Subject: Re: [PATCH V7 6/8] i40e: queue-specific settings for interrupt
moderation
On Fri, 2016-02-19 at 09:24 -0500, Kan Liang wrote:
> From: Kan Liang <kan.liang@...el.com>
>
> For i40e driver, each vector has its own ITR register. However, there
> are no concept of queue-specific settings in the driver proper. Only
> global variable is used to store ITR values. That will cause problems
> especially when resetting the vector. The specific ITR values could
> be
> lost.
> This patch move rx_itr_setting and tx_itr_setting to i40e_ring to
> store
> specific ITR register for each queue.
> i40e_get_coalesce and i40e_set_coalesce are also modified accordingly
> to
> support queue-specific settings. To make it compatible with old
> ethtool,
> if user doesn't specify the queue number, i40e_get_coalesce will
> return
> queue 0's value. While i40e_set_coalesce will apply value to all
> queues.
>
> Signed-off-by: Kan Liang <kan.liang@...el.com>
> Acked-by: Shannon Nelson <shannon.nelson@...el.com>
Acked-by: Jeff Kirsher <jeffrey.t.kirsher@...el.com>
There is one minor nitpick noted below, but that should not hold up
this patch.
> ---
> drivers/net/ethernet/intel/i40e/i40e.h | 7 --
> drivers/net/ethernet/intel/i40e/i40e_debugfs.c | 15 ++-
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 139
> ++++++++++++++++---------
> drivers/net/ethernet/intel/i40e/i40e_main.c | 12 +--
> drivers/net/ethernet/intel/i40e/i40e_txrx.c | 9 +-
> drivers/net/ethernet/intel/i40e/i40e_txrx.h | 8 ++
> 6 files changed, 120 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e.h
> b/drivers/net/ethernet/intel/i40e/i40e.h
> index e99be9f..2f6210a 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e.h
> +++ b/drivers/net/ethernet/intel/i40e/i40e.h
> @@ -521,13 +521,6 @@ struct i40e_vsi {
> struct i40e_ring **tx_rings;
>
> u16 work_limit;
> - /* high bit set means dynamic, use accessor routines to
> read/write.
> - * hardware only supports 2us resolution for the ITR
> registers.
> - * these values always store the USER setting, and must be
> converted
> - * before programming to a register.
> - */
> - u16 rx_itr_setting;
> - u16 tx_itr_setting;
> u16 int_rate_limit; /* value in usecs */
>
> u16 rss_table_size; /* HW RSS table size */
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> index 2a44f2e..0c97733 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_debugfs.c
> @@ -302,6 +302,10 @@ static void i40e_dbg_dump_vsi_seid(struct
> i40e_pf *pf, int seid)
> " rx_rings[%i]: vsi = %p, q_vector =
> %p\n",
> i, rx_ring->vsi,
> rx_ring->q_vector);
> + dev_info(&pf->pdev->dev,
> + " rx_rings[%i]: rx_itr_setting = %d
> (%s)\n",
> + i, rx_ring->rx_itr_setting,
> + ITR_IS_DYNAMIC(rx_ring->rx_itr_setting) ?
> "dynamic" : "fixed");
> }
> for (i = 0; i < vsi->num_queue_pairs; i++) {
> struct i40e_ring *tx_ring = ACCESS_ONCE(vsi-
> >tx_rings[i]);
> @@ -352,14 +356,15 @@ static void i40e_dbg_dump_vsi_seid(struct
> i40e_pf *pf, int seid)
> dev_info(&pf->pdev->dev,
> " tx_rings[%i]: DCB tc = %d\n",
> i, tx_ring->dcb_tc);
> + dev_info(&pf->pdev->dev,
> + " tx_rings[%i]: tx_itr_setting = %d
> (%s)\n",
> + i, tx_ring->tx_itr_setting,
> + ITR_IS_DYNAMIC(tx_ring->tx_itr_setting) ?
> "dynamic" : "fixed");
> }
> rcu_read_unlock();
> dev_info(&pf->pdev->dev,
> - " work_limit = %d, rx_itr_setting = %d (%s),
> tx_itr_setting = %d (%s)\n",
> - vsi->work_limit, vsi->rx_itr_setting,
> - ITR_IS_DYNAMIC(vsi->rx_itr_setting) ? "dynamic" :
> "fixed",
> - vsi->tx_itr_setting,
> - ITR_IS_DYNAMIC(vsi->tx_itr_setting) ? "dynamic" :
> "fixed");
> + " work_limit = %d\n",
> + vsi->work_limit);
> dev_info(&pf->pdev->dev,
> " max_frame = %d, rx_hdr_len = %d, rx_buf_len =
> %d dtype = %d\n",
> vsi->max_frame, vsi->rx_hdr_len, vsi->rx_buf_len,
> vsi->dtype);
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> index a85bc94..a470599 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
> @@ -1879,8 +1879,9 @@ static int i40e_set_phys_id(struct net_device
> *netdev,
> * 125us (8000 interrupts per second) == ITR(62)
> */
>
> -static int i40e_get_coalesce(struct net_device *netdev,
> - struct ethtool_coalesce *ec)
> +static int __i40e_get_coalesce(struct net_device *netdev,
> + struct ethtool_coalesce *ec,
> + int queue)
> {
> struct i40e_netdev_priv *np = netdev_priv(netdev);
> struct i40e_vsi *vsi = np->vsi;
> @@ -1888,14 +1889,24 @@ static int i40e_get_coalesce(struct
> net_device *netdev,
> ec->tx_max_coalesced_frames_irq = vsi->work_limit;
> ec->rx_max_coalesced_frames_irq = vsi->work_limit;
>
> - if (ITR_IS_DYNAMIC(vsi->rx_itr_setting))
> + /* rx and tx usecs has per queue value. If user doesn't
> specify the queue,
> + * return queue 0's value to represent.
> + */
> + if (queue < 0) {
> + queue = 0;
> + } else if (queue >= vsi->num_queue_pairs) {
> + return -EINVAL;
> + }
> +
> + if (ITR_IS_DYNAMIC(vsi->rx_rings[queue]->rx_itr_setting))
The curly braces are not needed in the above if/else if statement.
Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)
Powered by blists - more mailing lists