5f9769
From 2bafeec1b98cfa813fa75dfafa74fdacae8e32c4 Mon Sep 17 00:00:00 2001
5f9769
From: Dumitru Ceara <dceara@redhat.com>
5f9769
Date: Wed, 13 Jan 2021 10:23:19 +0100
5f9769
Subject: [PATCH 2/3] controller: Implement a generic barrier based on ofctrl
5f9769
 cur_cfg sync.
5f9769
5f9769
A new module, 'ofctrl-seqno', is added to implement this generic
5f9769
barrier.  Other modules can register their own types of seqno update
5f9769
requests.  The barrier implementation ensures that the a seqno update
5f9769
request is acked (returned by ofctrl_acked_seqnos_get()) only if the
5f9769
OVS flow operations that have been requested when the seqno update
5f9769
request was queued have been processed by OVS.
5f9769
5f9769
For now, the only user of this barrier is the main ovn-controller
5f9769
module but a future commit will use it too in order to mark
5f9769
Port_Bindings and OVS interfaces as "fully installed".
5f9769
5f9769
This commit also adds unit tests for the new 'ofctrl-seqno' module.
5f9769
The unit test structure is inspired by Mark Michelson's patch:
5f9769
http://patchwork.ozlabs.org/project/ovn/patch/20201216182421.234772-3-mmichels@redhat.com/
5f9769
5f9769
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
5f9769
Acked-by: Mark Michelson <mmichels@redhat.com>
5f9769
Signed-off-by: Numan Siddique <numans@ovn.org>
5f9769
(cherry picked from upstream master commit c93c626248c120eeaffafd323aef323d3b2507ab)
5f9769
5f9769
Change-Id: I500750d4756267e3f62746da33275a22cb1af26f
5f9769
---
5f9769
 controller/automake.mk         |   2 +
5f9769
 controller/ofctrl-seqno.c      | 254 +++++++++++++++++++++++++++++++++++++++++
5f9769
 controller/ofctrl-seqno.h      |  49 ++++++++
5f9769
 controller/ovn-controller.c    |  41 +++++--
5f9769
 controller/test-ofctrl-seqno.c | 194 +++++++++++++++++++++++++++++++
5f9769
 tests/automake.mk              |   8 +-
5f9769
 tests/ovn-ofctrl-seqno.at      | 226 ++++++++++++++++++++++++++++++++++++
5f9769
 tests/testsuite.at             |   1 +
5f9769
 8 files changed, 765 insertions(+), 10 deletions(-)
5f9769
 create mode 100644 controller/ofctrl-seqno.c
5f9769
 create mode 100644 controller/ofctrl-seqno.h
5f9769
 create mode 100644 controller/test-ofctrl-seqno.c
5f9769
 create mode 100644 tests/ovn-ofctrl-seqno.at
5f9769
5f9769
diff --git a/controller/automake.mk b/controller/automake.mk
5f9769
index 45e1bdd..480578e 100644
5f9769
--- a/controller/automake.mk
5f9769
+++ b/controller/automake.mk
5f9769
@@ -18,6 +18,8 @@ controller_ovn_controller_SOURCES = \
5f9769
 	controller/lport.h \
5f9769
 	controller/ofctrl.c \
5f9769
 	controller/ofctrl.h \
5f9769
+	controller/ofctrl-seqno.c \
5f9769
+	controller/ofctrl-seqno.h \
5f9769
 	controller/pinctrl.c \
5f9769
 	controller/pinctrl.h \
5f9769
 	controller/patch.c \
