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

[Jesse Gross]

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?

> * Warn if skb->dev is NULL on defragmentation and stop processing the skb.
>  - This fixes a crash bug
>  - But how can this occur?

The encap_rcv hook that you added to the TCP stack comes after the
skb->dev = NULL line.  This is why I wanted to push it is as close as
possible to the socket lookup.