Replace route list with vector. 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.
---
 northd/en-group-ecmp-route.c | 38 ++++++++++++++++--------------------
 northd/en-group-ecmp-route.h |  4 ++--
 northd/northd.c              | 11 +++++------
 3 files changed, 24 insertions(+), 29 deletions(-)

diff --git a/northd/en-group-ecmp-route.c b/northd/en-group-ecmp-route.c
index c55bbdd8b..ee1297947 100644
--- a/northd/en-group-ecmp-route.c
+++ b/northd/en-group-ecmp-route.c
@@ -39,11 +39,7 @@ ecmp_groups_node_free(struct ecmp_groups_node *eg)
         return;
     }
 
-    struct ecmp_route_list_node *er;
-    LIST_FOR_EACH_SAFE (er, list_node, &eg->route_list) {
-        ovs_list_remove(&er->list_node);
-        free(er);
-    }
+    vector_destroy(&eg->route_list);
     sset_destroy(&eg->selection_fields);
     free(eg);
 }
@@ -218,9 +214,10 @@ ecmp_groups_add_route(struct ecmp_groups_node *group,
         return;
     }
 
-    struct ecmp_route_list_node *er = xmalloc(sizeof *er);
-    er->route = route;
-    er->id = ++group->route_count;
+    struct ecmp_route_list_node er = (struct ecmp_route_list_node) {
+        .route = route,
+        .id = ++group->route_count,
+    };
 
     if (group->route_count == 1) {
         sset_clone(&group->selection_fields, &route->ecmp_selection_fields);
@@ -229,7 +226,7 @@ ecmp_groups_add_route(struct ecmp_groups_node *group,
                        &route->ecmp_selection_fields);
     }
 
-    ovs_list_insert(&group->route_list, &er->list_node);
+    vector_push(&group->route_list, &er);
 }
 
 /* Removes a route from an ecmp group. If the ecmp group should persist
@@ -240,13 +237,14 @@ ecmp_groups_remove_route(struct ecmp_groups_node *group,
                          const struct parsed_route *pr)
 {
     struct ecmp_route_list_node *er;
-    LIST_FOR_EACH (er, list_node, &group->route_list) {
+    size_t i = 0;
+    VECTOR_FOR_EACH_PTR (&group->route_list, er) {
         if (er->route == pr) {
             const struct parsed_route *found_route = er->route;
-            ovs_list_remove(&er->list_node);
-            free(er);
+            vector_remove_fast(&group->route_list, i, NULL);
             return found_route;
         }
+        i++;
     }
 
     return NULL;
@@ -257,9 +255,8 @@ ecmp_group_update_ids(struct ecmp_groups_node *group)
 {
     struct ecmp_route_list_node *er;
     size_t i = 0;
-    LIST_FOR_EACH (er, list_node, &group->route_list) {
-        er->id = i;
-        i++;
+    VECTOR_FOR_EACH_PTR (&group->route_list, er) {
+        er->id = i++;
     }
     group->route_count = i;
 }
@@ -283,8 +280,8 @@ ecmp_groups_add(struct group_ecmp_datapath *gn,
     eg->is_src_route = route->is_src_route;
     eg->source = route->source;
     eg->route_table_id = route->route_table_id;
+    eg->route_list = VECTOR_EMPTY_INITIALIZER(struct ecmp_route_list_node);
     sset_init(&eg->selection_fields);
-    ovs_list_init(&eg->route_list);
     ecmp_groups_add_route(eg, route);
 
     return eg;
@@ -311,7 +308,7 @@ static bool
 ecmp_group_has_symmetric_reply(struct ecmp_groups_node *eg)
 {
     struct ecmp_route_list_node *er;
-    LIST_FOR_EACH (er, list_node, &eg->route_list) {
+    VECTOR_FOR_EACH_PTR (&eg->route_list, er) {
         if (er->route->ecmp_symmetric_reply) {
             return true;
         }
@@ -430,7 +427,7 @@ handle_deleted_route(struct group_ecmp_route_data *data,
             return false;
         }
 
-        size_t ecmp_members = ovs_list_size(&eg->route_list);
+        size_t ecmp_members = vector_len(&eg->route_list);
         if (ecmp_members == 1) {
             /* The route is the only ecmp member, we remove the whole group. */
             hmap_remove(&node->ecmp_groups, &eg->hmap_node);