5f9769
diff --git a/controller/ofctrl-seqno.c b/controller/ofctrl-seqno.c
5f9769
new file mode 100644
5f9769
index 0000000..c9334b0
5f9769
--- /dev/null
5f9769
+++ b/controller/ofctrl-seqno.c
5f9769
@@ -0,0 +1,254 @@
5f9769
+/* Copyright (c) 2021, Red Hat, Inc.
5f9769
+ *
5f9769
+ * Licensed under the Apache License, Version 2.0 (the "License");
5f9769
+ * you may not use this file except in compliance with the License.
5f9769
+ * You may obtain a copy of the License at:
5f9769
+ *
5f9769
+ *     http://www.apache.org/licenses/LICENSE-2.0
5f9769
+ *
5f9769
+ * Unless required by applicable law or agreed to in writing, software
5f9769
+ * distributed under the License is distributed on an "AS IS" BASIS,
5f9769
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
5f9769
+ * See the License for the specific language governing permissions and
5f9769
+ * limitations under the License.
5f9769
+ */
5f9769
+
5f9769
+#include <config.h>
5f9769
+
5f9769
+#include "hash.h"
5f9769
+#include "ofctrl-seqno.h"
5f9769
+#include "openvswitch/list.h"
5f9769
+#include "util.h"
5f9769
+
5f9769
+/* A sequence number update request, i.e., when the barrier corresponding to
5f9769
+ * the 'flow_cfg' sequence number is replied to by OVS then it is safe
5f9769
+ * to inform the application that the 'req_cfg' seqno has been processed.
5f9769
+ */
5f9769
+struct ofctrl_seqno_update {
5f9769
+    struct ovs_list list_node; /* In 'ofctrl_seqno_updates'. */
5f9769
+    size_t seqno_type;         /* Application specific seqno type.
5f9769
+                                * Relevant only for 'req_cfg'.
5f9769
+                                */
5f9769
+    uint64_t flow_cfg;         /* The seqno that needs to be acked by OVS
5f9769
+                                * before 'req_cfg' can be acked for the
5f9769
+                                * application.
5f9769
+                                */
5f9769
+    uint64_t req_cfg;          /* Application specific seqno. */
5f9769
+};
5f9769
+
5f9769
+/* List of in flight sequence number updates. */
5f9769
+static struct ovs_list ofctrl_seqno_updates;
5f9769
+
5f9769
+/* Last sequence number request sent to OVS. */
5f9769
+static uint64_t ofctrl_req_seqno;
5f9769
+
5f9769
+/* State of seqno requests for a given application seqno type. */
5f9769
+struct ofctrl_seqno_state {
5f9769
+    struct ovs_list acked_cfgs; /* Acked requests since the last time the
5f9769
+                                 * application consumed acked requests.
5f9769
+                                 */
5f9769
+    uint64_t cur_cfg;           /* Last acked application seqno. */
5f9769
+    uint64_t req_cfg;           /* Last requested application seqno. */
5f9769
+};
5f9769
+
5f9769
+/* Per application seqno type states. */
5f9769
+static size_t n_ofctrl_seqno_states;
5f9769
+static struct ofctrl_seqno_state *ofctrl_seqno_states;
5f9769
+
5f9769
+/* ofctrl_acked_seqnos related static function prototypes. */
5f9769
+static void ofctrl_acked_seqnos_init(struct ofctrl_acked_seqnos *seqnos,
5f9769
+                                     uint64_t last_acked);
5f9769
+static void ofctrl_acked_seqnos_add(struct ofctrl_acked_seqnos *seqnos,
5f9769
+                                    uint32_t val);
5f9769
+
5f9769
+/* ofctrl_seqno_update related static function prototypes. */
5f9769
+static void ofctrl_seqno_update_create__(size_t seqno_type, uint64_t req_cfg);
5f9769
+static void ofctrl_seqno_update_list_destroy(struct ovs_list *seqno_list);
5f9769
+static void ofctrl_seqno_cfg_run(size_t seqno_type,
5f9769
+                                 struct ofctrl_seqno_update *update);
5f9769
+
5f9769
+/* Returns the collection of acked ofctrl_seqno_update requests of type
5f9769
+ * 'seqno_type'.  It's the responsibility of the caller to free memory by
5f9769
+ * calling ofctrl_acked_seqnos_destroy().
5f9769
+ */
5f9769
+struct ofctrl_acked_seqnos *
5f9769
+ofctrl_acked_seqnos_get(size_t seqno_type)
5f9769
+{
5f9769
+    struct ofctrl_acked_seqnos *acked_seqnos = xmalloc(sizeof *acked_seqnos);
5f9769
+    struct ofctrl_seqno_state *state = &ofctrl_seqno_states[seqno_type];
5f9769
+    struct ofctrl_seqno_update *update;
5f9769
+
5f9769
+    ofctrl_acked_seqnos_init(acked_seqnos, state->cur_cfg);
5f9769
+
5f9769
+    ovs_assert(seqno_type < n_ofctrl_seqno_states);
5f9769
+    LIST_FOR_EACH_POP (update, list_node, &state->acked_cfgs) {
5f9769
+        ofctrl_acked_seqnos_add(acked_seqnos, update->req_cfg);
5f9769
+        free(update);
5f9769
+    }
5f9769
+    return acked_seqnos;
5f9769
+}
5f9769
+
5f9769
+void
5f9769
+ofctrl_acked_seqnos_destroy(struct ofctrl_acked_seqnos *seqnos)
5f9769
+{
5f9769
+    if (!seqnos) {
5f9769
+        return;
5f9769
+    }
5f9769
+
5f9769
+    struct ofctrl_ack_seqno *seqno_node;
5f9769
+    HMAP_FOR_EACH_POP (seqno_node, node, &seqnos->acked) {
5f9769
+        free(seqno_node);
5f9769
+    }
5f9769
+    hmap_destroy(&seqnos->acked);
5f9769
+    free(seqnos);
5f9769
+}
5f9769
+
5f9769
+/* Returns true if 'val' is one of the acked sequence numbers in 'seqnos'. */
5f9769
+bool
5f9769
+ofctrl_acked_seqnos_contains(const struct ofctrl_acked_seqnos *seqnos,
5f9769
+                             uint32_t val)
5f9769
+{
5f9769
+    struct ofctrl_ack_seqno *sn;
5f9769
+
5f9769
+    HMAP_FOR_EACH_WITH_HASH (sn, node, hash_int(val, 0), &seqnos->acked) {
5f9769
+        if (sn->seqno == val) {
5f9769
+            return true;
5f9769
+        }
5f9769
+    }
5f9769
+    return false;
5f9769
+}
5f9769
+
5f9769
+void
5f9769
+ofctrl_seqno_init(void)
5f9769
+{
5f9769
+    ovs_list_init(&ofctrl_seqno_updates);
5f9769
+}
5f9769
+
5f9769
+/* Adds a new type of application specific seqno updates. */
5f9769
+size_t
5f9769
+ofctrl_seqno_add_type(void)
5f9769
+{
5f9769
+    size_t new_type = n_ofctrl_seqno_states;
5f9769
+    n_ofctrl_seqno_states++;
5f9769
+
5f9769
+    struct ofctrl_seqno_state *new_states =
5f9769
+        xzalloc(n_ofctrl_seqno_states * sizeof *new_states);
5f9769
+
5f9769
+    for (size_t i = 0; i < n_ofctrl_seqno_states - 1; i++) {
5f9769
+        ovs_list_move(&new_states[i].acked_cfgs,
5f9769
+                      &ofctrl_seqno_states[i].acked_cfgs);
5f9769
+    }
5f9769
+    ovs_list_init(&new_states[new_type].acked_cfgs);
5f9769
+
5f9769
+    free(ofctrl_seqno_states);
5f9769
+    ofctrl_seqno_states = new_states;
5f9769
+    return new_type;
5f9769
+}
5f9769
+
5f9769
+/* Creates a new seqno update request for an application specific
5f9769
+ * 'seqno_type'.
5f9769
+ */
5f9769
+void
5f9769
+ofctrl_seqno_update_create(size_t seqno_type, uint64_t new_cfg)
5f9769
+{
5f9769
+    ovs_assert(seqno_type < n_ofctrl_seqno_states);
5f9769
+
5f9769
+    struct ofctrl_seqno_state *state = &ofctrl_seqno_states[seqno_type];
5f9769
+
5f9769
+    /* If new_cfg didn't change since the last request there should already
5f9769
+     * be an update pending.
5f9769
+     */
5f9769
+    if (new_cfg == state->req_cfg) {
5f9769
+        return;
5f9769
+    }
5f9769
+
5f9769
+    state->req_cfg = new_cfg;
5f9769
+    ofctrl_seqno_update_create__(seqno_type, new_cfg);
5f9769
+}
5f9769
+
5f9769
+/* Should be called when the application is certain that all OVS flow updates
5f9769
+ * corresponding to 'flow_cfg' were processed.  Populates the application
5f9769
+ * specific lists of acked requests in 'ofctrl_seqno_states'.
5f9769
+ */
5f9769
+void
5f9769
+ofctrl_seqno_run(uint64_t flow_cfg)
5f9769
+{
5f9769
+    struct ofctrl_seqno_update *update, *prev;
5f9769
+    LIST_FOR_EACH_SAFE (update, prev, list_node, &ofctrl_seqno_updates) {
5f9769
+        if (flow_cfg < update->flow_cfg) {
5f9769
+            break;
5f9769
+        }
5f9769
+
5f9769
+        ovs_list_remove(&update->list_node);
5f9769
+        ofctrl_seqno_cfg_run(update->seqno_type, update);
5f9769
+    }
5f9769
+}
5f9769
+
5f9769
+/* Returns the seqno to be used when sending a barrier request to OVS. */
5f9769
+uint64_t
5f9769
+ofctrl_seqno_get_req_cfg(void)
5f9769
+{
5f9769
+    return ofctrl_req_seqno;
5f9769
+}
5f9769
+
5f9769
+/* Should be called whenever the openflow connection to OVS is lost.  Flushes
5f9769
+ * all pending 'ofctrl_seqno_updates'.
5f9769
+ */
5f9769
+void
5f9769
+ofctrl_seqno_flush(void)
5f9769
+{
5f9769
+    for (size_t i = 0; i < n_ofctrl_seqno_states; i++) {
5f9769
+        ofctrl_seqno_update_list_destroy(&ofctrl_seqno_states[i].acked_cfgs);
5f9769
+    }
5f9769
+    ofctrl_seqno_update_list_destroy(&ofctrl_seqno_updates);
5f9769
+    ofctrl_req_seqno = 0;
5f9769
+}
5f9769
+
5f9769
+static void
5f9769
+ofctrl_acked_seqnos_init(struct ofctrl_acked_seqnos *seqnos,
5f9769
+                         uint64_t last_acked)
5f9769
+{
5f9769
+    hmap_init(&seqnos->acked);
5f9769
+    seqnos->last_acked = last_acked;
5f9769
+}
5f9769
+
5f9769
+static void
5f9769
+ofctrl_acked_seqnos_add(struct ofctrl_acked_seqnos *seqnos, uint32_t val)
5f9769
+{
5f9769
+    seqnos->last_acked = val;
5f9769
+
5f9769
+    struct ofctrl_ack_seqno *sn = xmalloc(sizeof *sn);
5f9769
+    hmap_insert(&seqnos->acked, &sn->node, hash_int(val, 0));
5f9769
+    sn->seqno = val;
5f9769
+}
5f9769
+
5f9769
+static void
5f9769
+ofctrl_seqno_update_create__(size_t seqno_type, uint64_t req_cfg)
5f9769
+{
5f9769
+    struct ofctrl_seqno_update *update = xmalloc(sizeof *update);
5f9769
+
5f9769
+    ofctrl_req_seqno++;
5f9769
+    ovs_list_push_back(&ofctrl_seqno_updates, &update->list_node);
5f9769
+    update->seqno_type = seqno_type;
5f9769
+    update->flow_cfg = ofctrl_req_seqno;
5f9769
+    update->req_cfg = req_cfg;
5f9769
+}
5f9769
+
5f9769
+static void
5f9769
+ofctrl_seqno_update_list_destroy(struct ovs_list *seqno_list)
5f9769
+{
5f9769
+    struct ofctrl_seqno_update *update;
5f9769
+
5f9769
+    LIST_FOR_EACH_POP (update, list_node, seqno_list) {
5f9769
+        free(update);
5f9769
+    }
5f9769
+}
5f9769
+
5f9769
+static void
5f9769
+ofctrl_seqno_cfg_run(size_t seqno_type, struct ofctrl_seqno_update *update)
5f9769
+{
5f9769
+    ovs_assert(seqno_type < n_ofctrl_seqno_states);
5f9769
+    ovs_list_push_back(&ofctrl_seqno_states[seqno_type].acked_cfgs,
5f9769
+                       &update->list_node);
5f9769
+    ofctrl_seqno_states[seqno_type].cur_cfg = update->req_cfg;
5f9769
+}
5f9769
diff --git a/controller/ofctrl-seqno.h b/controller/ofctrl-seqno.h
5f9769
new file mode 100644
5f9769
index 0000000..876947c
5f9769
--- /dev/null
5f9769
+++ b/controller/ofctrl-seqno.h
5f9769
@@ -0,0 +1,49 @@
5f9769
+/* Copyright (c) 2021, Red Hat, Inc.
5f9769
+ *
5f9769
+ * Licensed under the Apache License, Version 2.0 (the "License");
5f9769
+ * you may not use this file except in compliance with the License.
5f9769
+ * You may obtain a copy of the License at:
5f9769
+ *
5f9769
+ *     http://www.apache.org/licenses/LICENSE-2.0
5f9769
+ *
5f9769
+ * Unless required by applicable law or agreed to in writing, software
5f9769
+ * distributed under the License is distributed on an "AS IS" BASIS,
5f9769
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
5f9769
+ * See the License for the specific language governing permissions and
5f9769
+ * limitations under the License.
5f9769
+ */
5f9769
+
5f9769
+#ifndef OFCTRL_SEQNO_H
5f9769
+#define OFCTRL_SEQNO_H 1
5f9769
+
5f9769
+#include <stdint.h>
5f9769
+
5f9769
+#include <openvswitch/hmap.h>
5f9769
+
5f9769
+/* Collection of acked ofctrl_seqno_update requests and the most recent
5f9769
+ * 'last_acked' value.
5f9769
+ */
5f9769
+struct ofctrl_acked_seqnos {
5f9769
+    struct hmap acked;
5f9769
+    uint64_t last_acked;
5f9769
+};
5f9769
+
5f9769
+/* Acked application specific seqno.  Stored in ofctrl_acked_seqnos.acked. */
5f9769
+struct ofctrl_ack_seqno {
5f9769
+    struct hmap_node node;
5f9769
+    uint64_t seqno;
5f9769
+};
5f9769
+
5f9769
+struct ofctrl_acked_seqnos *ofctrl_acked_seqnos_get(size_t seqno_type);
5f9769
+void ofctrl_acked_seqnos_destroy(struct ofctrl_acked_seqnos *seqnos);
5f9769
+bool ofctrl_acked_seqnos_contains(const struct ofctrl_acked_seqnos *seqnos,
5f9769
+                                  uint32_t val);
5f9769
+
5f9769
+void ofctrl_seqno_init(void);
5f9769
+size_t ofctrl_seqno_add_type(void);
5f9769
+void ofctrl_seqno_update_create(size_t seqno_type, uint64_t new_cfg);
5f9769
+void ofctrl_seqno_run(uint64_t flow_cfg);
5f9769
+uint64_t ofctrl_seqno_get_req_cfg(void);
5f9769
+void ofctrl_seqno_flush(void);
5f9769
+
5f9769
+#endif /* controller/ofctrl-seqno.h */
5f9769
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
5f9769
index 42883b4..bb1c659 100644
5f9769
--- a/controller/ovn-controller.c
5f9769
+++ b/controller/ovn-controller.c
5f9769
@@ -39,6 +39,7 @@
5f9769
 #include "lib/vswitch-idl.h"
