i40e: invert logic for checking incorrect cpu vs irq affinity

In commit 96db776a3682 ("i40e/vf: fix interrupt affinity bug")
we added some code to force exit of polling in case we did
not have the correct CPU. This is important since it was possible for
the IRQ affinity to be changed while the CPU is pegged at 100%. This can
result in the polling routine being stuck on the wrong CPU until
traffic finally stops.

Unfortunately, the implementation, "if the CPU is correct, exit as
normal, otherwise, fall-through to the end-polling exit" is incredibly
confusing to reason about. In this case, the normal flow looks like the
exception, while the exception actually occurs far away from the if
statement and comment.

We recently discovered and fixed a bug in this code because we were
incorrectly initializing the affinity mask.

Re-write the code so that the exceptional case is handled at the check,
rather than having the logic be spread through the regular exit flow.
This does end up with minor code duplication, but the resulting code is
much easier to reason about.

The new logic is identical, but inverted. If we are running on a CPU not
in our affinity mask, we'll exit polling. However, the code flow is much
easier to understand.

Note that we don't actually have to check for MSI-X, because in the MSI
case we'll only have one q_vector, but its default affinity mask should
be correct as it includes all CPUs when it's initialized. Further, we
could at some point add code to setup the notifier for the non-MSI-X
case and enable this workaround for that case too, if desired, though
there isn't much gain since its unlikely to be the common case.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Orabug: 26785018
(cherry picked from commit 6d9777298b54bf1212fcaa6ee6679a430ceca452)
Signed-off-by: Jack Vogel <jack.vogel@oracle.com>
Reviewed-by: Kyle Fortin <kyle.fortin@oracle.com>

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 8247d45c348bc81f2f8419fa00035c76bef9f1a5..cdd4f46dee47a65650cb50df389a35a0ae8277de 100644 (file)
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2144,6 +2144,7 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
        bool clean_complete = true;
        bool arm_wb = false;
        int budget_per_ring;
+       int work_done = 0;
 
        if (test_bit(__I40E_DOWN, &vsi->state)) {
                napi_complete(napi);
@@ -2174,6 +2175,7 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
        i40e_for_each_ring(ring, q_vector->rx) {
                int cleaned = i40e_clean_rx_irq(ring, budget_per_ring);
 
+               work_done += cleaned;
                /* if we clean as many as budgeted, we must not be done */
                if (cleaned >= budget_per_ring)
                        clean_complete = false;
@@ -2181,7 +2183,6 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
 
        /* If work not completed, return budget and polling will return */
        if (!clean_complete) {
-               const cpumask_t *aff_mask = &q_vector->affinity_mask;
                int cpu_id = smp_processor_id();
 
                /* It is possible that the interrupt affinity has changed but,
@@ -2191,15 +2192,22 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
                 * continue to poll, otherwise we must stop polling so the
                 * interrupt can move to the correct cpu.
                 */
-               if (likely(cpumask_test_cpu(cpu_id, aff_mask) ||
-                          !(vsi->back->flags & I40E_FLAG_MSIX_ENABLED))) {
+               if (!cpumask_test_cpu(cpu_id, &q_vector->affinity_mask)) {
+                       /* Tell napi that we are done polling */
+                       napi_complete_done(napi, work_done);
+
+                       /* Force an interrupt */
+                       i40e_force_wb(vsi, q_vector);
+
+                       /* Return budget-1 so that polling stops */
+                       return budget - 1;
+               }
 tx_only:
-                       if (arm_wb) {
-                               q_vector->tx.ring[0].tx_stats.tx_force_wb++;
-                               i40e_enable_wb_on_itr(vsi, q_vector);
-                       }
-                       return budget;
+               if (arm_wb) {
+                       q_vector->tx.ring[0].tx_stats.tx_force_wb++;
+                       i40e_enable_wb_on_itr(vsi, q_vector);
                }
+               return budget;
        }
 
        if (vsi->back->flags & I40E_TXR_FLAGS_WB_ON_ITR)
@@ -2208,14 +2216,7 @@ tx_only:
        /* Work is done so exit the polling mode and re-enable the interrupt */
        napi_complete(napi);
 
-       /* If we're prematurely stopping polling to fix the interrupt
-        * affinity we want to make sure polling starts back up so we
-        * issue a call to i40e_force_wb which triggers a SW interrupt.
-        */
-       if (!clean_complete)
-               i40e_force_wb(vsi, q_vector);
-       else
-               i40e_update_enable_itr(vsi, q_vector);
+       i40e_update_enable_itr(vsi, q_vector);
 
        return 0;
 }

diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index d385931003b2637915f2fca89a667f6e66fa90f2..026b11e9324f1fa08e11a0243c290cc754d7af0c 100644 (file)
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -1463,6 +1463,7 @@ int i40evf_napi_poll(struct napi_struct *napi, int budget)
        bool clean_complete = true;
        bool arm_wb = false;
        int budget_per_ring;
+       int work_done = 0;
 
        if (test_bit(__I40E_DOWN, &vsi->state)) {
                napi_complete(napi);
@@ -1493,6 +1494,7 @@ int i40evf_napi_poll(struct napi_struct *napi, int budget)
        i40e_for_each_ring(ring, q_vector->rx) {
                int cleaned = i40e_clean_rx_irq(ring, budget_per_ring);
 
+               work_done += cleaned;
                /* if we clean as many as budgeted, we must not be done */
                if (cleaned >= budget_per_ring)
                        clean_complete = false;
@@ -1500,7 +1502,6 @@ int i40evf_napi_poll(struct napi_struct *napi, int budget)
 
        /* If work not completed, return budget and polling will return */
        if (!clean_complete) {
-               const cpumask_t *aff_mask = &q_vector->affinity_mask;
                int cpu_id = smp_processor_id();
 
                /* It is possible that the interrupt affinity has changed but,
@@ -1510,14 +1511,22 @@ int i40evf_napi_poll(struct napi_struct *napi, int budget)
                 * continue to poll, otherwise we must stop polling so the
                 * interrupt can move to the correct cpu.
                 */
-               if (likely(cpumask_test_cpu(cpu_id, aff_mask))) {
+               if (!cpumask_test_cpu(cpu_id, &q_vector->affinity_mask)) {
+                       /* Tell napi that we are done polling */
+                       napi_complete_done(napi, work_done);
+
+                       /* Force an interrupt */
+                       i40evf_force_wb(vsi, q_vector);
+
+                       /* Return budget-1 so that polling stops */
+                       return budget - 1;
+               }
 tx_only:
-                       if (arm_wb) {
-                               q_vector->tx.ring[0].tx_stats.tx_force_wb++;
-                               i40e_enable_wb_on_itr(vsi, q_vector);
-                       }
-                       return budget;
+               if (arm_wb) {
+                       q_vector->tx.ring[0].tx_stats.tx_force_wb++;
+                       i40e_enable_wb_on_itr(vsi, q_vector);
                }
+               return budget;
        }
 
        if (vsi->back->flags & I40E_TXR_FLAGS_WB_ON_ITR)
@@ -1526,14 +1535,7 @@ tx_only:
        /* Work is done so exit the polling mode and re-enable the interrupt */
        napi_complete(napi);
 
-       /* If we're prematurely stopping polling to fix the interrupt
-        * affinity we want to make sure polling starts back up so we
-        * issue a call to i40evf_force_wb which triggers a SW interrupt.
-        */
-       if (!clean_complete)
-               i40evf_force_wb(vsi, q_vector);
-       else
-               i40e_update_enable_itr(vsi, q_vector);
+       i40e_update_enable_itr(vsi, q_vector);
 
        return 0;
 }