[Intel-wired-lan] [next PATCH S76-V2 01/13] i40e: Fix for unused value issue found by static analysis

[Intel-wired-lan] [next PATCH S76-V2 01/13] i40e: Fix for unused value issue found by static analysis

[Alice Michael]

From: Carolyn Wyborny <carolyn.wyborny@intel.com>

This patch fixes an issue where an error return value is
set, but without an immediate exit, the value can be overwritten
by the following code execution.  The condition  at this point
is not fatal, so remove the error assignment and comment the
intent for future code maintainers

Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 998ad96..9ee1d0f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -9865,13 +9865,15 @@ static int i40e_add_vsi(struct i40e_vsi *vsi)
 			 */
 			ret = i40e_vsi_config_tc(vsi, enabled_tc);
 			if (ret) {
+				/* Single TC condition is not fatal,
+				 * message and continue
+				 */
 				dev_info(&pf->pdev->dev,
 					 "failed to configure TCs for main VSI tc_map 0x%08x, err %s aq_err %s\n",
 					 enabled_tc,
 					 i40e_stat_str(&pf->hw, ret),
 					 i40e_aq_str(&pf->hw,
 						    pf->hw.aq.asq_last_status));
-				ret = -ENOENT;
 			}
 		}
 		break;
-- 
2.9.3

[Intel-wired-lan] [next PATCH S76-V2 02/13] i40e: remove workaround for resetting XPS on newer kernels

[Alice Michael]

From: Jacob Keller <jacob.e.keller@intel.com>

Since commit 3ffa037d7f78 ("i40e: Set XPS bit mask to zero in DCB mode")
we've tried to reset the XPS settings by building a custom
empty CPU mask.

This workaround is not necessary because we're not really removing the
XPS setting, but simply setting it so that no CPU is valid. Since v4.10
we've had this taken care of for us in the netdev core code.

We recently added a new macro to COMPAT which allows us to wrap the XPS
workaround when it's not necessary.

While we're doing this, we change the ordering of the calls, and add an
early return statement. This helps make the code easier to understand,
and reduces the headache of reading these #ifdefs. This should help
future maintainability.

Second, we shorten the code further by using zalloc_cpumask_var instead
of a separate call to bitmap_zero().

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 9ee1d0f..5004857 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -2875,22 +2875,15 @@ static void i40e_vsi_free_rx_resources(struct i40e_vsi *vsi)
 static void i40e_config_xps_tx_ring(struct i40e_ring *ring)
 {
 	struct i40e_vsi *vsi = ring->vsi;
-	cpumask_var_t mask;
 
 	if (!ring->q_vector || !ring->netdev)
 		return;
 
-	/* Single TC mode enable XPS */
-	if (vsi->tc_config.numtc <= 1) {
-		if (!test_and_set_bit(__I40E_TX_XPS_INIT_DONE, &ring->state))
-			netif_set_xps_queue(ring->netdev,
-					    &ring->q_vector->affinity_mask,
-					    ring->queue_index);
-	} else if (alloc_cpumask_var(&mask, GFP_KERNEL)) {
-		/* Disable XPS to allow selection based on TC */
-		bitmap_zero(cpumask_bits(mask), nr_cpumask_bits);
-		netif_set_xps_queue(ring->netdev, mask, ring->queue_index);
-		free_cpumask_var(mask);
+	if ((vsi->tc_config.numtc <= 1) &&
+	    !test_and_set_bit(__I40E_TX_XPS_INIT_DONE, &ring->state)) {
+		netif_set_xps_queue(ring->netdev,
+				    &ring->q_vector->affinity_mask,
+				    ring->queue_index);
 	}
 
 	/* schedule our worker thread which will take care of
-- 
2.9.3

[Intel-wired-lan] [next PATCH S76-V2 02/13] i40e: remove workaround for resetting XPS on newer kernels

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Friday, July 14, 2017 6:10 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S76-V2 02/13] i40e: remove
> workaround for resetting XPS on newer kernels
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> Since commit 3ffa037d7f78 ("i40e: Set XPS bit mask to zero in DCB mode")
> we've tried to reset the XPS settings by building a custom empty CPU mask.
> 
> This workaround is not necessary because we're not really removing the XPS
> setting, but simply setting it so that no CPU is valid. Since v4.10 we've had this
> taken care of for us in the netdev core code.
> 
> We recently added a new macro to COMPAT which allows us to wrap the XPS
> workaround when it's not necessary.
> 
> While we're doing this, we change the ordering of the calls, and add an early
> return statement. This helps make the code easier to understand, and
> reduces the headache of reading these #ifdefs. This should help future
> maintainability.
> 
> Second, we shorten the code further by using zalloc_cpumask_var instead of
> a separate call to bitmap_zero().
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S76-V2 03/13] i40e: move enabling icr0 into i40e_update_enable_itr

[Alice Michael]

From: Jacob Keller <jacob.e.keller@intel.com>

If we don't have MSI-X enabled, we handle interrupts on all icr0. This
is a special case, so let's move the conditional into
i40e_update_enable_itr() in order to make i40e_napi_poll easier to
read about.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index eda8da1..d96970f 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2241,6 +2241,12 @@ static inline void i40e_update_enable_itr(struct i40e_vsi *vsi,
 	int idx = q_vector->v_idx;
 	int rx_itr_setting, tx_itr_setting;
 
+	/* If we don't have MSIX, then we only need to re-enable icr0 */
+	if (!(vsi->back->flags & I40E_FLAG_MSIX_ENABLED)) {
+		i40e_irq_dynamic_enable_icr0(vsi->back, false);
+		return;
+	}
+
 	vector = (q_vector->v_idx + vsi->base_vector);
 
 	/* avoid dynamic calculation if in countdown mode OR if
@@ -2394,8 +2400,6 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
 	 */
 	if (!clean_complete)
 		i40e_force_wb(vsi, q_vector);
-	else if (!(vsi->back->flags & I40E_FLAG_MSIX_ENABLED))
-		i40e_irq_dynamic_enable_icr0(vsi->back, false);
 	else
 		i40e_update_enable_itr(vsi, q_vector);
 
-- 
2.9.3

[Intel-wired-lan] [next PATCH S76-V2 03/13] i40e: move enabling icr0 into i40e_update_enable_itr

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Friday, July 14, 2017 6:10 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S76-V2 03/13] i40e: move enabling icr0
> into i40e_update_enable_itr
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> If we don't have MSI-X enabled, we handle interrupts on all icr0. This is a
> special case, so let's move the conditional into
> i40e_update_enable_itr() in order to make i40e_napi_poll easier to read
> about.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S76-V2 04/13] i40e: initialize our affinity_mask based on cpu_possible_mask

[Alice Michael]

From: Jacob Keller <jacob.e.keller@intel.com>

On older kernels a call to irq_set_affinity_hint does not guarantee that
the IRQ affinity will be set. If nothing else on the system sets the IRQ
affinity this can result in a bug in the i40e_napi_poll() routine where
we notice that our interrupt fired on the "wrong" CPU according to our
internal affinity_mask variable.

This results in a bug where we continuously tell napi to stop polling to
move the interrupt to a new CPU, but the CPU never changes because our
affinity mask does not match the actual mask setup for the IRQ.

The root problem is a mismatched affinity mask value. So lets initialize
the value to cpu_possible_mask instead. This ensures that prior to the
first time we get an irq affinity notification we'll have the mask set
to include every possible CPU.

We use cpu_possible_mask instead of cpu_online_mask since the former is
almost certainly never going to change, while the later might change
after we've made a copy.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c     | 12 +++++++-----
 drivers/net/ethernet/intel/i40evf/i40evf_main.c |  7 +++++--
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 5004857..abf4025 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -2882,7 +2882,7 @@ static void i40e_config_xps_tx_ring(struct i40e_ring *ring)
 	if ((vsi->tc_config.numtc <= 1) &&
 	    !test_and_set_bit(__I40E_TX_XPS_INIT_DONE, &ring->state)) {
 		netif_set_xps_queue(ring->netdev,
-				    &ring->q_vector->affinity_mask,
+				    get_cpu_mask(ring->q_vector->v_idx),
 				    ring->queue_index);
 	}
 
