[ovs-dev] [PATCH ovn] northd: do not configure ECMP routes with wrong next-hop

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Mon Aug 2 10:59:08 UTC 2021


Check if the nexthop is reachable using router interfaces configuring
ECMP routes. DDlog northd implementation is already checking the
condition above.

https://bugzilla.redhat.com/show_bug.cgi?id=1978796

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
---
 northd/ovn-northd.c | 28 +++++++++++++---
 tests/ovn-northd.at | 79 +++++++++++++++++++++++++++++++++++++--------
 2 files changed, 89 insertions(+), 18 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index a0eaa1247..4c164a744 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -8337,10 +8337,16 @@ route_hash(struct parsed_route *route)
 
 static struct ovs_mutex bfd_lock = OVS_MUTEX_INITIALIZER;
 
+static bool
+find_static_route_outport(struct ovn_datapath *od, struct hmap *ports,
+    const struct nbrec_logical_router_static_route *route, bool is_ipv4,
+    const char **p_lrp_addr_s, struct ovn_port **p_out_port);
+
 /* Parse and validate the route. Return the parsed route if successful.
  * Otherwise return NULL. */
 static struct parsed_route *
-parsed_routes_add(struct ovs_list *routes,
+parsed_routes_add(struct ovn_datapath *od, struct hmap *ports,
+                  struct ovs_list *routes,
                   const struct nbrec_logical_router_static_route *route,
                   struct hmap *bfd_connections)
 {
@@ -8388,6 +8394,14 @@ parsed_routes_add(struct ovs_list *routes,
         }
     }
 
+    /* Verify that ip_prefix and nexthop are on the same network. */
+    if (!is_discard_route &&
+        !find_static_route_outport(od, ports, route,
+                                   IN6_IS_ADDR_V4MAPPED(&prefix),
+                                   NULL, NULL)) {
+        return NULL;
+    }
+
     const struct nbrec_bfd *nb_bt = route->bfd;
     if (nb_bt && !strcmp(nb_bt->dst_ip, route->nexthop)) {
         struct bfd_entry *bfd_e;
@@ -8662,8 +8676,12 @@ find_static_route_outport(struct ovn_datapath *od, struct hmap *ports,
                      route->ip_prefix, route->nexthop);
         return false;
     }
-    *p_out_port = out_port;
-    *p_lrp_addr_s = lrp_addr_s;
+    if (p_out_port) {
+        *p_out_port = out_port;
+    }
+    if (p_lrp_addr_s) {
+        *p_lrp_addr_s = lrp_addr_s;
+    }
 
     return true;
 }
@@ -10367,8 +10385,8 @@ build_static_route_flows_for_lrouter(
         struct ecmp_groups_node *group;
         for (int i = 0; i < od->nbr->n_static_routes; i++) {
             struct parsed_route *route =
-                parsed_routes_add(&parsed_routes, od->nbr->static_routes[i],
-                                  bfd_connections);
+                parsed_routes_add(od, ports, &parsed_routes,
+                                  od->nbr->static_routes[i], bfd_connections);
             if (!route) {
                 continue;
             }
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 474e88021..d4d3c9b65 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3015,16 +3015,16 @@ for i in $(seq 1 5); do
     check ovn-nbctl --wait=sb lsp-set-addresses sw$i-r0 00:00:00:00:00:0$i
 done
 
-uuid=$(ovn-nbctl create bfd logical_port=r0-sw1 dst_ip=192.168.10.2 status=down min_tx=250 min_rx=250 detect_mult=10)
-ovn-nbctl create bfd logical_port=r0-sw2 dst_ip=192.168.20.2 status=down min_tx=500 min_rx=500 detect_mult=20
-ovn-nbctl create bfd logical_port=r0-sw3 dst_ip=192.168.30.2 status=down
-ovn-nbctl create bfd logical_port=r0-sw4 dst_ip=192.168.40.2 status=down min_tx=0 detect_mult=0
+uuid=$(ovn-nbctl create bfd logical_port=r0-sw1 dst_ip=192.168.1.2 status=down min_tx=250 min_rx=250 detect_mult=10)
+ovn-nbctl create bfd logical_port=r0-sw2 dst_ip=192.168.2.2 status=down min_tx=500 min_rx=500 detect_mult=20
+ovn-nbctl create bfd logical_port=r0-sw3 dst_ip=192.168.3.2 status=down
+ovn-nbctl create bfd logical_port=r0-sw4 dst_ip=192.168.4.2 status=down min_tx=0 detect_mult=0
 
-wait_row_count bfd 1 logical_port=r0-sw1 detect_mult=10 dst_ip=192.168.10.2 \
+wait_row_count bfd 1 logical_port=r0-sw1 detect_mult=10 dst_ip=192.168.1.2 \
                      min_rx=250 min_tx=250 status=admin_down
-wait_row_count bfd 1 logical_port=r0-sw2 detect_mult=20 dst_ip=192.168.20.2 \
+wait_row_count bfd 1 logical_port=r0-sw2 detect_mult=20 dst_ip=192.168.2.2 \
                      min_rx=500 min_tx=500 status=admin_down
-wait_row_count bfd 1 logical_port=r0-sw3 detect_mult=5 dst_ip=192.168.30.2 \
+wait_row_count bfd 1 logical_port=r0-sw3 detect_mult=5 dst_ip=192.168.3.2 \
                      min_rx=1000 min_tx=1000 status=admin_down
 
 uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw1)
@@ -3035,17 +3035,17 @@ check ovn-nbctl clear bfd $uuid_2 min_rx
 wait_row_count bfd 1 logical_port=r0-sw2 min_rx=1000
 wait_row_count bfd 1 logical_port=r0-sw1 min_rx=1000 min_tx=1000 detect_mult=100
 
-check ovn-nbctl --bfd=$uuid lr-route-add r0 100.0.0.0/8 192.168.10.2
+check ovn-nbctl --bfd=$uuid lr-route-add r0 100.0.0.0/8 192.168.1.2
 wait_column down bfd status logical_port=r0-sw1
-AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.10.2 | grep -q bfd],[0])
+AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.1.2 | grep -q bfd],[0])
 
