i40e/i40evf: Drop outer checksum offload that was not requested
authorAlexander Duyck <aduyck@mirantis.com>
Mon, 25 Jan 2016 05:16:13 +0000 (21:16 -0800)
committerJeff Kirsher <jeffrey.t.kirsher@intel.com>
Thu, 18 Feb 2016 18:27:45 +0000 (10:27 -0800)
The i40e and i40evf drivers contained code for inserting an outer checksum
on UDP tunnels.  The issue however is that the upper levels of the stack
never requested such an offload and it results in possible errors.

In addition the same logic was being applied to the Rx side where it was
attempting to validate the outer checksum, but the logic there was
incorrect in that it was testing for the resultant sum to be equal to the
header checksum instead of being equal to 0.

Since this code is so massively flawed, and doing things that we didn't ask
for it to do I am just dropping it, and will bring it back later to use as
an offload for SKB_GSO_UDP_TUNNEL_CSUM which can make use of such a
feature.

As far as the Rx feature I am dropping it completely since it would need to
be massively expanded and applied to IPv4 and IPv6 checksums for all parts,
not just the one that supports Tx checksum offload for the outer.

Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
drivers/net/ethernet/intel/i40e/i40e_main.c
drivers/net/ethernet/intel/i40e/i40e_txrx.c
drivers/net/ethernet/intel/i40e/i40e_txrx.h
drivers/net/ethernet/intel/i40evf/i40e_txrx.c
drivers/net/ethernet/intel/i40evf/i40e_txrx.h

index 16e5e0b81bd04d1f2b37a0f02739eec9c7b1093d..0fa52ed1a89663fad02e956ad0bff5275f570646 100644 (file)
@@ -7474,8 +7474,6 @@ static int i40e_alloc_rings(struct i40e_vsi *vsi)
                tx_ring->dcb_tc = 0;
                if (vsi->back->flags & I40E_FLAG_WB_ON_ITR_CAPABLE)
                        tx_ring->flags = I40E_TXR_FLAGS_WB_ON_ITR;
-               if (vsi->back->flags & I40E_FLAG_OUTER_UDP_CSUM_CAPABLE)
-                       tx_ring->flags |= I40E_TXR_FLAGS_OUTER_UDP_CSUM;
                vsi->tx_rings[i] = tx_ring;
 
                rx_ring = &tx_ring[1];
index 65f2fd80aa79c25cca326e7660e4a549e5cd1780..d4364ec33022f225474bc0cda3814a8092fa0552 100644 (file)
@@ -1391,9 +1391,6 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
        struct i40e_rx_ptype_decoded decoded = decode_rx_desc_ptype(rx_ptype);
        bool ipv4 = false, ipv6 = false;
        bool ipv4_tunnel, ipv6_tunnel;
-       __wsum rx_udp_csum;
-       struct iphdr *iph;
-       __sum16 csum;
 
        ipv4_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT4_MAC_PAY3) &&
                     (rx_ptype <= I40E_RX_PTYPE_GRENAT4_MACVLAN_IPV6_ICMP_PAY4);
@@ -1443,37 +1440,12 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
        if (rx_error & BIT(I40E_RX_DESC_ERROR_PPRS_SHIFT))
                return;
 
-       /* If VXLAN/GENEVE traffic has an outer UDPv4 checksum we need to check
-        * it in the driver, hardware does not do it for us.
-        * Since L3L4P bit was set we assume a valid IHL value (>=5)
-        * so the total length of IPv4 header is IHL*4 bytes
-        * The UDP_0 bit *may* bet set if the *inner* header is UDP
+       /* 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 (!(vsi->back->flags & I40E_FLAG_OUTER_UDP_CSUM_CAPABLE) &&
-           (ipv4_tunnel)) {
-               skb->transport_header = skb->mac_header +
-                                       sizeof(struct ethhdr) +
-                                       (ip_hdr(skb)->ihl * 4);
-
-               /* Add 4 bytes for VLAN tagged packets */
-               skb->transport_header += (skb->protocol == htons(ETH_P_8021Q) ||
-                                         skb->protocol == htons(ETH_P_8021AD))
-                                         ? VLAN_HLEN : 0;
-
-               if ((ip_hdr(skb)->protocol == IPPROTO_UDP) &&
-                   (udp_hdr(skb)->check != 0)) {
-                       rx_udp_csum = udp_csum(skb);
-                       iph = ip_hdr(skb);
-                       csum = csum_tcpudp_magic(
-                                       iph->saddr, iph->daddr,
-                                       (skb->len - skb_transport_offset(skb)),
-                                       IPPROTO_UDP, rx_udp_csum);
-
-                       if (udp_hdr(skb)->check != csum)
-                               goto checksum_fail;
-
-               } /* else its GRE and so no outer UDP header */
-       }
 
        skb->ip_summed = CHECKSUM_UNNECESSARY;
        skb->csum_level = ipv4_tunnel || ipv6_tunnel;
@@ -2453,15 +2425,6 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
                        *tx_flags &= ~I40E_TX_FLAGS_IPV4;
                        *tx_flags |= I40E_TX_FLAGS_IPV6;
                }
