[ovs-dev] [PATCH ovn v1] northd: Load config before processing nbdb contents

[Russell Bryant]

Reorder ovnnb_db_run() such that configuration parameters are loaded
or initialized before processing the nbdb contents.

I found this bug because I noticed dynamic MAC addresses being
assigned at ovn-northd startup with an empty prefix.  Later, it would
switch to allocating MAC addresses with the random prefix that was
generated.

The impact of this bug is particularly bad if ovn-northd restarts in
an existing environment.  ovn-northd will check previously assigned
dynamic addresses for validity.  At startup, previously assigned MAC
addresses will all appear invalid because they have a non-empty
prefix, so it will reset them all.  In the case of IPv6, this also
causes the IPv6 addresses change, since OVN assigned dynamic IPv6
addresses are based on the MAC address.

With ovn-kubernetes, whatever first set of addresses were assigned is
what ends up cached on the Node object and used by the Pod.  This bug
can cause all of this to get out of sync, breaking network
connectivity for Pods on an OVN virtual network.

Signed-off-by: Russell Bryant <russell at ovn.org>
---
 northd/ovn-northd.c | 78 ++++++++++++++++++++++-----------------------
 1 file changed, 39 insertions(+), 39 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 33d3ff2ad..3a5cb7c91 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -10151,45 +10151,6 @@ ovnnb_db_run(struct northd_context *ctx,
     struct shash meter_groups = SHASH_INITIALIZER(&meter_groups);
     struct hmap lbs;
 
-    build_datapaths(ctx, datapaths, lr_list);
-    build_ports(ctx, sbrec_chassis_by_name, datapaths, ports);
-    build_ovn_lbs(ctx, ports, &lbs);
-    build_ipam(datapaths, ports);
-    build_port_group_lswitches(ctx, &port_groups, ports);
-    build_lrouter_groups(ports, lr_list);
-    build_ip_mcast(ctx, datapaths);
-    build_mcast_groups(ctx, datapaths, ports, &mcast_groups, &igmp_groups);
-    build_meter_groups(ctx, &meter_groups);
-    build_lflows(ctx, datapaths, ports, &port_groups, &mcast_groups,
-                 &igmp_groups, &meter_groups, &lbs);
-
-    sync_address_sets(ctx);
-    sync_port_groups(ctx);
-    sync_meters(ctx);
-    sync_dns_entries(ctx, datapaths);
-    destroy_ovn_lbs(&lbs);
-    hmap_destroy(&lbs);
-
-    struct ovn_igmp_group *igmp_group, *next_igmp_group;
-
-    HMAP_FOR_EACH_SAFE (igmp_group, next_igmp_group, hmap_node, &igmp_groups) {
-        ovn_igmp_group_destroy(&igmp_groups, igmp_group);
-    }
-
-    struct ovn_port_group *pg, *next_pg;
-    HMAP_FOR_EACH_SAFE (pg, next_pg, key_node, &port_groups) {
-        ovn_port_group_destroy(&port_groups, pg);
-    }
-    hmap_destroy(&igmp_groups);
-    hmap_destroy(&mcast_groups);
-    hmap_destroy(&port_groups);
-
-    struct shash_node *node, *next;
-    SHASH_FOR_EACH_SAFE (node, next, &meter_groups) {
-        shash_delete(&meter_groups, node);
-    }
-    shash_destroy(&meter_groups);
-
     /* Sync ipsec configuration.
      * Copy nb_cfg from northbound to southbound database.
      * Also set up to update sb_cfg once our southbound transaction commits. */
@@ -10263,6 +10224,45 @@ ovnnb_db_run(struct northd_context *ctx,
     controller_event_en = smap_get_bool(&nb->options,
                                         "controller_event", false);
 
+    build_datapaths(ctx, datapaths, lr_list);
+    build_ports(ctx, sbrec_chassis_by_name, datapaths, ports);
+    build_ovn_lbs(ctx, ports, &lbs);
+    build_ipam(datapaths, ports);
+    build_port_group_lswitches(ctx, &port_groups, ports);
+    build_lrouter_groups(ports, lr_list);
+    build_ip_mcast(ctx, datapaths);
+    build_mcast_groups(ctx, datapaths, ports, &mcast_groups, &igmp_groups);
+    build_meter_groups(ctx, &meter_groups);
+    build_lflows(ctx, datapaths, ports, &port_groups, &mcast_groups,
+                 &igmp_groups, &meter_groups, &lbs);
+
+    sync_address_sets(ctx);
+    sync_port_groups(ctx);
+    sync_meters(ctx);
+    sync_dns_entries(ctx, datapaths);
+    destroy_ovn_lbs(&lbs);
+    hmap_destroy(&lbs);
+
+    struct ovn_igmp_group *igmp_group, *next_igmp_group;
+
+    HMAP_FOR_EACH_SAFE (igmp_group, next_igmp_group, hmap_node, &igmp_groups) {
+        ovn_igmp_group_destroy(&igmp_groups, igmp_group);
+    }
+
+    struct ovn_port_group *pg, *next_pg;
+    HMAP_FOR_EACH_SAFE (pg, next_pg, key_node, &port_groups) {
+        ovn_port_group_destroy(&port_groups, pg);
+    }
+    hmap_destroy(&igmp_groups);
+    hmap_destroy(&mcast_groups);
+    hmap_destroy(&port_groups);
+
+    struct shash_node *node, *next;
+    SHASH_FOR_EACH_SAFE (node, next, &meter_groups) {
+        shash_delete(&meter_groups, node);
+    }
+    shash_destroy(&meter_groups);
+
     cleanup_macam(&macam);
 }
 
-- 
2.23.0