On 5/16/24 1:48 PM, Ilya Maximets wrote:
I didn't look at the code changes too closely, but in case you
wan't to re-spin the patch, I added a few comments for the
test cases below.
thanks for taking a look at this patch

diff --git a/tests/ovs-router.at b/tests/ovs-router.at
index b3314b3dff0d..f34dd3c99f5d 100644
--- a/tests/ovs-router.at
+++ b/tests/ovs-router.at
@@ -109,6 +109,39 @@ User: 2001:db8:beef::14/128 MARK 14 dev br0 GW 
2001:db8:cafe::1 SRC 2001:db8:caf
  OVS_VSWITCHD_STOP
  AT_CLEANUP

+AT_SETUP([appctl - route/add with ipv4 via ipv6])
We should probably also add a few cases for the lookup and del,
not just the add.

+AT_KEYWORDS([ovs_router])
+OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=dummy])
+AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 192.168.9.2/24], [0], [OK
+])
+AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 192.168.9.3/24], [0], [OK
+])
+
+# defualt pick the first IP, 192.168.9.2, as src
+AT_CHECK([ovs-appctl ovs/route/add 192.168.9.10/32 br0 
fe80::920a:84ff:beef:9510], [0], [OK
+])
+AT_CHECK([ovs-appctl ovs/route/add 192.168.9.11/32 br0 
fe80::920a:84ff:beef:9511 src=192.168.9.2], [0], [OK
+])
+AT_CHECK([ovs-appctl ovs/route/add 192.168.9.12/32 br0 
fe80::920a:84ff:beef:9512 src=192.168.9.3], [0], [OK
+])
+AT_CHECK([ovs-appctl ovs/route/add 192.168.9.13/32 br0 
fe80::920a:84ff:beef:9513 pkt_mark=13 src=192.168.9.3], [0], [OK
+])
Might be better to wrap some of the longer lines, e.g. after the bridge name.

+
+AT_CHECK([ovs-appctl ovs/route/show | grep User | grep beef | sort], [0], [dnl
+User: 192.168.9.10/32 dev br0 GW fe80::920a:84ff:beef:9510 SRC 192.168.9.2
+User: 192.168.9.11/32 dev br0 GW fe80::920a:84ff:beef:9511 SRC 192.168.9.2
+User: 192.168.9.12/32 dev br0 GW fe80::920a:84ff:beef:9512 SRC 192.168.9.3
+User: 192.168.9.13/32 MARK 13 dev br0 GW fe80::920a:84ff:beef:9513 SRC 
192.168.9.3
+])
+
+# DST and SRC must be in the same subnet domain
+AT_CHECK([ovs-appctl ovs/route/add 192.168.10.12/32 br0 
fe80::920a:84ff:beef:9512 src=192.168.9.3], [2], [], [dnl
+Error while inserting route.
+ovs-appctl: ovs-vswitchd: server returned an error
+])
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
  AT_SETUP([appctl - route/lookup])
  AT_KEYWORDS([ovs_router])
  OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=dummy])
diff --git a/tests/system-route.at b/tests/system-route.at
index c0ecad6cfb49..fd698321b401 100644
--- a/tests/system-route.at
+++ b/tests/system-route.at
@@ -65,6 +65,30 @@ Cached: fc00:db8:beef::13/128 dev br0 GW fc00:db8:cafe::1 
SRC fc00:db8:cafe::2])
  OVS_TRAFFIC_VSWITCHD_STOP
  AT_CLEANUP

+AT_SETUP([ovs-route - add system route ipv4 via ipv6])
+AT_KEYWORDS([route])
+OVS_TRAFFIC_VSWITCHD_START()
+AT_CHECK([ip link set br0 up])
+
+AT_CHECK([ip addr add 192.168.9.2/24 dev br0], [0], [stdout])
+AT_CHECK([ip addr add 192.168.9.3/24 dev br0], [0], [stdout])
+
+AT_CHECK([ip -6 addr add fc00:db8:cafe::2/64 dev br0], [0], [stdout])
+AT_CHECK([ip -6 addr add fc00:db8:cafe::3/64 dev br0], [0], [stdout])
+
+AT_CHECK([ip route add 192.168.9.12/32 dev br0 via inet6 
fe80::920a:84ff:fe9e:9512 src 192.168.9.2], [0], [stdout])
+AT_CHECK([ip route add 192.168.9.13/32 dev br0 via inet6 
fe80::920a:84ff:fe9e:9513 src 192.168.9.3], [0], [stdout])
maybe wrap these lines.

