diff mbox series

[ovs-dev,v2,2/3] controller: Implement a generic barrier based on ofctrl cur_cfg sync.

Message ID 20210106193819.17002.95906.stgit@dceara.remote.csb
State Changes Requested
Headers show
Series controller: Notify when an OVN port is ready to use. | expand

Commit Message

Dumitru Ceara Jan. 6, 2021, 7:38 p.m. UTC
A new module, 'ofctrl-seqno', is added to implement this generic
barrier.  Other modules can register their own types of seqno update
requests.  The barrier implementation ensures that the a seqno update
request is acked (returned by ofctrl_acked_seqnos_get()) only if the
OVS flow operations that have been requested when the seqno update
request was queued have been processed by OVS.

For now, the only user of this barrier is the main ovn-controller
module but a future commit will use it too in order to mark
Port_Bindings and OVS interfaces as "fully installed".

Signed-off-by: Dumitru Ceara <dceara@redhat.com>
---
 controller/automake.mk         |    2 
 controller/ofctrl-seqno.c      |  245 ++++++++++++++++++++++++++++++++++++++++
 controller/ofctrl-seqno.h      |   50 ++++++++
 controller/ovn-controller.c    |   41 +++++--
 controller/test-ofctrl-seqno.c |  194 ++++++++++++++++++++++++++++++++
 tests/automake.mk              |    8 +
 tests/ovn-ofctrl-seqno.at      |  228 +++++++++++++++++++++++++++++++++++++
 tests/testsuite.at             |    1 
 8 files changed, 759 insertions(+), 10 deletions(-)
 create mode 100644 controller/ofctrl-seqno.c
 create mode 100644 controller/ofctrl-seqno.h
 create mode 100644 controller/test-ofctrl-seqno.c
 create mode 100644 tests/ovn-ofctrl-seqno.at
diff mbox series

Patch

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