On 5 May 2017 at 10:54, Zoltán Balogh <[email protected]> wrote:
> Hi,
>
>> Thanks for taking a look. Andy and I have been throwing some thoughts
>> around about this, but I'm not sure we came to a concrete solution yet
>> either. My main thought is that I think that the 'flow' needs to be
>> modified in the similar way that the 'push_tnl' would work in the
>> datapath - updating the IP/UDP fields in a protocol-specific manner.
>> Then the shared code between patch ports and this tunneling
>> translation needs close attention to check everything is lined up
>> correctly, restored after output translation, etc.
>
> I have a patch that fixes tunneling over patch ports. The 14th
> system-userspace
> test still does fail, but now the packet size in the dump flow remains 242.
>
> ./system-traffic.at:554: ovs-ofctl dump-flows br0 | grep "in_port=2" | sed -n
> 's/.*\(n\_bytes=[0-9]*\).*/\1/p'
> ./system-traffic.at:558: ovs-ofctl dump-flows br-underlay | grep
> "in_port=LOCAL" | sed -n 's/.*\(n\_bytes=[0-9]*\).*/\1/p'
> --- - 2017-05-05 19:52:28.987111424 +0200
> +++
> /home/ezolbal/work/general_L3_tunneling/ovs/tests/system-userspace-testsuite.dir/at-groups/14/stdout
> 2017-05-05 19:52:28.983508027 +0200
> @@ -1,2 +1,2 @@
> -n_bytes=138
> +n_bytes=242
>
> I'm little bit lost with flow statistics. Could you have a look at the patch
> below, and help me to find out if it's only a statistics bug, please?
Ah, so the more interesting part of that test is that it also
truncates the packet during output to the tunnel. So the tunnel should
only see a 100B packet (encapped within GRE, so 138B). Commit
aaca4fe0ce9e ("ofp-actions: Add truncate action.") was where this
functionality was originally introduced, perhaps you can look at that
to determine how the truncation should be applied in this case?
<snip>
> @@ -3156,6 +3183,9 @@ build_tunnel_send(struct xlate_ctx *ctx, const struct
> xport *xport,
> int err;
> char buf_sip6[INET6_ADDRSTRLEN];
> char buf_dip6[INET6_ADDRSTRLEN];
> + /* Variables to backup Ethernet and IP data of flow and base_flow. */
> + struct flow old_flow = ctx->xin->flow;
> + struct flow old_base_flow = ctx->base_flow;
I think that the call to apply_nested_clone_actions() ends up doing
some flow/base_flow store/restore so please just check we don't double
this up unnecessarily.
By the way, it'd be nice to have a separate patch just to rename
apply_nested_clone_actions() since it has less to do with clone
actions and more to do with transiting a patch port.
Thanks,
Joe
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
