This version attempts to split northd engine processing
of advertised routes. The main motivation is to avoid
logical flow recomputation when NAT/LB routes change.
Signed-off-by: Martin Kalcok <martin.kal...@canonical.com>
---
lib/stopwatch-names.h | 1 +
northd/en-advertised-route-sync.c | 167 +++++++++++++++++++++++-------
northd/en-advertised-route-sync.h | 4 +
northd/en-northd-output.c | 8 ++
northd/en-northd-output.h | 2 +
northd/inc-proc-northd.c | 8 ++
northd/northd.c | 38 +++++++
northd/northd.h | 15 ++-
8 files changed, 205 insertions(+), 38 deletions(-)
diff --git a/lib/stopwatch-names.h b/lib/stopwatch-names.h
index c12dd28d0..13aa5e7bf 100644
--- a/lib/stopwatch-names.h
+++ b/lib/stopwatch-names.h
@@ -36,5 +36,6 @@
#define LS_STATEFUL_RUN_STOPWATCH_NAME "ls_stateful"
#define ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME "advertised_route_sync"
#define LEARNED_ROUTE_SYNC_RUN_STOPWATCH_NAME "learned_route_sync"
+#define DYNAMIC_ROUTES_RUN_STOPWATCH_NAME "dynamic_routes"
#endif
diff --git a/northd/en-advertised-route-sync.c
b/northd/en-advertised-route-sync.c
index 9e8636806..9d4fb308d 100644
--- a/northd/en-advertised-route-sync.c
+++ b/northd/en-advertised-route-sync.c
@@ -25,12 +25,14 @@
#include "openvswitch/hmap.h"
#include "ovn-util.h"
+
static void
advertised_route_table_sync(
struct ovsdb_idl_txn *ovnsb_txn,
const struct sbrec_advertised_route_table *sbrec_advertised_route_table,
const struct lr_stateful_table *lr_stateful_table,
- const struct hmap *parsed_routes,
+ const struct hmap *routes,
+ const struct hmap *dynamic_routes,
struct advertised_route_sync_data *data);
bool
@@ -141,6 +143,8 @@ en_advertised_route_sync_run(struct engine_node *node, void
*data OVS_UNUSED)
struct advertised_route_sync_data *routes_sync_data = data;
struct routes_data *routes_data
= engine_get_input_data("routes", node);
+ struct dynamic_routes_data *dynamic_routes_data
+ = engine_get_input_data("dynamic_routes", node);
const struct engine_context *eng_ctx = engine_get_context();
const struct sbrec_advertised_route_table *sbrec_advertised_route_table =
EN_OVSDB_GET(engine_get_input("SB_advertised_route", node));
@@ -153,12 +157,75 @@ en_advertised_route_sync_run(struct engine_node *node,
void *data OVS_UNUSED)
sbrec_advertised_route_table,
&lr_stateful_data->table,
&routes_data->parsed_routes,
+ &dynamic_routes_data->parsed_routes,
routes_sync_data);
stopwatch_stop(ADVERTISED_ROUTE_SYNC_RUN_STOPWATCH_NAME, time_msec());
engine_set_node_state(node, EN_UPDATED);
}
+void
+*en_dynamic_routes_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *arg OVS_UNUSED)
+{
+ struct dynamic_routes_data *data = xmalloc(sizeof *data);
+ *data = (struct dynamic_routes_data) {
+ .parsed_routes = HMAP_INITIALIZER(&data->parsed_routes),
+ };
+
+ return data;
+}
+
+void
+en_dynamic_routes_cleanup(void *data_)
+{
+ struct dynamic_routes_data *data = data_;
+
+ struct parsed_route *r;
+ HMAP_FOR_EACH_POP (r, key_node, &data->parsed_routes) {
+ parsed_route_free(r);
+ }
+ hmap_destroy(&data->parsed_routes);
+}
+
+void
+en_dynamic_routes_run(struct engine_node *node, void *data)
+{
+ struct dynamic_routes_data *dynamic_routes_data = data;
+ struct northd_data *northd_data = engine_get_input_data("northd", node);
+ struct ed_type_lr_stateful *lr_stateful_data =
+ engine_get_input_data("lr_stateful", node);
+
+ const struct lr_stateful_record *lr_stateful_rec;
+ HMAP_FOR_EACH (lr_stateful_rec, key_node,
+ &lr_stateful_data->table.entries) {
+ const struct ovn_datapath *od =
+ ovn_datapaths_find_by_index(&northd_data->lr_datapaths,
+ lr_stateful_rec->lr_index);
+ if (!od->dynamic_routing) {
+ continue;
+ }
+ build_lb_nat_parsed_routes(od, lr_stateful_rec->lrnat_rec,
+ &dynamic_routes_data->parsed_routes);
+
+ }
+ engine_set_node_state(node, EN_UPDATED);
+}
+
+bool
+dynamic_routes_change_handler(struct engine_node *node OVS_UNUSED,
+ void *data OVS_UNUSED)
+{
+ /* XXX: It was suggested to iterate through data in lr_stateful node
+ * (ed_type_lr_stateful). However the trk_data is only populated when NAT/LB
+ * changes. While this works for us when NAT/LB is is changed, we also need
+ * this handler to trigger when dynamic routing options are changed.
+ * I didn't find a node that would give us only the LR on which options
+ * changed, so for now I set it to recompute every time. */
+ return false;
+}
+
+
struct ar_entry {
struct hmap_node hmap_node;
@@ -335,12 +402,59 @@ publish_host_routes(struct hmap *sync_routes,
}
}
+static void
+advertised_route_table_sync_route_add(
+ const struct lr_stateful_table *lr_stateful_table,
+ struct advertised_route_sync_data *data,
+ struct uuidset *host_route_lrps,
+ struct hmap *sync_routes,
+ const struct parsed_route *route)
+{
+ if (route->is_discard_route) {
+ return;
+ }
+ if (prefix_is_link_local(&route->prefix, route->plen)) {
+ return;
+ }
+ if (!route->od->dynamic_routing) {
+ return;
+ }
+
+ enum dynamic_routing_redistribute_mode drr =
+ route->out_port->dynamic_routing_redistribute;
+ if (route->source == ROUTE_SOURCE_CONNECTED) {
+ if ((drr & DRRM_CONNECTED) == 0) {
+ return;
+ }
+ /* If we advertise host routes, we only need to do so once per
+ * LRP. */
+ const struct uuid *lrp_uuid = &route->out_port->nbrp->header_.uuid;
+ if (drr & DRRM_CONNECTED_AS_HOST &&
+ !uuidset_contains(host_route_lrps, lrp_uuid)) {
+ uuidset_insert(host_route_lrps, lrp_uuid);
+ publish_host_routes(sync_routes, lr_stateful_table, route, data);
+ return;
+ }
+ }
+ if (route->source == ROUTE_SOURCE_STATIC && (drr & DRRM_STATIC) == 0) {
+ return;
+ }
+ if (route->source == ROUTE_SOURCE_NAT && (drr & DRRM_NAT) == 0) {
+ return;
+ }
+
+ char *ip_prefix = normalize_v46_prefix(&route->prefix, route->plen);
+ ar_add_entry(sync_routes, route->od->sb, route->out_port->sb,
+ ip_prefix, NULL);
+}
+
static void
advertised_route_table_sync(
struct ovsdb_idl_txn *ovnsb_txn,
const struct sbrec_advertised_route_table *sbrec_advertised_route_table,
const struct lr_stateful_table *lr_stateful_table,
- const struct hmap *parsed_routes,
+ const struct hmap *routes,
+ const struct hmap *dynamic_routes,
struct advertised_route_sync_data *data)
{
struct hmap sync_routes = HMAP_INITIALIZER(&sync_routes);
@@ -348,46 +462,25 @@ advertised_route_table_sync(
const struct parsed_route *route;
struct ar_entry *route_e;
- const struct sbrec_advertised_route *sb_route;
- HMAP_FOR_EACH (route, key_node, parsed_routes) {
- if (route->is_discard_route) {
- continue;
- }
- if (prefix_is_link_local(&route->prefix, route->plen)) {
- continue;
- }
- if (!route->od->dynamic_routing) {
- continue;
- }
- enum dynamic_routing_redistribute_mode drr =
- route->out_port->dynamic_routing_redistribute;
- if (route->source == ROUTE_SOURCE_CONNECTED) {
- if ((drr & DRRM_CONNECTED) == 0) {
- continue;
- }
- /* If we advertise host routes, we only need to do so once per
- * LRP. */
- const struct uuid *lrp_uuid =
- &route->out_port->nbrp->header_.uuid;
- if (drr & DRRM_CONNECTED_AS_HOST &&
- !uuidset_contains(&host_route_lrps, lrp_uuid)) {
- uuidset_insert(&host_route_lrps, lrp_uuid);
- publish_host_routes(&sync_routes, lr_stateful_table,
- route, data);
- continue;
- }
- }
- if (route->source == ROUTE_SOURCE_STATIC && (drr & DRRM_STATIC) == 0) {
- continue;
- }
+ /* First build the set of non-dynamic routes that need sync-ing. */
+ HMAP_FOR_EACH (route, key_node, routes) {
+ advertised_route_table_sync_route_add(lr_stateful_table,
+ data, &host_route_lrps,
+ &sync_routes,
+ route);
+ }
- char *ip_prefix = normalize_v46_prefix(&route->prefix, route->plen);
- route_e = ar_add_entry(&sync_routes, route->od->sb,
- route->out_port->sb, ip_prefix, NULL);
+ /* First add the set of dynamic routes that need sync-ing. */
+ HMAP_FOR_EACH (route, key_node, dynamic_routes) {
+ advertised_route_table_sync_route_add(lr_stateful_table,
+ data, &host_route_lrps,
+ &sync_routes,
+ route);
}
uuidset_destroy(&host_route_lrps);
+ const struct sbrec_advertised_route *sb_route;
SBREC_ADVERTISED_ROUTE_TABLE_FOR_EACH_SAFE (sb_route,
sbrec_advertised_route_table) {
route_e = ar_find(&sync_routes, sb_route->datapath,
diff --git a/northd/en-advertised-route-sync.h
b/northd/en-advertised-route-sync.h
index 1f24fd329..94eee0eee 100644
--- a/northd/en-advertised-route-sync.h
+++ b/northd/en-advertised-route-sync.h
@@ -36,4 +36,8 @@ void *en_advertised_route_sync_init(struct engine_node *,
struct engine_arg *);
void en_advertised_route_sync_cleanup(void *data);
void en_advertised_route_sync_run(struct engine_node *, void *data);
+bool dynamic_routes_change_handler(struct engine_node *, void *data);
+void *en_dynamic_routes_init(struct engine_node *, struct engine_arg *);
+void en_dynamic_routes_cleanup(void *data);
+void en_dynamic_routes_run(struct engine_node *, void *data);
#endif /* EN_ADVERTISED_ROUTE_SYNC_H */
diff --git a/northd/en-northd-output.c b/northd/en-northd-output.c
index 715abd3ab..81c796974 100644
--- a/northd/en-northd-output.c
+++ b/northd/en-northd-output.c
@@ -97,3 +97,11 @@ northd_output_advertised_route_sync_handler(struct
engine_node *node,
return true;
}
+bool
+northd_output_dynamic_routes_handler(struct engine_node *node,
+ void *data OVS_UNUSED)
+{
+ engine_set_node_state(node, EN_UPDATED);
+ return true;
+}
+
diff --git a/northd/en-northd-output.h b/northd/en-northd-output.h
index 783587cb6..689640118 100644
--- a/northd/en-northd-output.h
+++ b/northd/en-northd-output.h
@@ -23,5 +23,7 @@ bool northd_output_acl_id_handler(struct engine_node *node,
void *data OVS_UNUSED);
bool northd_output_advertised_route_sync_handler(struct engine_node *node,
void *data OVS_UNUSED);
+bool northd_output_dynamic_routes_handler(struct engine_node *node,
+ void *data OVS_UNUSED);
#endif
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index ce7a89ec4..dc88ebd99 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -175,6 +175,7 @@ static ENGINE_NODE(multicast_igmp, "multicast_igmp");
static ENGINE_NODE(acl_id, "acl_id");
static ENGINE_NODE(advertised_route_sync, "advertised_route_sync");
static ENGINE_NODE(learned_route_sync, "learned_route_sync");
+static ENGINE_NODE(dynamic_routes, "dynamic_routes");
void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
struct ovsdb_idl_loop *sb)
@@ -287,7 +288,12 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
engine_add_input(&en_ecmp_nexthop, &en_sb_mac_binding,
ecmp_nexthop_mac_binding_handler);
+ engine_add_input(&en_dynamic_routes, &en_lr_stateful,
+ dynamic_routes_change_handler);
+ engine_add_input(&en_dynamic_routes, &en_northd, engine_noop_handler);
+
engine_add_input(&en_advertised_route_sync, &en_routes, NULL);
+ engine_add_input(&en_advertised_route_sync, &en_dynamic_routes, NULL);
engine_add_input(&en_advertised_route_sync, &en_sb_advertised_route,
NULL);
engine_add_input(&en_advertised_route_sync, &en_lr_stateful,
@@ -399,6 +405,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
northd_output_acl_id_handler);
engine_add_input(&en_northd_output, &en_advertised_route_sync,
northd_output_advertised_route_sync_handler);
+ engine_add_input(&en_northd_output, &en_dynamic_routes,
+ northd_output_dynamic_routes_handler);
struct engine_arg engine_arg = {
.nb_idl = nb->idl,
diff --git a/northd/northd.c b/northd/northd.c
index a6eb70948..4b5708059 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -846,6 +846,14 @@ parse_dynamic_routing_redistribute(
out |= DRRM_STATIC;
continue;
}
+ if (!strcmp(token, "nat")) {
+ out |= DRRM_NAT;
+ continue;
+ }
+ if (!strcmp(token, "lb")) {
+ out |= DRRM_LB;
+ continue;
+ }
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
VLOG_WARN_RL(&rl, "unkown dynamic-routing-redistribute option '%s'",
token);
@@ -11272,6 +11280,34 @@ build_parsed_routes(const struct ovn_datapath *od,
const struct hmap *lr_ports,
}
}
+void
+build_lb_nat_parsed_routes(const struct ovn_datapath *od,
+ const struct lr_nat_record *lr_nat,
+ struct hmap *routes)
+{
+ const struct ovn_port *op;
+ HMAP_FOR_EACH (op, dp_node, &od->ports) {
+ enum dynamic_routing_redistribute_mode drrm =
op->dynamic_routing_redistribute;
+ if (!(drrm & DRRM_NAT)) {
+ continue;
+ }
+ /* I'm thinking of extending parsed_route struct with "tracked_port".
+ * Since we are already parsing/iterating NATs here, it feels more
+ * efficinet to figure out the tracked_port here, rather than
+ * re-parsing NATs down the line in the advertised_route_table_sync
+ * function before calling "ar_add_entry".*/
+ for (size_t i = 0; i < lr_nat->n_nat_entries; i++) {
+ const struct ovn_nat *nat = &lr_nat->nat_entries[i];
+ int plen = nat_entry_is_v6(nat) ? 128 : 32;
+ struct in6_addr prefix;
+ ip46_parse(nat->nb->external_ip, &prefix);
+ parsed_route_add(od, NULL, &prefix, plen, false,
+ nat->nb->external_ip, op, 0, false, false,
+ NULL, ROUTE_SOURCE_NAT, &op->nbrp->header_,
routes);
+ }
+ }
+
+}
struct ecmp_route_list_node {
struct ovs_list list_node;
uint16_t id; /* starts from 1 */
@@ -11441,6 +11477,8 @@ route_source_to_offset(enum route_source source)
{
switch (source) {
case ROUTE_SOURCE_CONNECTED:
+ case ROUTE_SOURCE_NAT:
+ case ROUTE_SOURCE_LB:
return ROUTE_PRIO_OFFSET_CONNECTED;
case ROUTE_SOURCE_STATIC:
return ROUTE_PRIO_OFFSET_STATIC;
diff --git a/northd/northd.h b/northd/northd.h
index 11efa0136..95f2fe010 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -186,11 +186,15 @@ struct route_policy {
};
struct routes_data {
- struct hmap parsed_routes;
+ struct hmap parsed_routes; /* Stores struct parsed_route. */
struct simap route_tables;
struct hmap bfd_active_connections;
};
+struct dynamic_routes_data {
+ struct hmap parsed_routes; /* Stores struct parsed_route. */
+};
+
struct route_policies_data {
struct hmap route_policies;
struct hmap bfd_active_connections;
@@ -312,6 +316,8 @@ enum dynamic_routing_redistribute_mode_bits {
DRRM_CONNECTED_BIT = 0,
DRRM_CONNECTED_AS_HOST_BIT = 1,
DRRM_STATIC_BIT = 2,
+ DRRM_NAT_BIT = 3,
+ DRRM_LB_BIT = 4,
};
enum dynamic_routing_redistribute_mode {
@@ -319,6 +325,8 @@ enum dynamic_routing_redistribute_mode {
DRRM_CONNECTED = (1 << DRRM_CONNECTED_BIT),
DRRM_CONNECTED_AS_HOST = (1 << DRRM_CONNECTED_AS_HOST_BIT),
DRRM_STATIC = (1 << DRRM_STATIC_BIT),
+ DRRM_NAT = (1 << DRRM_NAT_BIT),
+ DRRM_LB = (1 << DRRM_LB_BIT),
};
/* The 'key' comes from nbs->header_.uuid or nbr->header_.uuid or
@@ -730,6 +738,10 @@ enum route_source {
ROUTE_SOURCE_STATIC,
/* The route is dynamically learned by an ovn-controller. */
ROUTE_SOURCE_LEARNED,
+ /* The route is derived from a NAT's external IP. */
+ ROUTE_SOURCE_NAT,
+ /* The route is derived from a LB's VIP. */
+ ROUTE_SOURCE_LB,
};
struct parsed_route {
@@ -811,6 +823,7 @@ void route_policies_destroy(struct route_policies_data *);
void build_parsed_routes(const struct ovn_datapath *, const struct hmap *,
const struct hmap *, struct hmap *, struct simap *,
struct hmap *);
+void build_lb_nat_parsed_routes(const struct ovn_datapath *, const struct
lr_nat_record *, struct hmap *);
uint32_t get_route_table_id(struct simap *, const char *);
void routes_init(struct routes_data *);
void routes_destroy(struct routes_data *);
--
2.43.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
