On Thu, Apr 19, 2012 at 7:39 PM, Simon Horman <ho...@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 <ho...@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 <ho...@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). _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
