We already determine the outport of a parsed_route in parsed_routes_add.
However we previously ignored it and recalculated it later on.

Now we just store it for later usage.

Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud>
---
 v1->v2:
   * fix memory leak
   * fix incorrect comparison of struct parsed_route

 lib/ovn-util.h  | 15 +++++++++
 northd/northd.c | 85 +++++++++++++++++++++++++------------------------
 northd/northd.h |  2 ++
 3 files changed, 61 insertions(+), 41 deletions(-)

diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 7b98b9b9a..2e2f31a36 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -484,4 +484,19 @@ void ovn_exit_args_finish(struct ovn_exit_args *exit_args);
 bool ovn_update_swconn_at(struct rconn *swconn, const char *target,
                           int probe_interval, const char *where);
 
+/* Return true if both strings are either NULL or equal to each other */
+static inline bool
+streq(const char *s1, const char *s2)
+{
+    if (s1) {
+        if (s2 && !strcmp(s1, s2)) {
+            return true;
+        }
+    } else if (!s2) {
+        return true;
+    }
+    return false;
+}
+
+
 #endif /* OVN_UTIL_H */
diff --git a/northd/northd.c b/northd/northd.c
index 8ba7b8c33..b208a3617 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -11066,12 +11066,29 @@ parsed_route_lookup(struct hmap *routes, size_t hash,
             continue;
         }
 
+        if (pr->out_port != new_pr->out_port) {
+            continue;
+        }
+
+        if (!streq(pr->lrp_addr_s, new_pr->lrp_addr_s)) {
+            continue;
+        }
+
         return pr;
     }
 
     return NULL;
 }
 
+static void
+parsed_route_free(struct parsed_route *pr) {
+    if (pr->lrp_addr_s) {
+        free(pr->lrp_addr_s);
+    }
+
+    free(pr);
+}
+
 static void
 parsed_routes_add(struct ovn_datapath *od, const struct hmap *lr_ports,
                   const struct nbrec_logical_router_static_route *route,
@@ -11125,10 +11142,12 @@ parsed_routes_add(struct ovn_datapath *od, const 
struct hmap *lr_ports,
     }
 
     /* Verify that ip_prefix and nexthop are on the same network. */
+    const char *lrp_addr_s = NULL;
+    struct ovn_port *out_port = NULL;
     if (!is_discard_route &&
         !find_static_route_outport(od, lr_ports, route,
                                    IN6_IS_ADDR_V4MAPPED(&prefix),
-                                   NULL, NULL)) {
+                                   &lrp_addr_s, &out_port)) {
         return;
     }
 
@@ -11175,6 +11194,10 @@ parsed_routes_add(struct ovn_datapath *od, const 
struct hmap *lr_ports,
                                                  "ecmp_symmetric_reply",
                                                  false);
     new_pr->is_discard_route = is_discard_route;
+    if (!is_discard_route) {
+        new_pr->lrp_addr_s = xstrdup(lrp_addr_s);
+    }
+    new_pr->out_port = out_port;
 
     size_t hash = uuid_hash(&od->key);
     struct parsed_route *pr = parsed_route_lookup(routes, hash, new_pr);
@@ -11182,7 +11205,7 @@ parsed_routes_add(struct ovn_datapath *od, const struct 
hmap *lr_ports,
         hmap_insert(routes, &new_pr->key_node, hash);
     } else {
         pr->stale = false;
-        free(new_pr);
+        parsed_route_free(new_pr);
     }
 }
 
@@ -11211,7 +11234,7 @@ build_parsed_routes(struct ovn_datapath *od, const 
struct hmap *lr_ports,
         }
 
         hmap_remove(routes, &pr->key_node);
-        free(pr);
+        parsed_route_free(pr);
     }
 }
 
