Re: [net-next v4 07/16] i40e: Rx checksum offload for VXLAN

On Thu, 2 Jan 2014, Or Gerlitz wrote:

On Sat, Dec 28, 2013 at 5:22 AM, Jeff Kirsher
[off-list ref] wrote:

quoted

From: Joseph Gasparakis <redacted>

This implements receive offload for VXLAN for i40e.  The hardware
supports checksum offload/verification of the inner/outer header.

Change-Id: I450db300af6713f2044fef1191a0d1d294c13369
Signed-off-by: Joseph Gasparakis <redacted>
Signed-off-by: Jesse Brandeburg <redacted>
Signed-off-by: Jeff Kirsher <redacted>
---
 drivers/net/ethernet/intel/i40e/i40e.h      |  1 +
 drivers/net/ethernet/intel/i40e/i40e_txrx.c | 57 ++++++++++++++++++++++++++---
 drivers/net/ethernet/intel/i40e/i40e_type.h | 51 ++++++++++++++------------
 3 files changed, 81 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index 6f1edc1..34a54e7 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h

@@ -29,6 +29,7 @@
 #define _I40E_H_

 #include <net/tcp.h>
+#include <net/udp.h>
 #include <linux/init.h>
 #include <linux/types.h>
 #include <linux/errno.h>

diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 01d0334..a978451 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c

@@ -860,12 +860,25 @@ static void i40e_receive_skb(struct i40e_ring *rx_ring,
  * @skb: skb currently being received and modified
  * @rx_status: status value of last descriptor in packet
  * @rx_error: error value of last descriptor in packet
+ * @rx_ptype: ptype value of last descriptor in packet
  **/
 static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
                                    struct sk_buff *skb,
                                    u32 rx_status,
-                                   u32 rx_error)
+                                   u32 rx_error,
+                                   u16 rx_ptype)
 {
+       bool ipv4_tunnel, ipv6_tunnel;
+       __wsum rx_udp_csum;
+       __sum16 csum;
+       struct iphdr *iph;
+
+       ipv4_tunnel = (rx_ptype > I40E_RX_PTYPE_GRENAT4_MAC_PAY3) &&
+                     (rx_ptype < I40E_RX_PTYPE_GRENAT4_MACVLAN_IPV6_ICMP_PAY4);
+       ipv6_tunnel = (rx_ptype > I40E_RX_PTYPE_GRENAT6_MAC_PAY3) &&
+                     (rx_ptype < I40E_RX_PTYPE_GRENAT6_MACVLAN_IPV6_ICMP_PAY4);
+
+       skb->encapsulation = ipv4_tunnel || ipv6_tunnel;

skb->encapulation is set (or not) here.

quoted

        skb->ip_summed = CHECKSUM_NONE;

        /* Rx csum enabled and ip headers found? */

@@ -873,13 +886,43 @@ static inline void i40e_rx_checksum(struct i40e_vsi *vsi,
              rx_status & (1 << I40E_RX_DESC_STATUS_L3L4P_SHIFT)))
                return;

-       /* IP or L4 checksum error */
+       /* IP or L4 or outmost IP checksum error */
        if (rx_error & ((1 << I40E_RX_DESC_ERROR_IPE_SHIFT) |
-                       (1 << I40E_RX_DESC_ERROR_L4E_SHIFT))) {
+                       (1 << I40E_RX_DESC_ERROR_L4E_SHIFT) |
+                       (1 << I40E_RX_DESC_ERROR_EIPE_SHIFT))) {
                vsi->back->hw_csum_rx_error++;
                return;
        }

+       if (ipv4_tunnel &&
+           !(rx_status & (1 << I40E_RX_DESC_STATUS_UDP_0_SHIFT))) {
+               /* 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 comment says that the driver has to compute the udp checksum
**if** the packet has an outer udp checksum,
but the driver call to issue checksumming is done unconditionally

This code runs when the right bits are set in the rx hardware descriptor 
field (rx_ptype and rx_status), which implies the hardware has detected 
ipv4 tunneling and there is non zero UDP csum.

quoted

+               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;
+
+               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) {
+                       vsi->back->hw_csum_rx_error++;
+                       return;
+               }
+       }
+
        skb->ip_summed = CHECKSUM_UNNECESSARY;
 }

Commit 0afb1666fe4 "vxlan: Add capability of Rx checksum offload for
inner packet" dictates that
the driver is expected to set the skb->encapsulation bit, this is
missing here, and indeed in vxlan_udp_encap_recv
your setting of ip_summed will be ignored and over-rided with
CHECKSUM_NONE, agree?

It is setting skb->encapsulation when hw detects it. I have pointed it out 
above.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html