From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:17
This series contains updates to i40e and i40evf only.
Sudheer updates code comments and state variable so that adminq_subtask
will have accutate information whenever it gets scheduled.
Mariusz stores information about FEC modes, to be used to printing link
states information, so that we do not need to call admin queue when
reporting link status. Adds VF support for controlling VLAN tag
stripping via ethtool.
Jake provides the majority of changes in this series, starting with
increasing the size of the prefix buffer so that it can hold enough
characters for every possible input, which prevents snprintf truncation.
Fixed other string truncation errors/warnings produced by GCC 7.x.
Removed an unnecessary workaround for resetting XPS. Fixed an issue
where there is a mismatched affinity mask value, so initialize the value
to cpu_possible_mask and invert the logic for checking incorrect CPU vs
IRQ affinity so that the exceptional case is handled at the check.
Removed ULTRA latency mode due to several issues found and will be
looking at better solution for small packet workloads.
Akeem fixes an issue where the incorrect flag was being used to set
promiscuous mode for unicast, which was enabling promiscuous mode only
for multicast instead of unicast.
Carolyn fixes an issue where an error return value is set, but this
value can be overwritten before we actually do exit the function. So
remove the error code assignment and add code comments for better
understanding on why we do not need to set and return the error.
The following are changes since commit ec15ecdee5eb9e33a565e1e8eaef39fd4de565cb:
net: mvpp2: fix the packet size configuration for 10G
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 40GbE
Akeem G Abodunrin (1):
i40e: Use correct flag to enable egress traffic for unicast promisc
Carolyn Wyborny (1):
i40e: Fix for unused value issue found by static analysis
Jacob Keller (9):
i40e: prevent snprintf format specifier truncation
i40evf: fix possible snprintf truncation of q_vector->name
i40e: force VMDQ device name truncation
i40e: remove workaround for resetting XPS
i40e: move enabling icr0 into i40e_update_enable_itr
i40e: initialize our affinity_mask based on cpu_possible_mask
i40e: invert logic for checking incorrect cpu vs irq affinity
i40e/i40evf: remove ULTRA latency mode
i40e/i40evf: avoid dynamic ITR updates when polling or low packet rate
Mariusz Stachura (3):
i40e: Store the requested FEC information
i40e/i40evf: support for VF VLAN tag stripping control
i40e: 25G FEC status improvements
Sudheer Mogilappagari (1):
i40e: Update state variable for adminq subtask
drivers/net/ethernet/intel/i40e/i40e_common.c | 8 ++-
drivers/net/ethernet/intel/i40e/i40e_main.c | 58 ++++++++++------
drivers/net/ethernet/intel/i40e/i40e_nvm.c | 10 ++-
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 78 +++++++++++-----------
drivers/net/ethernet/intel/i40e/i40e_txrx.h | 2 +-
drivers/net/ethernet/intel/i40e/i40e_type.h | 1 +
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 62 ++++++++++++++++-
drivers/net/ethernet/intel/i40evf/i40e_common.c | 4 +-
drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 69 +++++++++----------
drivers/net/ethernet/intel/i40evf/i40e_txrx.h | 2 +-
drivers/net/ethernet/intel/i40evf/i40e_type.h | 1 +
drivers/net/ethernet/intel/i40evf/i40evf.h | 6 +-
drivers/net/ethernet/intel/i40evf/i40evf_main.c | 61 +++++++++++++----
.../net/ethernet/intel/i40evf/i40evf_virtchnl.c | 40 +++++++++++
include/linux/avf/virtchnl.h | 5 ++
15 files changed, 285 insertions(+), 122 deletions(-)
--
2.14.1
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:18
From: Sudheer Mogilappagari <redacted>
During NVM update, state machine gets into unrecoverable state because
i40e_clean_adminq_subtask can get scheduled after the admin queue
command but before other state variables are updated. This causes
incorrect input to i40e_nvmupd_check_wait_event and state transitions
don't happen.
This fix updates the state variables so that adminq_subtask will have
accurate information whenever it gets scheduled.
Signed-off-by: Sudheer Mogilappagari <redacted>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/i40e/i40e_nvm.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:18
From: Akeem G Abodunrin <redacted>
Albeit, we usually set true promiscuous mode for both multicast and
unicast at the same time - however, it is possible to set it
individually, so using allmulti flag which is only for allmulticast might
caused unwanted behavior in mirroring egress traffic promiscuous for
unicast in VF.
Signed-off-by: Akeem G Abodunrin <redacted>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:18
From: Jacob Keller <jacob.e.keller@intel.com>
Increase the size of the prefix buffer so that it can hold enough
characters for every possible input. Although 20 is enough for all
expected inputs, it is possible for the values to be larger than
expected, resulting in a possibly truncated string. Additionally, lets
use sizeof(prefix) in order to ensure we use the correct size if we need
to change the array length in the future.
New versions of GCC starting at 7 now include warnings to prevent
truncation unless you handle the return code. At most 27 bytes can be
written here, so lets just increase the buffer size even if for all
expected hw->bus.* values we only needed 20.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/i40e/i40e_common.c | 4 ++--
drivers/net/ethernet/intel/i40evf/i40e_common.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:18
From: Mariusz Stachura <redacted>
Store information about FEC modes, that were requested. It will be used
in printing link status information function and this way there is no
need to call admin queue there.
Signed-off-by: Mariusz Stachura <redacted>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/i40e/i40e_common.c | 4 ++++
drivers/net/ethernet/intel/i40e/i40e_type.h | 1 +
drivers/net/ethernet/intel/i40evf/i40e_type.h | 1 +
3 files changed, 6 insertions(+)
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:19
From: Mariusz Stachura <redacted>
This patch gives VF capability to control VLAN tag stripping via
ethtool. As rx-vlan-offload was fixed before, now the VF is able to
change it using "ethtool --offload <IF> rxvlan on/off" settings.
Signed-off-by: Mariusz Stachura <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 60 ++++++++++++++++++++++
drivers/net/ethernet/intel/i40evf/i40evf.h | 4 ++
drivers/net/ethernet/intel/i40evf/i40evf_main.c | 33 ++++++++++++
.../net/ethernet/intel/i40evf/i40evf_virtchnl.c | 40 +++++++++++++++
include/linux/avf/virtchnl.h | 5 ++
5 files changed, 142 insertions(+)
@@ -2529,6 +2529,60 @@ static int i40e_vc_set_rss_hena(struct i40e_vf *vf, u8 *msg, u16 msglen)returni40e_vc_send_resp_to_vf(vf,VIRTCHNL_OP_SET_RSS_HENA,aq_ret);}+/**+*i40e_vc_enable_vlan_stripping+*@vf:pointertotheVFinfo+*@msg:pointertothemsgbuffer+*@msglen:msglength+*+*EnablevlanheaderstrippingfortheVF+**/+staticinti40e_vc_enable_vlan_stripping(structi40e_vf*vf,u8*msg,+u16msglen)+{+structi40e_vsi*vsi=vf->pf->vsi[vf->lan_vsi_idx];+i40e_statusaq_ret=0;++if(!test_bit(I40E_VF_STATE_ACTIVE,&vf->vf_states)){+aq_ret=I40E_ERR_PARAM;+gotoerr;+}++i40e_vlan_stripping_enable(vsi);++/* send the response to the VF */+err:+returni40e_vc_send_resp_to_vf(vf,VIRTCHNL_OP_ENABLE_VLAN_STRIPPING,+aq_ret);+}++/**+*i40e_vc_disable_vlan_stripping+*@vf:pointertotheVFinfo+*@msg:pointertothemsgbuffer+*@msglen:msglength+*+*DisablevlanheaderstrippingfortheVF+**/+staticinti40e_vc_disable_vlan_stripping(structi40e_vf*vf,u8*msg,+u16msglen)+{+structi40e_vsi*vsi=vf->pf->vsi[vf->lan_vsi_idx];+i40e_statusaq_ret=0;++if(!test_bit(I40E_VF_STATE_ACTIVE,&vf->vf_states)){+aq_ret=I40E_ERR_PARAM;+gotoerr;+}++i40e_vlan_stripping_disable(vsi);++/* send the response to the VF */+err:+returni40e_vc_send_resp_to_vf(vf,VIRTCHNL_OP_DISABLE_VLAN_STRIPPING,+aq_ret);+}+/***i40e_vc_process_vf_msg*@pf:pointertothePFstructure
@@ -820,6 +820,46 @@ void i40evf_set_rss_lut(struct i40evf_adapter *adapter)kfree(vrl);}+/**+*i40evf_enable_vlan_stripping+*@adapter:adapterstructure+*+*RequestVLANheaderstrippingtobeenabled+**/+voidi40evf_enable_vlan_stripping(structi40evf_adapter*adapter)+{+if(adapter->current_op!=VIRTCHNL_OP_UNKNOWN){+/* bail because we already have a command pending */+dev_err(&adapter->pdev->dev,"Cannot enable stripping, command %d pending\n",+adapter->current_op);+return;+}+adapter->current_op=VIRTCHNL_OP_ENABLE_VLAN_STRIPPING;+adapter->aq_required&=~I40EVF_FLAG_AQ_ENABLE_VLAN_STRIPPING;+i40evf_send_pf_msg(adapter,VIRTCHNL_OP_ENABLE_VLAN_STRIPPING,+NULL,0);+}++/**+*i40evf_disable_vlan_stripping+*@adapter:adapterstructure+*+*RequestVLANheaderstrippingtobedisabled+**/+voidi40evf_disable_vlan_stripping(structi40evf_adapter*adapter)+{+if(adapter->current_op!=VIRTCHNL_OP_UNKNOWN){+/* bail because we already have a command pending */+dev_err(&adapter->pdev->dev,"Cannot disable stripping, command %d pending\n",+adapter->current_op);+return;+}+adapter->current_op=VIRTCHNL_OP_DISABLE_VLAN_STRIPPING;+adapter->aq_required&=~I40EVF_FLAG_AQ_DISABLE_VLAN_STRIPPING;+i40evf_send_pf_msg(adapter,VIRTCHNL_OP_DISABLE_VLAN_STRIPPING,+NULL,0);+}+/***i40evf_print_link_message-printlinkupordown*@adapter:adapterstructure
@@ -133,6 +133,8 @@ enum virtchnl_ops {VIRTCHNL_OP_CONFIG_RSS_LUT=24,VIRTCHNL_OP_GET_RSS_HENA_CAPS=25,VIRTCHNL_OP_SET_RSS_HENA=26,+VIRTCHNL_OP_ENABLE_VLAN_STRIPPING=27,+VIRTCHNL_OP_DISABLE_VLAN_STRIPPING=28,};/* This macro is used to generate a compilation error if a structure
@@ -686,6 +688,9 @@ virtchnl_vc_validate_vf_msg(struct virtchnl_version_info *ver, u32 v_opcode,caseVIRTCHNL_OP_SET_RSS_HENA:valid_len=sizeof(structvirtchnl_rss_hena);break;+caseVIRTCHNL_OP_ENABLE_VLAN_STRIPPING:+caseVIRTCHNL_OP_DISABLE_VLAN_STRIPPING:+break;/* These are always errors coming from the VF. */caseVIRTCHNL_OP_EVENT:caseVIRTCHNL_OP_UNKNOWN:
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:19
From: Mariusz Stachura <redacted>
This patch improves the system log message. The log message will
be expanded to include the FEC mode the FW requested before link
was established.
Signed-off-by: Mariusz Stachura <redacted>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
@@ -5427,10 +5429,22 @@ void i40e_print_link_message(struct i40e_vsi *vsi, bool isup)elseif(vsi->back->hw.phy.link_info.fec_info&I40E_AQ_CONFIG_FEC_RS_ENA)fec=", FEC: CL108 RS-FEC";++/* 'CL108 RS-FEC' should be displayed when RS is requested, or+*bothRSandFCarerequested+*/+if(vsi->back->hw.phy.link_info.req_fec_info&+(I40E_AQ_REQUEST_FEC_KR|I40E_AQ_REQUEST_FEC_RS)){+if(vsi->back->hw.phy.link_info.req_fec_info&+I40E_AQ_REQUEST_FEC_RS)+req_fec=", Requested FEC: CL108 RS-FEC";+else+req_fec=", Requested FEC: CL74 FC-FEC/BASE-R";+}}-netdev_info(vsi->netdev,"NIC Link is Up, %sbps Full Duplex%s%s, Flow Control: %s\n",-speed,fec,an,fc);+netdev_info(vsi->netdev,"NIC Link is Up, %sbps Full Duplex%s%s%s, Flow Control: %s\n",+speed,req_fec,fec,an,fc);}/**
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:19
From: Jacob Keller <jacob.e.keller@intel.com>
In new versions of GCC since 7.x a new warning exists which warns when
a string is truncated before all of the format can be completed.
When we setup VMDQ netdev names we are copying a pre-existing interface
name which could be up to 15 characters in length. Since we also add
4 bytes, v, the literal %, the d and a \0 null, we would overrun the
available size unless snprintf truncated for us.
The snprintf call will of course truncate on the end, so lets instead
modify the code to force truncation of the copied netdev name by
4 characters, to create enough space for the 4 bytes we're adding.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
@@ -9690,8 +9690,13 @@ static int i40e_config_netdev(struct i40e_vsi *vsi)i40e_add_mac_filter(vsi,mac_addr);spin_unlock_bh(&vsi->mac_filter_hash_lock);}else{-/* relate the VSI_VMDQ name to the VSI_MAIN name */-snprintf(netdev->name,IFNAMSIZ,"%sv%%d",+/* Relate the VSI_VMDQ name to the VSI_MAIN name. Note that we+*arestilllimitedbyIFNAMSIZ,butwe'readding'v%d\0'to+*theend,whichis4byteslong,soforcetruncationofthe+*originalnamebyIFNAMSIZ-4+*/+snprintf(netdev->name,IFNAMSIZ,"%.*sv%%d",+IFNAMSIZ-4,pf->vsi[pf->lan_vsi]->netdev->name);random_ether_addr(mac_addr);
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:19
From: Jacob Keller <jacob.e.keller@intel.com>
The q_vector names are based on the interface name with a driver prefix,
the type of q_vector setup, and the queue number. We previously set the
size of this variable to IFNAMSIZ + 9, which is incorrect, because we
actually include a minimum of 14 characters extra beyond the interface
name size.
New versions of GCC since 7 include a new warning that detects this
possible truncation and complains. We can fix this by increasing the
size in case our interface name is too large to avoid truncation. We
don't need to go beyond 14 because the compiler is smart enough to
realize our values can never exceed size of 1. We do go up to 15 here
because possible future changes may increase the number of queues beyond
one digit.
While we are here, also change some variables to be unsigned (since they
are never negative) and stop using an extra unnecessary %s format
specifier.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/i40evf/i40evf.h | 2 +-
drivers/net/ethernet/intel/i40evf/i40evf_main.c | 21 +++++++++------------
2 files changed, 10 insertions(+), 13 deletions(-)
@@ -121,7 +121,7 @@ struct i40e_q_vector {#define ITR_COUNTDOWN_START 100u8itr_countdown;/* when 0 or 1 update ITR */intv_idx;/* vector index in list */-charname[IFNAMSIZ+9];+charname[IFNAMSIZ+15];boolarm_wb_state;cpumask_taffinity_mask;structirq_affinity_notifyaffinity_notify;
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:20
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.
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>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
@@ -2874,22 +2874,15 @@ static void i40e_vsi_free_rx_resources(struct i40e_vsi *vsi)staticvoidi40e_config_xps_tx_ring(structi40e_ring*ring){structi40e_vsi*vsi=ring->vsi;-cpumask_var_tmask;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);-}elseif(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
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:20
From: Carolyn Wyborny <redacted>
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 <redacted>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
@@ -9884,13 +9884,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,+*messageandcontinue+*/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;
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:20
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>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
@@ -2243,6 +2243,12 @@ static inline void i40e_update_enable_itr(struct i40e_vsi *vsi,intidx=q_vector->v_idx;intrx_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
@@ -2396,8 +2402,6 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)*/if(!clean_complete)i40e_force_wb(vsi,q_vector);-elseif(!(vsi->back->flags&I40E_FLAG_MSIX_ENABLED))-i40e_irq_dynamic_enable_icr0(vsi->back,false);elsei40e_update_enable_itr(vsi,q_vector);
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:21
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 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 <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
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(-)
@@ -2369,7 +2369,6 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)/* If work not completed, return budget and polling will return */if(!clean_complete){-constcpumask_t*aff_mask=&q_vector->affinity_mask;intcpu_id=smp_processor_id();/* It is possible that the interrupt affinity has changed but,
@@ -2379,15 +2378,22 @@ int i40e_napi_poll(struct napi_struct *napi, int budget)*continuetopoll,otherwisewemuststoppollingsothe*interruptcanmovetothecorrectcpu.*/-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 */+returnbudget-1;+}tx_only:-if(arm_wb){-q_vector->tx.ring[0].tx_stats.tx_force_wb++;-i40e_enable_wb_on_itr(vsi,q_vector);-}-returnbudget;+if(arm_wb){+q_vector->tx.ring[0].tx_stats.tx_force_wb++;+i40e_enable_wb_on_itr(vsi,q_vector);}+returnbudget;}if(vsi->back->flags&I40E_TXR_FLAGS_WB_ON_ITR)
@@ -2396,14 +2402,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-*affinitywewanttomakesurepollingstartsbackupsowe-*issueacalltoi40e_force_wbwhichtriggersaSWinterrupt.-*/-if(!clean_complete)-i40e_force_wb(vsi,q_vector);-else-i40e_update_enable_itr(vsi,q_vector);+i40e_update_enable_itr(vsi,q_vector);returnmin(work_done,budget-1);}
@@ -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){-constcpumask_t*aff_mask=&q_vector->affinity_mask;intcpu_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)*continuetopoll,otherwisewemuststoppollingsothe*interruptcanmovetothecorrectcpu.*/-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 */+returnbudget-1;+}tx_only:-if(arm_wb){-q_vector->tx.ring[0].tx_stats.tx_force_wb++;-i40e_enable_wb_on_itr(vsi,q_vector);-}-returnbudget;+if(arm_wb){+q_vector->tx.ring[0].tx_stats.tx_force_wb++;+i40e_enable_wb_on_itr(vsi,q_vector);}+returnbudget;}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-*affinitywewanttomakesurepollingstartsbackupsowe-*issueacalltoi40evf_force_wbwhichtriggersaSWinterrupt.-*/-if(!clean_complete)-i40evf_force_wb(vsi,q_vector);-else-i40e_update_enable_itr(vsi,q_vector);+i40e_update_enable_itr(vsi,q_vector);returnmin(work_done,budget-1);}
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:21
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>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
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(-)
@@ -994,24 +992,12 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)new_latency_range=I40E_LOWEST_LATENCY;break;caseI40E_BULK_LATENCY:-caseI40E_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.Thevalueof40000waspickedasitisjustbeyond-*whatthehardwarecanreceivepersecondifinlowlatency-*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){
@@ -392,24 +390,12 @@ static bool i40e_set_new_dynamic_itr(struct i40e_ring_container *rc)new_latency_range=I40E_LOWEST_LATENCY;break;caseI40E_BULK_LATENCY:-caseI40E_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.Thevalueof40000waspickedasitisjustbeyond-*whatthehardwarecanreceivepersecondifinlowlatency-*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){
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:21
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>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
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(-)
@@ -3506,8 +3506,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+*apermanentlifetimesoit'soktousehere.+*/+irq_set_affinity_hint(irq_num,get_cpu_mask(q_vector->v_idx));}vsi->irqs_ready=true;
@@ -4289,7 +4291,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]);
@@ -8235,7 +8237,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,
From: Jeff Kirsher <hidden> Date: 2017-08-28 00:16:22
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>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
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(-)
@@ -461,6 +461,7 @@ struct i40e_ring_container {structi40e_ring*ring;unsignedinttotal_bytes;/* total bytes processed this int */unsignedinttotal_packets;/* total packets processed this int */+unsignedlonglast_itr_update;/* jiffies of last ITR update */u16count;enumi40e_latency_rangelatency_range;u16itr;
@@ -432,6 +432,7 @@ struct i40e_ring_container {structi40e_ring*ring;unsignedinttotal_bytes;/* total bytes processed this int */unsignedinttotal_packets;/* total packets processed this int */+unsignedlonglast_itr_update;/* jiffies of last ITR update */u16count;enumi40e_latency_rangelatency_range;u16itr;