[ovs-dev] [PATCH ovn v7 5/5] ic: don't learn routes which have local GW

Vladislav Odintsov odivlad at gmail.com
Thu Nov 11 19:13:06 UTC 2021


In case we have ovn-ic-interconnected Logical_Routers
and install same ip_prefix route with GW in local AZ
in each LR in each AZ, this route would be learned in
other AZs and L3 loop is possible.
There could be next routes output:

[az1 ~]$ ovn-nbctl lr-route-list lr0
IPv4 Routes
Route Table global:
              128.0.0.0/1               169.254.1.1 dst-ip ecmp
              128.0.0.0/1             169.254.100.2 dst-ip (learned) ecmp

[az2 ~]$ ovn-nbctl lr-route-list lr0
IPv4 Routes
Route Table global:
              128.0.0.0/1               169.254.2.1 dst-ip ecmp
              128.0.0.0/1             169.254.100.1 dst-ip (learned) ecmp

So, there is a possible routing loop. Packets going
to 128.0.0.0/1 could go from AZ1 to AZ2 and on AZ2
they can be routed back.

This commit adds check for installed local (non-learned)
routes. If OVN IC route's ip_prefix, route_table are
the same with already installed non-learned NB route, such
route wouldn't be learned.

Signed-off-by: Vladislav Odintsov <odivlad at gmail.com>
---
 ic/ovn-ic.c           | 30 ++++++++++++++++++++++++--
 tests/ovn-ic.at       | 49 +++++++++++++++++++++++++++++++++++++++++++
 utilities/ovn-nbctl.c |  4 +++-
 3 files changed, 80 insertions(+), 3 deletions(-)

diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index f40468e92..a9b797af2 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -1209,7 +1209,25 @@ add_network_to_routes_ad(struct hmap *routes_ad, const char *network,
 }
 
 static bool
-route_need_learn(struct in6_addr *prefix, unsigned int plen,
+route_has_local_gw(const struct nbrec_logical_router *lr,
+                   const char *route_table, const char *ip_prefix) {
+
+    const struct nbrec_logical_router_static_route *route;
+    for (int i = 0; i < lr->n_static_routes; i++) {
+        route = lr->static_routes[i];
+        if (!smap_get(&route->external_ids, "ic-learned-route") &&
+            !strcmp(route->route_table, route_table) &&
+            !strcmp(route->ip_prefix, ip_prefix)) {
+            return true;
+        }
+    }
+    return false;
+}
+
+static bool
+route_need_learn(const struct nbrec_logical_router *lr,
+                 const struct icsbrec_route *isb_route,
+                 struct in6_addr *prefix, unsigned int plen,
                  const struct smap *nb_options)
 {
     if (!smap_get_bool(nb_options, "ic-route-learn", false)) {
@@ -1229,6 +1247,12 @@ route_need_learn(struct in6_addr *prefix, unsigned int plen,
         return false;
     }
 
+    if (route_has_local_gw(lr, isb_route->route_table, isb_route->ip_prefix)) {
+        VLOG_DBG("Skip learning %s (rtb:%s) route, as we've got one with "
+                 "local GW", isb_route->ip_prefix, isb_route->route_table);
+        return false;
+    }
+
     return true;
 }
 
@@ -1333,9 +1357,11 @@ sync_learned_routes(struct ic_context *ctx,
                              isb_route->nexthop);
                 continue;
             }
-            if (!route_need_learn(&prefix, plen, &nb_global->options)) {
+            if (!route_need_learn(ic_lr->lr, isb_route, &prefix, plen,
+                                  &nb_global->options)) {
                 continue;
             }
+
             struct ic_route_info *route_learned
                 = ic_route_find(&ic_lr->routes_learned, &prefix, plen,
                                 &nexthop, isb_route->origin,
diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
index 1340874d5..a189a8fed 100644
--- a/tests/ovn-ic.at
+++ b/tests/ovn-ic.at
@@ -928,3 +928,52 @@ OVN_CLEANUP_IC([az1], [az2])
 
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-ic -- same routes destination])
+
+ovn_init_ic_db
+ovn-ic-nbctl ts-add ts1
+
+for i in 1 2; do
+    ovn_start az$i
+    ovn_as az$i
+
+    # Enable route learning at AZ level
+    ovn-nbctl set nb_global . options:ic-route-learn=true
+    ovn-nbctl set nb_global . options:ic-route-learn-default=true
+    # Enable route advertising at AZ level
+    ovn-nbctl set nb_global . options:ic-route-adv=true
+    ovn-nbctl set nb_global . options:ic-route-adv-default=true
+
+    lr=lr1$i
+    ovn-nbctl lr-add $lr
+
+    lrp=lrp-$lr-ts1
+    lsp=lsp-ts1-$lr
+    # Create LRP and connect to TS
+    ovn-nbctl lrp-add $lr $lrp aa:aa:aa:aa:aa:0$i 169.254.100.$i/24
+    ovn-nbctl lsp-add ts1 $lsp \
+        -- lsp-set-addresses $lsp router \
+        -- lsp-set-type $lsp router \
+        -- lsp-set-options $lsp router-port=$lrp
+    ovn-nbctl lrp-add $lr lrp-local-subnet 00:00:00:00:00:0$i 192.168.$i.1/24
+    ovn-nbctl list logical-router-static-route
+    check ovn-nbctl lr-route-add $lr 10.0.0.0/24 192.168.$i.10
+    check ovn-nbctl lr-route-add $lr 0.0.0.0/0 192.168.$i.11
+done
+
+AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep dst-ip | sort], [0], [dnl
+                0.0.0.0/0              192.168.1.11 dst-ip
+              10.0.0.0/24              192.168.1.10 dst-ip
+           192.168.2.0/24             169.254.100.2 dst-ip (learned)
+])
+
+AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr12 | grep dst-ip | sort], [0], [dnl
+                0.0.0.0/0              192.168.2.11 dst-ip
+              10.0.0.0/24              192.168.2.10 dst-ip
+           192.168.1.0/24             169.254.100.1 dst-ip (learned)
+])
+
+AT_CLEANUP
+])
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 8bdcb19a3..17bb5d41d 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -4104,6 +4104,8 @@ nbctl_pre_lr_route_add(struct ctl_context *ctx)
                          &nbrec_logical_router_static_route_col_options);
     ovsdb_idl_add_column(ctx->idl,
                          &nbrec_logical_router_static_route_col_route_table);
+    ovsdb_idl_add_column(ctx->idl,
+                         &nbrec_logical_router_static_route_col_external_ids);
 }
 
 static char * OVS_WARN_UNUSED_RESULT
@@ -4233,7 +4235,7 @@ nbctl_lr_route_add(struct ctl_context *ctx)
     }
 
     if (!ecmp) {
-        if (route) {
+        if (route && !smap_get(&route->external_ids, "ic-learned-route")) {
             if (!may_exist) {
                 ctl_error(ctx, "duplicate prefix: %s (policy: %s). Use option"
                           " --ecmp to allow this for ECMP routing.",
-- 
2.30.0



More information about the dev mailing list