5f9769
 #include "lport.h"
5f9769
 #include "ofctrl.h"
5f9769
+#include "ofctrl-seqno.h"
5f9769
 #include "openvswitch/vconn.h"
5f9769
 #include "openvswitch/vlog.h"
5f9769
 #include "ovn/actions.h"
5f9769
@@ -98,6 +99,9 @@ struct pending_pkt {
5f9769
     char *flow_s;
5f9769
 };
5f9769
 
5f9769
+/* Registered ofctrl seqno type for nb_cfg propagation. */
5f9769
+static size_t ofctrl_seq_type_nb_cfg;
5f9769
+
5f9769
 struct local_datapath *
5f9769
 get_local_datapath(const struct hmap *local_datapaths, uint32_t tunnel_key)
5f9769
 {
5f9769
@@ -825,11 +829,14 @@ static void
5f9769
 store_nb_cfg(struct ovsdb_idl_txn *sb_txn, struct ovsdb_idl_txn *ovs_txn,
5f9769
              const struct sbrec_chassis_private *chassis,
5f9769
              const struct ovsrec_bridge *br_int,
5f9769
-             unsigned int delay_nb_cfg_report,
5f9769
-             uint64_t cur_cfg)
5f9769
+             unsigned int delay_nb_cfg_report)
5f9769
 {
5f9769
+    struct ofctrl_acked_seqnos *acked_nb_cfg_seqnos =
5f9769
+        ofctrl_acked_seqnos_get(ofctrl_seq_type_nb_cfg);
5f9769
+    uint64_t cur_cfg = acked_nb_cfg_seqnos->last_acked;
5f9769
+
5f9769
     if (!cur_cfg) {
5f9769
-        return;
5f9769
+        goto done;
5f9769
     }
5f9769
 
5f9769
     if (sb_txn && chassis && cur_cfg != chassis->nb_cfg) {
5f9769
@@ -850,6 +857,9 @@ store_nb_cfg(struct ovsdb_idl_txn *sb_txn, struct ovsdb_idl_txn *ovs_txn,
5f9769
                                                  cur_cfg_str);
5f9769
         free(cur_cfg_str);
5f9769
     }
5f9769
+
5f9769
+done:
5f9769
+    ofctrl_acked_seqnos_destroy(acked_nb_cfg_seqnos);
5f9769
 }
