From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerry Chu Subject: Re: [PATCH] net-gre-gro: Fix a bug that breaks the forwarding path Date: Tue, 4 Mar 2014 14:13:20 -0800 Message-ID: References: <1393536377-32243-1-git-send-email-hkchu@google.com> <20140228.165614.1112789771887381245.davem@davemloft.net> <5315FBAA.3030809@mellanox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: David Miller , Joseph Gasparakis , Pravin B Shelar , Eric Dumazet , "netdev@vger.kernel.org" , Tom Herbert To: Or Gerlitz Return-path: Received: from mail-yh0-f50.google.com ([209.85.213.50]:56763 "EHLO mail-yh0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755138AbaCDWNV (ORCPT ); Tue, 4 Mar 2014 17:13:21 -0500 Received: by mail-yh0-f50.google.com with SMTP id t59so190584yho.9 for ; Tue, 04 Mar 2014 14:13:20 -0800 (PST) In-Reply-To: <5315FBAA.3030809@mellanox.com> Sender: netdev-owner@vger.kernel.org List-ID: Hi Or, Thanks for writing this up. On Tue, Mar 4, 2014 at 8:13 AM, Or Gerlitz wrote: > On 28/02/2014 23:56, David Miller wrote: >> >> The topic of the skb->encapsulation semantics has come up several times in >> the past few weeks. We cannot move forward on any changes in this area until >> the semantics are well defined, and documented. Can someone work on a patch >> which documents skb->encapsulation properly, and then we can come back to >> fixing this bug? Thanks. > > > Lets try... the skb->encapsulation flag was introduced and used in 3.8 by > the > sequence of these three commits > > 0afb166 vxlan: Add capability of Rx checksum offload for inner packet > d6727fe vxlan: capture inner headers during encapsulation > 6a674e9 net: Add support for hardware-offloaded encapsulation > > When discussed earlier on the list in the context of the skb->ip_summed > field, > Tom Herbert came with the following interpretation for the semantics which > Joseph confirmed > > "when skb->encapsulation is set the ip_summed is valid for both the inner > and outer header > (e.g. CHECKSUM_COMPLETE is always assumed okay for both layers). If > skb->encapsulation is not set then ip_summed is only valid for outer header" For GRE encapped pkts is the following interpretation correct? 1) ip_summed == CHECKSUM_COMPLETE => csum covers IP payload csum of the outer IP datagram 2) ip_summed == CHECKSUM_UNNECESSARY 2.1. skb->encapsulation is on => both GRE csum (if one is present) and TCP/UDP csum have been validated (assuming inner is a TCP or UDP pkt) 2.2. skb->encapsulation is off => only GRE csum (if one is present) is validated. > > As for the TX side of things, the change-log of commit 6a674e9 states > > "For Tx encapsulation offload, the driver will need to set the right bits in > netdev->hw_enc_features. The protocol driver will have to set the > skb->encapsulation bit and populate the inner headers, so the NIC driver > will use those inner headers to calculate the csum in hardware." So we only support/care about csum offload for the inner pkts, which makes sense. > > So in higher level, it seems that the role of the skb->encapsulation field > is to mark the skb to carry encapsulated packet for the code path between > the time the packet is encapsulated by the protocol driver (e.g vxlan/ipip) > to the time driver xmit is called. Or from the time driver rx code runs till > the the time the packet is decapsulated. > > Further, my personal interpretation was that on the rx path, skb should > carry the encapsulation flag **only** if the HW was able to offload the > inner checksum. SGTM. Jerry > > Joseph, what's your thinking here? > > Or. > >