@@ -3507,8 +3507,10 @@ static int i40e_vsi_request_irq_msix(struct i40e_vsi *vsi, char *basename)
 		q_vector->affinity_notify.notify = i40e_irq_affinity_notify;
 		q_vector->affinity_notify.release = i40e_irq_affinity_release;
 		irq_set_affinity_notifier(irq_num, &q_vector->affinity_notify);
-		/* assign the mask for this irq */
-		irq_set_affinity_hint(irq_num, &q_vector->affinity_mask);
+		/* get_cpu_mask returns a static constant mask with
+		 * a permanent lifetime so it's ok to use here.
+		 */
+		irq_set_affinity_hint(irq_num, get_cpu_mask(q_vector->v_idx));
 	}
 
 	vsi->irqs_ready = true;
@@ -4290,7 +4292,7 @@ static void i40e_vsi_free_irq(struct i40e_vsi *vsi)
 
 			/* clear the affinity notifier in the IRQ descriptor */
 			irq_set_affinity_notifier(irq_num, NULL);
-			/* clear the affinity_mask in the IRQ descriptor */
+			/* remove our suggested affinity mask for this IRQ */
 			irq_set_affinity_hint(irq_num, NULL);
 			synchronize_irq(irq_num);
 			free_irq(irq_num, vsi->q_vectors[i]);
@@ -8221,7 +8223,7 @@ static int i40e_vsi_alloc_q_vector(struct i40e_vsi *vsi, int v_idx, int cpu)
 
 	q_vector->vsi = vsi;
 	q_vector->v_idx = v_idx;
-	cpumask_set_cpu(cpu, &q_vector->affinity_mask);
+	cpumask_copy(&q_vector->affinity_mask, cpu_possible_mask);
 
 	if (vsi->netdev)
 		netif_napi_add(vsi->netdev, &q_vector->napi,
diff --git a/drivers/net/ethernet/intel/i40evf/i40evf_main.c b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
index 77d2835..7ffe30d 100644
--- a/drivers/net/ethernet/intel/i40evf/i40evf_main.c
+++ b/drivers/net/ethernet/intel/i40evf/i40evf_main.c
@@ -587,8 +587,10 @@ i40evf_request_traffic_irqs(struct i40evf_adapter *adapter, char *basename)
 		q_vector->affinity_notify.release =
 						   i40evf_irq_affinity_release;
 		irq_set_affinity_notifier(irq_num, &q_vector->affinity_notify);
-		/* assign the mask for this irq */
-		irq_set_affinity_hint(irq_num, &q_vector->affinity_mask);
+		/* get_cpu_mask returns a static constant mask with
+		 * a permanent lifetime so it's ok to use here.
+		 */
+		irq_set_affinity_hint(irq_num, get_cpu_mask(q_vector->v_idx));
 	}
 
 	return 0;
@@ -1459,6 +1461,7 @@ static int i40evf_alloc_q_vectors(struct i40evf_adapter *adapter)
 		q_vector->adapter = adapter;
 		q_vector->vsi = &adapter->vsi;
 		q_vector->v_idx = q_idx;
+		cpumask_copy(&q_vector->affinity_mask, cpu_possible_mask);
 		netif_napi_add(adapter->netdev, &q_vector->napi,
 			       i40evf_napi_poll, NAPI_POLL_WEIGHT);
 	}
-- 
2.9.3

[Intel-wired-lan] [next PATCH S76-V2 04/13] i40e: initialize our affinity_mask based on cpu_possible_mask

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Friday, July 14, 2017 6:10 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S76-V2 04/13] i40e: initialize our
> affinity_mask based on cpu_possible_mask
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> On older kernels a call to irq_set_affinity_hint does not guarantee that the
> IRQ affinity will be set. If nothing else on the system sets the IRQ affinity this
> can result in a bug in the i40e_napi_poll() routine where we notice that our
> interrupt fired on the "wrong" CPU according to our internal affinity_mask
> variable.
> 
> This results in a bug where we continuously tell napi to stop polling to move
> the interrupt to a new CPU, but the CPU never changes because our affinity
> mask does not match the actual mask setup for the IRQ.
> 
> The root problem is a mismatched affinity mask value. So lets initialize the
> value to cpu_possible_mask instead. This ensures that prior to the first time
> we get an irq affinity notification we'll have the mask set to include every
> possible CPU.
> 
> We use cpu_possible_mask instead of cpu_online_mask since the former is
> almost certainly never going to change, while the later might change after
> we've made a copy.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c     | 12 +++++++-----
>  drivers/net/ethernet/intel/i40evf/i40evf_main.c |  7 +++++--
>  2 files changed, 12 insertions(+), 7 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S76-V2 05/13] i40e: invert logic for checking incorrect cpu vs irq affinity

[Alice Michael]

From: Jacob Keller <jacob.e.keller@intel.com>

In commit 96db776a3682 ("i40e/vf: fix interrupt affinity bug")
we added some code to force exit of polling incase 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>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 31 +++++++++++++--------------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 30 +++++++++++++-------------
 2 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index d96970f..b323677 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2367,7 +2367,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,
@@ -2377,15 +2376,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)
@@ -2394,14 +2400,7 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)
 	/* Work is done so exit the polling mode and re-enable the interrupt */
 	napi_complete_done(napi, work_done);
 
-	/* 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 min(work_done, budget - 1);
 }
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index 4bf7e35..a8dc1d9 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -1575,7 +1575,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,
@@ -1585,14 +1584,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)
@@ -1601,14 +1608,7 @@ int i40evf_napi_poll(struct napi_struct *napi, int budget)
 	/* Work is done so exit the polling mode and re-enable the interrupt */
 	napi_complete_done(napi, work_done);
 
-	/* 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 min(work_done, budget - 1);
 }
-- 
2.9.3

[Intel-wired-lan] [next PATCH S76-V2 05/13] i40e: invert logic for checking incorrect cpu vs irq affinity

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Friday, July 14, 2017 6:10 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S76-V2 05/13] i40e: invert logic for
> checking incorrect cpu vs irq affinity
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> In commit 96db776a3682 ("i40e/vf: fix interrupt affinity bug") we added
> some code to force exit of polling incase 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>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 31 +++++++++++++------------
> --
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 30 +++++++++++++----------
> ---
>  2 files changed, 30 insertions(+), 31 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S76-V2 06/13] i40e/i40evf: remove ULTRA latency mode

[Alice Michael]

From: Jacob Keller <jacob.e.keller@intel.com>

Since commit c56625d59726 ("i40e/i40evf: change dynamic interrupt
thresholds") a new higher latency ITR setting called I40E_ULTRA_LATENCY
was added with a cryptic comment about how it was meant for adjusting Rx
more aggressively when streaming small packets.

This mode was attempting to calculate packets per second and then kick
in when we have a huge number of small packets.

Unfortunately, the ULTRA setting was kicking in for workloads it wasn't
intended for including single-thread UDP_STREAM workloads.

This wasn't caught for a variety of reasons. First, the ip_defrag
routines were improved somewhat which makes the UDP_STREAM test still
reasonable at 10GbE, even when dropped down to 8k interrupts a second.
Additionally, some other obvious workloads appear to work fine, such
as TCP_STREAM.

The number 40k doesn't make sense for a number of reasons. First, we
absolutely can do more than 40k packets per second. Second, we calculate
the value inline in an integer, which sometimes can overflow resulting
in using incorrect values.

If we fix this overflow it makes it even more likely that we'll enter
ULTRA mode which is the opposite of what we want.

The ULTRA mode was added originally as a way to reduce CPU utilization
during a small packet workload where we weren't keeping up anyways. It
should never have been kicking in during these other workloads.

Given the issues outlined above, let's remove the ULTRA latency mode. If
necessary, a better solution to the CPU utilization issue for small
packet workloads will be added in a future patch.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 17 -----------------
 drivers/net/ethernet/intel/i40e/i40e_txrx.h   |  1 -
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 17 -----------------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.h |  1 -
 4 files changed, 36 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index b323677..c056f60 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -959,7 +959,6 @@ void i40e_force_wb(struct i40e_vsi *vsi, struct i40e_q_vector *q_vector)
 static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 {
 	enum i40e_latency_range new_latency_range = rc->latency_range;
-	struct i40e_q_vector *qv = rc->ring->q_vector;
 	u32 new_itr = rc->itr;
 	int bytes_per_int;
 	int usecs;
@@ -971,7 +970,6 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 	 *   0-10MB/s   lowest (50000 ints/s)
 	 *  10-20MB/s   low    (20000 ints/s)
 	 *  20-1249MB/s bulk   (18000 ints/s)
-	 *  > 40000 Rx packets per second (8000 ints/s)
 	 *
 	 * The math works out because the divisor is in 10^(-6) which
 	 * turns the bytes/us input value into MB/s values, but
@@ -994,24 +992,12 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 			new_latency_range = I40E_LOWEST_LATENCY;
 		break;
 	case I40E_BULK_LATENCY:
-	case I40E_ULTRA_LATENCY:
 	default:
 		if (bytes_per_int <= 20)
 			new_latency_range = I40E_LOW_LATENCY;
 		break;
 	}
 
-	/* this is to adjust RX more aggressively when streaming small
-	 * packets.  The value of 40000 was picked as it is just beyond
-	 * what the hardware can receive per second if in low latency
-	 * mode.
-	 */
-#define RX_ULTRA_PACKET_RATE 40000
-
-	if ((((rc->total_packets * 1000000) / usecs) > RX_ULTRA_PACKET_RATE) &&
-	    (&qv->rx == rc))
-		new_latency_range = I40E_ULTRA_LATENCY;
-
 	rc->latency_range = new_latency_range;
 
 	switch (new_latency_range) {
@@ -1024,9 +1010,6 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 	case I40E_BULK_LATENCY:
 		new_itr = I40E_ITR_18K;
 		break;
-	case I40E_ULTRA_LATENCY:
-		new_itr = I40E_ITR_8K;
-		break;
 	default:
 		break;
 	}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index f0a0eab..e6456e8 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -454,7 +454,6 @@ enum i40e_latency_range {
 	I40E_LOWEST_LATENCY = 0,
 	I40E_LOW_LATENCY = 1,
 	I40E_BULK_LATENCY = 2,
-	I40E_ULTRA_LATENCY = 3,
 };
 
 struct i40e_ring_container {
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index a8dc1d9..b9acb20 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -357,7 +357,6 @@ void i40evf_force_wb(struct i40e_vsi *vsi, struct i40e_q_vector *q_vector)
 static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 {
 	enum i40e_latency_range new_latency_range = rc->latency_range;
-	struct i40e_q_vector *qv = rc->ring->q_vector;
 	u32 new_itr = rc->itr;
 	int bytes_per_int;
 	int usecs;
@@ -369,7 +368,6 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 	 *   0-10MB/s   lowest (50000 ints/s)
 	 *  10-20MB/s   low    (20000 ints/s)
 	 *  20-1249MB/s bulk   (18000 ints/s)
-	 *  > 40000 Rx packets per second (8000 ints/s)
 	 *
 	 * The math works out because the divisor is in 10^(-6) which
 	 * turns the bytes/us input value into MB/s values, but
@@ -392,24 +390,12 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 			new_latency_range = I40E_LOWEST_LATENCY;
 		break;
 	case I40E_BULK_LATENCY:
-	case I40E_ULTRA_LATENCY:
 	default:
 		if (bytes_per_int <= 20)
 			new_latency_range = I40E_LOW_LATENCY;
 		break;
 	}
 
-	/* this is to adjust RX more aggressively when streaming small
-	 * packets.  The value of 40000 was picked as it is just beyond
-	 * what the hardware can receive per second if in low latency
-	 * mode.
-	 */
-#define RX_ULTRA_PACKET_RATE 40000
-
-	if ((((rc->total_packets * 1000000) / usecs) > RX_ULTRA_PACKET_RATE) &&
-	    (&qv->rx == rc))
-		new_latency_range = I40E_ULTRA_LATENCY;
-
 	rc->latency_range = new_latency_range;
 
 	switch (new_latency_range) {
@@ -422,9 +408,6 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 	case I40E_BULK_LATENCY:
 		new_itr = I40E_ITR_18K;
 		break;
-	case I40E_ULTRA_LATENCY:
-		new_itr = I40E_ITR_8K;
-		break;
 	default:
 		break;
 	}
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
index 4896840..fb506c9 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
@@ -425,7 +425,6 @@ enum i40e_latency_range {
 	I40E_LOWEST_LATENCY = 0,
 	I40E_LOW_LATENCY = 1,
 	I40E_BULK_LATENCY = 2,
-	I40E_ULTRA_LATENCY = 3,
 };
 
 struct i40e_ring_container {
-- 
2.9.3

[Intel-wired-lan] [next PATCH S76-V2 06/13] i40e/i40evf: remove ULTRA latency mode

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Friday, July 14, 2017 6:10 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S76-V2 06/13] i40e/i40evf: remove
> ULTRA latency mode
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> Since commit c56625d59726 ("i40e/i40evf: change dynamic interrupt
> thresholds") a new higher latency ITR setting called I40E_ULTRA_LATENCY
> was added with a cryptic comment about how it was meant for adjusting Rx
> more aggressively when streaming small packets.
> 
> This mode was attempting to calculate packets per second and then kick in
> when we have a huge number of small packets.
> 
> Unfortunately, the ULTRA setting was kicking in for workloads it wasn't
> intended for including single-thread UDP_STREAM workloads.
> 
> This wasn't caught for a variety of reasons. First, the ip_defrag routines were
> improved somewhat which makes the UDP_STREAM test still reasonable at
> 10GbE, even when dropped down to 8k interrupts a second.
> Additionally, some other obvious workloads appear to work fine, such as
> TCP_STREAM.
> 
> The number 40k doesn't make sense for a number of reasons. First, we
> absolutely can do more than 40k packets per second. Second, we calculate
> the value inline in an integer, which sometimes can overflow resulting in
> using incorrect values.
> 
> If we fix this overflow it makes it even more likely that we'll enter ULTRA
> mode which is the opposite of what we want.
> 
> The ULTRA mode was added originally as a way to reduce CPU utilization
> during a small packet workload where we weren't keeping up anyways. It
> should never have been kicking in during these other workloads.
> 
> Given the issues outlined above, let's remove the ULTRA latency mode. If
> necessary, a better solution to the CPU utilization issue for small packet
> workloads will be added in a future patch.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 17 -----------------
>  drivers/net/ethernet/intel/i40e/i40e_txrx.h   |  1 -
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 17 -----------------
> drivers/net/ethernet/intel/i40evf/i40e_txrx.h |  1 -
>  4 files changed, 36 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S76-V2 07/13] i40e/i40evf: avoid dynamic ITR updates when polling or low packet rate

[Alice Michael]

From: Jacob Keller <jacob.e.keller@intel.com>

The dynamic ITR algorithm depends on a calculation of usecs which
assumes that the interrupts have been firing constantly at the interrupt
throttle rate. This is not guaranteed because we could have a low packet
rate, or have been polling in software.

We'll estimate whether this is the case by using jiffies to determine if
we've been too long. If the time difference of jiffies is larger we are
guaranteed to have an incorrect calculation. If the time difference of
jiffies is smaller we might have been polling some but the difference
shouldn't affect the calculation too much.

This ensures that we don't get stuck in BULK latency during certain rare
situations where we receive bursts of packets that force us into napi
polling.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 22 +++++++++++++++++-----
 drivers/net/ethernet/intel/i40e/i40e_txrx.h   |  1 +
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 22 +++++++++++++++++-----
 drivers/net/ethernet/intel/i40evf/i40e_txrx.h |  1 +
 4 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index c056f60..74589ed 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -961,11 +961,25 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 	enum i40e_latency_range new_latency_range = rc->latency_range;
 	u32 new_itr = rc->itr;
 	int bytes_per_int;
-	int usecs;
+	unsigned int usecs, estimated_usecs;
 
 	if (rc->total_packets == 0 || !rc->itr)
 		return false;
 
+	usecs = (rc->itr << 1) * ITR_COUNTDOWN_START;
+	bytes_per_int = rc->total_bytes / usecs;
+
+	/* The calculations in this algorithm depend on interrupts actually
+	 * firing at the ITR rate. This may not happen if the packet rate is
+	 * really low, or if we've been napi polling. Check to make sure
+	 * that's not the case before we continue.
+	 */
+	estimated_usecs = jiffies_to_usecs(jiffies - rc->last_itr_update);
+	if (estimated_usecs > usecs) {
+		new_latency_range = I40E_LOW_LATENCY;
+		goto reset_latency;
+	}
+
 	/* simple throttlerate management
 	 *   0-10MB/s   lowest (50000 ints/s)
 	 *  10-20MB/s   low    (20000 ints/s)
@@ -977,9 +991,6 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 	 * are in 2 usec increments in the ITR registers, and make sure
 	 * to use the smoothed values that the countdown timer gives us.
 	 */
-	usecs = (rc->itr << 1) * ITR_COUNTDOWN_START;
-	bytes_per_int = rc->total_bytes / usecs;
-
 	switch (new_latency_range) {
 	case I40E_LOWEST_LATENCY:
 		if (bytes_per_int > 10)
@@ -998,6 +1009,7 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 		break;
 	}
 
+reset_latency:
 	rc->latency_range = new_latency_range;
 
 	switch (new_latency_range) {
@@ -1016,12 +1028,12 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 
 	rc->total_bytes = 0;
 	rc->total_packets = 0;
+	rc->last_itr_update = jiffies;
 
 	if (new_itr != rc->itr) {
 		rc->itr = new_itr;
 		return true;
 	}
-
 	return false;
 }
 
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index e6456e8..2f848bc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -461,6 +461,7 @@ struct i40e_ring_container {
 	struct i40e_ring *ring;
 	unsigned int total_bytes;	/* total bytes processed this int */
 	unsigned int total_packets;	/* total packets processed this int */
+	unsigned long last_itr_update;	/* jiffies of last ITR update */
 	u16 count;
 	enum i40e_latency_range latency_range;
 	u16 itr;
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index b9acb20..b56a193 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -359,11 +359,25 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 	enum i40e_latency_range new_latency_range = rc->latency_range;
 	u32 new_itr = rc->itr;
 	int bytes_per_int;
-	int usecs;
+	unsigned int usecs, estimated_usecs;
 
 	if (rc->total_packets == 0 || !rc->itr)
 		return false;
 
+	usecs = (rc->itr << 1) * ITR_COUNTDOWN_START;
+	bytes_per_int = rc->total_bytes / usecs;
+
+	/* The calculations in this algorithm depend on interrupts actually
+	 * firing at the ITR rate. This may not happen if the packet rate is
+	 * really low, or if we've been napi polling. Check to make sure
+	 * that's not the case before we continue.
+	 */
+	estimated_usecs = jiffies_to_usecs(jiffies - rc->last_itr_update);
+	if (estimated_usecs > usecs) {
+		new_latency_range = I40E_LOW_LATENCY;
+		goto reset_latency;
+	}
+
 	/* simple throttlerate management
 	 *   0-10MB/s   lowest (50000 ints/s)
 	 *  10-20MB/s   low    (20000 ints/s)
@@ -375,9 +389,6 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 	 * are in 2 usec increments in the ITR registers, and make sure
 	 * to use the smoothed values that the countdown timer gives us.
 	 */
-	usecs = (rc->itr << 1) * ITR_COUNTDOWN_START;
-	bytes_per_int = rc->total_bytes / usecs;
-
 	switch (new_latency_range) {
 	case I40E_LOWEST_LATENCY:
 		if (bytes_per_int > 10)
@@ -396,6 +407,7 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 		break;
 	}
 
+reset_latency:
 	rc->latency_range = new_latency_range;
 
 	switch (new_latency_range) {
@@ -414,12 +426,12 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 
 	rc->total_bytes = 0;
 	rc->total_packets = 0;
+	rc->last_itr_update = jiffies;
 
 	if (new_itr != rc->itr) {
 		rc->itr = new_itr;
 		return true;
 	}
-
 	return false;
 }
 
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
index fb506c9..0d9f98b 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
@@ -432,6 +432,7 @@ struct i40e_ring_container {
 	struct i40e_ring *ring;
 	unsigned int total_bytes;	/* total bytes processed this int */
 	unsigned int total_packets;	/* total packets processed this int */
+	unsigned long last_itr_update;	/* jiffies of last ITR update */
 	u16 count;
 	enum i40e_latency_range latency_range;
 	u16 itr;
-- 
2.9.3

[Intel-wired-lan] [next PATCH S76-V2 07/13] i40e/i40evf: avoid dynamic ITR updates when polling or low packet rate

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Friday, July 14, 2017 6:10 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S76-V2 07/13] i40e/i40evf: avoid
> dynamic ITR updates when polling or low packet rate
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> The dynamic ITR algorithm depends on a calculation of usecs which assumes
> that the interrupts have been firing constantly at the interrupt throttle rate.
> This is not guaranteed because we could have a low packet rate, or have
> been polling in software.
> 
> We'll estimate whether this is the case by using jiffies to determine if we've
> been too long. If the time difference of jiffies is larger we are guaranteed to
> have an incorrect calculation. If the time difference of jiffies is smaller we
> might have been polling some but the difference shouldn't affect the
> calculation too much.
> 
> This ensures that we don't get stuck in BULK latency during certain rare
> situations where we receive bursts of packets that force us into napi polling.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 22 +++++++++++++++++-----
>  drivers/net/ethernet/intel/i40e/i40e_txrx.h   |  1 +
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 22 +++++++++++++++++----
> -  drivers/net/ethernet/intel/i40evf/i40e_txrx.h |  1 +
>  4 files changed, 36 insertions(+), 10 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S76-V2 08/13] i40e/i40evf: rename bytes_per_int to bytes_per_usec

[Alice Michael]

From: Jacob Keller <jacob.e.keller@intel.com>

This value is not calculating bytes_per_int, which would actually just
be bytes/ITR_COUNTDOWN_START, but rather it's calculating bytes/usecs.

Rename the variable for clarity so that future developers understand
what the value is actually calculating.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 12 ++++++------
 drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 74589ed..8cd8203 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -960,14 +960,14 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 {
 	enum i40e_latency_range new_latency_range = rc->latency_range;
 	u32 new_itr = rc->itr;
-	int bytes_per_int;
+	int bytes_per_usec;
 	unsigned int usecs, estimated_usecs;
 
 	if (rc->total_packets == 0 || !rc->itr)
 		return false;
 
 	usecs = (rc->itr << 1) * ITR_COUNTDOWN_START;
-	bytes_per_int = rc->total_bytes / usecs;
+	bytes_per_usec = rc->total_bytes / usecs;
 
 	/* The calculations in this algorithm depend on interrupts actually
 	 * firing at the ITR rate. This may not happen if the packet rate is
@@ -993,18 +993,18 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 	 */
 	switch (new_latency_range) {
 	case I40E_LOWEST_LATENCY:
-		if (bytes_per_int > 10)
+		if (bytes_per_usec > 10)
 			new_latency_range = I40E_LOW_LATENCY;
 		break;
 	case I40E_LOW_LATENCY:
-		if (bytes_per_int > 20)
+		if (bytes_per_usec > 20)
 			new_latency_range = I40E_BULK_LATENCY;
-		else if (bytes_per_int <= 10)
+		else if (bytes_per_usec <= 10)
 			new_latency_range = I40E_LOWEST_LATENCY;
 		break;
 	case I40E_BULK_LATENCY:
 	default:
-		if (bytes_per_int <= 20)
+		if (bytes_per_usec <= 20)
 			new_latency_range = I40E_LOW_LATENCY;
 		break;
 	}
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index b56a193..8167617 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -358,14 +358,14 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 {
 	enum i40e_latency_range new_latency_range = rc->latency_range;
 	u32 new_itr = rc->itr;
-	int bytes_per_int;
+	int bytes_per_usec;
 	unsigned int usecs, estimated_usecs;
 
 	if (rc->total_packets == 0 || !rc->itr)
 		return false;
 
 	usecs = (rc->itr << 1) * ITR_COUNTDOWN_START;
-	bytes_per_int = rc->total_bytes / usecs;
+	bytes_per_usec = rc->total_bytes / usecs;
 
 	/* The calculations in this algorithm depend on interrupts actually
 	 * firing at the ITR rate. This may not happen if the packet rate is
@@ -391,18 +391,18 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)
 	 */
 	switch (new_latency_range) {
 	case I40E_LOWEST_LATENCY:
-		if (bytes_per_int > 10)
+		if (bytes_per_usec > 10)
 			new_latency_range = I40E_LOW_LATENCY;
 		break;
 	case I40E_LOW_LATENCY:
-		if (bytes_per_int > 20)
+		if (bytes_per_usec > 20)
 			new_latency_range = I40E_BULK_LATENCY;
-		else if (bytes_per_int <= 10)
+		else if (bytes_per_usec <= 10)
 			new_latency_range = I40E_LOWEST_LATENCY;
 		break;
 	case I40E_BULK_LATENCY:
 	default:
-		if (bytes_per_int <= 20)
+		if (bytes_per_usec <= 20)
 			new_latency_range = I40E_LOW_LATENCY;
 		break;
 	}
-- 
2.9.3

[Intel-wired-lan] [next PATCH S76-V2 08/13] i40e/i40evf: rename bytes_per_int to bytes_per_usec

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Friday, July 14, 2017 6:10 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S76-V2 08/13] i40e/i40evf: rename
> bytes_per_int to bytes_per_usec
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> This value is not calculating bytes_per_int, which would actually just be
> bytes/ITR_COUNTDOWN_START, but rather it's calculating bytes/usecs.
> 
> Rename the variable for clarity so that future developers understand what
> the value is actually calculating.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   | 12 ++++++------
>  drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 12 ++++++------
>  2 files changed, 12 insertions(+), 12 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S76-V2 09/13] i40e: Fix unqualified module message while bringing link up

[Alice Michael]

From: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>

In current driver, when ifconfig ethx up is done, the link state
doesn't transition to UP inside i40e_open(). It changes after AQ
command response is handled in i40e_handle_link_event().

When pf->hw.phy.link_info.link_info is DOWN inside i40e_open(),
The state is transient and invalid. So log message gets printed
based on incorrect info (i.e link_info and an_info).

This commit removes check for unqualified module inside
i40e_up_complete(). The existing check in i40e_handle_link_event()
logs the error message based on correct link state information.

Signed-off-by: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index abf4025..beeeeeb 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -5457,15 +5457,6 @@ static int i40e_up_complete(struct i40e_vsi *vsi)
 		i40e_print_link_message(vsi, true);
 		netif_tx_start_all_queues(vsi->netdev);
 		netif_carrier_on(vsi->netdev);
-	} else if (vsi->netdev) {
-		i40e_print_link_message(vsi, false);
-		/* need to check for qualified module here*/
-		if ((pf->hw.phy.link_info.link_info &
-			I40E_AQ_MEDIA_AVAILABLE) &&
-		    (!(pf->hw.phy.link_info.an_info &
-			I40E_AQ_QUALIFIED_MODULE)))
-			netdev_err(vsi->netdev,
-				   "the driver failed to link because an unqualified module was detected.");
 	}
 
 	/* replay FDIR SB filters */