5f9769
 
5f9769
 static const char *
5f9769
@@ -967,6 +977,11 @@ en_ofctrl_is_connected_run(struct engine_node *node, void *data)
5f9769
     struct ed_type_ofctrl_is_connected *of_data = data;
5f9769
     if (of_data->connected != ofctrl_is_connected()) {
5f9769
         of_data->connected = !of_data->connected;
5f9769
+
5f9769
+        /* Flush ofctrl seqno requests when the ofctrl connection goes down. */
5f9769
+        if (!of_data->connected) {
5f9769
+            ofctrl_seqno_flush();
5f9769
+        }
5f9769
         engine_set_node_state(node, EN_UPDATED);
5f9769
         return;
5f9769
     }
5f9769
@@ -2393,6 +2408,9 @@ main(int argc, char *argv[])
5f9769
     pinctrl_init();
5f9769
     lflow_init();
5f9769
 
5f9769
+    /* Register ofctrl seqno types. */
5f9769
+    ofctrl_seq_type_nb_cfg = ofctrl_seqno_add_type();
5f9769
+
5f9769
     /* Connect to OVS OVSDB instance. */
5f9769
     struct ovsdb_idl_loop ovs_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
5f9769
         ovsdb_idl_create(ovs_remote, &ovsrec_idl_class, false, true));
