Re: [PATCH] net-gre-gro: Fix a bug that breaks the forwarding path

[Joseph Gasparakis <joseph.gasparakis@intel.com>]

On Tue, 4 Mar 2014, 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"
> 

Agree. This should be valid for both Rx and Tx. 

> 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 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.
Correct. Here is a little bit more explanation about the though behind 
these statements:

When the packet gets decapsulated skb->encapsulation should be reset to 0 as
all that is left is the (previously to decap) inner packet. For the same reason
the inner headers also will not be valid any more: there are no inner headers as such.
Personaly in Rx I assume that when the skb leaves the driver, and the 
hardware has detected encapsulation and hence the csums have been verified 
(or not), the skb->encapsulation is on and skb->ip_summed is set accordingly for both 
layers, but the inner headers are not set and even if they are they are not valid.

Also for Tx, skb->encapsulation should be the indication to the 
driver that it can use the inner headers (i.e. they are valid) in the skb 
in order to offload the inner csum.

> 
> 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.
> 
> Joseph, what's your thinking here?

Yes, I agree. If the hardware cannot offload the inner checksum most 
probably it couldn't even detect the encapsulation.

> 
> Or.
> 
> 
>