On Mon, Jan 11, 2021 at 5:37 PM Dumitru Ceara <[email protected]> wrote:

> 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".
>
> This commit also adds unit tests for the new 'ofctrl-seqno' module.
> The unit test structure is inspired by Mark Michelson's patch:
>
> http://patchwork.ozlabs.org/project/ovn/patch/[email protected]/
>
> Signed-off-by: Dumitru Ceara <[email protected]>
>

Hi Dumitru,

Thanks for working on this feature.

I applied this series and tested it on sandbox, and looks like there is
some issue when I  run this script -
https://github.com/ovn-org/ovn/blob/master/tutorial/ovn-setup.sh

ovn-controller is not claiming all the lports.

I have still not reviewed the patches thoroughly.

I have few comments now.

1. tests/ovn-ofctrl-seqno.at requires a license stuff at the beginning ?

2. For the ofctrl-seqno.c file, can you please re-arrange the public
functions and
    static functions as per the coding guidelines. Since it is a new file
being added,
   I think it will be helpful so that the code is organized as per the
guidelines.


https://github.com/ovn-org/ovn/blob/master/Documentation/internals/contributing/coding-style.rst
  See section - Functions.

   What do you think ?

Thanks
Numan


> ---
>  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      |  226
> +++++++++++++++++++++++++++++++++++++
>  tests/testsuite.at             |    1
>  8 files changed, 757 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 --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 42883b4..bb1c659 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,
> @@ -2853,17 +2872,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());
>                  }
>
>              }
> @@ -2889,7 +2914,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..59dfea9
> --- /dev/null
> +++ b/tests/ovn-ofctrl-seqno.at
> @@ -0,0 +1,226 @@
> +#
> +# 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_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])
> +
> +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])
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to