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

[Simon Horman]

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.

> The best way to think about the fields in the STT header is as an
> encoding of information in the skb metadata or the transmit
> descriptors passed to the NIC.

Thanks.