[ovs-dev] [PATCH 2/3] [RFC] datapath: tunnelling: Pass l4_offset to update_header callback

[Simon Horman]

On Mon, Apr 09, 2012 at 02:39:55PM +0900, Simon Horman wrote:
> On Fri, Apr 06, 2012 at 10:44:18AM -0700, Jesse Gross wrote:
> > On Thu, Apr 5, 2012 at 7:10 PM, Simon Horman <horms at verge.net.au> wrote:
> > > On Thu, Apr 05, 2012 at 05:34:21PM -0700, Jesse Gross wrote:
> > >> On Tue, Apr 3, 2012 at 10:14 PM, Simon Horman <horms at verge.net.au> wrote:
> > >> > The STT protocol's header includes a field for the offset
> > >> > to the start of the l4 header. It seems that this
> > >> > is the value of the transport_offset of the original SKB and
> > >> > making that value available to the update_header callback avoids
> > >> > STT needing to calculate the offset.
> > >> >
> > >> > Signed-off-by: Simon Horman <horms at verge.net.au>
> > >>
> > >> Isn't it possible to quickly compute the L4 offset if you know the
> > >> size of the header (which in STT is fixed)?  I'm just trying to
> > >> minimize the amount of protocol-specific code in the generic tunneling
> > >> code as we add more protocols.
> > >
> > > To be honest I'm not sure that I really understand the motivation for the
> > > l4_offset in STT. But it seems to me the difficulty in calculating
> > > l4_offset lies in calculating the size of the IP header in the inner
> > > packet, especially in the case of IPv6. I'm quite happy to accept that I
> > > have missed the point somehow.
> > 
> > It is to be able to efficiently skip over the IP header on the receive
> > side.  However, on the transmit side we already have computed this
> > information.  What I'm suggesting is essentially the same as what you
> > have done here, just a slightly different implementation.  I think the
> > L4 offset can be computed with the information that we already have by
> > doing:
> > 
> > csum_start - (skb_headroom(skb) + skb_network_offset(skb) +
> > mutable->tunnel_hlen)
> 
> I'm assuming this is skb->csum_start, however, it seems to be 0
> when entering stt_update_head() when testing using an emulated
> interface with OVS running inside a KVM guest.
> 
> I have also tested using a virtio interface, in which case
> the calculation above gives the correct result on occasions where
> csum_start is non-zero.
> 
> I have also noticed that sometimes my current implementation
> yields a negative value, clearly incorrect. I will look into that.

This turned out to be ARP (skb->proto = 0x0806). I guess
that l4_offset should be set to 0 in the stt header if skb->proto is
neither ETH_P_IP nor ETH_P_IPV6.