-- 
2.9.3

[Intel-wired-lan] [next PATCH S76-V2 09/13] i40e: Fix unqualified module message while bringing link up

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Friday, July 14, 2017 6:10 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S76-V2 09/13] i40e: Fix unqualified
> module message while bringing link up
> 
> From: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>
> 
> In current driver, when ifconfig ethx up is done, the link state doesn't
> transition to UP inside i40e_open(). It changes after AQ command response
> is handled in i40e_handle_link_event().
> 
> When pf->hw.phy.link_info.link_info is DOWN inside i40e_open(), The state
> is transient and invalid. So log message gets printed based on incorrect info
> (i.e link_info and an_info).
> 
> This commit removes check for unqualified module inside
> i40e_up_complete(). The existing check in i40e_handle_link_event() logs the
> error message based on correct link state information.
> 
> Signed-off-by: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 9 ---------
>  1 file changed, 9 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S76-V2 10/13] i40e: Fix link down message when interface is brought up

[Alice Michael]

From: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>

i40e_print_link_message() is intended to compare new
link state with current link state and print log message
only if the new state is different from current state.

However in current driver the new state does not get updated
when link is going down because of the if condition. When an
interface is brought down, vsi->state is set to I40E_VSI_DOWN
in i40e_vsi_close() and later i40e_print_link_message() does
not get invoked in i40e_link_event due to if condition. Hence
link down message doesn't appear when link is going down. The
down state is seen  later during i40e_open() and old state
gets printed. The actual link state doesn't get updated in
i40e_close() or i40e_open() but when i40e_handle_link_event is
called inside i40e_clean_adminq_subtask.

