From: Jeff Kirsher <hidden> Date: 2016-07-15 07:02:47
This series contains fixes to i40e and ixgbe.
Alex fixes issues found in i40e_rx_checksum() which was broken, where the
checksum was being returned valid when it was not.
Kiran fixes a bug which was found when we abruptly remove a cable which
caused a panic. Set the VSI broadcast promiscuous mode during VSI add
sequence and prevents adding MAC filter if specified MAC address is
broadcast.
Paolo Abeni fixes a bug by returning the actual work done, capped to
weight - 1, since the core doesn't allow to return the full budget when
the driver modifies the NAPI status.
Guilherme Piccoli fixes an issue where the q_vector initialization
routine sets the affinity _mask of a q_vector based on v_idx value.
This means a loop iterates on v_idx, which is an incremental value, and
the cpumask is created based on this value. This is a problem in
systems with multiple logical CPUs per core (like in SMT scenarios).
Changed the way q_vector's affinity_mask is created to resolve the issue.
The following are changes since commit 005db31d5f5f7c31cfdc43505d77eb3ca5cf8ec6:
bonding: set carrier off for devices created through netlink
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-queue master
Alexander Duyck (1):
i40e/i40evf: Fix i40e_rx_checksum
Guilherme G. Piccoli (1):
i40e: use valid online CPU on q_vector initialization
Kiran Patil (1):
i40e: enable VSI broadcast promiscuous mode instead of adding
broadcast filter
Paolo Abeni (1):
ixgbe: napi_poll must return the work done
drivers/net/ethernet/intel/i40e/i40e_main.c | 48 +++++++++++++++++----------
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 30 +++++++++--------
drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 30 +++++++++--------
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
4 files changed, 66 insertions(+), 44 deletions(-)
--
2.5.5
From: Jeff Kirsher <hidden> Date: 2016-07-15 07:02:47
From: Alexander Duyck <redacted>
There are a couple of issues I found in i40e_rx_checksum while doing some
recent testing. As a result I have found the Rx checksum logic is pretty
much broken and returning that the checksum is valid for tunnels in cases
where it is not.
First the inner types are not the correct values to use to test for if a
tunnel is present or not. In addition the inner protocol types are not a
bitmask as such performing an OR of the values doesn't make sense. I have
instead changed the code so that the inner protocol types are used to
determine if we report CHECKSUM_UNNECESSARY or not. For anything that does
not end in UDP, TCP, or SCTP it doesn't make much sense to report a
checksum offload since it won't contain a checksum anyway.
This leaves us with the need to set the csum_level based on some value.
For that purpose I am using the tunnel_type field. If the tunnel type is
GRENAT or greater then this means we have a GRE or UDP tunnel with an inner
header. In the case of GRE or UDP we will have a possible checksum present
so for this reason it should be safe to set the csum_level to 1 to indicate
that we are reporting the state of the inner header.
Signed-off-by: Alexander Duyck <redacted>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 30 +++++++++++++++------------
drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 30 +++++++++++++++------------
2 files changed, 34 insertions(+), 26 deletions(-)
@@ -1336,19 +1336,23 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,if(rx_error&BIT(I40E_RX_DESC_ERROR_PPRS_SHIFT))return;-/* The hardware supported by this driver does not validate outer-*checksumsfortunneledVXLANorGENEVEframes.Idon'tagree-*withitbutthespecificationstatesthatyou"MAY validate",it-*doesn'tmakeitahardrequirementsoifwehavevalidatedthe-*innerchecksumreportCHECKSUM_UNNECESSARY.+/* If there is an outer header present that might contain a checksum+*weneedtobumpthechecksumlevelby1toreflectthefactthat+*weareindicatingwevalidatedtheinnerchecksum.*/-if(decoded.inner_prot&(I40E_RX_PTYPE_INNER_PROT_TCP|-I40E_RX_PTYPE_INNER_PROT_UDP|-I40E_RX_PTYPE_INNER_PROT_SCTP))-tunnel=true;--skb->ip_summed=CHECKSUM_UNNECESSARY;-skb->csum_level=tunnel?1:0;+if(decoded.tunnel_type>=I40E_RX_PTYPE_TUNNEL_IP_GRENAT)+skb->csum_level=1;++/* Only report checksum unnecessary for TCP, UDP, or SCTP */+switch(decoded.inner_prot){+caseI40E_RX_PTYPE_INNER_PROT_TCP:+caseI40E_RX_PTYPE_INNER_PROT_UDP:+caseI40E_RX_PTYPE_INNER_PROT_SCTP:+skb->ip_summed=CHECKSUM_UNNECESSARY;+/* fall though */+default:+break;+}return;
@@ -808,19 +808,23 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,if(rx_error&BIT(I40E_RX_DESC_ERROR_PPRS_SHIFT))return;-/* The hardware supported by this driver does not validate outer-*checksumsfortunneledVXLANorGENEVEframes.Idon'tagree-*withitbutthespecificationstatesthatyou"MAY validate",it-*doesn'tmakeitahardrequirementsoifwehavevalidatedthe-*innerchecksumreportCHECKSUM_UNNECESSARY.+/* If there is an outer header present that might contain a checksum+*weneedtobumpthechecksumlevelby1toreflectthefactthat+*weareindicatingwevalidatedtheinnerchecksum.*/-if(decoded.inner_prot&(I40E_RX_PTYPE_INNER_PROT_TCP|-I40E_RX_PTYPE_INNER_PROT_UDP|-I40E_RX_PTYPE_INNER_PROT_SCTP))-tunnel=true;--skb->ip_summed=CHECKSUM_UNNECESSARY;-skb->csum_level=tunnel?1:0;+if(decoded.tunnel_type>=I40E_RX_PTYPE_TUNNEL_IP_GRENAT)+skb->csum_level=1;++/* Only report checksum unnecessary for TCP, UDP, or SCTP */+switch(decoded.inner_prot){+caseI40E_RX_PTYPE_INNER_PROT_TCP:+caseI40E_RX_PTYPE_INNER_PROT_UDP:+caseI40E_RX_PTYPE_INNER_PROT_SCTP:+skb->ip_summed=CHECKSUM_UNNECESSARY;+/* fall though */+default:+break;+}return;
From: Jeff Kirsher <hidden> Date: 2016-07-15 07:02:48
From: Paolo Abeni <pabeni@redhat.com>
Currently the function ixgbe_poll() returns 0 when it clean completely
the rx rings, but this foul budget accounting in core code.
Fix this returning the actual work done, capped to weight - 1, since
the core doesn't allow to return the full budget when the driver modifies
the napi status
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Venkatesh Srinivas <redacted>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -2887,7 +2887,7 @@ int ixgbe_poll(struct napi_struct *napi, int budget)if(!test_bit(__IXGBE_DOWN,&adapter->state))ixgbe_irq_enable_queues(adapter,BIT_ULL(q_vector->v_idx));-return0;+returnmin(work_done,budget-1);}/**
From: Jeff Kirsher <hidden> Date: 2016-07-15 07:02:48
From: Kiran Patil <redacted>
This patch sets VSI broadcast promiscuous mode during VSI add sequence
and prevents adding MAC filter if specified MAC address is broadcast.
Change-ID: Ia62251fca095bc449d0497fc44bec3a5a0136773
CC: stable@kernel.org
Signed-off-by: Kiran Patil <redacted>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 32 ++++++++++++++++++-----------
1 file changed, 20 insertions(+), 12 deletions(-)
@@ -1344,6 +1344,13 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi,if(!vsi||!macaddr)returnNULL;+/* Do not allow broadcast filter to be added since broadcast filter+*isaddedaspartofaddVSIforanynewlycreatedVSIexcept+*FDIRVSI+*/+if(is_broadcast_ether_addr(macaddr))+returnNULL;+f=i40e_find_filter(vsi,macaddr,vlan,is_vf,is_netdev);if(!f){f=kzalloc(sizeof(*f),GFP_ATOMIC);
@@ -2151,18 +2158,6 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)aq_ret,pf->hw.aq.asq_last_status);}}-aq_ret=i40e_aq_set_vsi_broadcast(&vsi->back->hw,-vsi->seid,-cur_promisc,NULL);-if(aq_ret){-retval=i40e_aq_rc_to_posix(aq_ret,-pf->hw.aq.asq_last_status);-dev_info(&pf->pdev->dev,-"set brdcast promisc failed, err %s, aq_err %s\n",-i40e_stat_str(&pf->hw,aq_ret),-i40e_aq_str(&pf->hw,-pf->hw.aq.asq_last_status));-}}out:/* if something went wrong then set the changed flag so we try again */
@@ -9224,6 +9219,7 @@ int i40e_is_vsi_uplink_mode_veb(struct i40e_vsi *vsi)staticinti40e_add_vsi(structi40e_vsi*vsi){intret=-ENODEV;+i40e_statusaq_ret=0;u8laa_macaddr[ETH_ALEN];boolfound_laa_mac_filter=false;structi40e_pf*pf=vsi->back;
@@ -9413,6 +9409,18 @@ static int i40e_add_vsi(struct i40e_vsi *vsi)vsi->seid=ctxt.seid;vsi->id=ctxt.vsi_number;}+/* Except FDIR VSI, for all othet VSI set the broadcast filter */+if(vsi->type!=I40E_VSI_FDIR){+aq_ret=i40e_aq_set_vsi_broadcast(hw,vsi->seid,true,NULL);+if(aq_ret){+ret=i40e_aq_rc_to_posix(aq_ret,+hw->aq.asq_last_status);+dev_info(&pf->pdev->dev,+"set brdcast promisc failed, err %s, aq_err %s\n",+i40e_stat_str(hw,aq_ret),+i40e_aq_str(hw,hw->aq.asq_last_status));+}+}spin_lock_bh(&vsi->mac_filter_list_lock);/* If macvlan filters already exist, force them to get loaded */
From: Jeff Kirsher <hidden> Date: 2016-07-15 07:02:49
From: "Guilherme G. Piccoli" <redacted>
Currently, the q_vector initialization routine sets the affinity_mask
of a q_vector based on v_idx value. Meaning a loop iterates on v_idx,
which is an incremental value, and the cpumask is created based on
this value.
This is a problem in systems with multiple logical CPUs per core (like in
SMT scenarios). If we disable some logical CPUs, by turning SMT off for
example, we will end up with a sparse cpu_online_mask, i.e., only the first
CPU in a core is online, and incremental filling in q_vector cpumask might
lead to multiple offline CPUs being assigned to q_vectors.
Example: if we have a system with 8 cores each one containing 8 logical
CPUs (SMT == 8 in this case), we have 64 CPUs in total. But if SMT is
disabled, only the 1st CPU in each core remains online, so the
cpu_online_mask in this case would have only 8 bits set, in a sparse way.
In general case, when SMT is off the cpu_online_mask has only C bits set:
0, 1*N, 2*N, ..., C*(N-1) where
C == # of cores;
N == # of logical CPUs per core.
In our example, only bits 0, 8, 16, 24, 32, 40, 48, 56 would be set.
This patch changes the way q_vector's affinity_mask is created: it iterates
on v_idx, but consumes the CPU index from the cpu_online_mask instead of
just using the v_idx incremental value.
No functional changes were introduced.
Signed-off-by: Guilherme G Piccoli <redacted>
Tested-by: Andrew Bowers <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
@@ -7721,10 +7721,11 @@ static int i40e_init_msix(struct i40e_pf *pf)*i40e_vsi_alloc_q_vector-Allocatememoryforasingleinterruptvector*@vsi:theVSIbeingconfigured*@v_idx:indexofthevectorinthevsistruct+*@cpu:cputobeusedonaffinity_mask**Weallocateoneq_vector.Ifallocationfailswereturn-ENOMEM.**/-staticinti40e_vsi_alloc_q_vector(structi40e_vsi*vsi,intv_idx)+staticinti40e_vsi_alloc_q_vector(structi40e_vsi*vsi,intv_idx,intcpu){structi40e_q_vector*q_vector;
@@ -7735,7 +7736,8 @@ static int i40e_vsi_alloc_q_vector(struct i40e_vsi *vsi, int v_idx)q_vector->vsi=vsi;q_vector->v_idx=v_idx;-cpumask_set_cpu(v_idx,&q_vector->affinity_mask);+cpumask_set_cpu(cpu,&q_vector->affinity_mask);+if(vsi->netdev)netif_napi_add(vsi->netdev,&q_vector->napi,i40e_napi_poll,NAPI_POLL_WEIGHT);
@@ -7759,8 +7761,7 @@ static int i40e_vsi_alloc_q_vector(struct i40e_vsi *vsi, int v_idx)staticinti40e_vsi_alloc_q_vectors(structi40e_vsi*vsi){structi40e_pf*pf=vsi->back;-intv_idx,num_q_vectors;-interr;+interr,v_idx,num_q_vectors,current_cpu;/* if not MSIX, give the one vector only to the LAN VSI */if(pf->flags&I40E_FLAG_MSIX_ENABLED)
@@ -7770,10 +7771,15 @@ static int i40e_vsi_alloc_q_vectors(struct i40e_vsi *vsi)elsereturn-EINVAL;+current_cpu=cpumask_first(cpu_online_mask);+for(v_idx=0;v_idx<num_q_vectors;v_idx++){-err=i40e_vsi_alloc_q_vector(vsi,v_idx);+err=i40e_vsi_alloc_q_vector(vsi,v_idx,current_cpu);if(err)gotoerr_out;+current_cpu=cpumask_next(current_cpu,cpu_online_mask);+if(unlikely(current_cpu>=nr_cpu_ids))+current_cpu=cpumask_first(cpu_online_mask);}return0;