[ovs-dev] [PATCH v5] tunneling: Avoid recirculation on datapath by computing the recirculate actions at translate time.

[Joe Stringer]

On 11 April 2017 at 03:14, Sugesh Chandran <sugesh.chandran at intel.com> wrote:
> Openvswitch datapath recirculates packets for tunneling, i.e.
> the incoming packets are encapsulated at first pass. Further actions are
> applied on encapsulated packets on the second pass after recirculating.
> The proposed patch compute and append the post tunnel actions at the time of
> translation itself instead of recirculating at datapath. These actions are solely
> depends on tunnel attributes so there is no need of datapath recirculation.
> By avoiding the recirculation at datapath, the patch offers upto 30%
> performance improvement for VxLAN tunneling in our testing.
> The action execution logic is using the new CLONE action to define
> the packet cloning when the actions are combined. The lenght in the CLONE
> action specifies the size of nested action set.
>
> It also fixing the test suites failures that are introduced by nested CLONE
> action in tunneling.
>
> v5
> - Fix the OVN test case failure by commenting the test validation as its not
>   relevant with the new tunnel CLONE action.
> - Code changes for applying CLONE action on a batch than individual packets
>   are already pushed to the master. V5 patch is now only doing CLONE at tunnel
>   push.
> v4
> - Rename the function to compute post tunnel nested function.
> - Use the clone action syntax itself for the flow display.
> - Use nl_msg functions for handling the nested attribute.
> - Modify the CLONE action to process packets in batch than individually.
> v3
> - Rebase with newely clone action and use it for tunneling.
> v2
> - Use only single CLONE action with length to mark the tunnel combine action set.
> - Update the datapath trace display functions to handle CLONE.
> - Fixed test cases to work with CLONE action.
>
> Signed-off-by: Sugesh Chandran <sugesh.chandran at intel.com>
> Signed-off-by: Zoltán Balogh <zoltan.balogh at ericsson.com>
> Co-authored-by: Zoltán Balogh <zoltan.balogh at ericsson.com>

Hi Sugesh, Zoltán,

Since this patch, the "make check-system-userspace" test for "datapath
- truncate and output to gre tunnel" has been failing. Specifically,
the test expects that 138B of truncated packet transits through the
LOCAL port of the br-underlay bridge and hits a flow there; however,
the flow has no stats attributed, presumably because the match is
wrong. I suspect that the problem exists for any test that makes
modifications to the packet after tunnel encapsulation, ie if the
underlay bridge does more than basic L2.

I think that what's happening is that when build_tunnel_send()
serializes the ODP action for push_tunnel, it doesn't update the
'flow' to reflect the new encapsulated state of the packet. Then, when
calling apply_nested_clone_actions() it performs lookup in the second
bridge using the unmodified flow, ie as though it hasn't been
encapsulated at all. If, for instance, on the underlay bridge you
match on the tunnel protocol type or port and attempt to forward such
packets directly to the external interface then have a default drop
for packets that don't match, you'll see the traffic get dropped. The
resulting datapath flows can end up generating a set of actions with
"clone(drop),push_tnl(...)", which also seems wrong.

William and I have been looking at this a bit, but it'd be good if you
had a chance to look too. I think that William is working on a simpler
test case to reproduce. For reference if you are not familiar with
"make check-system-userspace" tests, there is some documentation
available below---the system-traffic.at tests are the same for 'make
check-kernel' and 'make check-system-userspace'.

http://docs.openvswitch.org/en/latest/topics/testing/#datapath-testing

Cheers,
Joe