[net PATCH 0/3] netpoll fixes for Intel drivers

[net PATCH 0/3] netpoll fixes for Intel drivers

[Alexander Duyck]

While reviewing some other patches related to netpoll I started looking at
how the drivers were handling their budgets.  It turns out that the ixgbe,
fm10k, and i40e drivers contained cases where they would increase the
budget from 0 and as a result it was possible for them to clean Rx frames
inside of netpoll context.

This patch set corrects that by adding a check before the Rx portion of the
polling routines for a budget less than or equal to 0.  If the value is in
that range we simply exit and leave ourselves on the polling list rather
than attempt to clean any Rx frames.

---

Alexander Duyck (3):
      i40e: Fix handling of napi budget
      fm10k: Fix handling of napi budget when multiple queues are enabled per vector
      ixgbe: Fix handling of napi budget when multiple queues are enabled per vector


 drivers/net/ethernet/intel/fm10k/fm10k_main.c |    4 ++++
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   |    5 +++++
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    3 ++-
 3 files changed, 11 insertions(+), 1 deletion(-)

--

[net PATCH 1/3] i40e: Fix handling of napi budget

[Alexander Duyck]

The polling routine for i40e was rounding up the budget for Rx cleanup to
1.  This is incorrect as netpoll will call is expecting no Rx to be
processed as the budget passed was 0.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 738aca68f665..4ca73a4be4b3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -1923,6 +1923,10 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
 		arm_wb |= ring->arm_wb;
 	}
 
+	/* Handle case where we are called by netpoll with a budget of 0 */
+	if (budget <= 0)
+		goto tx_only;
+
 	/* We attempt to distribute budget to each Rx queue fairly, but don't
 	 * allow the budget to go below 1 because that would exit polling early.
 	 */
