ovn-controller: Fix nb_cfg update with monitor_cond_change in flight.

It is not correct for ovn-controller to pass the current SB_Global.nb_cfg
value to ofctrl_put() if there are pending changes to conditional
monitoring clauses (local or in flight).  It might be that after the
monitor condition is acked by the SB, records that were added to the SB
before SB_Global.nb_cfg was set are now sent as updates to
ovn-controller.  These should be first installed in OVS before
ovn-controller reports that it caught up with the current
SB_Global.nb_cfg value.

Also, ofctrl_put should not advance cur_cfg if there are flow updates in
flight.

Signed-off-by: Dumitru Ceara <dceara@redhat.com>

---
v2:
- use new IDL *set_condition() return value.
- fix ofctrl_put to not advance cur_cfg if there are flow updates in
  flight.

This patch depends on OVS patches:
- https://patchwork.ozlabs.org/project/openvswitch/patch/20201110121811.2205350-1-i.maximets@ovn.org/
- https://patchwork.ozlabs.org/project/openvswitch/patch/1605018868-30691-1-git-send-email-dceara@redhat.com/

controller/ofctrl.c

@@ -2033,7 +2033,7 @@ ofctrl_put(struct ovn_desired_flow_table *flow_table,
         need_put = true;
     } else if (nb_cfg != old_nb_cfg) {
         /* nb_cfg changed since last ofctrl_put() call */
-        if (cur_cfg == old_nb_cfg) {
+        if (cur_cfg == old_nb_cfg && ovs_list_is_empty(&flow_updates)) {
             /* we were up-to-date already, so just update with the
              * new nb_cfg */
             cur_cfg = nb_cfg;

controller/ovn-controller.c

@@ -130,7 +130,7 @@ get_bridge(const struct ovsrec_bridge_table *bridge_table, const char *br_name)
     return NULL;
 }
 
-static void
+static unsigned int
 update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
                    const struct sbrec_chassis *chassis,
                    const struct sset *local_ifaces,
@@ -236,16 +236,24 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
         }
     }
 
-out:
-    sbrec_port_binding_set_condition(ovnsb_idl, &pb);
-    sbrec_logical_flow_set_condition(ovnsb_idl, &lf);
-    sbrec_mac_binding_set_condition(ovnsb_idl, &mb);
-    sbrec_multicast_group_set_condition(ovnsb_idl, &mg);
-    sbrec_dns_set_condition(ovnsb_idl, &dns);
-    sbrec_controller_event_set_condition(ovnsb_idl, &ce);
-    sbrec_ip_multicast_set_condition(ovnsb_idl, &ip_mcast);
-    sbrec_igmp_group_set_condition(ovnsb_idl, &igmp);
-    sbrec_chassis_private_set_condition(ovnsb_idl, &chprv);
+out:;
+    unsigned int cond_seqnos[] = {
+        sbrec_port_binding_set_condition(ovnsb_idl, &pb),
+        sbrec_logical_flow_set_condition(ovnsb_idl, &lf),
+        sbrec_mac_binding_set_condition(ovnsb_idl, &mb),
+        sbrec_multicast_group_set_condition(ovnsb_idl, &mg),
+        sbrec_dns_set_condition(ovnsb_idl, &dns),
+        sbrec_controller_event_set_condition(ovnsb_idl, &ce),
+        sbrec_ip_multicast_set_condition(ovnsb_idl, &ip_mcast),
+        sbrec_igmp_group_set_condition(ovnsb_idl, &igmp),
+        sbrec_chassis_private_set_condition(ovnsb_idl, &chprv),
+    };
+
+    unsigned int expected_cond_seqno = 0;
+    for (size_t i = 0; i < ARRAY_SIZE(cond_seqnos); i++) {
+        expected_cond_seqno = MAX(expected_cond_seqno, cond_seqnos[i]);
+    }
+
     ovsdb_idl_condition_destroy(&pb);
     ovsdb_idl_condition_destroy(&lf);
     ovsdb_idl_condition_destroy(&mb);
@@ -255,6 +263,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
     ovsdb_idl_condition_destroy(&ip_mcast);
     ovsdb_idl_condition_destroy(&igmp);
     ovsdb_idl_condition_destroy(&chprv);
+    return expected_cond_seqno;
 }
 
 static const char *
@@ -726,11 +735,23 @@ restore_ct_zones(const struct ovsrec_bridge_table *bridge_table,
 }
 
 static int64_t
-get_nb_cfg(const struct sbrec_sb_global_table *sb_global_table)
+get_nb_cfg(const struct sbrec_sb_global_table *sb_global_table,
+           unsigned int cond_seqno, unsigned int expected_cond_seqno)
 {
+    static int64_t nb_cfg = 0;
+
+    /* Delay getting nb_cfg if there are monitor condition changes
+     * in flight.  It might be that those changes would instruct the
+     * server to send updates that happened before SB_Global.nb_cfg.
+     */
+    if (cond_seqno < expected_cond_seqno) {
+        return nb_cfg;
+    }
+
     const struct sbrec_sb_global *sb
         = sbrec_sb_global_table_first(sb_global_table);
-    return sb ? sb->nb_cfg : 0;
+    nb_cfg = sb ? sb->nb_cfg : 0;
+    return nb_cfg;
 }
 
 static const char *
@@ -2423,6 +2444,7 @@ main(int argc, char *argv[])
 
     unsigned int ovs_cond_seqno = UINT_MAX;
     unsigned int ovnsb_cond_seqno = UINT_MAX;
+    unsigned int ovnsb_expected_cond_seqno = UINT_MAX;
 
     struct controller_engine_ctx ctrl_engine_ctx = {
         .enable_lflow_cache = true
@@ -2574,7 +2596,9 @@ main(int argc, char *argv[])
                                    &ct_zones_data->pending,
                                    sbrec_meter_table_get(ovnsb_idl_loop.idl),
                                    get_nb_cfg(sbrec_sb_global_table_get(
-                                                   ovnsb_idl_loop.idl)),
+                                                   ovnsb_idl_loop.idl),
+                                              ovnsb_cond_seqno,
+                                              ovnsb_expected_cond_seqno),
                                    engine_node_changed(&en_flow_output));
                     }
                     runtime_data = engine_get_data(&en_runtime_data);
@@ -2602,10 +2626,12 @@ main(int argc, char *argv[])
                                     &runtime_data->local_datapaths,
                                     &runtime_data->active_tunnels);
                         if (engine_node_changed(&en_runtime_data)) {
-                            update_sb_monitors(ovnsb_idl_loop.idl, chassis,
-                                               &runtime_data->local_lports,
-                                               &runtime_data->local_datapaths,
-                                               sb_monitor_all);
+                            ovnsb_expected_cond_seqno =
+                                update_sb_monitors(
+                                    ovnsb_idl_loop.idl, chassis,
+                                    &runtime_data->local_lports,
+                                    &runtime_data->local_datapaths,
+                                    sb_monitor_all);
                         }
                     }
                 }