5f9769
@@ -2624,6 +2642,7 @@ main(int argc, char *argv[])
5f9769
     ofctrl_init(&flow_output_data->group_table,
5f9769
                 &flow_output_data->meter_table,
5f9769
                 get_ofctrl_probe_interval(ovs_idl_loop.idl));
5f9769
+    ofctrl_seqno_init();
5f9769
 
5f9769
     unixctl_command_register("group-table-list", "", 0, 0,
5f9769
                              extend_table_list,
5f9769
@@ -2853,17 +2872,23 @@ main(int argc, char *argv[])
5f9769
                                     sb_monitor_all);
5f9769
                         }
5f9769
                     }
5f9769
+
5f9769
+                    ofctrl_seqno_update_create(
5f9769
+                        ofctrl_seq_type_nb_cfg,
5f9769
+                        get_nb_cfg(sbrec_sb_global_table_get(
5f9769
+                                                       ovnsb_idl_loop.idl),
5f9769
+                                              ovnsb_cond_seqno,
5f9769
+                                              ovnsb_expected_cond_seqno));
5f9769
+
5f9769
                     flow_output_data = engine_get_data(&en_flow_output);
5f9769
                     if (flow_output_data && ct_zones_data) {
5f9769
                         ofctrl_put(&flow_output_data->flow_table,
5f9769
                                    &ct_zones_data->pending,
5f9769
                                    sbrec_meter_table_get(ovnsb_idl_loop.idl),
5f9769
-                                   get_nb_cfg(sbrec_sb_global_table_get(
5f9769
-                                                   ovnsb_idl_loop.idl),
5f9769
-                                              ovnsb_cond_seqno,
5f9769
-                                              ovnsb_expected_cond_seqno),
5f9769
+                                   ofctrl_seqno_get_req_cfg(),
5f9769
                                    engine_node_changed(&en_flow_output));
5f9769
                     }
5f9769
+                    ofctrl_seqno_run(ofctrl_get_cur_cfg());
5f9769
                 }
5f9769
 
5f9769
             }
5f9769
@@ -2889,7 +2914,7 @@ main(int argc, char *argv[])
5f9769
             }
5f9769
 
5f9769
             store_nb_cfg(ovnsb_idl_txn, ovs_idl_txn, chassis_private,
5f9769
-                         br_int, delay_nb_cfg_report, ofctrl_get_cur_cfg());
5f9769
+                         br_int, delay_nb_cfg_report);
5f9769
 