@@ -1939,6 +1943,7 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
 
 	/* If work not completed, return budget and polling will return */
 	if (!clean_complete) {
+tx_only:
 		if (arm_wb)
 			i40e_force_wb(vsi, q_vector);
 		return budget;

[net PATCH 2/3] fm10k: Fix handling of napi budget when multiple queues are enabled per vector

[Alexander Duyck]

This patch corrects an issue in which the polling routine would increase
the budget for Rx to at least 1 per queue if multiple queues were present.
This would result in Rx packets being processed when the budget was 0 which
is meant to indicate that no Rx can be handled.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_main.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 92d415584749..b5bcb8255260 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -1413,6 +1413,10 @@ static int fm10k_poll(struct napi_struct *napi, int budget)
 	fm10k_for_each_ring(ring, q_vector->tx)
 		clean_complete &= fm10k_clean_tx_irq(q_vector, ring);
 
+	/* Handle case where we are called by netpoll with a budget of 0 */
+	if (budget <= 0)
+		return budget;
+
 	/* attempt to distribute budget to each queue fairly, but don't
 	 * allow the budget to go below 1 because we'll exit polling
 	 */

[net PATCH 3/3] ixgbe: Fix handling of napi budget when multiple queues are enabled per vector

[Alexander Duyck]

This patch corrects an issue in which the polling routine would increase
the budget for Rx to at least 1 per queue if multiple queues were present.
This would result in Rx packets being processed when the budget was 0 which
is meant to indicate that no Rx can be handled.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index acb1b91408ec..506b0944253c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2772,7 +2772,8 @@ int ixgbe_poll(struct napi_struct *napi, int budget)
 	ixgbe_for_each_ring(ring, q_vector->tx)
 		clean_complete &= !!ixgbe_clean_tx_irq(q_vector, ring);
 
-	if (!ixgbe_qv_lock_napi(q_vector))
+	/* Exit if we are called by netpoll or busy polling is active */
+	if ((budget <= 0) || !ixgbe_qv_lock_napi(q_vector))
 		return budget;
 
 	/* attempt to distribute budget to each queue fairly, but don't allow

Re: [net PATCH 1/3] i40e: Fix handling of napi budget

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 485 bytes --]

On Tue, 2015-09-22 at 14:35 -0700, Alexander Duyck wrote:
> The polling routine for i40e was rounding up the budget for Rx
> cleanup to
> 1.  This is incorrect as netpoll will call is expecting no Rx to be
> processed as the budget passed was 0.
> 
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c |    5 +++++
>  1 file changed, 5 insertions(+)

This does not apply cleanly to my next-queue tree, dev-queue branch.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [Intel-wired-lan] [net PATCH 2/3] fm10k: Fix handling of napi budget when multiple queues are enabled per vector

[Singh, Krishneil K]

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Tuesday, September 22, 2015 2:36 PM
To: netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
Subject: [Intel-wired-lan] [net PATCH 2/3] fm10k: Fix handling of napi budget when multiple queues are enabled per vector

This patch corrects an issue in which the polling routine would increase the budget for Rx to at least 1 per queue if multiple queues were present.
This would result in Rx packets being processed when the budget was 0 which is meant to indicate that no Rx can be handled.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---

Tested-by: Krishneil Singh <Krishneil.k.singh@intel.com>

Re: [net PATCH 3/3] ixgbe: Fix handling of napi budget when multiple queues are enabled per vector

[William Dauchy]

Hi Alexander,

On Tue, Sep 22, 2015 at 11:35 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch corrects an issue in which the polling routine would increase
> the budget for Rx to at least 1 per queue if multiple queues were present.
> This would result in Rx packets being processed when the budget was 0 which
> is meant to indicate that no Rx can be handled.

We may logically have the same issue with VF, right?

>From 4521374ae746543a925982d59a8ba73b6aaee59c Mon Sep 17 00:00:00 2001
From: William Dauchy <william@gandi.net>
Date: Fri, 30 Oct 2015 15:48:43 +0100
Subject: [PATCH] ixgbevf: Fix handling of napi budget when multiple queues are
 enabled per vector

This is the same patch as for ixgbe but applied differently according to
busy polling

Signed-off-by: William Dauchy <william@gandi.net>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 149a0b4..ff6e21d 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -1014,6 +1014,8 @@ static int ixgbevf_poll(struct napi_struct
*napi, int budget)
  ixgbevf_for_each_ring(ring, q_vector->tx)
  clean_complete &= ixgbevf_clean_tx_irq(q_vector, ring);

+ if (budget <= 0)
+ return budget;
 #ifdef CONFIG_NET_RX_BUSY_POLL
  if (!ixgbevf_qv_lock_napi(q_vector))
  return budget;
-- 
William

Re: [net PATCH 3/3] ixgbe: Fix handling of napi budget when multiple queues are enabled per vector

[Alexander Duyck]

On 10/30/2015 07:55 AM, William Dauchy wrote:
> Hi Alexander,
>
> On Tue, Sep 22, 2015 at 11:35 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
>> This patch corrects an issue in which the polling routine would increase
>> the budget for Rx to at least 1 per queue if multiple queues were present.
>> This would result in Rx packets being processed when the budget was 0 which
>> is meant to indicate that no Rx can be handled.
> We may logically have the same issue with VF, right?
>
>  From 4521374ae746543a925982d59a8ba73b6aaee59c Mon Sep 17 00:00:00 2001
> From: William Dauchy <william@gandi.net>
> Date: Fri, 30 Oct 2015 15:48:43 +0100
> Subject: [PATCH] ixgbevf: Fix handling of napi budget when multiple queues are
>   enabled per vector
>
> This is the same patch as for ixgbe but applied differently according to
> busy polling
>
> Signed-off-by: William Dauchy <william@gandi.net>
> ---
>   drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index 149a0b4..ff6e21d 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -1014,6 +1014,8 @@ static int ixgbevf_poll(struct napi_struct
> *napi, int budget)
>    ixgbevf_for_each_ring(ring, q_vector->tx)
>    clean_complete &= ixgbevf_clean_tx_irq(q_vector, ring);
>
> + if (budget <= 0)
> + return budget;
>   #ifdef CONFIG_NET_RX_BUSY_POLL
>    if (!ixgbevf_qv_lock_napi(q_vector))
>    return budget;

Yes the same issue applies to ixgbevf, but it looks like your patch was 
mangled by your mail client.  Can you please resubmit with the white 
spaces fixed?

- Alex