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

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(-)

--

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 <redacted>
---
 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;

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 <redacted>
---
 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
 	 */

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 <redacted>
---
 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

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 <redacted>
---
 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.

-----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 <redacted>
---

Tested-by: Krishneil Singh <redacted>

Hi Alexander,

On Tue, Sep 22, 2015 at 11:35 PM, Alexander Duyck [off-list ref] 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 <redacted>
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 <redacted>
---
 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

On 10/30/2015 07:55 AM, William Dauchy wrote:

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