5f9769
             if (pending_pkt.conn) {
5f9769
                 struct ed_type_addr_sets *as_data =
5f9769
diff --git a/controller/test-ofctrl-seqno.c b/controller/test-ofctrl-seqno.c
5f9769
new file mode 100644
5f9769
index 0000000..fce88d4
5f9769
--- /dev/null
5f9769
+++ b/controller/test-ofctrl-seqno.c
5f9769
@@ -0,0 +1,194 @@
5f9769
+/* Copyright (c) 2021, Red Hat, Inc.
5f9769
+ *
5f9769
+ * Licensed under the Apache License, Version 2.0 (the "License");
5f9769
+ * you may not use this file except in compliance with the License.
5f9769
+ * You may obtain a copy of the License at:
5f9769
+ *
5f9769
+ *     http://www.apache.org/licenses/LICENSE-2.0
5f9769
+ *
5f9769
+ * Unless required by applicable law or agreed to in writing, software
5f9769
+ * distributed under the License is distributed on an "AS IS" BASIS,
5f9769
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
5f9769
+ * See the License for the specific language governing permissions and
5f9769
+ * limitations under the License.
5f9769
+ */
5f9769
+
5f9769
+#include <config.h>
5f9769
+
5f9769
+#include "tests/ovstest.h"
5f9769
+#include "sort.h"
5f9769
+#include "util.h"
5f9769
+
5f9769
+#include "ofctrl-seqno.h"
5f9769
+
5f9769
+static void
5f9769
+test_init(void)
5f9769
+{
5f9769
+    ofctrl_seqno_init();
5f9769
+}
5f9769
+
5f9769
+static bool
5f9769
+test_read_uint_value(struct ovs_cmdl_context *ctx, unsigned int index,
5f9769
+                     const char *descr, unsigned int *result)
5f9769
+{
5f9769
+    if (index >= ctx->argc) {
5f9769
+        fprintf(stderr, "Missing %s argument\n", descr);
5f9769
+        return false;
5f9769
+    }
5f9769
+
5f9769
+    const char *arg = ctx->argv[index];
5f9769
+    if (!str_to_uint(arg, 10, result)) {
5f9769
+        fprintf(stderr, "Invalid %s: %s\n", descr, arg);
5f9769
+        return false;
5f9769
+    }
5f9769
+    return true;
5f9769
+}
5f9769
+
5f9769
+static int
5f9769
+test_seqno_compare(size_t a, size_t b, void *values_)
5f9769
+{
5f9769
+    uint64_t *values = values_;
5f9769
+
5f9769
+    return values[a] == values[b] ? 0 : (values[a] < values[b] ? -1 : 1);
5f9769
+}
5f9769
+
5f9769
+static void
5f9769
+test_seqno_swap(size_t a, size_t b, void *values_)
5f9769
+{
5f9769
+    uint64_t *values = values_;
5f9769
+    uint64_t tmp = values[a];
5f9769
+
5f9769
+    values[a] = values[b];
5f9769
+    values[b] = tmp;
5f9769
+}
5f9769
+
5f9769
+static void
5f9769
+test_dump_acked_seqnos(size_t seqno_type)
5f9769
+{
5f9769
+    struct ofctrl_acked_seqnos * acked_seqnos =
5f9769
+        ofctrl_acked_seqnos_get(seqno_type);
5f9769
+
5f9769
+    printf("ofctrl-seqno-type: %"PRIuSIZE"\n", seqno_type);
5f9769
+    printf("  last-acked %"PRIu64"\n", acked_seqnos->last_acked);
5f9769
+
5f9769
+    size_t n_acked = hmap_count(&acked_seqnos->acked);
5f9769
+    uint64_t *acked = xmalloc(n_acked * sizeof *acked);
5f9769
+    struct ofctrl_ack_seqno *ack_seqno;
5f9769
+    size_t i = 0;
5f9769
+
5f9769
+    /* A bit hacky but ignoring overflows the "total of all seqno + 1" should
5f9769
+     * be a number that is not part of the acked seqnos.
5f9769
+     */
5f9769
+    uint64_t total_seqno = 1;
5f9769
+    HMAP_FOR_EACH (ack_seqno, node, &acked_seqnos->acked) {
5f9769
+        ovs_assert(ofctrl_acked_seqnos_contains(acked_seqnos,
5f9769
+                                                ack_seqno->seqno));
5f9769
+        total_seqno += ack_seqno->seqno;
5f9769
+        acked[i++] = ack_seqno->seqno;
5f9769
+    }
5f9769
+    ovs_assert(!ofctrl_acked_seqnos_contains(acked_seqnos, total_seqno));
5f9769
+
5f9769
+    sort(n_acked, test_seqno_compare, test_seqno_swap, acked);
5f9769
+
5f9769
+    for (i = 0; i < n_acked; i++) {
5f9769
+        printf("  %"PRIu64"\n", acked[i]);
5f9769
+    }
5f9769
+
5f9769
+    free(acked);
5f9769
+    ofctrl_acked_seqnos_destroy(acked_seqnos);
5f9769
+}
5f9769
+
5f9769
+static void
5f9769
+test_ofctrl_seqno_add_type(struct ovs_cmdl_context *ctx)
5f9769
+{
5f9769
+    unsigned int n_types;
5f9769
+
5f9769
+    test_init();
5f9769
+
5f9769
+    if (!test_read_uint_value(ctx, 1, "n_types", &n_types)) {
5f9769
+        return;
5f9769
+    }
5f9769
+    for (unsigned int i = 0; i < n_types; i++) {
5f9769
+        printf("%"PRIuSIZE"\n", ofctrl_seqno_add_type());
5f9769
+    }
5f9769
+}
5f9769
+
5f9769
+static void
5f9769
+test_ofctrl_seqno_ack_seqnos(struct ovs_cmdl_context *ctx)
5f9769
+{
5f9769
+    unsigned int n_reqs = 0;
5f9769
+    unsigned int shift = 2;
5f9769
+    unsigned int n_types;
5f9769
+    unsigned int n_acks;
5f9769
+
5f9769
+    test_init();
5f9769
+    bool batch_acks = !strcmp(ctx->argv[1], "true");
5f9769
+
5f9769
+    if (!test_read_uint_value(ctx, shift++, "n_types", &n_types)) {
5f9769
+        return;
5f9769
+    }
5f9769
+
5f9769
+    for (unsigned int i = 0; i < n_types; i++) {
5f9769
+        ovs_assert(ofctrl_seqno_add_type() == i);
5f9769
+
5f9769
+        /* Read number of app specific seqnos. */
5f9769
+        unsigned int n_app_seqnos;
5f9769
+
5f9769
+        if (!test_read_uint_value(ctx, shift++, "n_app_seqnos",
5f9769
+                                  &n_app_seqnos)) {
5f9769
+            return;
5f9769
+        }
5f9769
+
5f9769
+        for (unsigned int j = 0; j < n_app_seqnos; j++, n_reqs++) {
5f9769
+            unsigned int app_seqno;
5f9769
+
5f9769
+            if (!test_read_uint_value(ctx, shift++, "app_seqno", &app_seqno)) {
5f9769
+                return;
5f9769
+            }
5f9769
+            ofctrl_seqno_update_create(i, app_seqno);
5f9769
+        }
5f9769
+    }
5f9769
+    printf("ofctrl-seqno-req-cfg: %u\n", n_reqs);
5f9769
+
5f9769
+    if (!test_read_uint_value(ctx, shift++, "n_acks", &n_acks)) {
5f9769
+        return;
5f9769
+    }
5f9769
+    for (unsigned int i = 0; i < n_acks; i++) {
5f9769
+        unsigned int ack_seqno;
5f9769
+
5f9769
+        if (!test_read_uint_value(ctx, shift++, "ack_seqno", &ack_seqno)) {
5f9769
+            return;
5f9769
+        }
5f9769
+        ofctrl_seqno_run(ack_seqno);
5f9769
+
5f9769
+        if (!batch_acks) {
5f9769
+            for (unsigned int st = 0; st < n_types; st++) {
5f9769
+                test_dump_acked_seqnos(st);
5f9769
+            }
5f9769
+        }
5f9769
+    }
5f9769
+    if (batch_acks) {
5f9769
+        for (unsigned int st = 0; st < n_types; st++) {
5f9769
+            test_dump_acked_seqnos(st);
5f9769
+        }
5f9769
+    }
5f9769
+}
5f9769
+
5f9769
+static void
5f9769
+test_ofctrl_seqno_main(int argc, char *argv[])
5f9769
+{
5f9769
+    set_program_name(argv[0]);
5f9769
+    static const struct ovs_cmdl_command commands[] = {
5f9769
+        {"ofctrl_seqno_add_type", NULL, 1, 1,
5f9769
+         test_ofctrl_seqno_add_type, OVS_RO},
5f9769
+        {"ofctrl_seqno_ack_seqnos", NULL, 2, INT_MAX,
5f9769
+         test_ofctrl_seqno_ack_seqnos, OVS_RO},
5f9769
+        {NULL, NULL, 0, 0, NULL, OVS_RO},
5f9769
+    };
5f9769
+    struct ovs_cmdl_context ctx;
5f9769
+    ctx.argc = argc - 1;
5f9769
+    ctx.argv = argv + 1;
5f9769
+    ovs_cmdl_run_command(&ctx, commands);
5f9769
+}
5f9769
+
5f9769
+OVSTEST_REGISTER("test-ofctrl-seqno", test_ofctrl_seqno_main);
5f9769
diff --git a/tests/automake.mk b/tests/automake.mk
5f9769
index c5c286e..c09f615 100644
5f9769
--- a/tests/automake.mk
5f9769
+++ b/tests/automake.mk
5f9769
@@ -31,7 +31,8 @@ TESTSUITE_AT = \
5f9769
 	tests/ovn-controller-vtep.at \
5f9769
 	tests/ovn-ic.at \
5f9769
 	tests/ovn-macros.at \
5f9769
-	tests/ovn-performance.at
5f9769
+	tests/ovn-performance.at \
5f9769
+	tests/ovn-ofctrl-seqno.at
5f9769
 
5f9769
 SYSTEM_KMOD_TESTSUITE_AT = \
5f9769
 	tests/system-common-macros.at \
5f9769
@@ -202,7 +203,10 @@ noinst_PROGRAMS += tests/ovstest
5f9769
 tests_ovstest_SOURCES = \
5f9769
 	tests/ovstest.c \
5f9769
 	tests/ovstest.h \
5f9769
-	tests/test-ovn.c
5f9769
+	tests/test-ovn.c \
5f9769
+	controller/test-ofctrl-seqno.c \
5f9769
+	controller/ofctrl-seqno.c \
5f9769
+	controller/ofctrl-seqno.h
5f9769
 
5f9769
 tests_ovstest_LDADD = $(OVS_LIBDIR)/daemon.lo \
5f9769
     $(OVS_LIBDIR)/libopenvswitch.la lib/libovn.la
5f9769
diff --git a/tests/ovn-ofctrl-seqno.at b/tests/ovn-ofctrl-seqno.at
5f9769
new file mode 100644
5f9769
index 0000000..59dfea9
5f9769
--- /dev/null
5f9769
+++ b/tests/ovn-ofctrl-seqno.at
5f9769
@@ -0,0 +1,226 @@
5f9769
+#
5f9769
+# Unit tests for the controller/ofctrl-seqno.c module.
5f9769
+#
5f9769
+AT_BANNER([OVN unit tests - ofctrl-seqno])
5f9769
+
5f9769
+AT_SETUP([ovn -- unit test -- ofctrl-seqno add-type])
5f9769
+
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_add_type 1], [0], [dnl
5f9769
+0
5f9769
+])
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_add_type 2], [0], [dnl
5f9769
+0
5f9769
+1
5f9769
+])
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_add_type 3], [0], [dnl
5f9769
+0
5f9769
+1
5f9769
+2
5f9769
+])
5f9769
+AT_CLEANUP
5f9769
+
5f9769
+AT_SETUP([ovn -- unit test -- ofctrl-seqno ack-seqnos])
5f9769
+
5f9769
+AS_BOX([No Ack Batching, 1 seqno type])
5f9769
+n_types=1
5f9769
+n_app_seqnos=3
5f9769
+app_seqnos="40 41 42"
5f9769
+
5f9769
+n_acks=1
5f9769
+acks="1"
5f9769
+echo "ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos false ${n_types} ${n_app_seqnos} ${app_seqnos} ${n_acks} ${acks}"
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos false ${n_types} \
5f9769
+          ${n_app_seqnos} ${app_seqnos} ${n_acks} ${acks}], [0], [dnl
5f9769
+ofctrl-seqno-req-cfg: 3
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 40
5f9769
+  40
5f9769
+])
5f9769
+
5f9769
+n_acks=2
5f9769
+acks="1 2"
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos false ${n_types} \
5f9769
+          ${n_app_seqnos} ${app_seqnos} ${n_acks} ${acks}], [0], [dnl
5f9769
+ofctrl-seqno-req-cfg: 3
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 40
5f9769
+  40
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 41
5f9769
+  41
5f9769
+])
5f9769
+
5f9769
+n_acks=3
5f9769
+acks="1 2 3"
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos false ${n_types} \
5f9769
+          ${n_app_seqnos} ${app_seqnos} ${n_acks} ${acks}], [0], [dnl
5f9769
+ofctrl-seqno-req-cfg: 3
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 40
5f9769
+  40
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 41
5f9769
+  41
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 42
5f9769
+  42
5f9769
+])
5f9769
+
5f9769
+AS_BOX([Ack Batching, 1 seqno type])
5f9769
+n_types=1
5f9769
+n_app_seqnos=3
5f9769
+app_seqnos="40 41 42"
5f9769
+
5f9769
+n_acks=1
5f9769
+acks="1"
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos true ${n_types} \
5f9769
+          ${n_app_seqnos} ${app_seqnos} ${n_acks} ${acks}], [0], [dnl
5f9769
+ofctrl-seqno-req-cfg: 3
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 40
5f9769
+  40
5f9769
+])
5f9769
+
5f9769
+n_acks=2
5f9769
+acks="1 2"
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos true ${n_types} \
5f9769
+          ${n_app_seqnos} ${app_seqnos} ${n_acks} ${acks}], [0], [dnl
5f9769
+ofctrl-seqno-req-cfg: 3
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 41
5f9769
+  40
5f9769
+  41
5f9769
+])
5f9769
+
5f9769
+n_acks=3
5f9769
+acks="1 2 3"
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos true ${n_types} \
5f9769
+          ${n_app_seqnos} ${app_seqnos} ${n_acks} ${acks}], [0], [dnl
5f9769
+ofctrl-seqno-req-cfg: 3
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 42
5f9769
+  40
5f9769
+  41
5f9769
+  42
5f9769
+])
5f9769
+
5f9769
+AS_BOX([No Ack Batching, 2 seqno types])
5f9769
+n_types=2
5f9769
+n_app_seqnos=3
5f9769
+app_seqnos1="40 41 42"
5f9769
+app_seqnos2="50 51 52"
5f9769
+
5f9769
+n_acks=1
5f9769
+acks="1"
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos false ${n_types} \
5f9769
+          ${n_app_seqnos} ${app_seqnos1} ${n_app_seqnos} ${app_seqnos2} \
5f9769
+          ${n_acks} ${acks}], [0], [dnl
5f9769
+ofctrl-seqno-req-cfg: 6
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 40
5f9769
+  40
5f9769
+ofctrl-seqno-type: 1
5f9769
+  last-acked 0
5f9769
+])
5f9769
+
5f9769
+n_acks=3
5f9769
+acks="1 2 3"
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos false ${n_types} \
5f9769
+          ${n_app_seqnos} ${app_seqnos1} ${n_app_seqnos} ${app_seqnos2} \
5f9769
+          ${n_acks} ${acks}], [0], [dnl
5f9769
+ofctrl-seqno-req-cfg: 6
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 40
5f9769
+  40
5f9769
+ofctrl-seqno-type: 1
5f9769
+  last-acked 0
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 41
5f9769
+  41
5f9769
+ofctrl-seqno-type: 1
5f9769
+  last-acked 0
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 42
5f9769
+  42
5f9769
+ofctrl-seqno-type: 1
5f9769
+  last-acked 0
5f9769
+])
5f9769
+
5f9769
+n_acks=3
5f9769
+acks="4 5 6"
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos false ${n_types} \
5f9769
+          ${n_app_seqnos} ${app_seqnos1} ${n_app_seqnos} ${app_seqnos2} \
5f9769
+          ${n_acks} ${acks}], [0], [dnl
5f9769
+ofctrl-seqno-req-cfg: 6
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 42
5f9769
+  40
5f9769
+  41
5f9769
+  42
5f9769
+ofctrl-seqno-type: 1
5f9769
+  last-acked 50
5f9769
+  50
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 42
5f9769
+ofctrl-seqno-type: 1
5f9769
+  last-acked 51
5f9769
+  51
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 42
5f9769
+ofctrl-seqno-type: 1
5f9769
+  last-acked 52
5f9769
+  52
5f9769
+])
5f9769
+
5f9769
+AS_BOX([Ack Batching, 2 seqno types])
5f9769
+n_types=2
5f9769
+n_app_seqnos=3
5f9769
+app_seqnos1="40 41 42"
5f9769
+app_seqnos2="50 51 52"
5f9769
+
5f9769
+n_acks=1
5f9769
+acks="1"
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos true ${n_types} \
5f9769
+          ${n_app_seqnos} ${app_seqnos1} ${n_app_seqnos} ${app_seqnos2} \
5f9769
+          ${n_acks} ${acks}], [0], [dnl
5f9769
+ofctrl-seqno-req-cfg: 6
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 40
5f9769
+  40
5f9769
+ofctrl-seqno-type: 1
5f9769
+  last-acked 0
5f9769
+])
5f9769
+
5f9769
+n_acks=3
5f9769
+acks="1 2 3"
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos true ${n_types} \
5f9769
+          ${n_app_seqnos} ${app_seqnos1} ${n_app_seqnos} ${app_seqnos2} \
5f9769
+          ${n_acks} ${acks}], [0], [dnl
5f9769
+ofctrl-seqno-req-cfg: 6
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 42
5f9769
+  40
5f9769
+  41
5f9769
+  42
5f9769
+ofctrl-seqno-type: 1
5f9769
+  last-acked 0
5f9769
+])
5f9769
+
5f9769
+n_acks=3
5f9769
+acks="4 5 6"
5f9769
+AT_CHECK([ovstest test-ofctrl-seqno ofctrl_seqno_ack_seqnos true ${n_types} \
5f9769
+          ${n_app_seqnos} ${app_seqnos1} ${n_app_seqnos} ${app_seqnos2} \
5f9769
+          ${n_acks} ${acks}], [0], [dnl
5f9769
+ofctrl-seqno-req-cfg: 6
5f9769
+ofctrl-seqno-type: 0
5f9769
+  last-acked 42
5f9769
+  40
5f9769
+  41
5f9769
+  42
5f9769
+ofctrl-seqno-type: 1
5f9769
+  last-acked 52
5f9769
+  50
5f9769
+  51
5f9769
+  52
5f9769
+])
5f9769
+AT_CLEANUP
5f9769
diff --git a/tests/testsuite.at b/tests/testsuite.at
5f9769
index 960227d..3eba785 100644
5f9769
--- a/tests/testsuite.at
5f9769
+++ b/tests/testsuite.at
5f9769
@@ -26,6 +26,7 @@ m4_include([tests/ovn.at])
5f9769
 m4_include([tests/ovn-performance.at])
5f9769
 m4_include([tests/ovn-northd.at])
5f9769
 m4_include([tests/ovn-nbctl.at])
5f9769
+m4_include([tests/ovn-ofctrl-seqno.at])
5f9769
 m4_include([tests/ovn-sbctl.at])
5f9769
 m4_include([tests/ovn-ic-nbctl.at])
5f9769
 m4_include([tests/ovn-ic-sbctl.at])
5f9769
-- 
5f9769
1.8.3.1
5f9769