[ovs-dev] [PATCH] tnl-neigh: Use outgoing ofproto version.

Flavio Leitner fbl at sysclose.org
Wed Aug 28 14:19:35 UTC 2019


Hi Ben and Ilya,

This patch has two reviews, so any chance for you to take a look
soon as well?

Thanks in advance!
fbl

On Tue, Aug 13, 2019 at 01:34:04PM -0300, Flavio Leitner via dev wrote:
> When a packet needs to be encapsulated in userspace, the endpoint
> address needs to be resolved to fill in the headers. If it is not,
> then currently OvS sends either a Neighbor Solicitation (IPv6)
> or an ARP Query (IPv4) to resolve it.
> 
> The problem is that the NS/ARP packet will go through the flow
> rules in the new bridge, but inheriting the ofproto table version
> from the original packet to be encapsulated. When those versions
> don't match, the result is unexpected because no flow rules might
> be visible, which would cause the default table rule to be used
> to drop the packet. Or only part of the flow rules would be visible
> and so on.
> 
> Since the NS/ARP packet is created by OvS and will be injected in
> the outgoing bridge, use the corresponding ofproto version instead.
> 
> Signed-off-by: Flavio Leitner <fbl at sysclose.org>
> ---
>  ofproto/ofproto-dpif-xlate.c |  4 +--
>  tests/tunnel.at              | 62 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 64 insertions(+), 2 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 28a7fdd84..5a8a46370 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3414,6 +3414,7 @@ compose_table_xlate(struct xlate_ctx *ctx, const struct xport *out_dev,
>                      struct dp_packet *packet)
>  {
>      struct xbridge *xbridge = out_dev->xbridge;
> +    ovs_version_t version = ofproto_dpif_get_tables_version(xbridge->ofproto);
>      struct ofpact_output output;
>      struct flow flow;
>  
> @@ -3423,8 +3424,7 @@ compose_table_xlate(struct xlate_ctx *ctx, const struct xport *out_dev,
>      output.port = OFPP_TABLE;
>      output.max_len = 0;
>  
> -    return ofproto_dpif_execute_actions__(xbridge->ofproto,
> -                                          ctx->xin->tables_version, &flow,
> +    return ofproto_dpif_execute_actions__(xbridge->ofproto, version, &flow,
>                                            NULL, &output.ofpact, sizeof output,
>                                            ctx->depth, ctx->resubmits, packet);
>  }
> diff --git a/tests/tunnel.at b/tests/tunnel.at
> index fc6f87936..faffb4149 100644
> --- a/tests/tunnel.at
> +++ b/tests/tunnel.at
> @@ -394,6 +394,68 @@ AT_CHECK([ovs-appctl dpif/show | tail -n +3], [0], [dnl
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
>  
> +AT_SETUP([tunnel - table version])
> +dnl check if changes in the egress bridge flow table affects
> +dnl discovering the link layer address of tunnel endpoints.
> +OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=dummy ofport_request=1 other-config:hwaddr=aa:55:aa:55:00:00])
> +AT_CHECK([ovs-vsctl add-br int-br -- set bridge int-br datapath_type=dummy], [0])
> +AT_CHECK([ovs-vsctl add-port int-br v1 -- set Interface v1 type=vxlan \
> +                       options:remote_ip=172.31.1.2 options:key=123 \
> +                       ofport_request=2 \
> +                 -- add-port int-br v2 -- set Interface v2 type=internal \
> +                       ofport_request=3 \
> +                       ], [0])
> +
> +AT_CHECK([ovs-appctl dpif/show], [0], [dnl
> +dummy at ovs-dummy: hit:0 missed:0
> +  br0:
> +    br0 65534/100: (dummy-internal)
> +    p0 1/1: (dummy)
> +  int-br:
> +    int-br 65534/2: (dummy-internal)
> +    v1 2/4789: (vxlan: key=123, remote_ip=172.31.1.2)
> +    v2 3/3: (dummy-internal)
> +])
> +
> +dnl First setup dummy interface IP address, then add the route
> +dnl so that tnl-port table can get valid IP address for the device.
> +AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 172.31.1.1/24], [0], [OK
> +])
> +AT_CHECK([ovs-appctl ovs/route/add 172.31.1.0/24 br0], [0], [OK
> +])
> +
> +dnl change the flow table to bump the internal table version
> +AT_CHECK([ovs-ofctl add-flow int-br action=normal])
> +AT_CHECK([ovs-ofctl add-flow br0 action=normal])
> +AT_CHECK([ovs-ofctl del-flows br0])
> +AT_CHECK([ovs-ofctl add-flow br0 action=normal])
> +AT_CHECK([ovs-ofctl del-flows br0])
> +AT_CHECK([ovs-ofctl add-flow br0 action=normal])
> +AT_CHECK([ovs-ofctl del-flows br0])
> +AT_CHECK([ovs-ofctl add-flow br0 action=normal])
> +AT_CHECK([ovs-ofctl del-flows br0])
> +AT_CHECK([ovs-ofctl add-flow br0 action=normal])
> +AT_CHECK([ovs-ofctl del-flows br0])
> +AT_CHECK([ovs-ofctl add-flow br0 action=normal])
> +AT_CHECK([ovs-ofctl del-flows br0])
> +AT_CHECK([ovs-ofctl add-flow br0 action=normal])
> +AT_CHECK([ovs-ofctl del-flows br0])
> +AT_CHECK([ovs-ofctl add-flow br0 action=normal])
> +
> +dnl Check Neighbour discovery.
> +AT_CHECK([ovs-vsctl -- set Interface p0 options:pcap=p0.pcap])
> +
> +AT_CHECK([ovs-appctl netdev-dummy/receive int-br 'in_port(2),eth(src=aa:55:aa:55:00:00,dst=f8:bc:12:ff:ff:ff),eth_type(0x0800),ipv4(src=1.1.3.92,dst=1.1.3.88,proto=1,tos=0,ttl=64,frag=no),icmp(type=0,code=0)'])
> +AT_CHECK([ovs-pcap p0.pcap > p0.pcap.txt 2>&1])
> +
> +dnl When the wrong version is used, the flow is not visible and the
> +dnl packet is dropped.
> +AT_CHECK([cat p0.pcap.txt | grep ffffffffffffaa55aa55000008060001080006040001aa55aa550000ac1f0101000000000000ac1f0102 | uniq], [0], [dnl
> +ffffffffffffaa55aa55000008060001080006040001aa55aa550000ac1f0101000000000000ac1f0102
> +])
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
>  AT_SETUP([tunnel - LISP])
>  OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=lisp \
>                      options:remote_ip=1.1.1.1 ofport_request=1])
> -- 
> 2.20.1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list