This change allows i40e_print_link_message() to be called when
interface is going down and keeps the state information updated.

Signed-off-by: Mogilappagari, Sudheer <sudheer.mogilappagari@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index beeeeeb..b4844e2 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -6406,8 +6406,7 @@ static void i40e_link_event(struct i40e_pf *pf)
 	     new_link == netif_carrier_ok(vsi->netdev)))
 		return;
 
-	if (!test_bit(__I40E_VSI_DOWN, vsi->state))
-		i40e_print_link_message(vsi, new_link);
+	i40e_print_link_message(vsi, new_link);
 
 	/* Notify the base of the switch tree connected to
 	 * the link.  Floating VEBs are not notified.
-- 
2.9.3

[Intel-wired-lan] [next PATCH S76-V2 10/13] i40e: Fix link down message when interface is brought up

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Friday, July 14, 2017 6:10 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Cc: Mogilappagari at osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S76-V2 10/13] i40e: Fix link down
> message when interface is brought up
> 
> From: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>
> 
> i40e_print_link_message() is intended to compare new link state with
> current link state and print log message only if the new state is different from
> current state.
> 
> However in current driver the new state does not get updated when link is
> going down because of the if condition. When an interface is brought down,
> vsi->state is set to I40E_VSI_DOWN in i40e_vsi_close() and later
> i40e_print_link_message() does not get invoked in i40e_link_event due to if
> condition. Hence link down message doesn't appear when link is going down.
> The down state is seen  later during i40e_open() and old state gets printed.
> The actual link state doesn't get updated in
> i40e_close() or i40e_open() but when i40e_handle_link_event is called inside
> i40e_clean_adminq_subtask.
> 
> This change allows i40e_print_link_message() to be called when interface is
> going down and keeps the state information updated.
> 
> Signed-off-by: Mogilappagari, Sudheer <sudheer.mogilappagari@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S76-V2 11/13] i40e: Fix unrecognized PHY type warning message for 25G AOC/ACC

[Alice Michael]

From: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>

For 25G ACC and AOC PHY types a warning message appears like
"Link is up but PHY type 0x23 is not recognized". This commit
adds a case statement in i40e_get_link_settings_link_up() for
25G AOC/ACC PHY types to set correct parameters.

Signed-off-by: Mogilappagari, Sudheer <sudheer.mogilappagari@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index a868c8d..9212c35 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -665,6 +665,15 @@ static int i40e_get_link_ksettings(struct net_device *netdev,
 		ethtool_link_ksettings_add_link_mode(cmd, advertising,
 						     Asym_Pause);
 		break;
+	case I40E_PHY_TYPE_25GBASE_AOC:
+	case I40E_PHY_TYPE_25GBASE_ACC:
+		ethtool_link_ksettings_add_link_mode(ks, supported, Autoneg);
+		ethtool_link_ksettings_add_link_mode(ks, advertising, Autoneg);
+		ethtool_link_ksettings_add_link_mode(ks, supported,
+						     25000baseCR_Full);
+		ethtool_link_ksettings_add_link_mode(ks, advertising,
+						     25000baseCR_Full);
+		break;
 	default:
 		ethtool_convert_link_mode_to_legacy_u32(
 			&advertising, cmd->link_modes.advertising);
-- 
2.9.3

[Intel-wired-lan] [next PATCH S76-V2 11/13] i40e: Fix unrecognized PHY type warning message for 25G AOC/ACC

[kbuild test robot]

Hi Sudheer,

[auto build test ERROR on jkirsher-next-queue/dev-queue]
[also build test ERROR on v4.12 next-20170714]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Alice-Michael/i40e-Fix-for-unused-value-issue-found-by-static-analysis/20170715-162444
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue.git dev-queue
config: x86_64-randconfig-x013-201728 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/net/ethernet/intel/i40e/i40e_ethtool.c: In function 'i40e_get_link_ksettings':
>> drivers/net/ethernet/intel/i40e/i40e_ethtool.c:668:7: error: 'I40E_PHY_TYPE_25GBASE_AOC' undeclared (first use in this function)
     case I40E_PHY_TYPE_25GBASE_AOC:
          ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c:668:7: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/net/ethernet/intel/i40e/i40e_ethtool.c:669:7: error: 'I40E_PHY_TYPE_25GBASE_ACC' undeclared (first use in this function)
     case I40E_PHY_TYPE_25GBASE_ACC:
          ^~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/netdevice.h:42:0,
                    from include/net/sock.h:51,
                    from include/linux/tcp.h:23,
                    from include/net/tcp.h:24,
                    from drivers/net/ethernet/intel/i40e/i40e.h:30,
                    from drivers/net/ethernet/intel/i40e/i40e_ethtool.c:29:
>> drivers/net/ethernet/intel/i40e/i40e_ethtool.c:670:40: error: 'ks' undeclared (first use in this function)
      ethtool_link_ksettings_add_link_mode(ks, supported, Autoneg);
                                           ^
   include/linux/ethtool.h:137:49: note: in definition of macro 'ethtool_link_ksettings_add_link_mode'
     __set_bit(ETHTOOL_LINK_MODE_ ## mode ## _BIT, (ptr)->link_modes.name)
                                                    ^~~

vim +/I40E_PHY_TYPE_25GBASE_AOC +668 drivers/net/ethernet/intel/i40e/i40e_ethtool.c

   589	
   590	/**
   591	 * i40e_get_settings - Get Link Speed and Duplex settings
   592	 * @netdev: network interface device structure
   593	 * @ecmd: ethtool command
   594	 *
   595	 * Reports speed/duplex settings based on media_type
   596	 **/
   597	static int i40e_get_link_ksettings(struct net_device *netdev,
   598					   struct ethtool_link_ksettings *cmd)
   599	{
   600		struct i40e_netdev_priv *np = netdev_priv(netdev);
   601		struct i40e_pf *pf = np->vsi->back;
   602		struct i40e_hw *hw = &pf->hw;
   603		struct i40e_link_status *hw_link_info = &hw->phy.link_info;
   604		bool link_up = hw_link_info->link_info & I40E_AQ_LINK_UP;
   605		u32 advertising;
   606	
   607		if (link_up)
   608			i40e_get_settings_link_up(hw, cmd, netdev, pf);
   609		else
   610			i40e_get_settings_link_down(hw, cmd, pf);
   611	
   612		/* Now set the settings that don't rely on link being up/down */
   613		/* Set autoneg settings */
   614		cmd->base.autoneg = ((hw_link_info->an_info & I40E_AQ_AN_COMPLETED) ?
   615				  AUTONEG_ENABLE : AUTONEG_DISABLE);
   616	
   617		switch (hw->phy.media_type) {
   618		case I40E_MEDIA_TYPE_BACKPLANE:
   619			ethtool_link_ksettings_add_link_mode(cmd, supported,
   620							     Autoneg);
   621			ethtool_link_ksettings_add_link_mode(cmd, supported,
   622							     Backplane);
   623			ethtool_link_ksettings_add_link_mode(cmd, advertising,
   624							     Autoneg);
   625			ethtool_link_ksettings_add_link_mode(cmd, advertising,
   626							     Backplane);
   627			cmd->base.port = PORT_NONE;
   628			break;
   629		case I40E_MEDIA_TYPE_BASET:
   630			ethtool_link_ksettings_add_link_mode(cmd, supported, TP);
   631			ethtool_link_ksettings_add_link_mode(cmd, advertising, TP);
   632			cmd->base.port = PORT_TP;
   633			break;
   634		case I40E_MEDIA_TYPE_DA:
   635		case I40E_MEDIA_TYPE_CX4:
   636			ethtool_link_ksettings_add_link_mode(cmd, supported, FIBRE);
   637			ethtool_link_ksettings_add_link_mode(cmd, advertising, FIBRE);
   638			cmd->base.port = PORT_DA;
   639			break;
   640		case I40E_MEDIA_TYPE_FIBER:
   641			ethtool_link_ksettings_add_link_mode(cmd, supported, FIBRE);
   642			cmd->base.port = PORT_FIBRE;
   643			break;
   644		case I40E_MEDIA_TYPE_UNKNOWN:
   645		default:
   646			cmd->base.port = PORT_OTHER;
   647			break;
   648		}
   649	
   650		/* Set flow control settings */
   651		ethtool_link_ksettings_add_link_mode(cmd, supported, Pause);
   652	
   653		switch (hw->fc.requested_mode) {
   654		case I40E_FC_FULL:
   655			ethtool_link_ksettings_add_link_mode(cmd, advertising,
   656							     Pause);
   657			break;
   658		case I40E_FC_TX_PAUSE:
   659			ethtool_link_ksettings_add_link_mode(cmd, advertising,
   660							     Asym_Pause);
   661			break;
   662		case I40E_FC_RX_PAUSE:
   663			ethtool_link_ksettings_add_link_mode(cmd, advertising,
   664							     Pause);
   665			ethtool_link_ksettings_add_link_mode(cmd, advertising,
   666							     Asym_Pause);
   667			break;
 > 668		case I40E_PHY_TYPE_25GBASE_AOC:
 > 669		case I40E_PHY_TYPE_25GBASE_ACC:
 > 670			ethtool_link_ksettings_add_link_mode(ks, supported, Autoneg);
   671			ethtool_link_ksettings_add_link_mode(ks, advertising, Autoneg);
   672			ethtool_link_ksettings_add_link_mode(ks, supported,
   673							     25000baseCR_Full);
   674			ethtool_link_ksettings_add_link_mode(ks, advertising,
   675							     25000baseCR_Full);
   676			break;
   677		default:
   678			ethtool_convert_link_mode_to_legacy_u32(
   679				&advertising, cmd->link_modes.advertising);
   680	
   681			advertising &= ~(ADVERTISED_Pause | ADVERTISED_Asym_Pause);
   682	
   683			ethtool_convert_legacy_u32_to_link_mode(
   684				cmd->link_modes.advertising, advertising);
   685			break;
   686		}
   687	
   688		return 0;
   689	}
   690	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 31779 bytes
