[ovs-dev] [PATCH 2/2] [RFC] tunnelling: stt: Prototype Implementation

[Simon Horman]

On Fri, Apr 20, 2012 at 01:11:39PM -0700, Jesse Gross wrote:
> On Thu, Apr 19, 2012 at 7:39 PM, Simon Horman <horms at verge.net.au> wrote:
> > On Thu, Apr 19, 2012 at 03:53:35PM -0700, Jesse Gross wrote:
> >> On Wed, Apr 18, 2012 at 9:50 PM, Simon Horman <horms at verge.net.au> wrote:
> >> > This is a not yet well exercised implementation of STT intended for review,
> >> > I am sure there are numerous areas that need improvement.
> >> >
> >> > In particular:
> >> > - The transmit path's generation of partial checksums needs to be tested
> >> > - The VLAN stripping code needs to be excercised
> >> > - The code needs to be exercised in the presence of HW checksumming
> >> > - In general, the code has been exercised by running Open vSwtich in
> >> >  KVM guests on the same host. Testing between physucal hosts is needed.
> >> >
> >> > This implementation is based on the CAPWAP implementation and in particular
> >> > includes defragmentation code almost identical to CAPWAP. It seems to me
> >> > that while fragmentation can be handled by GSO/TSO, defragmentation code is
> >> > needed in STT in the case where LRO/GRO doesn't reassemble an entire STT
> >> > frame for some reason.
> >> >
> >> > If the defragmentation code, which is of non-trivial length, remains more
> >> > or less in its present state then there is some scope for consolidation
> >> > with CAPWAP. Other code that may possibly be consolidated with CAPWAP has
> >> > been marked accordingly.
> >> >
> >> > This code depends on a encap_rcv hook being added to the Linux Kernel's TCP
> >> > stack. A patch to add such a hook will be posted separately. Ultimately
> >> > this change or some alternative will need to be applied to the mainline
> >> > Linux kernel's TCP stack if STT is to be widely deployed. Motivating this
> >> > change to the TCP stack is part of the purpose of this prototype STT
> >> > implementation.
> >> >
> >> > The configuration of STT is analogous to that of other tunneling
> >> > protocols such as GRE which are supported by Open vSwtich.
> >> >
> >> > e.g.
> >> >
> >> > ovs-vsctl add bridge project0 ports @newport \
> >> >        -- --id=@newport create port name=stt0 interfaces=[@newinterface] \
> >> >        -- --id=@newinterface create interface name=stt0 type=stt options="remote_ip=10.0.99.192,key=64"
> >> >
> >> > Signed-off-by: Simon Horman <horms at verge.net.au>
> >> >
> >> > ---
> >> >
> >> > v3
> >> > * Correct stripping of vlan tag on transmit
> >> > * Correct setting of vlan TCI on recieve
> >> >  - Use __vlan_hwaccel_put_tag instead of vlan_put_tag
> >> > * Use encap_rcv_enable() to enable receiving packets from the TCP stack
> >> >  - This is an update for the new implementation of the TCP stack
> >> >    patch that adds encap_rcv
> >> > * call pskb_may_pull() for STT_FRAME_HLEN + ETH_HLEN bytes in
> >> >  process_stt_proto() as this is required by ovs_flow_extract()
> >> > * Include "stt: " in pr_fmt
> >> > * Make use of pr_* instead of printk
> >> > * Rate limit all packet-generated pr_* messages
> >> > * STT flags are 8bits wide so don't define them using __cpu_to_be16()
> >> > * Only include l4_offset if
> >> >  1. get_ip_summed(skb) is OVS_CSUM_PARTIAL
> >> >  2. skb->csum_start is non-zero
> >> >  3. it is between 0 and 255
> >> >  - Warn if the first two conditions are met but not the third one.
> >> > * Only set STT_FLAG_CHECKSUM_VERIFIED if
> >> >  get_ip_summed(skb) is * OVS_CSUM_UNNECESSARY
> >> > * Print a debug message if get_ip_summed(skb) is OVS_CSUM_UNNECESSARY,
> >> >  this case is yet to be exercised
> >> > * In the rx path, adjust skb->csum_start to take into account pulling
> >> >  STT_FRAME_HLEN if get_ip_summed(skb) is OVS_CSUM_PARTIAL
> >>
> >> Hi Simon,
> >>
> >> It looks like most of things that I mentioned in comments on the
> >> previous version are still present here.  Is this just an intermediate
> >> version?
> >
> > Sorry, I hadn't noticed this email until now.
> >
> > In general my aim was to make a functionally correct implementation and
> > then concentrate on acceleration. As it happens the implementation is not
> > yet correct and I have ended up implementing some acceleration. So things
> > are somewhat work in progress.
> >
> > My intention in marking this as [RFC] was to indicate that I don't think
> > it is ready for merging yet. Ideally I would like STT considered for
> > merging once it is correct, even if there is still (ample) scope for
> > performance improvements.
> 
> I agree that it makes sense to focus on correctness first although in
> the context of STT offloading is really part of the core protocol.  I
> could see not doing the atomic ops stuff at first but offloading
> really needs to be handled (if only for the reason that the other side
> expects to be able to send offloaded packets).

Sure, I think that is reasonable. Offloading is at the heart of STT.