@@ -444,9 +441,8 @@ handle_deleted_route(struct group_ecmp_route_data *data,
             if (ecmp_group_has_symmetric_reply(eg)) {
                 ecmp_group_update_ids(eg);
             } else {
-                struct ecmp_route_list_node *er = CONTAINER_OF(
-                    ovs_list_front(&eg->route_list),
-                    struct ecmp_route_list_node, list_node);
+                const struct ecmp_route_list_node *er =
+                    vector_get_ptr(&eg->route_list, 0);
                 unique_routes_add(node, er->route);
                 hmap_remove(&node->ecmp_groups, &eg->hmap_node);
                 ecmp_groups_node_free(eg);
diff --git a/northd/en-group-ecmp-route.h b/northd/en-group-ecmp-route.h
index b85e6014e..a7040addd 100644
--- a/northd/en-group-ecmp-route.h
+++ b/northd/en-group-ecmp-route.h
@@ -20,10 +20,10 @@
 #include "openvswitch/hmap.h"
 #include "openvswitch/list.h"
 #include "northd/northd.h"
+#include "vec.h"
 #include <netinet/in.h>
 
 struct ecmp_route_list_node {
-    struct ovs_list list_node;
     uint16_t id; /* starts from 1 */
     const struct parsed_route *route;
 };
@@ -37,7 +37,7 @@ struct ecmp_groups_node {
     enum route_source source;
     uint32_t route_table_id;
     uint16_t route_count;
-    struct ovs_list route_list; /* Contains ecmp_route_list_node */
+    struct vector route_list; /* Contains ecmp_route_list_node */
     struct sset selection_fields;
 };
 
diff --git a/northd/northd.c b/northd/northd.c
index 9a49fb8f1..c20ddb786 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -11941,7 +11941,7 @@ build_ecmp_route_flow(struct lflow_table *lflows,
 {
     bool is_ipv4_prefix = IN6_IS_ADDR_V4MAPPED(&eg->prefix);
     uint16_t priority;
-    struct ecmp_route_list_node *er;
+    const struct ecmp_route_list_node *er;
     struct ds route_match = DS_EMPTY_INITIALIZER;
 
     char *prefix_s = build_route_prefix_s(&eg->prefix, eg->plen);
@@ -11955,7 +11955,7 @@ build_ecmp_route_flow(struct lflow_table *lflows,
     ds_put_format(&actions, "ip.ttl--; flags.loopback = 1; %s = %"PRIu16
                   "; %s = ", REG_ECMP_GROUP_ID, eg->id, REG_ECMP_MEMBER_ID);
 
-    if (!ovs_list_is_singleton(&eg->route_list)) {
+    if (vector_len(&eg->route_list) > 1) {
         if (protocol && !sset_is_empty(&eg->selection_fields)) {
             ds_put_format(&route_match, " && %s", protocol);
         }
@@ -11963,7 +11963,7 @@ build_ecmp_route_flow(struct lflow_table *lflows,
         struct ds values = DS_EMPTY_INITIALIZER;
         bool is_first = true;
 
-        LIST_FOR_EACH (er, list_node, &eg->route_list) {
+        VECTOR_FOR_EACH_PTR (&eg->route_list, er) {
             if (is_first) {
                 is_first = false;
             } else {
@@ -11997,8 +11997,7 @@ build_ecmp_route_flow(struct lflow_table *lflows,
         ds_put_cstr(&actions, ");");
         ds_destroy(&values);
     } else {
-        er = CONTAINER_OF(ovs_list_front(&eg->route_list),
-                          struct ecmp_route_list_node, list_node);
+        er = vector_get_ptr(&eg->route_list, 0);
         ds_put_format(&actions, "%"PRIu16"; next;", er->id);
     }
 
@@ -12009,7 +12008,7 @@ build_ecmp_route_flow(struct lflow_table *lflows,
     /* Add per member flow */
     struct ds match = DS_EMPTY_INITIALIZER;
     struct sset visited_ports = SSET_INITIALIZER(&visited_ports);
-    LIST_FOR_EACH (er, list_node, &eg->route_list) {
+    VECTOR_FOR_EACH_PTR (&eg->route_list, er) {
         const struct parsed_route *route = er->route;
         bool is_ipv4_nexthop = IN6_IS_ADDR_V4MAPPED(route->nexthop);
         /* Symmetric ECMP reply is only usable on gateway routers.
-- 
2.49.0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to