ic: Fix NULL ptr deref on log of duplicate routes.

Commit ac1dc2b introduced re-use of the add_to_routes_ad() function for both connected and static routes, however it did not add conditionals for handling warning level logging of duplicate routes. Reported-at: ovn-org#268 Fixes: ac1dc2b ("ic: prevent advertising/learning multiple same routes") Signed-off-by: Frode Nordahl <fnordahl@ubuntu.com>
fnordahl · Jan 7, 2025 · 76ccb54 · 76ccb54
1 parent dbdd8ea
commit 76ccb54
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 3 deletions.
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
@@ -1128,6 +1128,8 @@ add_to_routes_ad(struct hmap *routes_ad, const struct in6_addr prefix,
                  const struct nbrec_logical_router *nb_lr,
                  const char *route_tag)
 {
+    ovs_assert(nb_route || nb_lrp);
+
     if (route_table == NULL) {
         route_table = "";
     }
@@ -1149,9 +1151,15 @@ add_to_routes_ad(struct hmap *routes_ad, const struct in6_addr prefix,
         hmap_insert(routes_ad, &ic_route->node, hash);
     } else {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
-        VLOG_WARN_RL(&rl, "Duplicate route advertisement was suppressed! NB "
-                     "route uuid: "UUID_FMT,
-                     UUID_ARGS(&nb_route->header_.uuid));
+        const char *msg_fmt = "Duplicate %s route advertisement was "
+                              "suppressed! NB %s uuid: "UUID_FMT;
+        if (nb_route) {
+            VLOG_WARN_RL(&rl, msg_fmt, origin, "route",
+                         UUID_ARGS(&nb_route->header_.uuid));
+        } else {
+            VLOG_WARN_RL(&rl, msg_fmt, origin, "lrp",
+                         UUID_ARGS(&nb_lrp->header_.uuid));
+        }
     }
 }
 

diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
@@ -282,6 +282,67 @@ OVN_CLEANUP_IC
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-ic -- local duplicate connected route])
+
+ovn_init_ic_db
+net_add n1
+
+# 1 GW per AZ
+for i in 1 2; do
+    az=az$i
+    ovn_start $az
+    sim_add gw-$az
+    as gw-$az
+    check ovs-vsctl add-br br-phys
+    ovn_az_attach $az n1 br-phys 192.168.1.$i
+    check ovs-vsctl set open . external-ids:ovn-is-interconn=true
+    check ovn-nbctl set nb-global . \
+        options:ic-route-adv=true \
+        options:ic-route-adv-default=true \
+        options:ic-route-learn=true \
+        options:ic-route-learn-default=true
+done
+
+ovn_as az1
+
+# create transit switch and connect to LR
+check ovn-ic-nbctl --wait=sb ts-add ts1
+for i in 1 2; do
+    ovn_as az$i
+
+    check \
+       grep -vq "connected route advertisement was suppressed! NB lrp" \
+       az$i/ic/ovn-ic.log
+
+    check ovn-nbctl lr-add lr1
+    check ovn-nbctl lrp-add lr1 lrp$i 00:00:00:00:0$i:01 10.0.$i.1/24
+    check ovn-nbctl lrp-add lr1 lrp-dup1 00:00:00:00:0$i:42 10.0.42.1/24
+    check ovn-nbctl lrp-add lr1 lrp-dup2 00:00:00:00:0$i:51 10.0.42.1/24
+    check ovn-nbctl lrp-set-gateway-chassis lrp$i gw-az$i
+
+    check ovn-nbctl --wait=sb lsp-add ts1 lsp$i -- \
+        lsp-set-addresses lsp$i router -- \
+        lsp-set-type lsp$i router -- \
+        lsp-set-options lsp$i router-port=lrp$i
+    check ovn-ic-nbctl --wait=sb sync
+
+    check \
+       grep -q "connected route advertisement was suppressed! NB lrp" \
+       az$i/ic/ovn-ic.log
+done
+
+for i in 1 2; do
+    az=az$i
+    ovn_as $az
+    OVN_CLEANUP_SBOX(gw-$az)
+    OVN_CLEANUP_AZ([$az])
+done
+
+OVN_CLEANUP_IC
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([ovn-ic -- gateway sync])