-check ovn-nbctl --bfd lr-route-add r0 200.0.0.0/8 192.168.20.2
+check ovn-nbctl --bfd lr-route-add r0 200.0.0.0/8 192.168.2.2
 wait_column down bfd status logical_port=r0-sw2
-AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.20.2 | grep -q bfd],[0])
+AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.2.2 | grep -q bfd],[0])
 
-check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8 192.168.50.2 r0-sw5
+check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8 192.168.5.2 r0-sw5
 wait_column down bfd status logical_port=r0-sw5
-AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.50.2 | grep -q bfd],[0])
+AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.5.2 | grep -q bfd],[0])
 
 route_uuid=$(fetch_column nb:logical_router_static_route _uuid ip_prefix="100.0.0.0/8")
 check ovn-nbctl clear logical_router_static_route $route_uuid bfd
@@ -4976,3 +4976,56 @@ AT_CHECK([grep -e "chk_pkt_len" -e "lr_in_larger_pkts" lr0flows | sort], [0], [d
 
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn -- ecmp routes flows])
+AT_KEYWORDS([ecmp-routes-flows])
+ovn_start
+
+check ovn-sbctl chassis-add ch1 geneve 127.0.0.1
+
+check ovn-nbctl lr-add lr0
+check ovn-nbctl ls-add public
+check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 192.168.0.1/24
+check ovn-nbctl lsp-add public public-lr0
+check ovn-nbctl lsp-set-type public-lr0 router
+check ovn-nbctl lsp-set-addresses public-lr0 router
+check ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public
+
+check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 192.168.0.10
+
+ovn-sbctl dump-flows lr0 > lr0flows
+
+AT_CHECK([grep -e "lr_in_ip_routing.*select" lr0flows |sort], [0], [dnl
+])
+AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows |sort], [0], [dnl
+  table=11(lr_in_ip_routing_ecmp), priority=150  , match=(reg8[[0..15]] == 0), action=(next;)
+])
+
+check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 192.168.0.20
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CHECK([grep -e "lr_in_ip_routing.*select" lr0flows |sort], [0], [dnl
+  table=10(lr_in_ip_routing   ), priority=65   , match=(ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1; reg8[[16..31]] = select(1, 2);)
+])
+AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | sed 's/192\.168\.0\..0/192.168.0.??/' |sort], [0], [dnl
+  table=11(lr_in_ip_routing_ecmp), priority=100  , match=(reg8[[0..15]] == 1 && reg8[[16..31]] == 1), action=(reg0 = 192.168.0.??; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; next;)
+  table=11(lr_in_ip_routing_ecmp), priority=100  , match=(reg8[[0..15]] == 1 && reg8[[16..31]] == 2), action=(reg0 = 192.168.0.??; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; next;)
+  table=11(lr_in_ip_routing_ecmp), priority=150  , match=(reg8[[0..15]] == 0), action=(next;)
+])
+
+# add ecmp route with wrong nexthop
+check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 192.168.1.20
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CHECK([grep -e "lr_in_ip_routing.*select" lr0flows |sort], [0], [dnl
+  table=10(lr_in_ip_routing   ), priority=65   , match=(ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1; reg8[[16..31]] = select(1, 2);)
+])
+AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | sed 's/192\.168\.0\..0/192.168.0.??/' |sort], [0], [dnl
+  table=11(lr_in_ip_routing_ecmp), priority=100  , match=(reg8[[0..15]] == 1 && reg8[[16..31]] == 1), action=(reg0 = 192.168.0.??; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; next;)
+  table=11(lr_in_ip_routing_ecmp), priority=100  , match=(reg8[[0..15]] == 1 && reg8[[16..31]] == 2), action=(reg0 = 192.168.0.??; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; next;)
+  table=11(lr_in_ip_routing_ecmp), priority=150  , match=(reg8[[0..15]] == 0), action=(next;)
+])
+
+AT_CLEANUP
+])
-- 
2.31.1



More information about the dev mailing list