public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Joseph Gasparakis <joseph.gasparakis@intel•com>
To: Or Gerlitz <or.gerlitz@gmail•com>
Cc: "Kirsher, Jeffrey T" <jeffrey.t.kirsher@intel•com>,
	"Gasparakis, Joseph" <joseph.gasparakis@intel•com>,
	David Miller <davem@davemloft•net>,
	"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
	"gospo@redhat•com" <gospo@redhat•com>,
	"sassmann@redhat•com" <sassmann@redhat•com>,
	"Brandeburg, Jesse" <jesse.brandeburg@intel•com>
Subject: Re: [net-next v4 07/16] i40e: Rx checksum offload for VXLAN
Date: Thu, 2 Jan 2014 12:57:56 -0800 (PST)	[thread overview]
Message-ID: <alpine.LFD.2.03.1401021252500.4979@intel.com> (raw)
In-Reply-To: <CAJZOPZ+XpHNcMdOKYpALvUXq5-yYuxcwAqG2AEe4DU=d=VGPtg@mail.gmail.com>



On Thu, 2 Jan 2014, Or Gerlitz wrote:

> On Sat, Dec 28, 2013 at 5:22 AM, Jeff Kirsher
> <jeffrey.t.kirsher@intel•com> wrote:
> > From: Joseph Gasparakis <joseph.gasparakis@intel•com>
> >
> > 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 <joseph.gasparakis@intel•com>
> > Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel•com>
> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel•com>
> > ---
> >  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.

> >         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.

> > +               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
> 

  reply	other threads:[~2014-01-02 20:41 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-28  3:22 [net-next v4 00/16][pull request] Intel Wired LAN Driver Updates Jeff Kirsher
2013-12-28  3:22 ` [net-next v4 01/16] i40e: using for_each_set_bit to simplify the code Jeff Kirsher
2013-12-28  3:22 ` [net-next v4 02/16] i40e: Suppress HMC error to Interrupt message level Jeff Kirsher
2013-12-28  6:11   ` David Miller
2013-12-28  3:22 ` [net-next v4 03/16] i40e: Populate and check pci bus speed and width Jeff Kirsher
2013-12-28  6:10   ` David Miller
2013-12-28  3:22 ` [net-next v4 04/16] i40e: add wake-on-lan support Jeff Kirsher
2013-12-28  3:22 ` [net-next v4 05/16] i40e: fix curly brace use and return type Jeff Kirsher
2013-12-28  3:22 ` [net-next v4 06/16] i40e: Implementation of VXLAN ndo's Jeff Kirsher
2013-12-28  6:17   ` David Miller
2013-12-28  3:22 ` [net-next v4 07/16] i40e: Rx checksum offload for VXLAN Jeff Kirsher
2014-01-02 10:20   ` Or Gerlitz
2014-01-02 20:57     ` Joseph Gasparakis [this message]
2014-01-02 21:31       ` Or Gerlitz
2014-01-03 19:07         ` Joseph Gasparakis
2013-12-28  3:22 ` [net-next v4 08/16] i40e: move i40e_reset_vf Jeff Kirsher
2013-12-28  3:22 ` [net-next v4 09/16] i40e: refactor VF reset flow Jeff Kirsher
2013-12-28  3:22 ` [net-next v4 10/16] i40e: remove redundant code Jeff Kirsher
2013-12-28  3:22 ` [net-next v4 11/16] i40e: remove chatty log messages Jeff Kirsher
2013-12-28  3:22 ` [net-next v4 12/16] i40e: fix error return Jeff Kirsher
2013-12-28  3:22 ` [net-next v4 13/16] i40e: be more informative Jeff Kirsher
2013-12-28  3:22 ` [net-next v4 14/16] i40e: make a define from a large constant Jeff Kirsher
2013-12-28  3:22 ` [net-next v4 15/16] i40e: update led set args Jeff Kirsher
2013-12-28  3:22 ` [net-next v4 16/16] i40e: report VF MAC addresses correctly Jeff Kirsher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.2.03.1401021252500.4979@intel.com \
    --to=joseph.gasparakis@intel$(echo .)com \
    --cc=davem@davemloft$(echo .)net \
    --cc=gospo@redhat$(echo .)com \
    --cc=jeffrey.t.kirsher@intel$(echo .)com \
    --cc=jesse.brandeburg@intel$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=or.gerlitz@gmail$(echo .)com \
    --cc=sassmann@redhat$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox