[ovs-dev] [PATCH ovn v2] ic: learn routes to LR only from corresponding transit switch

[Vladislav Odintsov]

This commit fixes an error where in case of LRs were connected
between different AZs with ovn-ic, their routes were leaking
from one LR attached to its transit switch to another LR
attached to another transit switch.

Testcase, which reproduces described problem is added as well.

Signed-off-by: Vladislav Odintsov <odivlad at gmail.com>
---
v1 -> v2:
  - Address Han's review comments:
    - Make comments and test more clear to understand.
    - Use ovsdb_idl_index to optimize transit switch lookup.
---
 ic/ovn-ic.c     | 17 ++++++++++++++-
 tests/ovn-ic.at | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index d69583956..a771ab8d7 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -67,6 +67,7 @@ struct ic_context {
     struct ovsdb_idl_index *sbrec_chassis_by_name;
     struct ovsdb_idl_index *sbrec_port_binding_by_name;
     struct ovsdb_idl_index *icsbrec_port_binding_by_ts;
+    struct ovsdb_idl_index *icsbrec_route_by_ts;
 };
 
 struct ic_state {
@@ -1156,7 +1157,14 @@ sync_learned_route(struct ic_context *ctx,
 {
     ovs_assert(ctx->ovnnb_txn);
     const struct icsbrec_route *isb_route;
-    ICSBREC_ROUTE_FOR_EACH (isb_route, ctx->ovnisb_idl) {
+    const struct icsbrec_route *isb_route_key =
+        icsbrec_route_index_init_row(ctx->icsbrec_route_by_ts);
+
+    icsbrec_route_index_set_transit_switch(isb_route_key,
+                                           ic_lr->isb_pb->transit_switch);
+
+    ICSBREC_ROUTE_FOR_EACH_EQUAL(isb_route, isb_route_key,
+                                 ctx->icsbrec_route_by_ts) {
         if (isb_route->availability_zone == az) {
             continue;
         }
@@ -1208,6 +1216,8 @@ sync_learned_route(struct ic_context *ctx,
                 ic_lr->lr, nb_route);
         }
     }
+    icsbrec_route_index_destroy_row(isb_route_key);
+
     /* Delete extra learned routes. */
     struct ic_route_info *route_learned, *next;
     HMAP_FOR_EACH_SAFE (route_learned, next, node, &ic_lr->routes_learned) {
@@ -1678,6 +1688,10 @@ main(int argc, char *argv[])
         = ovsdb_idl_index_create1(ovnisb_idl_loop.idl,
                                   &icsbrec_port_binding_col_transit_switch);
 
+    struct ovsdb_idl_index *icsbrec_route_by_ts
+        = ovsdb_idl_index_create1(ovnisb_idl_loop.idl,
+                                  &icsbrec_route_col_transit_switch);
+
     /* Main loop. */
     exiting = false;
     state.had_lock = false;
@@ -1718,6 +1732,7 @@ main(int argc, char *argv[])
                 .sbrec_port_binding_by_name = sbrec_port_binding_by_name,
                 .sbrec_chassis_by_name = sbrec_chassis_by_name,
                 .icsbrec_port_binding_by_ts = icsbrec_port_binding_by_ts,
+                .icsbrec_route_by_ts = icsbrec_route_by_ts,
             };
 
             if (!state.had_lock && ovsdb_idl_has_lock(ovnsb_idl_loop.idl)) {
diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
index 2a4fba031..ee78f4794 100644
--- a/tests/ovn-ic.at
+++ b/tests/ovn-ic.at
@@ -315,6 +315,61 @@ OVS_WAIT_WHILE([ovn_as az2 ovn-nbctl lr-route-list lr2 | grep learned | grep 192
 # AZ1 shouldn't learn 10.11 any more.
 OVS_WAIT_WHILE([ovn_as az1 ovn-nbctl lr-route-list lr1 | grep learned | grep 10.11])
 
+# cleanup
+ovn-ic-nbctl --if-exists ts-del ts1
+ovn_as az1 ovn-nbctl lr-del lr1
+ovn_as az2 ovn-nbctl lr-del lr2
+
+# Create new transit switches and LRs. Test topology is next:
+# logical router (lr11) - transit switch (ts11) - logical router (lr12)
+# logical router (lr21) - transit switch (ts22) - logical router (lr22)
+#
+# lr12 has static route 10.0.0.0/24 and directly connected network 192.168.0.0/24
+for i in 1 2; do
+    ovn_as az$i
+
+    # Ensure route learning at AZ level
+    ovn-nbctl set nb_global . options:ic-route-learn=true
+    # Ensure route advertising at AZ level
+    ovn-nbctl set nb_global . options:ic-route-adv=true
+    # Drop blacklist
+    ovn-nbctl remove nb_global . options ic-route-blacklist
+
+    for j in 1 2; do
+        ts=ts$j$j
+        ovn-ic-nbctl --may-exist ts-add $ts
+
+        # Create LRP and connect to TS
+        lr=lr$j$i
+        echo lr: $lr, ts: $ts
+        ovn-nbctl lr-add $lr
+        ovn-nbctl lrp-add $lr lrp-$lr-$ts aa:aa:aa:aa:aa:0$j 169.254.100.$i/24
+        ovn-nbctl lsp-add $ts lsp-$ts-$lr \
+                -- lsp-set-addresses lsp-$ts-$lr router \
+                -- lsp-set-type lsp-$ts-$lr router \
+                -- lsp-set-options lsp-$ts-$lr router-port=lrp-$lr-$ts
+    done
+done
+
+# Create directly-connected routes
+ovn_as az2 ovn-nbctl lrp-add lr12 lrp-lr12-ls2 aa:aa:aa:aa:bb:01 "192.168.0.1/24"
+ovn_as az2 ovn-nbctl lr-route-add lr12 10.10.10.0/24 192.168.0.10
+
+echo az1
+ovn_as az1 ovn-nbctl show
+echo az2
+ovn_as az2 ovn-nbctl show
+
+# Test routes from lr12 were learned to lr11
+AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr11 |
+             grep learned | awk '{print $1, $2}' | sort], [0], [dnl
+10.10.10.0/24 169.254.100.2
+192.168.0.0/24 169.254.100.2
+])
+
+# Test routes from lr12 didn't leak as learned to lr21
+AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr21], [0], [])
+
 OVN_CLEANUP_IC([az1], [az2])
 
 AT_CLEANUP
-- 
2.30.0