Desc: not available
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20170715/292d4003/attachment-0001.bin>

[Intel-wired-lan] [next PATCH S76-V2 11/13] i40e: Fix unrecognized PHY type warning message for 25G AOC/ACC

[kbuild test robot]

Hi Sudheer,

[auto build test WARNING on jkirsher-next-queue/dev-queue]
[also build test WARNING on v4.12 next-20170714]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Alice-Michael/i40e-Fix-for-unused-value-issue-found-by-static-analysis/20170715-162444
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue.git dev-queue
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   include/linux/compiler.h:260:8: sparse: attribute 'no_sanitize_address': unknown attribute
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c:668:14: sparse: undefined identifier 'I40E_PHY_TYPE_25GBASE_AOC'
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c:669:14: sparse: undefined identifier 'I40E_PHY_TYPE_25GBASE_ACC'
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c:670:17: sparse: undefined identifier 'ks'
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c:671:17: sparse: undefined identifier 'ks'
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c:672:17: sparse: undefined identifier 'ks'
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c:674:17: sparse: undefined identifier 'ks'
>> drivers/net/ethernet/intel/i40e/i40e_ethtool.c:668:14: sparse: incompatible types for 'case' statement
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c:669:14: sparse: incompatible types for 'case' statement
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c:668:14: sparse: Expected constant expression in case statement
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c:669:14: sparse: Expected constant expression in case statement
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c: In function 'i40e_get_link_ksettings':
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c:668:7: error: 'I40E_PHY_TYPE_25GBASE_AOC' undeclared (first use in this function)
     case I40E_PHY_TYPE_25GBASE_AOC:
          ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c:668:7: note: each undeclared identifier is reported only once for each function it appears in
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c:669:7: error: 'I40E_PHY_TYPE_25GBASE_ACC' undeclared (first use in this function)
     case I40E_PHY_TYPE_25GBASE_ACC:
          ^~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/netdevice.h:42:0,
                    from include/net/sock.h:51,
                    from include/linux/tcp.h:23,
                    from include/net/tcp.h:24,
                    from drivers/net/ethernet/intel/i40e/i40e.h:30,
                    from drivers/net/ethernet/intel/i40e/i40e_ethtool.c:29:
   drivers/net/ethernet/intel/i40e/i40e_ethtool.c:670:40: error: 'ks' undeclared (first use in this function)
      ethtool_link_ksettings_add_link_mode(ks, supported, Autoneg);
                                           ^
   include/linux/ethtool.h:137:49: note: in definition of macro 'ethtool_link_ksettings_add_link_mode'
     __set_bit(ETHTOOL_LINK_MODE_ ## mode ## _BIT, (ptr)->link_modes.name)
                                                    ^~~

vim +/case +668 drivers/net/ethernet/intel/i40e/i40e_ethtool.c

   589	
   590	/**
   591	 * i40e_get_settings - Get Link Speed and Duplex settings
   592	 * @netdev: network interface device structure
   593	 * @ecmd: ethtool command
   594	 *
   595	 * Reports speed/duplex settings based on media_type
   596	 **/
   597	static int i40e_get_link_ksettings(struct net_device *netdev,
   598					   struct ethtool_link_ksettings *cmd)
   599	{
   600		struct i40e_netdev_priv *np = netdev_priv(netdev);
   601		struct i40e_pf *pf = np->vsi->back;
   602		struct i40e_hw *hw = &pf->hw;
   603		struct i40e_link_status *hw_link_info = &hw->phy.link_info;
   604		bool link_up = hw_link_info->link_info & I40E_AQ_LINK_UP;
   605		u32 advertising;
   606	
   607		if (link_up)
   608			i40e_get_settings_link_up(hw, cmd, netdev, pf);
   609		else
   610			i40e_get_settings_link_down(hw, cmd, pf);
   611	
   612		/* Now set the settings that don't rely on link being up/down */
   613		/* Set autoneg settings */
   614		cmd->base.autoneg = ((hw_link_info->an_info & I40E_AQ_AN_COMPLETED) ?
   615				  AUTONEG_ENABLE : AUTONEG_DISABLE);
   616	
   617		switch (hw->phy.media_type) {
   618		case I40E_MEDIA_TYPE_BACKPLANE:
   619			ethtool_link_ksettings_add_link_mode(cmd, supported,
   620							     Autoneg);
   621			ethtool_link_ksettings_add_link_mode(cmd, supported,
   622							     Backplane);
   623			ethtool_link_ksettings_add_link_mode(cmd, advertising,
   624							     Autoneg);
   625			ethtool_link_ksettings_add_link_mode(cmd, advertising,
   626							     Backplane);
   627			cmd->base.port = PORT_NONE;
   628			break;
   629		case I40E_MEDIA_TYPE_BASET:
   630			ethtool_link_ksettings_add_link_mode(cmd, supported, TP);
   631			ethtool_link_ksettings_add_link_mode(cmd, advertising, TP);
   632			cmd->base.port = PORT_TP;
   633			break;
   634		case I40E_MEDIA_TYPE_DA:
   635		case I40E_MEDIA_TYPE_CX4:
   636			ethtool_link_ksettings_add_link_mode(cmd, supported, FIBRE);
   637			ethtool_link_ksettings_add_link_mode(cmd, advertising, FIBRE);
   638			cmd->base.port = PORT_DA;
   639			break;
   640		case I40E_MEDIA_TYPE_FIBER:
   641			ethtool_link_ksettings_add_link_mode(cmd, supported, FIBRE);
   642			cmd->base.port = PORT_FIBRE;
   643			break;
   644		case I40E_MEDIA_TYPE_UNKNOWN:
   645		default:
   646			cmd->base.port = PORT_OTHER;
   647			break;
   648		}
   649	
   650		/* Set flow control settings */
   651		ethtool_link_ksettings_add_link_mode(cmd, supported, Pause);
   652	
   653		switch (hw->fc.requested_mode) {
   654		case I40E_FC_FULL:
   655			ethtool_link_ksettings_add_link_mode(cmd, advertising,
   656							     Pause);
   657			break;
   658		case I40E_FC_TX_PAUSE:
   659			ethtool_link_ksettings_add_link_mode(cmd, advertising,
   660							     Asym_Pause);
   661			break;
   662		case I40E_FC_RX_PAUSE:
   663			ethtool_link_ksettings_add_link_mode(cmd, advertising,
   664							     Pause);
   665			ethtool_link_ksettings_add_link_mode(cmd, advertising,
   666							     Asym_Pause);
   667			break;
 > 668		case I40E_PHY_TYPE_25GBASE_AOC:
   669		case I40E_PHY_TYPE_25GBASE_ACC:
   670			ethtool_link_ksettings_add_link_mode(ks, supported, Autoneg);
   671			ethtool_link_ksettings_add_link_mode(ks, advertising, Autoneg);
   672			ethtool_link_ksettings_add_link_mode(ks, supported,
   673							     25000baseCR_Full);
   674			ethtool_link_ksettings_add_link_mode(ks, advertising,
   675							     25000baseCR_Full);
   676			break;
   677		default:
   678			ethtool_convert_link_mode_to_legacy_u32(
   679				&advertising, cmd->link_modes.advertising);
   680	
   681			advertising &= ~(ADVERTISED_Pause | ADVERTISED_Asym_Pause);
   682	
   683			ethtool_convert_legacy_u32_to_link_mode(
   684				cmd->link_modes.advertising, advertising);
   685			break;
   686		}
   687	
   688		return 0;
   689	}
   690	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[Intel-wired-lan] [next PATCH S76-V2 11/13] i40e: Fix unrecognized PHY type warning message for 25G AOC/ACC

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Friday, July 14, 2017 6:10 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Cc: Mogilappagari at osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S76-V2 11/13] i40e: Fix unrecognized
> PHY type warning message for 25G AOC/ACC
> 
> From: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>
> 
> For 25G ACC and AOC PHY types a warning message appears like "Link is up
> but PHY type 0x23 is not recognized". This commit adds a case statement in
> i40e_get_link_settings_link_up() for 25G AOC/ACC PHY types to set correct
> parameters.
> 
> Signed-off-by: Mogilappagari, Sudheer <sudheer.mogilappagari@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S76-V2 12/13] i40e: simplify member variable accesses