+
+OVS_WAIT_UNTIL_EQUAL([ovs-appctl ovs/route/show | grep -E 
'192.168.9.1[[23]]/32' | sort], [dnl
+Cached: 192.168.9.12/32 dev br0 GW fe80::920a:84ff:fe9e:9512 SRC 192.168.9.2
+Cached: 192.168.9.13/32 dev br0 GW fe80::920a:84ff:fe9e:9513 SRC 192.168.9.3])
+
+AT_CHECK([ovs-appctl ovs/route/add 192.168.9.14/32 br0 
fe80::920a:84ff:fe9e:9514], [0], [OK
+])
Not sure why this add is here as we're not checking the result.

+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
+
  dnl Checks that OVS doesn't use routes from non-standard tables.
  AT_SETUP([ovs-route - route tables])
  AT_KEYWORDS([route])
diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
index 508737c53ec6..0c8f3862cce9 100644
--- a/tests/tunnel-push-pop.at
+++ b/tests/tunnel-push-pop.at
@@ -196,6 +196,54 @@ OVS_WAIT_UNTIL([test `ovs-pcap p0.pcap | grep 
100022eb0000000120000237 | wc -l`
  OVS_VSWITCHD_STOP
  AT_CLEANUP

+AT_SETUP([tunnel_push_pop - v4 via v6 route])
Overall, I'd suggest to format this test in a way some of the
newer tests are written.  See 'recirculation after encapsulation'
or 'local_ip configuration' tests for example.

+
+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 t1 -- set Interface t1 type=vxlan \
+                       options:remote_ip=1.1.2.92 options:key=123 
ofport_request=1\
+                       ], [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)
+    t1 1/4789: (vxlan: key=123, remote_ip=1.1.2.92)
+])
+
+AT_CHECK([ovs-ofctl add-flow br0 action=normal])
+
+dnl Setup dummy interface IP addresses.
+AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 1.1.2.88/24], [0], [OK
Hmm.  I'm a little confused by this.  The whole point of v4 over v6
routing is that the network we're routing the packets over doesn't
have IPv4 routing, i.e. br0 bridge that is containing an external port
should not be part of IPv4 network, i.e. the address should not be
routable.  Otherwise, why do we even need to use v4 over v6, if there
is a native v4?

Or am I missing something?
you're right. I discussed with Derrick and the address should not be routable.
In the reporter's case there was an IPv4 address on br-phy, but it
was a /32 address and there was no actual route between tunnel
endpoints, except via v6 gateway.

Best regards, Ilya Maixmets.

+])
+AT_CHECK([ovs-appctl netdev-dummy/ip6addr br0 2001:cafe::88/64], [0], [OK
+])
+dnl Add a static v4 via v6 route
+AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/32 br0 2001:cafe::10], [0], [OK
+])
+
+AT_CHECK([ovs-appctl ovs/route/show | grep br0 | sort], [0], [dnl
+Cached: 1.1.2.0/24 dev br0 SRC 1.1.2.88 local
+Cached: 2001:cafe::/64 dev br0 SRC 2001:cafe::88 local
+User: 1.1.2.92/32 dev br0 GW 2001:cafe::10 SRC 1.1.2.88
+])
+
+AT_CHECK([ovs-appctl tnl/neigh/set br0 1.1.2.92 f8:bc:12:44:34:c8], [0], [OK
+])
Beside the other comments, we should not set neighbor cache manually
here.  We should generate Neighbor Advertisement packet from p0 port
so the bridge learns where the 2001:cafe::10 is.  Tunnel should be
able to work with only this information.
OK!
+
+dnl Check VXLAN tunnel push
+AT_CHECK([ovs-ofctl add-flow int-br action=1])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 
'in_port(2),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'],
 [0], [stdout])
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions: 
tnl_push(tnl_port(4789),header(size=50,type=4,eth(dst=f8:bc:12:44:34:c8,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=17,tos=0,ttl=64,frag=0x4000),udp(src=0,dst=4789,csum=0x0),vxlan(flags=0x8000000,vni=0x7b)),out_port(100)),1
+])
We should preserve more information on the trace here including the
routing decisions and addresses chosen.  See the previously mentioned
tests for example.
Got it, thanks
William

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to