Add support for I+P handling of en-learned-route-sync for the
sb_learned_routes input.
Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud>
---
v2->v3:
* Addressed review comments.
* Ignore routes that are added and deleted in the same handler run.
v1-v2: Fix test error
northd/en-learned-route-sync.c | 121 ++++++++++++++++++++++++++++++---
northd/en-learned-route-sync.h | 21 ++++++
northd/inc-proc-northd.c | 6 +-
northd/northd.c | 24 ++++++-
northd/northd.h | 34 +++++----
tests/ovn-northd.at | 16 ++---
6 files changed, 186 insertions(+), 36 deletions(-)
diff --git a/northd/en-learned-route-sync.c b/northd/en-learned-route-sync.c
index 4e87b3265..bde9ffa79 100644
--- a/northd/en-learned-route-sync.c
+++ b/northd/en-learned-route-sync.c
@@ -60,6 +60,19 @@ learned_route_sync_northd_change_handler(struct engine_node
*node,
return true;
}
+static void
+routes_sync_clear_tracked(struct learned_route_sync_data *data)
+{
+ hmapx_clear(&data->trk_data.trk_created_parsed_route);
+ struct hmapx_node *hmapx_node;
+ HMAPX_FOR_EACH_SAFE (hmapx_node,
+ &data->trk_data.trk_deleted_parsed_route) {
+ parsed_route_free(hmapx_node->data);
+ hmapx_delete(&data->trk_data.trk_deleted_parsed_route, hmapx_node);
+ }
+ data->tracked = false;
+}
+
static void
routes_sync_clear(struct learned_route_sync_data *data)
{
@@ -67,12 +80,16 @@ routes_sync_clear(struct learned_route_sync_data *data)
HMAP_FOR_EACH_POP (r, key_node, &data->parsed_routes) {
parsed_route_free(r);
}
+ routes_sync_clear_tracked(data);
}
static void
routes_sync_init(struct learned_route_sync_data *data)
{
hmap_init(&data->parsed_routes);
+ hmapx_init(&data->trk_data.trk_created_parsed_route);
+ hmapx_init(&data->trk_data.trk_deleted_parsed_route);
+ data->tracked = false;
}
static void
@@ -80,6 +97,8 @@ routes_sync_destroy(struct learned_route_sync_data *data)
{
routes_sync_clear(data);
hmap_destroy(&data->parsed_routes);
+ hmapx_destroy(&data->trk_data.trk_created_parsed_route);
+ hmapx_destroy(&data->trk_data.trk_deleted_parsed_route);
}
void *
@@ -97,6 +116,12 @@ en_learned_route_sync_cleanup(void *data)
routes_sync_destroy(data);
}
+void
+en_learned_route_sync_clear_tracked_data(void *data)
+{
+ routes_sync_clear_tracked(data);
+}
+
void
en_learned_route_sync_run(struct engine_node *node, void *data)
{
@@ -122,7 +147,7 @@ en_learned_route_sync_run(struct engine_node *node, void
*data)
}
-static void
+static struct parsed_route *
parse_route_from_sbrec_route(struct hmap *parsed_routes_out,
const struct hmap *lr_ports,
const struct hmap *lr_datapaths,
@@ -132,7 +157,7 @@ parse_route_from_sbrec_route(struct hmap *parsed_routes_out,
NULL, lr_datapaths, route->datapath);
if (!od || ovn_datapath_is_stale(od)) {
- return;
+ return NULL;
}
/* Verify that the next hop is an IP address with an all-ones mask. */
@@ -144,7 +169,7 @@ parse_route_from_sbrec_route(struct hmap *parsed_routes_out,
UUID_FMT, route->nexthop,
UUID_ARGS(&route->header_.uuid));
free(nexthop);
- return;
+ return NULL;
}
if ((IN6_IS_ADDR_V4MAPPED(nexthop) && plen != 32) ||
(!IN6_IS_ADDR_V4MAPPED(nexthop) && plen != 128)) {
@@ -153,7 +178,7 @@ parse_route_from_sbrec_route(struct hmap *parsed_routes_out,
UUID_FMT, route->nexthop,
UUID_ARGS(&route->header_.uuid));
free(nexthop);
- return;
+ return NULL;
}
/* Parse ip_prefix */
@@ -164,7 +189,7 @@ parse_route_from_sbrec_route(struct hmap *parsed_routes_out,
UUID_FMT, route->ip_prefix,
UUID_ARGS(&route->header_.uuid));
free(nexthop);
- return;
+ return NULL;
}
/* Verify that ip_prefix and nexthop are on the same network. */
@@ -175,14 +200,18 @@ parse_route_from_sbrec_route(struct hmap
*parsed_routes_out,
IN6_IS_ADDR_V4MAPPED(&prefix),
false,
&out_port, &lrp_addr_s)) {
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
+ VLOG_WARN_RL(&rl, "could not find output port %s for learned route "
+ UUID_FMT, route->logical_port->logical_port,
+ UUID_ARGS(&route->header_.uuid));
free(nexthop);
- return;
+ return NULL;
}
- parsed_route_add(od, nexthop, &prefix, plen, false, lrp_addr_s,
- out_port, 0, false, false, NULL,
- ROUTE_SOURCE_LEARNED, &route->header_, NULL,
- parsed_routes_out);
+ return parsed_route_add(od, nexthop, &prefix, plen, false, lrp_addr_s,
+ out_port, 0, false, false, NULL,
+ ROUTE_SOURCE_LEARNED, &route->header_, NULL,
+ parsed_routes_out);
}
static void
@@ -212,3 +241,75 @@ routes_table_sync(
parsed_route_hash(route));
}
}
+
+static struct parsed_route *
+find_learned_route(const struct sbrec_learned_route *learned_route,
+ const struct ovn_datapaths *lr_datapaths,
+ const struct hmap *routes)
+{
+ const struct ovn_datapath *od = ovn_datapath_from_sbrec(
+ NULL, &lr_datapaths->datapaths, learned_route->datapath);
+ if (!od) {
+ return NULL;
+ }
+ return parsed_route_lookup_by_source(od, ROUTE_SOURCE_LEARNED,
+ &learned_route->header_, routes);
+}
+
+bool
+learned_route_sync_sb_learned_route_change_handler(struct engine_node *node,
+ void *data_)
+{
+ struct learned_route_sync_data *data = data_;
+ data->tracked = true;
+
+ const struct sbrec_learned_route_table *sbrec_learned_route_table =
+ EN_OVSDB_GET(engine_get_input("SB_learned_route", node));
+ struct northd_data *northd_data = engine_get_input_data("northd", node);
+
+ const struct sbrec_learned_route *changed_route;
+ SBREC_LEARNED_ROUTE_TABLE_FOR_EACH_TRACKED (changed_route,
+ sbrec_learned_route_table) {
+ if (sbrec_learned_route_is_new(changed_route) &&
+ sbrec_learned_route_is_deleted(changed_route)) {
+ continue;
+ }
+
+ if (sbrec_learned_route_is_new(changed_route)) {
+ struct parsed_route *route = parse_route_from_sbrec_route(
+ &data->parsed_routes, &northd_data->lr_ports,
+ &northd_data->lr_datapaths.datapaths, changed_route);
+ if (route) {
+ hmapx_add(&data->trk_data.trk_created_parsed_route, route);
+ continue;
+ }
+ }
+
+ if (sbrec_learned_route_is_deleted(changed_route)) {
+ struct parsed_route *route = find_learned_route(
+ changed_route, &northd_data->lr_datapaths,
+ &data->parsed_routes);
+ if (!route) {
+ goto fail;
+ }
+ hmap_remove(&data->parsed_routes, &route->key_node);
+ hmapx_add(&data->trk_data.trk_deleted_parsed_route, route);
+ continue;
+ }
+
+ /* The learned routes should never be updated, so we just fail the
+ * handler here if that happens. */
+ goto fail;
+ }
+
+ if (!hmapx_is_empty(&data->trk_data.trk_created_parsed_route) ||
+ !hmapx_is_empty(&data->trk_data.trk_deleted_parsed_route)) {
+ engine_set_node_state(node, EN_UPDATED);
+ }
+
+ return true;
+
+fail:
+ routes_sync_clear_tracked(data);
+ return false;
+}
diff --git a/northd/en-learned-route-sync.h b/northd/en-learned-route-sync.h
index 1a5d6ff23..b2d079621 100644
--- a/northd/en-learned-route-sync.h
+++ b/northd/en-learned-route-sync.h
@@ -18,15 +18,36 @@
#include "lib/inc-proc-eng.h"
#include "openvswitch/hmap.h"
+#include "hmapx.h"
+
+/* Track what's changed in the learned routes engine node.
+ * For now only tracks changed learned routes. */
+struct learned_routes_tracked_data {
+ /* Tracked created routes based on sb_learned_routes.
+ * hmapx node is 'struct parsed_route *'. */
+ struct hmapx trk_created_parsed_route;
+
+ /* Tracked deleted routes based on sb_learned_routes.
+ * hmapx node is 'struct parsed_route *'. */
+ struct hmapx trk_deleted_parsed_route;
+};
struct learned_route_sync_data {
struct hmap parsed_routes;
+
+ /* 'tracked' is set to true if there is information available for
+ * incremental processing. If true then trk_data is valid. */
+ bool tracked;
+ struct learned_routes_tracked_data trk_data;
};
bool learned_route_sync_northd_change_handler(struct engine_node *,
void *data);
+bool learned_route_sync_sb_learned_route_change_handler(struct engine_node *,
+ void *data);
void *en_learned_route_sync_init(struct engine_node *, struct engine_arg *);
void en_learned_route_sync_cleanup(void *data);
+void en_learned_route_sync_clear_tracked_data(void *data);
void en_learned_route_sync_run(struct engine_node *, void *data);
#endif /* EN_LEARNED_ROUTE_SYNC_H */
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 3fa99f637..b22e5fef3 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -174,7 +174,8 @@ static ENGINE_NODE(ecmp_nexthop, "ecmp_nexthop");
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_WITH_CLEAR_TRACK_DATA(learned_route_sync,
+ "learned_route_sync");
static ENGINE_NODE(dynamic_routes, "dynamic_routes");
void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
@@ -307,7 +308,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
advertised_route_sync_northd_change_handler);
engine_add_input(&en_learned_route_sync, &en_routes, NULL);
- engine_add_input(&en_learned_route_sync, &en_sb_learned_route, NULL);
+ engine_add_input(&en_learned_route_sync, &en_sb_learned_route,
+ learned_route_sync_sb_learned_route_change_handler);
engine_add_input(&en_learned_route_sync, &en_northd,
learned_route_sync_northd_change_handler);
diff --git a/northd/northd.c b/northd/northd.c
index a91e48ac2..cfa2cf915 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -11074,6 +11074,26 @@ parsed_route_clone(const struct parsed_route *pr)
return new_pr;
}
+/* Searches for a parsed_route in a hmap based on datapath, source and
+ * source_hint. */
+struct parsed_route *
+parsed_route_lookup_by_source(const struct ovn_datapath *od,
+ enum route_source source,
+ const struct ovsdb_idl_row *source_hint,
+ const struct hmap *routes)
+{
+ size_t hash = uuid_hash(&od->key);
+ struct parsed_route *route;
+ HMAP_FOR_EACH_WITH_HASH (route, key_node, hash, routes) {
+ if (route->source == source &&
+ uuid_equals(&route->source_hint->uuid,
+ &source_hint->uuid)) {
+ return route;
+ }
+ }
+ return NULL;
+}
+
/* This hash needs to be equal to the one used in
* build_route_flows_for_lrouter to iterate over all routes of a datapath.
* This is distinct from route_hash which is stored in parsed_route->hash. */
@@ -11097,7 +11117,7 @@ parsed_route_free(struct parsed_route *pr) {
* in there.
* Takes ownership of the provided nexthop. All other parameters are cloned.
* Elements of the routes hmap need to be freed using parsed_route_free. */
-void
+struct parsed_route *
parsed_route_add(const struct ovn_datapath *od,
struct in6_addr *nexthop,
const struct in6_addr *prefix,
@@ -11126,9 +11146,11 @@ parsed_route_add(const struct ovn_datapath *od,
struct parsed_route *pr = parsed_route_lookup(routes, hash, new_pr);
if (!pr) {
hmap_insert(routes, &new_pr->key_node, hash);
+ return new_pr;
} else {
pr->stale = false;
parsed_route_free(new_pr);
+ return pr;
}
}
diff --git a/northd/northd.h b/northd/northd.h
index de0d924c5..f74719cc9 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -782,24 +782,28 @@ struct parsed_route {
};
struct parsed_route *parsed_route_clone(const struct parsed_route *);
+struct parsed_route *parsed_route_lookup_by_source(
+ const struct ovn_datapath *od, enum route_source source,
+ const struct ovsdb_idl_row *source_hint, const struct hmap *routes);
size_t parsed_route_hash(const struct parsed_route *);
void parsed_route_free(struct parsed_route *);
-void parsed_route_add(const struct ovn_datapath *od,
- struct in6_addr *nexthop,
- const struct in6_addr *prefix,
- unsigned int plen,
- bool is_discard_route,
- const char *lrp_addr_s,
- const struct ovn_port *out_port,
- uint32_t route_table_id,
- bool is_src_route,
- bool ecmp_symmetric_reply,
- const struct sset *ecmp_selection_fields,
- enum route_source source,
- const struct ovsdb_idl_row *source_hint,
- const struct ovn_port *tracked_port,
- struct hmap *routes);
+struct parsed_route *parsed_route_add(
+ const struct ovn_datapath *od,
+ struct in6_addr *nexthop,
+ const struct in6_addr *prefix,
+ unsigned int plen,
+ bool is_discard_route,
+ const char *lrp_addr_s,
+ const struct ovn_port *out_port,
+ uint32_t route_table_id,
+ bool is_src_route,
+ bool ecmp_symmetric_reply,
+ const struct sset *ecmp_selection_fields,
+ enum route_source source,
+ const struct ovsdb_idl_row *source_hint,
+ const struct ovn_port *tracked_port,
+ struct hmap *routes);
bool
find_route_outport(const struct hmap *lr_ports, const char *output_port,
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index c74d57f7e..65f2c9950 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -15370,8 +15370,8 @@ ovn-sbctl dump-flows lr0 > lr0flows
AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], [0], [dnl
table=??(lr_in_ip_routing ), priority=0 , match=(1), action=(drop;)
table=??(lr_in_ip_routing ), priority=10550, match=(nd_rs || nd_ra),
action=(drop;)
- table=??(lr_in_ip_routing ), priority=194 , match=(reg7 == 0 && ip4.dst
== 192.168.1.0/24), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1;
reg8[[16..31]] = select(1, 2);)
- table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 && ip4.dst
== 192.168.0.0/24), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 2;
reg8[[16..31]] = select(1, 2);)
+ table=??(lr_in_ip_routing ), priority=194 , match=(reg7 == 0 && ip4.dst
== 192.168.1.0/24), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 2;
reg8[[16..31]] = select(1, 2);)
+ table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 && ip4.dst
== 192.168.0.0/24), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1;
reg8[[16..31]] = select(1, 2);)
table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
10.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5 =
10.0.0.1; eth.src = 00:00:00:00:ff:01; outport = "lr0-sw0"; flags.loopback = 1;
reg9[[9]] = 1; next;)
table=??(lr_in_ip_routing ), priority=198 , match=(ip4.dst ==
10.0.1.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg5 =
10.0.1.1; eth.src = 00:00:00:00:ff:02; outport = "lr0-sw1"; flags.loopback = 1;
reg9[[9]] = 1; next;)
table=??(lr_in_ip_routing ), priority=518 , match=(inport == "lr0-sw0" &&
ip6.dst == fe80::/64), action=(ip.ttl--; reg8[[0..15]] = 0; xxreg0 = ip6.dst;
xxreg1 = fe80::200:ff:fe00:ff01; eth.src = 00:00:00:00:ff:01; outport =
"lr0-sw0"; flags.loopback = 1; reg9[[9]] = 0; next;)
@@ -15379,10 +15379,10 @@ AT_CHECK([grep -w "lr_in_ip_routing" lr0flows |
ovn_strip_lflows], [0], [dnl
])
AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | sed -e
's/10\.0\..\./10.0.??./g' -e 's/lr0-sw./lr0-sw??/' -e 's/00:ff:0./00:ff:0?/' |
ovn_strip_lflows], [0], [dnl
table=??(lr_in_ip_routing_ecmp), priority=0 , match=(1), action=(drop;)
- table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1
&& reg8[[16..31]] == 1), action=(reg0 = 10.0.??.20; reg5 = 10.0.??.1; eth.src =
00:00:00:00:ff:0?; outport = "lr0-sw??"; reg9[[9]] = 1; next;)
- table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1
&& reg8[[16..31]] == 2), action=(reg0 = 10.0.??.20; reg5 = 10.0.??.1; eth.src =
00:00:00:00:ff:0?; outport = "lr0-sw??"; reg9[[9]] = 1; next;)
- table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 2
&& reg8[[16..31]] == 1), action=(reg0 = 10.0.??.10; reg5 = 10.0.??.1; eth.src =
00:00:00:00:ff:0?; outport = "lr0-sw??"; reg9[[9]] = 1; next;)
- table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 2
&& reg8[[16..31]] == 2), action=(reg0 = 10.0.??.10; reg5 = 10.0.??.1; eth.src =
00:00:00:00:ff:0?; outport = "lr0-sw??"; reg9[[9]] = 1; next;)
+ table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1
&& reg8[[16..31]] == 1), action=(reg0 = 10.0.??.10; reg5 = 10.0.??.1; eth.src =
00:00:00:00:ff:0?; outport = "lr0-sw??"; reg9[[9]] = 1; next;)
+ table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1
&& reg8[[16..31]] == 2), action=(reg0 = 10.0.??.10; reg5 = 10.0.??.1; eth.src =
00:00:00:00:ff:0?; outport = "lr0-sw??"; reg9[[9]] = 1; next;)
+ table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 2
&& reg8[[16..31]] == 1), action=(reg0 = 10.0.??.20; reg5 = 10.0.??.1; eth.src =
00:00:00:00:ff:0?; outport = "lr0-sw??"; reg9[[9]] = 1; next;)
+ table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 2
&& reg8[[16..31]] == 2), action=(reg0 = 10.0.??.20; reg5 = 10.0.??.1; eth.src =
00:00:00:00:ff:0?; outport = "lr0-sw??"; reg9[[9]] = 1; next;)
table=??(lr_in_ip_routing_ecmp), priority=150 , match=(reg8[[0..15]] == 0),
action=(next;)
])
@@ -15676,7 +15676,7 @@ check ovn-nbctl --wait=sb sync
check_engine_compute northd unchanged
check_engine_compute routes unchanged
check_engine_compute advertised_route_sync unchanged
-check_engine_compute learned_route_sync recompute
+check_engine_compute learned_route_sync incremental
check_engine_compute lflow recompute
CHECK_NO_CHANGE_AFTER_RECOMPUTE
@@ -15686,7 +15686,7 @@ check ovn-nbctl --wait=sb sync
check_engine_compute northd unchanged
check_engine_compute routes unchanged
check_engine_compute advertised_route_sync unchanged
-check_engine_compute learned_route_sync recompute
+check_engine_compute learned_route_sync incremental
check_engine_compute lflow recompute
CHECK_NO_CHANGE_AFTER_RECOMPUTE
--
2.48.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
