From 858296c8784bf98450765cbc6b1bc2e44175cc01 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Tue, 14 Jun 2016 15:45:42 -0700 Subject: i40e/i40evf: Fix i40e_rx_checksum 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 Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- 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(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index 55f151fca1dc..a8868e1bf832 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c @@ -1280,8 +1280,8 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi, union i40e_rx_desc *rx_desc) { struct i40e_rx_ptype_decoded decoded; - bool ipv4, ipv6, tunnel = false; u32 rx_error, rx_status; + bool ipv4, ipv6; u8 ptype; u64 qword; @@ -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 - * checksums for tunneled VXLAN or GENEVE frames. I don't agree - * with it but the specification states that you "MAY validate", it - * doesn't make it a hard requirement so if we have validated the - * inner checksum report CHECKSUM_UNNECESSARY. + /* If there is an outer header present that might contain a checksum + * we need to bump the checksum level by 1 to reflect the fact that + * we are indicating we validated the inner checksum. */ - 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) { + case I40E_RX_PTYPE_INNER_PROT_TCP: + case I40E_RX_PTYPE_INNER_PROT_UDP: + case I40E_RX_PTYPE_INNER_PROT_SCTP: + skb->ip_summed = CHECKSUM_UNNECESSARY; + /* fall though */ + default: + break; + } return; diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c index be99189da925..79d99cd91b24 100644 --- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c @@ -752,8 +752,8 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi, union i40e_rx_desc *rx_desc) { struct i40e_rx_ptype_decoded decoded; - bool ipv4, ipv6, tunnel = false; u32 rx_error, rx_status; + bool ipv4, ipv6; u8 ptype; u64 qword; @@ -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 - * checksums for tunneled VXLAN or GENEVE frames. I don't agree - * with it but the specification states that you "MAY validate", it - * doesn't make it a hard requirement so if we have validated the - * inner checksum report CHECKSUM_UNNECESSARY. + /* If there is an outer header present that might contain a checksum + * we need to bump the checksum level by 1 to reflect the fact that + * we are indicating we validated the inner checksum. */ - 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) { + case I40E_RX_PTYPE_INNER_PROT_TCP: + case I40E_RX_PTYPE_INNER_PROT_UDP: + case I40E_RX_PTYPE_INNER_PROT_SCTP: + skb->ip_summed = CHECKSUM_UNNECESSARY; + /* fall though */ + default: + break; + } return; -- cgit From f6bd09625ba66446821d55c61891bea9e2cdc5b3 Mon Sep 17 00:00:00 2001 From: Kiran Patil Date: Mon, 20 Jun 2016 09:10:34 -0700 Subject: i40e: enable VSI broadcast promiscuous mode instead of adding broadcast filter 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 Signed-off-by: Kiran Patil Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_main.c | 32 ++++++++++++++++++----------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 5ea22008d721..1592dcbed790 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -1344,6 +1344,13 @@ struct i40e_mac_filter *i40e_add_filter(struct i40e_vsi *vsi, if (!vsi || !macaddr) return NULL; + /* Do not allow broadcast filter to be added since broadcast filter + * is added as part of add VSI for any newly created VSI except + * FDIR VSI + */ + if (is_broadcast_ether_addr(macaddr)) + return NULL; + 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) static int i40e_add_vsi(struct i40e_vsi *vsi) { int ret = -ENODEV; + i40e_status aq_ret = 0; u8 laa_macaddr[ETH_ALEN]; bool found_laa_mac_filter = false; struct i40e_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 */ -- cgit From 4b732cd4bb6006ad7fd4d5cdba27fcb751cdf4b7 Mon Sep 17 00:00:00 2001 From: Paolo Abeni Date: Wed, 15 Jun 2016 15:37:59 +0200 Subject: ixgbe: napi_poll must return the work done 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 Reviewed-by: Venkatesh Srinivas Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 088c47cf27d9..8bebd862a54c 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -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)); - return 0; + return min(work_done, budget - 1); } /** -- cgit From 7f6c553902bfa1c4e3f6cfa955c5ea036c7fe8e4 Mon Sep 17 00:00:00 2001 From: "Guilherme G. Piccoli" Date: Mon, 27 Jun 2016 12:16:43 -0300 Subject: i40e: use valid online CPU on q_vector initialization 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 Tested-by: Andrew Bowers Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/i40e/i40e_main.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 1592dcbed790..501f15d9f4d6 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -7721,10 +7721,11 @@ static int i40e_init_msix(struct i40e_pf *pf) * i40e_vsi_alloc_q_vector - Allocate memory for a single interrupt vector * @vsi: the VSI being configured * @v_idx: index of the vector in the vsi struct + * @cpu: cpu to be used on affinity_mask * * We allocate one q_vector. If allocation fails we return -ENOMEM. **/ -static int i40e_vsi_alloc_q_vector(struct i40e_vsi *vsi, int v_idx) +static int i40e_vsi_alloc_q_vector(struct i40e_vsi *vsi, int v_idx, int cpu) { struct i40e_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) static int i40e_vsi_alloc_q_vectors(struct i40e_vsi *vsi) { struct i40e_pf *pf = vsi->back; - int v_idx, num_q_vectors; - int err; + int err, 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) else return -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) goto err_out; + current_cpu = cpumask_next(current_cpu, cpu_online_mask); + if (unlikely(current_cpu >= nr_cpu_ids)) + current_cpu = cpumask_first(cpu_online_mask); } return 0; -- cgit