@@ -11559,8 +11582,7 @@ add_ecmp_symmetric_reply_flows(struct lflow_table 
*lflows,
 
 static void
 build_ecmp_route_flow(struct lflow_table *lflows, struct ovn_datapath *od,
-                      const struct hmap *lr_ports, struct ecmp_groups_node *eg,
-                      struct lflow_ref *lflow_ref)
+                      struct ecmp_groups_node *eg, struct lflow_ref *lflow_ref)
 
 {
     bool is_ipv4 = IN6_IS_ADDR_V4MAPPED(&eg->prefix);
@@ -11608,20 +11630,14 @@ build_ecmp_route_flow(struct lflow_table *lflows, 
struct ovn_datapath *od,
     LIST_FOR_EACH (er, list_node, &eg->route_list) {
         const struct parsed_route *route_ = er->route;
         const struct nbrec_logical_router_static_route *route = route_->route;
-        /* Find the outgoing port. */
-        const char *lrp_addr_s = NULL;
-        struct ovn_port *out_port = NULL;
-        if (!find_static_route_outport(od, lr_ports, route, is_ipv4,
-                                       &lrp_addr_s, &out_port)) {
-            continue;
-        }
         /* Symmetric ECMP reply is only usable on gateway routers.
          * It is NOT usable on distributed routers with a gateway port.
          */
         if (smap_get(&od->nbr->options, "chassis") &&
             route_->ecmp_symmetric_reply && sset_add(&visited_ports,
-                                                     out_port->key)) {
-            add_ecmp_symmetric_reply_flows(lflows, od, lrp_addr_s, out_port,
+                                                     route_->out_port->key)) {
+            add_ecmp_symmetric_reply_flows(lflows, od, route_->lrp_addr_s,
+                                           route_->out_port,
                                            route_, &route_match,
                                            lflow_ref);
         }
@@ -11638,9 +11654,9 @@ build_ecmp_route_flow(struct lflow_table *lflows, 
struct ovn_datapath *od,
                       is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6,
                       route->nexthop,
                       is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6,
-                      lrp_addr_s,
-                      out_port->lrp_networks.ea_s,
-                      out_port->json_key);
+                      route_->lrp_addr_s,
+                      route_->out_port->lrp_networks.ea_s,
+                      route_->out_port->json_key);
         ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP, 100,
                                 ds_cstr(&match), ds_cstr(&actions),
                                 &route->header_, lflow_ref);
@@ -11720,35 +11736,22 @@ add_route(struct lflow_table *lflows, struct 
ovn_datapath *od,
 
 static void
 build_static_route_flow(struct lflow_table *lflows, struct ovn_datapath *od,
-                        const struct hmap *lr_ports,
                         const struct parsed_route *route_,
                         const struct sset *bfd_ports,
                         struct lflow_ref *lflow_ref)
 {
-    const char *lrp_addr_s = NULL;
-    struct ovn_port *out_port = NULL;
-
     const struct nbrec_logical_router_static_route *route = route_->route;
 
-    /* Find the outgoing port. */
-    if (!route_->is_discard_route) {
-        if (!find_static_route_outport(od, lr_ports, route,
-                                       IN6_IS_ADDR_V4MAPPED(&route_->prefix),
-                                       &lrp_addr_s, &out_port)) {
-            return;
-        }
-    }
-
     int ofs = !strcmp(smap_get_def(&route->options, "origin", ""),
                       ROUTE_ORIGIN_CONNECTED) ? ROUTE_PRIO_OFFSET_CONNECTED
                                               : ROUTE_PRIO_OFFSET_STATIC;
 
     char *prefix_s = build_route_prefix_s(&route_->prefix, route_->plen);
-    add_route(lflows, route_->is_discard_route ? od : out_port->od, out_port,
-              lrp_addr_s, prefix_s, route_->plen, route->nexthop,
-              route_->is_src_route, route_->route_table_id,
-              bfd_ports, &route->header_, route_->is_discard_route,
-              ofs, lflow_ref);
+    add_route(lflows, route_->is_discard_route ? od : route_->out_port->od,
+              route_->out_port, route_->lrp_addr_s, prefix_s,
+              route_->plen, route->nexthop, route_->is_src_route,
+              route_->route_table_id, bfd_ports, &route->header_,
+              route_->is_discard_route, ofs, lflow_ref);
 
     free(prefix_s);
 }
@@ -13523,7 +13526,7 @@ build_ip_routing_flows_for_lrp(struct ovn_port *op,
 static void
 build_static_route_flows_for_lrouter(
         struct ovn_datapath *od, struct lflow_table *lflows,
-        const struct hmap *lr_ports, struct hmap *parsed_routes,
+        struct hmap *parsed_routes,
         struct simap *route_tables, const struct sset *bfd_ports,
         struct lflow_ref *lflow_ref)
 {
@@ -13572,11 +13575,11 @@ build_static_route_flows_for_lrouter(
     HMAP_FOR_EACH (group, hmap_node, &ecmp_groups) {
         /* add a flow in IP_ROUTING, and one flow for each member in
          * IP_ROUTING_ECMP. */
-        build_ecmp_route_flow(lflows, od, lr_ports, group, lflow_ref);
+        build_ecmp_route_flow(lflows, od, group, lflow_ref);
     }
     const struct unique_routes_node *ur;
     HMAP_FOR_EACH (ur, hmap_node, &unique_routes) {
-        build_static_route_flow(lflows, od, lr_ports, ur->route,
+        build_static_route_flow(lflows, od, ur->route,
                                 bfd_ports, lflow_ref);
     }
     ecmp_groups_destroy(&ecmp_groups);
@@ -17062,7 +17065,7 @@ build_lswitch_and_lrouter_iterate_by_lr(struct 
ovn_datapath *od,
                                            lsi->meter_groups, NULL);
     build_ND_RA_flows_for_lrouter(od, lsi->lflows, NULL);
     build_ip_routing_pre_flows_for_lrouter(od, lsi->lflows, NULL);
-    build_static_route_flows_for_lrouter(od, lsi->lflows, lsi->lr_ports,
+    build_static_route_flows_for_lrouter(od, lsi->lflows,
                                          lsi->parsed_routes, lsi->route_tables,
                                          lsi->bfd_ports, NULL);
     build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match,
@@ -17653,7 +17656,7 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn,
     /* Parallel build may result in a suboptimal hash. Resize the
      * lflow map to a correct size before doing lookups */
     lflow_table_expand(lflows);
-    
+
     stopwatch_start(LFLOWS_TO_SB_STOPWATCH_NAME, time_msec());
     lflow_table_sync_to_sb(lflows, ovnsb_txn, input_data->ls_datapaths,
                            input_data->lr_datapaths,
@@ -18833,7 +18836,7 @@ static_routes_destroy(struct static_routes_data *data)
 {
     struct parsed_route *r;
     HMAP_FOR_EACH_POP (r, key_node, &data->parsed_routes) {
-        free(r);
+        parsed_route_free(r);
     }
     hmap_destroy(&data->parsed_routes);
 
diff --git a/northd/northd.h b/northd/northd.h
index 8f76d642d..728fa6740 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -707,6 +707,8 @@ struct parsed_route {
     bool is_discard_route;
     const struct nbrec_logical_router *nbr;
     bool stale;
+    char *lrp_addr_s;
+    struct ovn_port *out_port;
 };
 
 void ovnnb_db_run(struct northd_input *input_data,
-- 
2.47.0


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

Reply via email to