On Wed, Apr 30, 2025 at 7:54 AM Ales Musil <amu...@redhat.com> wrote:
> Replace several lists in the controller code with vectors. The change
> allows us to avoid small allocations just to keep to have the list
> reference. The vector can store elements directly and allocates in
> batches rather than one by one. This also simplifies the code to some
> extent.
>
> Signed-off-by: Ales Musil <amu...@redhat.com>
> Acked-by: Mark Michelson <mmich...@redhat.com>
> ---
> v4: Rebase on top of current main.
> Add ack from Mark.
> v3: Rebase on top of current main.
> v2: Rebase on top of current main.
> ---
> controller/binding.c | 70 ++++------
> controller/mirror.c | 60 ++++-----
> controller/ofctrl-seqno.c | 193 +++++++++++++---------------
> controller/ofctrl-seqno.h | 11 +-
> controller/ovn-controller.c | 2 +-
> controller/physical.c | 79 ++++--------
> controller/route-exchange-netlink.c | 33 ++---
> controller/route-exchange-netlink.h | 6 +-
> controller/route-exchange.c | 10 +-
> controller/test-ofctrl-seqno.c | 27 ++--
> 10 files changed, 204 insertions(+), 287 deletions(-)
>
> diff --git a/controller/binding.c b/controller/binding.c
> index 9aec6708f..f5091d2cf 100644
> --- a/controller/binding.c
> +++ b/controller/binding.c
> @@ -2168,16 +2168,14 @@ binding_run(struct binding_ctx_in *b_ctx_in,
> struct binding_ctx_out *b_ctx_out)
> build_local_bindings(b_ctx_in, b_ctx_out);
> }
>
> - struct ovs_list localnet_lports =
> OVS_LIST_INITIALIZER(&localnet_lports);
> - struct ovs_list external_lports =
> OVS_LIST_INITIALIZER(&external_lports);
> - struct ovs_list multichassis_ports = OVS_LIST_INITIALIZER(
> -
> &multichassis_ports);
> - struct ovs_list vtep_lports = OVS_LIST_INITIALIZER(&vtep_lports);
> -
> - struct lport {
> - struct ovs_list list_node;
> - const struct sbrec_port_binding *pb;
> - };
> + struct vector localnet_lports =
> + VECTOR_EMPTY_INITIALIZER(struct sbrec_port_binding *);
> + struct vector external_lports =
> + VECTOR_EMPTY_INITIALIZER(struct sbrec_port_binding *);
> + struct vector multichassis_ports =
> + VECTOR_EMPTY_INITIALIZER(struct sbrec_port_binding *);
> + struct vector vtep_lports =
> + VECTOR_EMPTY_INITIALIZER(struct sbrec_port_binding *);
>
> /* Run through each binding record to see if it is resident on this
> * chassis and update the binding accordingly. This includes both
> @@ -2202,9 +2200,7 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct
> binding_ctx_out *b_ctx_out)
>
> case LP_VTEP:
> update_related_lport(pb, b_ctx_out);
> - struct lport *vtep_lport = xmalloc(sizeof *vtep_lport);
> - vtep_lport->pb = pb;
> - ovs_list_push_back(&vtep_lports, &vtep_lport->list_node);
> + vector_push(&vtep_lports, &pb);
> break;
>
> case LP_LOCALPORT:
> @@ -2214,11 +2210,7 @@ binding_run(struct binding_ctx_in *b_ctx_in, struct
> binding_ctx_out *b_ctx_out)
> case LP_VIF:
> consider_vif_lport(pb, b_ctx_in, b_ctx_out, NULL);
> if (pb->additional_chassis) {
> - struct lport *multichassis_lport = xmalloc(
> - sizeof *multichassis_lport);
> - multichassis_lport->pb = pb;
> - ovs_list_push_back(&multichassis_ports,
> - &multichassis_lport->list_node);
> + vector_push(&multichassis_ports, &pb);
> }
> break;
>
> @@ -2248,17 +2240,12 @@ binding_run(struct binding_ctx_in *b_ctx_in,
> struct binding_ctx_out *b_ctx_out)
>
> case LP_EXTERNAL:
> consider_external_lport(pb, b_ctx_in, b_ctx_out);
> - struct lport *ext_lport = xmalloc(sizeof *ext_lport);
> - ext_lport->pb = pb;
> - ovs_list_push_back(&external_lports, &ext_lport->list_node);
> + vector_push(&external_lports, &pb);
> break;
>
> - case LP_LOCALNET: {
> - struct lport *lnet_lport = xmalloc(sizeof *lnet_lport);
> - lnet_lport->pb = pb;
> - ovs_list_push_back(&localnet_lports, &lnet_lport->list_node);
> + case LP_LOCALNET:
> + vector_push(&localnet_lports, &pb);
> break;
> - }
>
> case LP_REMOTE:
> /* Remote ports are related, since we can have rules in the
> @@ -2283,41 +2270,36 @@ binding_run(struct binding_ctx_in *b_ctx_in,
> struct binding_ctx_out *b_ctx_out)
> /* Run through each localnet lport list to see if it is a localnet
> port
> * on local datapaths discovered from above loop, and handles it
> * accordingly. */
> - struct lport *lnet_lport;
> - LIST_FOR_EACH_POP (lnet_lport, list_node, &localnet_lports) {
> - consider_localnet_lport(lnet_lport->pb, b_ctx_out);
> - update_ld_localnet_port(lnet_lport->pb, &bridge_mappings,
> + VECTOR_FOR_EACH (&localnet_lports, pb) {
> + consider_localnet_lport(pb, b_ctx_out);
> + update_ld_localnet_port(pb, &bridge_mappings,
> b_ctx_out->local_datapaths);
> - free(lnet_lport);
> }
> + vector_destroy(&localnet_lports);
>
> /* Run through external lport list to see if there are external ports
> * on local datapaths discovered from above loop, and update the
> * corresponding local datapath accordingly. */
> - struct lport *ext_lport;
> - LIST_FOR_EACH_POP (ext_lport, list_node, &external_lports) {
> - update_ld_external_ports(ext_lport->pb,
> b_ctx_out->local_datapaths);
> - free(ext_lport);
> + VECTOR_FOR_EACH (&external_lports, pb) {
> + update_ld_external_ports(pb, b_ctx_out->local_datapaths);
> }
> + vector_destroy(&external_lports);
>
> /* Run through multichassis lport list to see if there are ports
> * on local datapaths discovered from above loop, and update the
> * corresponding local datapath accordingly. */
> - struct lport *multichassis_lport;
> - LIST_FOR_EACH_POP (multichassis_lport, list_node,
> &multichassis_ports) {
> - update_ld_multichassis_ports(multichassis_lport->pb,
> - b_ctx_out->local_datapaths);
> - free(multichassis_lport);
> + VECTOR_FOR_EACH (&multichassis_ports, pb) {
> + update_ld_multichassis_ports(pb, b_ctx_out->local_datapaths);
> }
> + vector_destroy(&multichassis_ports);
>
> /* Run through vtep lport list to see if there are vtep ports
> * on local datapaths discovered from above loop, and update the
> * corresponding local datapath accordingly. */
> - struct lport *vtep_lport;
> - LIST_FOR_EACH_POP (vtep_lport, list_node, &vtep_lports) {
> - update_ld_vtep_port(vtep_lport->pb, b_ctx_out->local_datapaths);
> - free(vtep_lport);
> + VECTOR_FOR_EACH (&vtep_lports, pb) {
> + update_ld_vtep_port(pb, b_ctx_out->local_datapaths);
> }
> + vector_destroy(&vtep_lports);
>
> shash_destroy(&bridge_mappings);
> /* Remove stale QoS entries. */
> diff --git a/controller/mirror.c b/controller/mirror.c
> index 2f1c811a0..62b4ac98a 100644
> --- a/controller/mirror.c
> +++ b/controller/mirror.c
> @@ -30,6 +30,7 @@
> #include "binding.h"
> #include "lib/ovn-sb-idl.h"
> #include "mirror.h"
> +#include "vec.h"
>
> VLOG_DEFINE_THIS_MODULE(port_mirror);
>
> @@ -37,14 +38,8 @@ struct ovn_mirror {
> char *name;
> const struct sbrec_mirror *sb_mirror;
> const struct ovsrec_mirror *ovs_mirror;
> - struct ovs_list mirror_src_lports;
> - struct ovs_list mirror_dst_lports;
> -};
> -
> -struct mirror_lport {
> - struct ovs_list list_node;
> -
> - struct local_binding *lbinding;
> + struct vector mirror_src_lports; /* Vector of struct local_binding *.
> */
> + struct vector mirror_dst_lports; /* Vector of struct local_binding *.
> */
> };
>
> static struct ovn_mirror *ovn_mirror_create(char *mirror_name);
> @@ -238,8 +233,8 @@ ovn_mirror_create(char *mirror_name)
> {
> struct ovn_mirror *m = xzalloc(sizeof *m);
> m->name = xstrdup(mirror_name);
> - ovs_list_init(&m->mirror_src_lports);
> - ovs_list_init(&m->mirror_dst_lports);
> + m->mirror_src_lports = VECTOR_EMPTY_INITIALIZER(struct local_binding
> *);
> + m->mirror_dst_lports = VECTOR_EMPTY_INITIALIZER(struct local_binding
> *);
> return m;
> }
>
> @@ -259,15 +254,8 @@ static void
> ovn_mirror_delete(struct ovn_mirror *m)
> {
> free(m->name);
> - struct mirror_lport *m_lport;
> - LIST_FOR_EACH_POP (m_lport, list_node, &m->mirror_src_lports) {
> - free(m_lport);
> - }
> -
> - LIST_FOR_EACH_POP (m_lport, list_node, &m->mirror_dst_lports) {
> - free(m_lport);
> - }
> -
> + vector_destroy(&m->mirror_src_lports);
> + vector_destroy(&m->mirror_dst_lports);
> free(m);
> }
>
> @@ -276,16 +264,12 @@ ovn_mirror_add_lport(struct ovn_mirror *m, struct
> local_binding *lbinding)
> {
> if (!strcmp(m->sb_mirror->filter, "from-lport") ||
> !strcmp(m->sb_mirror->filter, "both")) {
> - struct mirror_lport *m_lport = xzalloc(sizeof *m_lport);
> - m_lport->lbinding = lbinding;
> - ovs_list_push_back(&m->mirror_src_lports, &m_lport->list_node);
> + vector_push(&m->mirror_src_lports, &lbinding);
> }
>
> if (!strcmp(m->sb_mirror->filter, "to-lport") ||
> !strcmp(m->sb_mirror->filter, "both")) {
> - struct mirror_lport *m_lport = xzalloc(sizeof *m_lport);
> - m_lport->lbinding = lbinding;
> - ovs_list_push_back(&m->mirror_dst_lports, &m_lport->list_node);
> + vector_push(&m->mirror_dst_lports, &lbinding);
> }
> }
>
> @@ -350,8 +334,8 @@ sync_ovn_mirror(struct ovn_mirror *m, struct
> ovsdb_idl_txn *ovs_idl_txn,
> return;
> }
>
> - if (ovs_list_is_empty(&m->mirror_src_lports) &&
> - ovs_list_is_empty(&m->mirror_dst_lports)) {
> + if (vector_is_empty(&m->mirror_src_lports) &&
> + vector_is_empty(&m->mirror_dst_lports)) {
> /* Nothing to do. */
> return;
> }
> @@ -399,8 +383,8 @@ should_delete_ovs_mirror(struct ovn_mirror *m)
> return true;
> }
>
> - return (ovs_list_is_empty(&m->mirror_src_lports) &&
> - ovs_list_is_empty(&m->mirror_dst_lports));
> + return (vector_is_empty(&m->mirror_src_lports) &&
> + vector_is_empty(&m->mirror_dst_lports));
> }
>
> static const struct ovsrec_port *
> @@ -474,18 +458,18 @@ create_ovs_mirror(struct ovn_mirror *m, struct
> ovsdb_idl_txn *ovs_idl_txn,
> static void
> sync_ovs_mirror_ports(struct ovn_mirror *m, const struct ovsrec_bridge
> *br_int)
> {
> - struct mirror_lport *m_lport;
> + struct local_binding *lbinding;
>
> - if (ovs_list_is_empty(&m->mirror_src_lports)) {
> + if (vector_is_empty(&m->mirror_src_lports)) {
> ovsrec_mirror_set_select_src_port(m->ovs_mirror, NULL, 0);
> } else {
> - size_t n_lports = ovs_list_size(&m->mirror_src_lports);
> + size_t n_lports = vector_len(&m->mirror_src_lports);
> struct ovsrec_port **ovs_ports = xmalloc(sizeof *ovs_ports *
> n_lports);
>
> size_t i = 0;
> - LIST_FOR_EACH (m_lport, list_node, &m->mirror_src_lports) {
> + VECTOR_FOR_EACH (&m->mirror_src_lports, lbinding) {
> const struct ovsrec_port *p =
> - get_iface_port(m_lport->lbinding->iface, br_int);
> + get_iface_port(lbinding->iface, br_int);
> ovs_assert(p);
> ovs_ports[i++] = (struct ovsrec_port *) p;
> }
> @@ -494,16 +478,16 @@ sync_ovs_mirror_ports(struct ovn_mirror *m, const
> struct ovsrec_bridge *br_int)
> free(ovs_ports);
> }
>
> - if (ovs_list_is_empty(&m->mirror_dst_lports)) {
> + if (vector_is_empty(&m->mirror_dst_lports)) {
> ovsrec_mirror_set_select_dst_port(m->ovs_mirror, NULL, 0);
> } else {
> - size_t n_lports = ovs_list_size(&m->mirror_dst_lports);
> + size_t n_lports = vector_len(&m->mirror_dst_lports);
> struct ovsrec_port **ovs_ports = xmalloc(sizeof *ovs_ports *
> n_lports);
>
> size_t i = 0;
> - LIST_FOR_EACH (m_lport, list_node, &m->mirror_dst_lports) {
> + VECTOR_FOR_EACH (&m->mirror_dst_lports, lbinding) {
> const struct ovsrec_port *p =
> - get_iface_port(m_lport->lbinding->iface, br_int);
> + get_iface_port(lbinding->iface, br_int);
> ovs_assert(p);
> ovs_ports[i++] = (struct ovsrec_port *) p;
> }
> diff --git a/controller/ofctrl-seqno.c b/controller/ofctrl-seqno.c
> index 95373aa57..83c17c0e5 100644
> --- a/controller/ofctrl-seqno.c
> +++ b/controller/ofctrl-seqno.c
> @@ -19,13 +19,15 @@
> #include "ofctrl-seqno.h"
> #include "openvswitch/list.h"
> #include "util.h"
> +#include "vec.h"
> +
> +#define VECTOR_THRESHOLD 1024
>
> /* 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'.
> */
> @@ -37,14 +39,15 @@ struct ofctrl_seqno_update {
> };
>
> /* List of in flight sequence number updates. */
> -static struct ovs_list ofctrl_seqno_updates;
> +static struct vector ofctrl_seqno_updates =
> + VECTOR_EMPTY_INITIALIZER(struct ofctrl_seqno_update);
>
> /* 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
> + struct vector acked_cfgs; /* Acked requests since the last time the
> * application consumed acked requests.
> */
> uint64_t cur_cfg; /* Last acked application seqno. */
> @@ -52,20 +55,11 @@ struct ofctrl_seqno_state {
> };
>
> /* Per application seqno type states. */
> -static size_t n_ofctrl_seqno_states;
> -static struct ofctrl_seqno_state *ofctrl_seqno_states;
> -
> -/* ofctrl_acked_seqnos related static function prototypes. */
> -static void ofctrl_acked_seqnos_init(struct ofctrl_acked_seqnos *seqnos,
> - uint64_t last_acked);
> -static void ofctrl_acked_seqnos_add(struct ofctrl_acked_seqnos *seqnos,
> - uint64_t val);
> +static struct vector ofctrl_seqno_states =
> + VECTOR_EMPTY_INITIALIZER(struct ofctrl_seqno_state);
>
> -/* ofctrl_seqno_update related static function prototypes. */
> -static void ofctrl_seqno_update_create__(size_t seqno_type, uint64_t
> req_cfg);
> -static void ofctrl_seqno_update_list_destroy(struct ovs_list *seqno_list);
> -static void ofctrl_seqno_cfg_run(size_t seqno_type,
> - struct ofctrl_seqno_update *update);
> +/* ofctrl_seqno_state related static function prototypes. */
> +static struct ofctrl_seqno_state *ofctrl_seqno_state_get(size_t
> seqno_type);
>
> /* Returns the collection of acked ofctrl_seqno_update requests of type
> * 'seqno_type'. It's the responsibility of the caller to free memory by
> @@ -74,17 +68,17 @@ static void ofctrl_seqno_cfg_run(size_t seqno_type,
> 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;
> + struct ofctrl_seqno_state *state = ofctrl_seqno_state_get(seqno_type);
>
> - ofctrl_acked_seqnos_init(acked_seqnos, state->cur_cfg);
> + struct ofctrl_acked_seqnos *acked_seqnos = xmalloc(sizeof
> *acked_seqnos);
> + acked_seqnos->acked = vector_clone(&state->acked_cfgs);
> + acked_seqnos->last_acked = 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);
> + vector_clear(&state->acked_cfgs);
> + if (vector_capacity(&state->acked_cfgs) >= VECTOR_THRESHOLD) {
> + vector_shrink_to_fit(&state->acked_cfgs);
> }
> +
> return acked_seqnos;
> }
>
> @@ -95,11 +89,7 @@ ofctrl_acked_seqnos_destroy(struct ofctrl_acked_seqnos
> *seqnos)
> return;
> }
>
> - struct ofctrl_ack_seqno *seqno_node;
> - HMAP_FOR_EACH_POP (seqno_node, node, &seqnos->acked) {
> - free(seqno_node);
> - }
> - hmap_destroy(&seqnos->acked);
> + vector_destroy(&seqnos->acked);
> free(seqnos);
> }
>
> @@ -108,40 +98,52 @@ bool
> ofctrl_acked_seqnos_contains(const struct ofctrl_acked_seqnos *seqnos,
> uint64_t val)
> {
> - struct ofctrl_ack_seqno *sn;
> + if (vector_is_empty(&seqnos->acked)) {
> + return false;
> + }
> +
> + size_t low = 0;
> + size_t high = vector_len(&seqnos->acked) -1;
> + uint64_t *acked = vector_get_array(&seqnos->acked);
>
> - HMAP_FOR_EACH_WITH_HASH (sn, node, hash_uint64(val), &seqnos->acked) {
> - if (sn->seqno == val) {
> + while (low <= high) {
> + size_t mid = low + (high - low) / 2;
> +
> + if (acked[mid] == val) {
> return true;
> }
> +
> + if (acked[mid] >= acked[low]) {
> + if (val >= acked[low] && val < acked[mid]) {
> + high = mid - 1;
> + } else {
> + low = mid + 1;
> + }
> + } else {
> + if (val > acked[mid] && val <= acked[high]) {
> + low = mid + 1;
> + } else {
> + high = mid - 1;
> + }
> + }
> }
> - return false;
> -}
>
> -void
> -ofctrl_seqno_init(void)
> -{
> - ovs_list_init(&ofctrl_seqno_updates);
> + return false;
> }
>
> /* 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);
> + size_t new_type = vector_len(&ofctrl_seqno_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);
> + struct ofctrl_seqno_state state = (struct ofctrl_seqno_state) {
> + .acked_cfgs = VECTOR_EMPTY_INITIALIZER(uint64_t),
> + .cur_cfg = 0,
> + .req_cfg = 0,
> + };
> + vector_push(&ofctrl_seqno_states, &state);
>
> - free(ofctrl_seqno_states);
> - ofctrl_seqno_states = new_states;
> return new_type;
> }
>
> @@ -151,9 +153,7 @@ ofctrl_seqno_add_type(void)
> 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];
> + struct ofctrl_seqno_state *state = ofctrl_seqno_state_get(seqno_type);
>
> /* If new_cfg didn't change since the last request there should
> already
> * be an update pending.
> @@ -163,7 +163,14 @@ ofctrl_seqno_update_create(size_t seqno_type,
> uint64_t new_cfg)
> }
>
> state->req_cfg = new_cfg;
> - ofctrl_seqno_update_create__(seqno_type, new_cfg);
> +
> + ofctrl_req_seqno++;
> + struct ofctrl_seqno_update update = (struct ofctrl_seqno_update) {
> + .seqno_type = seqno_type,
> + .flow_cfg = ofctrl_req_seqno,
> + .req_cfg = new_cfg,
> + };
> + vector_push(&ofctrl_seqno_updates, &update);
> }
>
> /* Should be called when the application is certain that all OVS flow
> updates
> @@ -173,14 +180,25 @@ ofctrl_seqno_update_create(size_t seqno_type,
> uint64_t new_cfg)
> void
> ofctrl_seqno_run(uint64_t flow_cfg)
> {
> + size_t index = 0;
> +
> struct ofctrl_seqno_update *update;
> - LIST_FOR_EACH_SAFE (update, list_node, &ofctrl_seqno_updates) {
> + VECTOR_FOR_EACH_PTR (&ofctrl_seqno_updates, update) {
> if (flow_cfg < update->flow_cfg) {
> break;
> }
> + struct ofctrl_seqno_state *state =
> + ofctrl_seqno_state_get(update->seqno_type);
> + state->cur_cfg = update->req_cfg;
> + vector_push(&state->acked_cfgs, &update->req_cfg);
> +
> + index++;
> + }
> +
> + vector_remove_block(&ofctrl_seqno_updates, 0, index);
>
> - ovs_list_remove(&update->list_node);
> - ofctrl_seqno_cfg_run(update->seqno_type, update);
> + if (vector_capacity(&ofctrl_seqno_updates) >= VECTOR_THRESHOLD) {
> + vector_shrink_to_fit(&ofctrl_seqno_updates);
> }
> }
>
> @@ -197,58 +215,31 @@ ofctrl_seqno_get_req_cfg(void)
> void
> ofctrl_seqno_flush(void)
> {
> - for (size_t i = 0; i < n_ofctrl_seqno_states; i++) {
> -
> ofctrl_seqno_update_list_destroy(&ofctrl_seqno_states[i].acked_cfgs);
> - }
> - ofctrl_seqno_update_list_destroy(&ofctrl_seqno_updates);
> - ofctrl_req_seqno = 0;
> -}
> -
> -static void
> -ofctrl_acked_seqnos_init(struct ofctrl_acked_seqnos *seqnos,
> - uint64_t last_acked)
> -{
> - hmap_init(&seqnos->acked);
> - seqnos->last_acked = last_acked;
> -}
> + vector_clear(&ofctrl_seqno_updates);
>
> -static void
> -ofctrl_acked_seqnos_add(struct ofctrl_acked_seqnos *seqnos, uint64_t val)
> -{
> - seqnos->last_acked = val;
> -
> - struct ofctrl_ack_seqno *sn = xmalloc(sizeof *sn);
> - hmap_insert(&seqnos->acked, &sn->node, hash_uint64(val));
> - sn->seqno = val;
> -}
> -
> -static void
> -ofctrl_seqno_update_create__(size_t seqno_type, uint64_t req_cfg)
> -{
> - struct ofctrl_seqno_update *update = xmalloc(sizeof *update);
> + struct ofctrl_seqno_state *state;
> + VECTOR_FOR_EACH_PTR (&ofctrl_seqno_states, state) {
> + vector_clear(&state->acked_cfgs);
> + }
>
> - 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;
> + ofctrl_req_seqno = 0;
> }
>
> -static void
> -ofctrl_seqno_update_list_destroy(struct ovs_list *seqno_list)
> +void
> +ofctrl_seqno_destroy(void)
> {
> - struct ofctrl_seqno_update *update;
> + vector_destroy(&ofctrl_seqno_updates);
>
> - LIST_FOR_EACH_POP (update, list_node, seqno_list) {
> - free(update);
> + struct ofctrl_seqno_state *state;
> + VECTOR_FOR_EACH_PTR (&ofctrl_seqno_states, state) {
> + vector_destroy(&state->acked_cfgs);
> }
> + vector_destroy(&ofctrl_seqno_states);
> }
>
> -static void
> -ofctrl_seqno_cfg_run(size_t seqno_type, struct ofctrl_seqno_update
> *update)
> +static struct ofctrl_seqno_state *
> +ofctrl_seqno_state_get(size_t seqno_type)
> {
> - 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;
> + ovs_assert(seqno_type < vector_len(&ofctrl_seqno_states));
> + return vector_get_ptr(&ofctrl_seqno_states, seqno_type);
> }
> diff --git a/controller/ofctrl-seqno.h b/controller/ofctrl-seqno.h
> index adcc5a0d3..faa97cc53 100644
> --- a/controller/ofctrl-seqno.h
> +++ b/controller/ofctrl-seqno.h
> @@ -19,31 +19,26 @@
> #include <stdint.h>
>
> #include <openvswitch/hmap.h>
> +#include "vec.h"
>
> /* Collection of acked ofctrl_seqno_update requests and the most recent
> * 'last_acked' value.
> */
> struct ofctrl_acked_seqnos {
> - struct hmap acked;
> + struct vector 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,
> uint64_t val);
>
> -void ofctrl_seqno_init(void);
> size_t ofctrl_seqno_add_type(void);
> void ofctrl_seqno_update_create(size_t seqno_type, uint64_t new_cfg);
> void ofctrl_seqno_run(uint64_t flow_cfg);
> uint64_t ofctrl_seqno_get_req_cfg(void);
> void ofctrl_seqno_flush(void);
> +void ofctrl_seqno_destroy(void);
>
> #endif /* controller/ofctrl-seqno.h */
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 180b8a8d2..1cf5bb45f 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -5926,7 +5926,6 @@ main(int argc, char *argv[])
>
> ofctrl_init(&lflow_output_data->group_table,
> &lflow_output_data->meter_table);
> - ofctrl_seqno_init();
>
> unixctl_command_register("group-table-list", "", 0, 0,
> extend_table_list,
> @@ -6640,6 +6639,7 @@ loop_done:
> free(ovn_version);
> lflow_destroy();
> ofctrl_destroy();
> + ofctrl_seqno_destroy();
> pinctrl_destroy();
> binding_destroy();
> patch_destroy();
> diff --git a/controller/physical.c b/controller/physical.c
> index a17d792c3..65b4c7335 100644
> --- a/controller/physical.c
> +++ b/controller/physical.c
> @@ -64,11 +64,6 @@ struct zone_ids {
> int snat; /* MFF_LOG_SNAT_ZONE. */
> };
>
> -struct tunnel {
> - struct ovs_list list_node;
> - const struct chassis_tunnel *tun;
> -};
> -
> static void
> load_logical_ingress_metadata(const struct sbrec_port_binding *binding,
> const struct zone_ids *zone_ids,
> @@ -415,16 +410,14 @@ find_additional_encap_for_chassis(const struct
> sbrec_port_binding *pb,
> return NULL;
> }
>
> -static struct ovs_list *
> +static struct vector
> get_remote_tunnels(const struct sbrec_port_binding *binding,
> const struct physical_ctx *ctx,
> const char *local_encap_ip)
> {
> + struct vector tunnels =
> + VECTOR_EMPTY_INITIALIZER(const struct chassis_tunnel *);
> const struct chassis_tunnel *tun;
> -
> - struct ovs_list *tunnels = xmalloc(sizeof *tunnels);
> - ovs_list_init(tunnels);
> -
> if (binding->chassis && binding->chassis != ctx->chassis) {
> tun = get_port_binding_tun(binding->encap, binding->chassis,
> ctx->chassis_tunnels, local_encap_ip);
> @@ -435,9 +428,7 @@ get_remote_tunnels(const struct sbrec_port_binding
> *binding,
> "for port %s.",
> binding->chassis->name, binding->logical_port);
> } else {
> - struct tunnel *tun_elem = xmalloc(sizeof *tun_elem);
> - tun_elem->tun = tun;
> - ovs_list_push_back(tunnels, &tun_elem->list_node);
> + vector_push(&tunnels, &tun);
> }
> }
>
> @@ -459,9 +450,7 @@ get_remote_tunnels(const struct sbrec_port_binding
> *binding,
> binding->additional_chassis[i]->name,
> binding->logical_port);
> continue;
> }
> - struct tunnel *tun_elem = xmalloc(sizeof *tun_elem);
> - tun_elem->tun = tun;
> - ovs_list_push_back(tunnels, &tun_elem->list_node);
> + vector_push(&tunnels, &tun);
> }
> return tunnels;
> }
> @@ -482,8 +471,8 @@ put_remote_port_redirect_overlay(const struct
> sbrec_port_binding *binding,
>
> match_set_reg_masked(match, MFF_LOG_ENCAP_ID - MFF_REG0, i << 16,
> (uint32_t) 0xFFFF << 16);
> - struct ovs_list *tuns = get_remote_tunnels(binding, ctx,
> encap_ip);
> - if (!ovs_list_is_empty(tuns)) {
> + struct vector tuns = get_remote_tunnels(binding, ctx, encap_ip);
> + if (!vector_is_empty(&tuns)) {
> bool is_vtep_port = type == LP_VTEP;
> /* rewrite MFF_IN_PORT to bypass OpenFlow loopback check for
> ARP/ND
> * responder in L3 networks. */
> @@ -492,24 +481,18 @@ put_remote_port_redirect_overlay(const struct
> sbrec_port_binding *binding,
> ofpacts_clone);
> }
>
> - struct tunnel *tun;
> - LIST_FOR_EACH (tun, list_node, tuns) {
> - put_encapsulation(ctx->mff_ovn_geneve, tun->tun,
> - binding->datapath, port_key,
> is_vtep_port,
> - ofpacts_clone);
> - ofpact_put_OUTPUT(ofpacts_clone)->port = tun->tun->ofport;
> + const struct chassis_tunnel *tun;
> + VECTOR_FOR_EACH (&tuns, tun) {
> + put_encapsulation(ctx->mff_ovn_geneve, tun,
> binding->datapath,
> + port_key, is_vtep_port, ofpacts_clone);
> + ofpact_put_OUTPUT(ofpacts_clone)->port = tun->ofport;
> }
> put_resubmit(OFTABLE_LOCAL_OUTPUT, ofpacts_clone);
> ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 100,
> binding->header_.uuid.parts[0], match,
> ofpacts_clone, &binding->header_.uuid);
> }
> - struct tunnel *tun_elem;
> - LIST_FOR_EACH_POP (tun_elem, list_node, tuns) {
> - free(tun_elem);
> - }
> - free(tuns);
> -
> + vector_destroy(&tuns);
> ofpbuf_delete(ofpacts_clone);
> }
> }
> @@ -1612,7 +1595,7 @@ get_tunnel_overhead(struct chassis_tunnel const *tun)
>
> static uint16_t
> get_effective_mtu(const struct sbrec_port_binding *mcp,
> - struct ovs_list *remote_tunnels,
> + struct vector *remote_tunnels,
> const struct if_status_mgr *if_mgr)
> {
> /* Use interface MTU as a base for calculation */
> @@ -1624,9 +1607,9 @@ get_effective_mtu(const struct sbrec_port_binding
> *mcp,
>
> /* Iterate over all peer tunnels and find the biggest tunnel overhead
> */
> uint16_t overhead = 0;
> - struct tunnel *tun;
> - LIST_FOR_EACH (tun, list_node, remote_tunnels) {
> - overhead = MAX(overhead, get_tunnel_overhead(tun->tun));
> + const struct chassis_tunnel *tun;
> + VECTOR_FOR_EACH (remote_tunnels, tun) {
> + overhead = MAX(overhead, get_tunnel_overhead(tun));
> }
> if (!overhead) {
> return 0;
> @@ -1656,7 +1639,7 @@ handle_pkt_too_big_for_ip_version(struct
> ovn_desired_flow_table *flow_table,
>
> static void
> handle_pkt_too_big(struct ovn_desired_flow_table *flow_table,
> - struct ovs_list *remote_tunnels,
> + struct vector *remote_tunnels,
> const struct sbrec_port_binding *binding,
> const struct sbrec_port_binding *mcp,
> const struct if_status_mgr *if_mgr)
> @@ -1681,9 +1664,9 @@ enforce_tunneling_for_multichassis_ports(
> return;
> }
>
> - struct ovs_list *tuns = get_remote_tunnels(binding, ctx, NULL);
> - if (ovs_list_is_empty(tuns)) {
> - free(tuns);
> + struct vector tuns = get_remote_tunnels(binding, ctx, NULL);
> + if (vector_is_empty(&tuns)) {
> + vector_destroy(&tuns);
> return;
> }
>
> @@ -1708,26 +1691,20 @@ enforce_tunneling_for_multichassis_ports(
> match_outport_dp_and_port_keys(&match, dp_key, port_key);
> match_set_reg(&match, MFF_LOG_INPORT - MFF_REG0, mcp->tunnel_key);
>
> - struct tunnel *tun;
> - LIST_FOR_EACH (tun, list_node, tuns) {
> - put_encapsulation(ctx->mff_ovn_geneve, tun->tun,
> - binding->datapath, port_key, is_vtep_port,
> - &ofpacts);
> - ofpact_put_OUTPUT(&ofpacts)->port = tun->tun->ofport;
> + const struct chassis_tunnel *tun;
> + VECTOR_FOR_EACH (&tuns, tun) {
> + put_encapsulation(ctx->mff_ovn_geneve, tun, binding->datapath,
> + port_key, is_vtep_port, &ofpacts);
> + ofpact_put_OUTPUT(&ofpacts)->port = tun->ofport;
> }
> ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 110,
> binding->header_.uuid.parts[0], &match, &ofpacts,
> &binding->header_.uuid);
> ofpbuf_uninit(&ofpacts);
>
> - handle_pkt_too_big(flow_table, tuns, binding, mcp, ctx->if_mgr);
> - }
> -
> - struct tunnel *tun_elem;
> - LIST_FOR_EACH_POP (tun_elem, list_node, tuns) {
> - free(tun_elem);
> + handle_pkt_too_big(flow_table, &tuns, binding, mcp, ctx->if_mgr);
> }
> - free(tuns);
> + vector_destroy(&tuns);
> }
>
> static void
> diff --git a/controller/route-exchange-netlink.c
> b/controller/route-exchange-netlink.c
> index ad0c25989..83deb13e6 100644
> --- a/controller/route-exchange-netlink.c
> +++ b/controller/route-exchange-netlink.c
> @@ -31,6 +31,7 @@
> #include "ovn-util.h"
> #include "route-table.h"
> #include "route.h"
> +#include "vec.h"
>
> #include "route-exchange-netlink.h"
>
> @@ -188,19 +189,10 @@ re_nl_delete_route(uint32_t table_id, const struct
> in6_addr *dst,
> return modify_route(RTM_DELROUTE, 0, table_id, dst, plen, priority);
> }
>
> -void
> -re_nl_learned_routes_destroy(struct ovs_list *learned_routes)
> -{
> - struct re_nl_received_route_node *rr;
> - LIST_FOR_EACH_POP (rr, list_node, learned_routes) {
> - free(rr);
> - }
> -}
> -
> struct route_msg_handle_data {
> const struct sbrec_datapath_binding *db;
> struct hmapx *routes_to_advertise;
> - struct ovs_list *learned_routes;
> + struct vector *learned_routes;
> const struct hmap *routes;
> };
>
> @@ -227,14 +219,17 @@ handle_route_msg(const struct route_table_msg *msg,
> void *data)
> * As we just want to learn remote routes we do not need
> it.*/
> continue;
> }
> - struct re_nl_received_route_node *rr = xmalloc(sizeof *rr);
> - rr->db = handle_data->db;
> - rr->prefix = rd->rta_dst;
> - rr->plen = rd->rtm_dst_len;
> - rr->nexthop = nexthop->addr;
> - memcpy(rr->ifname, nexthop->ifname, IFNAMSIZ);
> - rr->ifname[IFNAMSIZ] = 0;
> - ovs_list_push_back(handle_data->learned_routes,
> &rr->list_node);
> + struct re_nl_received_route_node rr;
> + rr = (struct re_nl_received_route_node) {
> + .db = handle_data->db,
> + .prefix = rd->rta_dst,
> + .plen = rd->rtm_dst_len,
> + .nexthop = nexthop->addr,
> + };
> + memcpy(rr.ifname, nexthop->ifname, IFNAMSIZ);
> + rr.ifname[IFNAMSIZ] = '\0';
> +
> + vector_push(handle_data->learned_routes, &rr);
> }
> return;
> }
> @@ -266,7 +261,7 @@ handle_route_msg(const struct route_table_msg *msg,
> void *data)
>
> void
> re_nl_sync_routes(uint32_t table_id, const struct hmap *routes,
> - struct ovs_list *learned_routes,
> + struct vector *learned_routes,
> const struct sbrec_datapath_binding *db)
> {
> struct hmapx routes_to_advertise =
> HMAPX_INITIALIZER(&routes_to_advertise);
> diff --git a/controller/route-exchange-netlink.h
> b/controller/route-exchange-netlink.h
> index de59d87a5..e37cfe711 100644
> --- a/controller/route-exchange-netlink.h
> +++ b/controller/route-exchange-netlink.h
> @@ -19,7 +19,6 @@
> #define ROUTE_EXCHANGE_NETLINK_H 1
>
> #include <stdint.h>
> -#include "openvswitch/list.h"
> #include <netinet/in.h>
> #include <net/if.h>
>
> @@ -31,10 +30,10 @@
>
> struct in6_addr;
> struct hmap;
> +struct vector;
>
> struct re_nl_received_route_node {
> const struct sbrec_datapath_binding *db;
> - struct ovs_list list_node;
> struct in6_addr prefix;
> unsigned int plen;
> struct in6_addr nexthop;
> @@ -52,9 +51,8 @@ int re_nl_delete_route(uint32_t table_id, const struct
> in6_addr *dst,
>
> void re_nl_dump(uint32_t table_id);
>
> -void re_nl_learned_routes_destroy(struct ovs_list *learned_routes);
> void re_nl_sync_routes(uint32_t table_id, const struct hmap *routes,
> - struct ovs_list *learned_routes,
> + struct vector *learned_routes,
> const struct sbrec_datapath_binding *db);
>
> void re_nl_cleanup_routes(uint32_t table_id);
> diff --git a/controller/route-exchange.c b/controller/route-exchange.c
> index 98ce93d43..79a046f60 100644
> --- a/controller/route-exchange.c
> +++ b/controller/route-exchange.c
> @@ -132,7 +132,7 @@ route_lookup(struct hmap *route_map,
> }
>
> static void
> -sb_sync_learned_routes(const struct ovs_list *learned_routes,
> +sb_sync_learned_routes(const struct vector *learned_routes,
> const struct sbrec_datapath_binding *datapath,
> const struct smap *bound_ports,
> struct ovsdb_idl_txn *ovnsb_idl_txn,
> @@ -160,7 +160,7 @@ sb_sync_learned_routes(const struct ovs_list
> *learned_routes,
> sbrec_learned_route_index_destroy_row(filter);
>
> struct re_nl_received_route_node *learned_route;
> - LIST_FOR_EACH (learned_route, list_node, learned_routes) {
> + VECTOR_FOR_EACH_PTR (learned_routes, learned_route) {
> char *ip_prefix = normalize_v46_prefix(&learned_route->prefix,
> learned_route->plen);
> char *nexthop = normalize_v46(&learned_route->nexthop);
> @@ -240,8 +240,8 @@ route_exchange_run(const struct route_exchange_ctx_in
> *r_ctx_in,
>
> maintained_route_table_add(table_id);
>
> - struct ovs_list received_routes =
> - OVS_LIST_INITIALIZER(&received_routes);
> + struct vector received_routes =
> + VECTOR_EMPTY_INITIALIZER(struct re_nl_received_route_node);
>
> re_nl_sync_routes(ad->db->tunnel_key, &ad->routes,
> &received_routes, ad->db);
> @@ -254,7 +254,7 @@ route_exchange_run(const struct route_exchange_ctx_in
> *r_ctx_in,
> route_table_add_watch_request(&r_ctx_out->route_table_watches,
> table_id);
>
> - re_nl_learned_routes_destroy(&received_routes);
> + vector_destroy(&received_routes);
> }
>
> /* Remove routes in tables previously maintained by us. */
> diff --git a/controller/test-ofctrl-seqno.c
> b/controller/test-ofctrl-seqno.c
> index cd3821e04..7d478c033 100644
> --- a/controller/test-ofctrl-seqno.c
> +++ b/controller/test-ofctrl-seqno.c
> @@ -22,12 +22,6 @@
>
> #include "ofctrl-seqno.h"
>
> -static void
> -test_init(void)
> -{
> - ofctrl_seqno_init();
> -}
> -
> static int
> test_seqno_compare(size_t a, size_t b, void *values_)
> {
> @@ -55,20 +49,20 @@ test_dump_acked_seqnos(size_t 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);
> + size_t n_acked = vector_len(&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;
> +
> + uint64_t ack_seqno;
> + VECTOR_FOR_EACH (&acked_seqnos->acked, ack_seqno) {
> + ovs_assert(ofctrl_acked_seqnos_contains(acked_seqnos, ack_seqno));
> + total_seqno += ack_seqno;
> + acked[i++] = ack_seqno;
> }
> ovs_assert(!ofctrl_acked_seqnos_contains(acked_seqnos, total_seqno));
>
> @@ -87,14 +81,14 @@ 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());
> }
> +
> + ofctrl_seqno_destroy();
> }
>
> static void
> @@ -105,7 +99,6 @@ test_ofctrl_seqno_ack_seqnos(struct ovs_cmdl_context
> *ctx)
> 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)) {
> @@ -157,6 +150,8 @@ test_ofctrl_seqno_ack_seqnos(struct ovs_cmdl_context
> *ctx)
> test_dump_acked_seqnos(st);
> }
> }
> +
> + ofctrl_seqno_destroy();
> }
>
> static void
> --
> 2.49.0
>
>
Recheck-request: github-robot
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
