[ovs-dev] tnl-neigh: Use outgoing ofproto version.
diff mbox series

Message ID 20190813163404.19126-1-fbl@sysclose.org
State New
Headers show
Series
  • [ovs-dev] tnl-neigh: Use outgoing ofproto version.
Related show

Commit Message

Sriram Vatala via dev Aug. 13, 2019, 4:34 p.m. UTC
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@sysclose.org>
---
 ofproto/ofproto-dpif-xlate.c |  4 +--
 tests/tunnel.at              | 62 ++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 2 deletions(-)

Comments

Sriram Vatala via dev Aug. 13, 2019, 4:45 p.m. UTC | #1
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.

Hi,

Please backport this up to 2.9 at least.
Thanks,
fbl
Vasu Dasari Aug. 14, 2019, 3:43 p.m. UTC | #2
Looks good Flavio.

Acked-By: Vasu Dasari <vdasari@gmail.com>

Thanks
-Vasu

*Vasu Dasari*


On Tue, Aug 13, 2019 at 12:45 PM Flavio Leitner via dev <
ovs-dev@openvswitch.org> wrote:

> 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.
>
> Hi,
>
> Please backport this up to 2.9 at least.
> Thanks,
> fbl
>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

Patch
diff mbox series

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@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])