-               if ((tx_ring->flags & I40E_TXR_FLAGS_OUTER_UDP_CSUM) &&
-                   (l4_tunnel == I40E_TXD_CTX_UDP_TUNNELING)        &&
-                   (*cd_tunneling & I40E_TXD_CTX_QW0_EXT_IP_MASK)) {
-                       oudph->check = ~csum_tcpudp_magic(oiph->saddr,
-                                       oiph->daddr,
-                                       (skb->len - skb_transport_offset(skb)),
-                                       IPPROTO_UDP, 0);
-                       *cd_tunneling |= I40E_TXD_CTX_QW0_L4T_CS_MASK;
-               }
        } else {
                network_hdr_len = skb_network_header_len(skb);
                this_ip_hdr = ip_hdr(skb);
index 3acc9244134d9609da0b16f34c4fe5788e741498..fb065d4fe15cddee8ef1265ff39a96b20fe01a4e 100644 (file)
@@ -277,7 +277,6 @@ struct i40e_ring {
 
        u16 flags;
 #define I40E_TXR_FLAGS_WB_ON_ITR       BIT(0)
-#define I40E_TXR_FLAGS_OUTER_UDP_CSUM  BIT(1)
 #define I40E_TXR_FLAGS_LAST_XMIT_MORE_SET BIT(2)
 
        /* stats structs */
index fb6cd7e5d3be2737176b137e79f43f2c58b8586f..8b20ed3f20d16f279c5636e326c7a3a9114cc738 100644 (file)
@@ -863,9 +863,6 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
        struct i40e_rx_ptype_decoded decoded = decode_rx_desc_ptype(rx_ptype);
        bool ipv4 = false, ipv6 = false;
        bool ipv4_tunnel, ipv6_tunnel;
-       __wsum rx_udp_csum;
-       struct iphdr *iph;
-       __sum16 csum;
 
        ipv4_tunnel = (rx_ptype >= I40E_RX_PTYPE_GRENAT4_MAC_PAY3) &&
                     (rx_ptype <= I40E_RX_PTYPE_GRENAT4_MACVLAN_IPV6_ICMP_PAY4);
@@ -915,36 +912,12 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
        if (rx_error & BIT(I40E_RX_DESC_ERROR_PPRS_SHIFT))
                return;
 
-       /* If VXLAN traffic has an outer UDPv4 checksum we need to check
-        * it in the driver, hardware does not do it for us.
-        * Since L3L4P bit was set we assume a valid IHL value (>=5)
-        * so the total length of IPv4 header is IHL*4 bytes
-        * The UDP_0 bit *may* bet set if the *inner* header is UDP
+       /* 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 (ipv4_tunnel) {
-               skb->transport_header = skb->mac_header +
-                                       sizeof(struct ethhdr) +
-                                       (ip_hdr(skb)->ihl * 4);
-
-               /* Add 4 bytes for VLAN tagged packets */
-               skb->transport_header += (skb->protocol == htons(ETH_P_8021Q) ||
-                                         skb->protocol == htons(ETH_P_8021AD))
-                                         ? VLAN_HLEN : 0;
-
-               if ((ip_hdr(skb)->protocol == IPPROTO_UDP) &&
-                   (udp_hdr(skb)->check != 0)) {
-                       rx_udp_csum = udp_csum(skb);
-                       iph = ip_hdr(skb);
-                       csum = csum_tcpudp_magic(iph->saddr, iph->daddr,
-                                                (skb->len -
-                                                 skb_transport_offset(skb)),
-                                                IPPROTO_UDP, rx_udp_csum);
-
-                       if (udp_hdr(skb)->check != csum)
-                               goto checksum_fail;
-
-               } /* else its GRE and so no outer UDP header */
-       }
 
        skb->ip_summed = CHECKSUM_UNNECESSARY;
        skb->csum_level = ipv4_tunnel || ipv6_tunnel;
@@ -1667,15 +1640,6 @@ static void i40e_tx_enable_csum(struct sk_buff *skb, u32 *tx_flags,
                        *tx_flags |= I40E_TX_FLAGS_IPV6;
                }
 
-               if ((tx_ring->flags & I40E_TXR_FLAGS_OUTER_UDP_CSUM) &&
-                   (l4_tunnel == I40E_TXD_CTX_UDP_TUNNELING)        &&
-                   (*cd_tunneling & I40E_TXD_CTX_QW0_EXT_IP_MASK)) {
-                       oudph->check = ~csum_tcpudp_magic(oiph->saddr,
-                                       oiph->daddr,
-                                       (skb->len - skb_transport_offset(skb)),
-                                       IPPROTO_UDP, 0);
-                       *cd_tunneling |= I40E_TXD_CTX_QW0_L4T_CS_MASK;
-               }
        } else {
                network_hdr_len = skb_network_header_len(skb);
                this_ip_hdr = ip_hdr(skb);
index 81c96619287bc45f2abc6e06c33c3404e07e6ede..043b9556834f5fe3b8ba836e234d77e3a5b46dbe 100644 (file)
@@ -275,7 +275,6 @@ struct i40e_ring {
 
        u16 flags;
 #define I40E_TXR_FLAGS_WB_ON_ITR       BIT(0)
-#define I40E_TXR_FLAGS_OUTER_UDP_CSUM  BIT(1)
 
        /* stats structs */
        struct i40e_queue_stats stats;