[Alice Michael]

From: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>

This commit replaces usage of vsi->back in i40e_print_link_message()
(which is actually a PF pointer) with temp variable.

Signed-off-by: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index b4844e2..d83a9e6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -5347,12 +5347,13 @@ static int i40e_init_pf_dcb(struct i40e_pf *pf)
 void i40e_print_link_message(struct i40e_vsi *vsi, bool isup)
 {
 	enum i40e_aq_link_speed new_speed;
+	struct i40e_pf *pf = vsi->back;
 	char *speed = "Unknown";
 	char *fc = "Unknown";
 	char *fec = "";
 	char *an = "";
 
-	new_speed = vsi->back->hw.phy.link_info.link_speed;
+	new_speed = pf->hw.phy.link_info.link_speed;
 
 	if ((vsi->current_isup == isup) && (vsi->current_speed == new_speed))
 		return;
@@ -5366,13 +5367,13 @@ void i40e_print_link_message(struct i40e_vsi *vsi, bool isup)
 	/* Warn user if link speed on NPAR enabled partition is not at
 	 * least 10GB
 	 */
-	if (vsi->back->hw.func_caps.npar_enable &&
-	    (vsi->back->hw.phy.link_info.link_speed == I40E_LINK_SPEED_1GB ||
-	     vsi->back->hw.phy.link_info.link_speed == I40E_LINK_SPEED_100MB))
+	if (pf->hw.func_caps.npar_enable &&
+	    (pf->hw.phy.link_info.link_speed == I40E_LINK_SPEED_1GB ||
+	     pf->hw.phy.link_info.link_speed == I40E_LINK_SPEED_100MB))
 		netdev_warn(vsi->netdev,
 			    "The partition detected link speed that is less than 10Gbps\n");
 
-	switch (vsi->back->hw.phy.link_info.link_speed) {
+	switch (pf->hw.phy.link_info.link_speed) {
 	case I40E_LINK_SPEED_40GB:
 		speed = "40 G";
 		break;
@@ -5395,7 +5396,7 @@ void i40e_print_link_message(struct i40e_vsi *vsi, bool isup)
 		break;
 	}
 
-	switch (vsi->back->hw.fc.current_mode) {
+	switch (pf->hw.fc.current_mode) {
 	case I40E_FC_FULL:
 		fc = "RX/TX";
 		break;
@@ -5410,17 +5411,17 @@ void i40e_print_link_message(struct i40e_vsi *vsi, bool isup)
 		break;
 	}
 
-	if (vsi->back->hw.phy.link_info.link_speed == I40E_LINK_SPEED_25GB) {
+	if (pf->hw.phy.link_info.link_speed == I40E_LINK_SPEED_25GB) {
 		fec = ", FEC: None";
 		an = ", Autoneg: False";
 
-		if (vsi->back->hw.phy.link_info.an_info & I40E_AQ_AN_COMPLETED)
+		if (pf->hw.phy.link_info.an_info & I40E_AQ_AN_COMPLETED)
 			an = ", Autoneg: True";
 
-		if (vsi->back->hw.phy.link_info.fec_info &
+		if (pf->hw.phy.link_info.fec_info &
 		    I40E_AQ_CONFIG_FEC_KR_ENA)
 			fec = ", FEC: CL74 FC-FEC/BASE-R";
-		else if (vsi->back->hw.phy.link_info.fec_info &
+		else if (pf->hw.phy.link_info.fec_info &
 			 I40E_AQ_CONFIG_FEC_RS_ENA)
 			fec = ", FEC: CL108 RS-FEC";
 	}
-- 
2.9.3

[Intel-wired-lan] [next PATCH S76-V2 12/13] i40e: simplify member variable accesses

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Friday, July 14, 2017 6:10 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S76-V2 12/13] i40e: simplify member
> variable accesses
> 
> From: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>
> 
> This commit replaces usage of vsi->back in i40e_print_link_message() (which
> is actually a PF pointer) with temp variable.
> 
> Signed-off-by: Sudheer Mogilappagari <sudheer.mogilappagari@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S76-V2 13/13] i40e: relax warning message in case of version mismatch

[Alice Michael]

From: Mariusz Stachura <mariusz.stachura@intel.com>

FVL and FPK devices are often on different firmware release
schedules. This change relaxes the minor version warning message,
so it is only displayed for older FW warning version for old
firmware FVL3 or earlier.

Signed-off-by: Mariusz Stachura <mariusz.stachura@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index d83a9e6..93fa941 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -11355,8 +11355,7 @@ static int i40e_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	    hw->aq.api_min_ver > I40E_FW_API_VERSION_MINOR)
 		dev_info(&pdev->dev,
 			 "The driver for the device detected a newer version of the NVM image than expected. Please install the most recent version of the network driver.\n");
-	else if (hw->aq.api_maj_ver < I40E_FW_API_VERSION_MAJOR ||
-		 hw->aq.api_min_ver < (I40E_FW_API_VERSION_MINOR - 1))
+	else if (hw->aq.api_maj_ver == 1 && hw->aq.api_min_ver < 4)
 		dev_info(&pdev->dev,
 			 "The driver for the device detected an older version of the NVM image than expected. Please update the NVM image.\n");
 
-- 
2.9.3

[Intel-wired-lan] [next PATCH S76-V2 13/13] i40e: relax warning message in case of version mismatch

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Friday, July 14, 2017 6:10 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Cc: Stachura, Mariusz <mariusz.stachura@intel.com>
> Subject: [Intel-wired-lan] [next PATCH S76-V2 13/13] i40e: relax warning
> message in case of version mismatch
> 
> From: Mariusz Stachura <mariusz.stachura@intel.com>
> 
> FVL and FPK devices are often on different firmware release schedules. This
> change relaxes the minor version warning message, so it is only displayed for
> older FW warning version for old firmware FVL3 or earlier.
> 
> Signed-off-by: Mariusz Stachura <mariusz.stachura@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

[Intel-wired-lan] [next PATCH S76-V2 01/13] i40e: Fix for unused value issue found by static analysis

[Bowers, AndrewX]

> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On
> Behalf Of Alice Michael
> Sent: Friday, July 14, 2017 6:10 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan at lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S76-V2 01/13] i40e: Fix for unused
> value issue found by static analysis
> 
> From: Carolyn Wyborny <carolyn.wyborny@intel.com>
> 
> This patch fixes an issue where an error return value is set, but without an
> immediate exit, the value can be overwritten by the following code
> execution.  The condition  at this point is not fatal, so remove the error
> assignment and comment the intent for future code maintainers
> 
> Signed-off-by: Carolyn Wyborny <carolyn.wyborny@intel.com>
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>