On 7/9/26 4:09 PM, Lucas Vargas Dias wrote:
> Create a handler for static routes. It handles update, create or
> delete.
> Full recompute is triggered when update bfd column for static route.
>
> Test with 2000 static routes created in the same logical router
> and add a new one:
> Without the incremental processing:
> ovn-nbctl --print-wait-time --wait=sb lr-route-add lr1-2 10.0.0.1/32
> 192.168.20.2
> Time spent on processing nb_cfg 4:
> ovn-northd delay before processing: 4ms
> ovn-northd completion: 62ms
>
> With the incremental processing:
> ovn-nbctl --print-wait-time --wait=sb lr-route-add lr1-2 10.0.0.1/32
> 192.168.20.2
> Time spent on processing nb_cfg 6:
> ovn-northd delay before processing: 1ms
> ovn-northd completion: 18ms
>
> Test with 2000 static routes created in the same logical router
> and delete one:
> Without the incremental processing:
> ovn-nbctl --print-wait-time --wait=sb lr-route-del lr1-2 10.0.0.1/32
> 192.168.20.2
> Time spent on processing nb_cfg 5:
> ovn-northd delay before processing: 3ms
> ovn-northd completion: 62ms
>
> With the incremental processing:
> ovn-nbctl --print-wait-time --wait=sb lr-route-del lr1-2 10.0.0.1/32
> 192.168.20.2
> Time spent on processing nb_cfg 9:
> ovn-northd delay before processing: 10ms
> ovn-northd completion: 28ms
>
> Signed-off-by: Lucas Vargas Dias <[email protected]>
> ---
Hi Lucas,
Thanks for the quick respin!
> northd/en-group-ecmp-route.c | 52 ++++++++++
> northd/en-group-ecmp-route.h | 4 +
> northd/en-northd.c | 163 +++++++++++++++++++++++++++++--
> northd/en-northd.h | 7 ++
> northd/inc-proc-northd.c | 21 ++--
> northd/northd.c | 102 +++++++++++++++----
> northd/northd.h | 26 +++++
> tests/ovn-inc-proc-graph-dump.at | 8 +-
> tests/ovn-northd.at | 133 +++++++++++++++++++++++--
> 9 files changed, 470 insertions(+), 46 deletions(-)
>
> diff --git a/northd/en-group-ecmp-route.c b/northd/en-group-ecmp-route.c
> index c4c93fd84..87dade486 100644
> --- a/northd/en-group-ecmp-route.c
> +++ b/northd/en-group-ecmp-route.c
> @@ -519,3 +519,55 @@ group_ecmp_route_learned_route_change_handler(struct
> engine_node *eng_node,
> }
> return EN_HANDLED_UNCHANGED;
> }
> +
> +enum engine_input_handler_result
> +group_ecmp_route_routes_change_handler(struct engine_node *eng_node,
> + void *_data)
> +{
> + struct routes_data *routes_data
> + = engine_get_input_data("routes", eng_node);
> + struct group_ecmp_route_data *data = _data;
> + if (!routes_data->tracked) {
> + return EN_UNHANDLED;
> + }
> +
> + data->tracked = true;
> +
> + struct parsed_route *pr;
> + struct hmapx updated_routes = HMAPX_INITIALIZER(&updated_routes);
> +
> + const struct hmapx_node *hmapx_node;
> + HMAPX_FOR_EACH (hmapx_node,
> + &routes_data->trk_data.trk_deleted_parsed_route) {
> + pr = hmapx_node->data;
> + if (!handle_deleted_route(data, pr, &updated_routes)) {
> + hmapx_destroy(&updated_routes);
> + return EN_UNHANDLED;
> + }
> + }
> +
> + HMAPX_FOR_EACH (hmapx_node,
> + &routes_data->trk_data.trk_crupdated_parsed_route) {
> + pr = hmapx_node->data;
> + handle_added_route(data, pr, &updated_routes);
> + }
> +
> + HMAPX_FOR_EACH (hmapx_node, &updated_routes) {
> + struct group_ecmp_datapath *node = hmapx_node->data;
> + if (hmap_is_empty(&node->unique_routes) &&
> + hmap_is_empty(&node->ecmp_groups)) {
> + hmapx_add(&data->trk_data.deleted_datapath_routes, node);
> + hmap_remove(&data->datapaths, &node->hmap_node);
> + } else {
> + hmapx_add(&data->trk_data.crupdated_datapath_routes, node);
> + }
> + }
> +
> + hmapx_destroy(&updated_routes);
> +
> + if (!hmapx_is_empty(&data->trk_data.crupdated_datapath_routes) ||
> + !hmapx_is_empty(&data->trk_data.deleted_datapath_routes)) {
> + return EN_HANDLED_UPDATED;
> + }
> + return EN_HANDLED_UNCHANGED;
> +}
> diff --git a/northd/en-group-ecmp-route.h b/northd/en-group-ecmp-route.h
> index d4a3248d0..aa6b26015 100644
> --- a/northd/en-group-ecmp-route.h
> +++ b/northd/en-group-ecmp-route.h
> @@ -98,6 +98,10 @@ enum engine_input_handler_result
> group_ecmp_route_learned_route_change_handler(struct engine_node *,
> void *data);
>
> +enum engine_input_handler_result
> +group_ecmp_route_routes_change_handler(struct engine_node *,
> + void *data);
> +
> struct group_ecmp_datapath *group_ecmp_datapath_lookup(
> const struct group_ecmp_route_data *data,
> const struct ovn_datapath *od);
> diff --git a/northd/en-northd.c b/northd/en-northd.c
> index c34818dba..1102bd322 100644
> --- a/northd/en-northd.c
> +++ b/northd/en-northd.c
> @@ -207,7 +207,8 @@ northd_nb_logical_router_handler(struct engine_node *node,
> }
>
> if (northd_has_lr_nats_in_tracked_data(&nd->trk_data) ||
> - northd_has_lrouters_in_tracked_data(&nd->trk_data)) {
> + northd_has_lrouters_in_tracked_data(&nd->trk_data) ||
> + northd_has_lr_route_in_tracked_data(&nd->trk_data)) {
> return EN_HANDLED_UPDATED;
> }
>
> @@ -329,7 +330,7 @@ en_route_policies_run(struct engine_node *node, void
> *data)
>
> enum engine_input_handler_result
> routes_northd_change_handler(struct engine_node *node,
> - void *data OVS_UNUSED)
> + void *data OVS_UNUSED)
> {
> struct northd_data *northd_data = engine_get_input_data("northd", node);
> if (!northd_has_tracked_data(&northd_data->trk_data)) {
> @@ -347,15 +348,145 @@ routes_northd_change_handler(struct engine_node *node,
> * Note: When we add I-P to the created/deleted logical routers or
> * logical router ports, we need to revisit this handler.
> *
> - * This node also accesses the static routes of the logical router.
> - * When these static routes gets updated, en_northd engine
> recomputes
> - * and so does this node.
> - * Note: When we add I-P to handle static routes changes, we need
> - * to revisit this handler.
> */
> return EN_HANDLED_UNCHANGED;
> }
>
> +static bool
> +static_route_is_relevant_updated(
> + const struct nbrec_logical_router_static_route *sr)
> +{
> + return nbrec_logical_router_static_route_is_updated(sr,
> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_BFD)
> + || nbrec_logical_router_static_route_is_updated(sr,
> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_IP_PREFIX)
> + || nbrec_logical_router_static_route_is_updated(sr,
> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_NEXTHOP)
> + || nbrec_logical_router_static_route_is_updated(sr,
> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_OPTIONS)
> + || nbrec_logical_router_static_route_is_updated(sr,
> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_OUTPUT_PORT)
> + || nbrec_logical_router_static_route_is_updated(sr,
> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_POLICY)
> + || nbrec_logical_router_static_route_is_updated(sr,
> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_ROUTE_TABLE)
> + || nbrec_logical_router_static_route_is_updated(sr,
> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_SELECTION_FIELDS);
Nit: column order mismatch with the NB table schema.
> +}
> +
> +static struct parsed_route *
> +static_route_lookup_parsed(struct routes_data *routes_data,
> + const struct nbrec_logical_router_static_route
> *sr)
> +{
> + struct hmap *routes = &routes_data->parsed_routes;
> + struct parsed_route *pr = parsed_route_lookup_by_source(
> + ROUTE_SOURCE_STATIC, &sr->header_, routes);
> + if (!pr) {
> + pr = parsed_route_lookup_by_source(
> + ROUTE_SOURCE_IC_DYNAMIC, &sr->header_, routes);
> + }
> + return pr;
> +}
> +
> +enum engine_input_handler_result
> +routes_static_route_change_handler(struct engine_node *node,
> + void *data)
> +{
> + struct routes_data *routes_data = data;
> + const struct nbrec_logical_router_static_route_table *
> + nb_lr_static_route_table =
> + EN_OVSDB_GET(engine_get_input("NB_logical_router_static_route",
> node));
> + struct northd_data *northd_data = engine_get_input_data("northd", node);
> + struct bfd_data *bfd_data = engine_get_input_data("bfd", node);
> + struct parsed_route *pr;
> +
> + /* Logical routers reference their static routes, so a route change also
> + * marks the router as updated. Iterate the updated routers and their
> + * routes to handle created and updated routes, avoiding a reverse lookup
> + * from route to router. */
> + struct hmapx_node *hmapx_node;
> + HMAPX_FOR_EACH (hmapx_node, &northd_data->trk_data.trk_lrs_routes) {
> + struct ovn_datapath *od = hmapx_node->data;
> + for (size_t i = 0; i < od->nbr->n_static_routes; i++) {
> + const struct nbrec_logical_router_static_route *sr =
> + od->nbr->static_routes[i];
> +
> + if (nbrec_logical_router_static_route_is_new(sr)) {
> + pr = parsed_routes_add_static(od, &northd_data->lr_ports, sr,
> + &bfd_data->bfd_connections,
> + &routes_data->parsed_routes,
> + &routes_data->route_tables,
> + &routes_data->bfd_active_connections);
> + if (!pr) {
> + return EN_UNHANDLED;
> + }
> + hmapx_add(&routes_data->trk_data.trk_crupdated_parsed_route,
> + pr);
> + continue;
> + }
> +
> + /* A BFD column change requires a full recompute. */
> + if (nbrec_logical_router_static_route_is_updated(sr,
> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_BFD)) {
> + return EN_UNHANDLED;
> + }
> +
> + if (!static_route_is_relevant_updated(sr)) {
> + continue;
> + }
> +
> + pr = static_route_lookup_parsed(routes_data, sr);
> + if (!pr || !pr->od) {
> + return EN_UNHANDLED;
> + }
> + hmapx_add(&routes_data->trk_data.trk_deleted_parsed_route, pr);
> + hmap_remove(&routes_data->parsed_routes, &pr->key_node);
> + pr = parsed_routes_add_static(od, &northd_data->lr_ports, sr,
> + &bfd_data->bfd_connections,
> + &routes_data->parsed_routes,
> + &routes_data->route_tables,
> + &routes_data->bfd_active_connections);
> + if (!pr) {
> + /* engine_recompute() will clear the tracked data and free
> the
> + * route we just moved to trk_deleted_parsed_route. */
> + return EN_UNHANDLED;
> + }
> + hmapx_add(&routes_data->trk_data.trk_crupdated_parsed_route, pr);
> + }
> + }
> +
> + /* Deleted routes are no longer referenced by any router, so handle them
> + * by iterating the tracked static route table. */
> + const struct nbrec_logical_router_static_route *changed_static_route;
> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_TABLE_FOR_EACH_TRACKED (
> + changed_static_route, nb_lr_static_route_table) {
> + if (!nbrec_logical_router_static_route_is_deleted(
> + changed_static_route)) {
> + continue;
> + }
> + /* A route created and deleted within the same transaction never got
> a
> + * parsed route. */
> + if (nbrec_logical_router_static_route_is_new(changed_static_route)) {
> + continue;
> + }
> +
> + pr = static_route_lookup_parsed(routes_data, changed_static_route);
> + if (!pr) {
> + return EN_UNHANDLED;
> + }
> + hmapx_add(&routes_data->trk_data.trk_deleted_parsed_route, pr);
> + hmap_remove(&routes_data->parsed_routes, &pr->key_node);
> + }
> +
> + if (!hmapx_is_empty(&routes_data->trk_data.trk_crupdated_parsed_route) ||
> + !hmapx_is_empty(&routes_data->trk_data.trk_deleted_parsed_route)) {
> + routes_data->tracked = true;
Like in the other change handler, this can go in the beginning of the
function. On failure to incrementally process this will be set to
false. A no-op change of routes (e.g., add+delete) shouldn't cause a
failure to incrementally process dependent nodes, e.g.,
group_ecmp_route_routes_change_handler().
> + return EN_HANDLED_UPDATED;
> + }
> +
> + return EN_HANDLED_UNCHANGED;
> +}
> +
> enum engine_node_state
> en_routes_run(struct engine_node *node, void *data)
> {
> @@ -421,6 +552,18 @@ bfd_sync_northd_change_handler(struct engine_node *node,
> void *data OVS_UNUSED)
> return EN_HANDLED_UNCHANGED;
> }
>
> +enum engine_input_handler_result
> +bfd_sync_routes_change_handler(struct engine_node *node,
> + void *data OVS_UNUSED)
> +{
> + struct routes_data *routes_data
> + = engine_get_input_data("routes", node);
> + if (!routes_data->tracked) {
> + return EN_UNHANDLED;
> + }
> + return EN_HANDLED_UNCHANGED;
> +}
> +
> enum engine_node_state
> en_bfd_sync_run(struct engine_node *node, void *data)
> {
> @@ -590,6 +733,12 @@ en_routes_cleanup(void *data)
> routes_destroy(data);
> }
>
> +void
> +en_routes_clear_tracked_data(void *data)
> +{
> + routes_clear_tracked(data);
> +}
> +
> void
> en_bfd_cleanup(void *data)
> {
> diff --git a/northd/en-northd.h b/northd/en-northd.h
> index 7794739b9..c62631008 100644
> --- a/northd/en-northd.h
> +++ b/northd/en-northd.h
> @@ -39,9 +39,12 @@ enum engine_node_state en_route_policies_run(struct
> engine_node *node,
> void *data);
> void *en_route_policies_init(struct engine_node *node OVS_UNUSED,
> struct engine_arg *arg OVS_UNUSED);
> +void en_routes_clear_tracked_data(void *data);
> void en_routes_cleanup(void *data);
> enum engine_input_handler_result
> routes_northd_change_handler(struct engine_node *node, void *data
> OVS_UNUSED);
> +enum engine_input_handler_result
> +routes_static_route_change_handler(struct engine_node *node, void *data);
> enum engine_node_state en_routes_run(struct engine_node *node, void *data);
> void *en_bfd_init(struct engine_node *node OVS_UNUSED,
> struct engine_arg *arg OVS_UNUSED);
> @@ -52,6 +55,10 @@ void *en_bfd_sync_init(struct engine_node *node OVS_UNUSED,
> enum engine_input_handler_result
> bfd_sync_northd_change_handler(struct engine_node *node,
> void *data OVS_UNUSED);
> +enum engine_input_handler_result
> +bfd_sync_routes_change_handler(struct engine_node *node,
> + void *data OVS_UNUSED);
> +
> enum engine_node_state en_bfd_sync_run(struct engine_node *node, void *data);
> void en_bfd_sync_cleanup(void *data OVS_UNUSED);
> void *en_ic_learned_svc_monitors_init(struct engine_node *node OVS_UNUSED,
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index 8376c2e1d..b0b151348 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -75,7 +75,8 @@ static unixctl_cb_func chassis_features_list;
> NB_NODE(sampling_app) \
> NB_NODE(network_function) \
> NB_NODE(network_function_group) \
> - NB_NODE(logical_switch_port_health_check)
> + NB_NODE(logical_switch_port_health_check) \
> + NB_NODE(logical_router_static_route)
>
> enum nb_engine_node {
> #define NB_NODE(NAME) NB_##NAME,
> @@ -177,7 +178,7 @@ static ENGINE_NODE(lr_nat, CLEAR_TRACKED_DATA);
> static ENGINE_NODE(lr_stateful, CLEAR_TRACKED_DATA);
> static ENGINE_NODE(ls_stateful, CLEAR_TRACKED_DATA);
> static ENGINE_NODE(route_policies);
> -static ENGINE_NODE(routes);
> +static ENGINE_NODE(routes, CLEAR_TRACKED_DATA);
> static ENGINE_NODE(bfd);
> static ENGINE_NODE(bfd_sync, SB_WRITE);
> static ENGINE_NODE(ecmp_nexthop, SB_WRITE);
> @@ -336,10 +337,12 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_routes, &en_bfd, NULL);
> engine_add_input(&en_routes, &en_northd,
> routes_northd_change_handler);
> + engine_add_input(&en_routes, &en_nb_logical_router_static_route,
> + routes_static_route_change_handler);
>
> engine_add_input(&en_bfd_sync, &en_bfd, NULL);
> engine_add_input(&en_bfd_sync, &en_nb_bfd, NULL);
> - engine_add_input(&en_bfd_sync, &en_routes, NULL);
> + engine_add_input(&en_bfd_sync, &en_routes,
> bfd_sync_routes_change_handler);
> engine_add_input(&en_bfd_sync, &en_route_policies, NULL);
> engine_add_input(&en_bfd_sync, &en_northd,
> bfd_sync_northd_change_handler);
>
> @@ -375,7 +378,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_learned_route_sync, &en_northd,
> learned_route_sync_northd_change_handler);
>
> - engine_add_input(&en_group_ecmp_route, &en_routes, NULL);
> + engine_add_input(&en_group_ecmp_route, &en_routes,
> + group_ecmp_route_routes_change_handler);
> engine_add_input(&en_group_ecmp_route, &en_learned_route_sync,
> group_ecmp_route_learned_route_change_handler);
>
> @@ -394,9 +398,12 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> engine_add_input(&en_lflow, &en_sb_logical_dp_group, NULL);
> engine_add_input(&en_lflow, &en_bfd_sync, NULL);
> engine_add_input(&en_lflow, &en_route_policies, NULL);
> - engine_add_input(&en_lflow, &en_routes, NULL);
> - /* XXX: The incremental processing only supports changes to learned
> routes.
> - * All other changes trigger a full recompute. */
> + /* Route changes are propagated to en_lflow through the
> en_group_ecmp_route
> + * input. Any change to en_routes also triggers en_group_ecmp_route (via
> + * group_ecmp_route_routes_change_handler), which then triggers en_lflow.
> + * If en_routes falls back to a recompute, en_group_ecmp_route recomputes
> + * as well and so does en_lflow. Hence a noop handler is enough here. */
> + engine_add_input(&en_lflow, &en_routes, engine_noop_handler);
> engine_add_input(&en_lflow, &en_group_ecmp_route,
> lflow_group_ecmp_route_change_handler);
> engine_add_input(&en_lflow, &en_global_config,
> diff --git a/northd/northd.c b/northd/northd.c
> index e4cd93969..4f005c1a7 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -4563,6 +4563,7 @@ destroy_northd_data_tracked_changes(struct northd_data
> *nd)
> destroy_tracked_ovn_ports(&trk_changes->trk_lsps);
> destroy_tracked_lbs(&trk_changes->trk_lbs);
> hmapx_clear(&trk_changes->trk_nat_lrs);
> + hmapx_clear(&trk_changes->trk_lrs_routes);
> hmapx_clear(&trk_changes->ls_with_changed_lbs);
> hmapx_clear(&trk_changes->ls_with_changed_acls);
> hmapx_clear(&trk_changes->ls_with_changed_ipam);
> @@ -4586,6 +4587,7 @@ init_northd_tracked_data(struct northd_data *nd)
> hmapx_init(&trk_data->trk_lbs.crupdated);
> hmapx_init(&trk_data->trk_lbs.deleted);
> hmapx_init(&trk_data->trk_nat_lrs);
> + hmapx_init(&trk_data->trk_lrs_routes);
> hmapx_init(&trk_data->ls_with_changed_lbs);
> hmapx_init(&trk_data->ls_with_changed_acls);
> hmapx_init(&trk_data->ls_with_changed_ipam);
> @@ -4604,6 +4606,7 @@ destroy_northd_tracked_data(struct northd_data *nd)
> hmapx_destroy(&trk_data->trk_lbs.crupdated);
> hmapx_destroy(&trk_data->trk_lbs.deleted);
> hmapx_destroy(&trk_data->trk_nat_lrs);
> + hmapx_destroy(&trk_data->trk_lrs_routes);
> hmapx_destroy(&trk_data->ls_with_changed_lbs);
> hmapx_destroy(&trk_data->ls_with_changed_acls);
> hmapx_destroy(&trk_data->ls_with_changed_ipam);
> @@ -5428,7 +5431,8 @@ lr_changes_can_be_handled(const struct
> nbrec_logical_router *lr)
> if (nbrec_logical_router_is_updated(lr, col)) {
> if (col == NBREC_LOGICAL_ROUTER_COL_LOAD_BALANCER
> || col == NBREC_LOGICAL_ROUTER_COL_LOAD_BALANCER_GROUP
> - || col == NBREC_LOGICAL_ROUTER_COL_NAT) {
> + || col == NBREC_LOGICAL_ROUTER_COL_NAT
> + || col == NBREC_LOGICAL_ROUTER_COL_STATIC_ROUTES) {
> continue;
> }
> return false;
> @@ -5453,12 +5457,7 @@ lr_changes_can_be_handled(const struct
> nbrec_logical_router *lr)
> return false;
> }
> }
> - for (size_t i = 0; i < lr->n_static_routes; i++) {
> - if (nbrec_logical_router_static_route_row_get_seqno(
> - lr->static_routes[i], OVSDB_IDL_CHANGE_MODIFY) > 0) {
> - return false;
> - }
> - }
> +
> return true;
> }
>
> @@ -5484,6 +5483,26 @@ is_lr_nats_changed(const struct nbrec_logical_router
> *nbr) {
> || is_lr_nats_seqno_changed(nbr));
> }
>
> +static bool
> +is_lr_static_routes_seqno_changed(const struct nbrec_logical_router *nbr)
> +{
> + for (size_t i = 0; i < nbr->n_static_routes; i++) {
> + if (nbrec_logical_router_static_route_row_get_seqno(
> + nbr->static_routes[i], OVSDB_IDL_CHANGE_MODIFY) > 0)
> {
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> +static bool
> +is_lr_static_routes_changed(const struct nbrec_logical_router *nbr) {
The curly brace should be on the next line.
> + return nbrec_logical_router_is_updated(nbr,
> + NBREC_LOGICAL_ROUTER_COL_STATIC_ROUTES)
> + || is_lr_static_routes_seqno_changed(nbr);
> +}
> +
> /* Return true if changes are handled incrementally, false otherwise.
> *
> * Note: Changes to load balancer and load balancer groups associated with
> @@ -5555,6 +5574,22 @@ northd_handle_lr_changes(const struct northd_input *ni,
>
> hmapx_add(&nd->trk_data.trk_nat_lrs, od);
> }
> +
> + /* Static Route was added or deleted. */
> + if (is_lr_static_routes_changed(changed_lr)) {
> + struct ovn_datapath *od = ovn_datapath_find_(
> + &nd->lr_datapaths.datapaths,
> + &changed_lr->header_.uuid);
> +
> + if (!od) {
> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1,
> 1);
> + VLOG_WARN_RL(&rl, "Internal error: a tracked updated LR "
> + "doesn't exist in lr_datapaths: "UUID_FMT,
> + UUID_ARGS(&changed_lr->header_.uuid));
> + goto fail;
> + }
> + hmapx_add(&nd->trk_data.trk_lrs_routes, od);
> + }
> }
>
> HMAPX_FOR_EACH (node, &ni->synced_lrs->deleted) {
> @@ -5595,6 +5630,9 @@ northd_handle_lr_changes(const struct northd_input *ni,
> if (!hmapx_is_empty(&nd->trk_data.trk_nat_lrs)) {
> nd->trk_data.type |= NORTHD_TRACKED_LR_NATS;
> }
> + if (!hmapx_is_empty(&nd->trk_data.trk_lrs_routes)) {
> + nd->trk_data.type |= NORTHD_TRACKED_LR_ROUTES;
> + }
> if (!hmapx_is_empty(&nd->trk_data.trk_routers.crupdated) ||
> !hmapx_is_empty(&nd->trk_data.trk_routers.deleted)) {
> nd->trk_data.type |= NORTHD_TRACKED_ROUTERS;
> @@ -12461,7 +12499,7 @@ parsed_route_add(const struct ovn_datapath *od,
> }
> }
>
> -static void
> +struct parsed_route *
> parsed_routes_add_static(const struct ovn_datapath *od,
> const struct hmap *lr_ports,
> const struct nbrec_logical_router_static_route
> *route,
> @@ -12482,8 +12520,9 @@ parsed_routes_add_static(const struct ovn_datapath
> *od,
> 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)) {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> @@ -12491,7 +12530,7 @@ parsed_routes_add_static(const struct ovn_datapath
> *od,
> UUID_FMT, route->nexthop,
> UUID_ARGS(&route->header_.uuid));
> free(nexthop);
> - return;
> + return NULL;
> }
> }
>
> @@ -12503,7 +12542,7 @@ parsed_routes_add_static(const struct ovn_datapath
> *od,
> 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. */
> @@ -12515,7 +12554,7 @@ parsed_routes_add_static(const struct ovn_datapath
> *od,
> : IN6_IS_ADDR_V4MAPPED(&prefix),
> &lrp_addr_s, &out_port)) {
> free(nexthop);
> - return;
> + return NULL;
> }
>
> const struct nbrec_bfd *nb_bt = route->bfd;
> @@ -12525,7 +12564,7 @@ parsed_routes_add_static(const struct ovn_datapath
> *od,
> nb_bt->dst_ip);
> if (!bfd_e) {
> free(nexthop);
> - return;
> + return NULL;
> }
>
> /* This static route is linked to an active bfd session. */
> @@ -12542,10 +12581,9 @@ parsed_routes_add_static(const struct ovn_datapath
> *od,
> bfd_set_status(bfd_sr, "down");
> }
>
> -
> if (!strcmp(bfd_sr->status, "down")) {
> free(nexthop);
> - return;
> + return NULL;
> }
> }
>
> @@ -12589,12 +12627,17 @@ parsed_routes_add_static(const struct ovn_datapath
> *od,
> bool dynamic_routing_advertise = smap_get_bool(&route->options,
>
> "dynamic-routing-advertise",
> true);
> - parsed_route_add(od, nexthop, &prefix, plen, is_discard_route,
> lrp_addr_s,
> - out_port, route_table_id, is_src_route,
> - ecmp_symmetric_reply, override_connected,
> - &ecmp_selection_fields, source,
> dynamic_routing_advertise,
> - &route->header_, NULL, routes);
> + struct parsed_route *pr = parsed_route_add(od, nexthop, &prefix, plen,
> + is_discard_route, lrp_addr_s,
> + out_port, route_table_id,
> + is_src_route,
> + ecmp_symmetric_reply,
> + override_connected,
> + &ecmp_selection_fields,
> source,
> + dynamic_routing_advertise,
> + &route->header_, NULL,
> routes);
> sset_destroy(&ecmp_selection_fields);
> + return pr;
> }
>
> static void
> @@ -21128,6 +21171,9 @@ routes_init(struct routes_data *data)
> hmap_init(&data->parsed_routes);
> simap_init(&data->route_tables);
> hmap_init(&data->bfd_active_connections);
> + data->tracked = false;
This should go to the end now as we changed the order of the fields in
this revision.
> + hmapx_init(&data->trk_data.trk_deleted_parsed_route);
> + hmapx_init(&data->trk_data.trk_crupdated_parsed_route);
> }
>
> void
> @@ -21258,6 +21304,22 @@ routes_destroy(struct routes_data *data)
>
> simap_destroy(&data->route_tables);
> __bfd_destroy(&data->bfd_active_connections);
> + data->tracked = false;
No real need to reset this, the whole 'data' will be freed soon.
> + hmapx_destroy(&data->trk_data.trk_crupdated_parsed_route);
> + hmapx_destroy(&data->trk_data.trk_deleted_parsed_route);
> +}
> +
> +void
> +routes_clear_tracked(struct routes_data *data)
> +{
> + data->tracked = false;
> + hmapx_clear(&data->trk_data.trk_crupdated_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);
> + }
> }
>
> void
> diff --git a/northd/northd.h b/northd/northd.h
> index bb86d3dac..aeea8961b 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -159,6 +159,7 @@ enum northd_tracked_data_type {
> NORTHD_TRACKED_LS_ACLS = (1 << 4),
> NORTHD_TRACKED_SWITCHES = (1 << 5),
> NORTHD_TRACKED_ROUTERS = (1 << 6),
> + NORTHD_TRACKED_LR_ROUTES = (1 << 7),
> };
>
> /* Track what's changed in the northd engine node.
> @@ -176,6 +177,10 @@ struct northd_tracked_data {
> * hmapx node is 'struct ovn_datapath *'. */
> struct hmapx trk_nat_lrs;
>
> + /* Tracked logical routers whose static routes have changed.
> + * hmapx node is 'struct ovn_datapath *'. */
> + struct hmapx trk_lrs_routes;
> +
> /* Tracked logical switches whose load balancers have changed.
> * hmapx node is 'struct ovn_datapath *'. */
> struct hmapx ls_with_changed_lbs;
> @@ -214,10 +219,17 @@ struct route_policy {
> uint32_t jump_chain_id;
> };
>
> +struct route_tracked_data {
> + struct hmapx trk_crupdated_parsed_route;
> + struct hmapx trk_deleted_parsed_route;
> +};
> +
> struct routes_data {
> struct hmap parsed_routes; /* Stores struct parsed_route. */
> struct simap route_tables;
> struct hmap bfd_active_connections;
> + struct route_tracked_data trk_data;
> + bool tracked;
> };
>
> struct route_policies_data {
> @@ -894,6 +906,14 @@ struct parsed_route *parsed_route_add(
> const struct ovn_port *tracked_port,
> struct hmap *routes);
>
> +struct parsed_route *parsed_routes_add_static(
> + const struct ovn_datapath *od,
> + const struct hmap *lr_ports,
> + const struct nbrec_logical_router_static_route *route,
> + const struct hmap *bfd_connections,
> + struct hmap *routes, struct simap *route_tables,
> + struct hmap *bfd_active_connections);
> +
> struct svc_monitors_map_data {
> const struct hmap *local_svc_monitors_map;
> const struct hmap *ic_learned_svc_monitors_map;
> @@ -938,6 +958,7 @@ void build_parsed_routes(const struct ovn_datapath *,
> const struct hmap *,
> uint32_t get_route_table_id(struct simap *, const char *);
> void routes_init(struct routes_data *);
> void routes_destroy(struct routes_data *);
> +void routes_clear_tracked(struct routes_data *);
>
> void bfd_init(struct bfd_data *);
> void bfd_destroy(struct bfd_data *);
> @@ -1050,6 +1071,11 @@ northd_has_lr_nats_in_tracked_data(struct
> northd_tracked_data *trk_nd_changes)
> {
> return trk_nd_changes->type & NORTHD_TRACKED_LR_NATS;
> }
Nit: missing empty line.
> +static inline bool
> +northd_has_lr_route_in_tracked_data(struct northd_tracked_data
> *trk_nd_changes)
> +{
> + return trk_nd_changes->type & NORTHD_TRACKED_LR_ROUTES;
> +}
>
> static inline bool
> northd_has_ls_lbs_in_tracked_data(struct northd_tracked_data *trk_nd_changes)
> diff --git a/tests/ovn-inc-proc-graph-dump.at
> b/tests/ovn-inc-proc-graph-dump.at
> index a1fc1d289..269766ded 100644
> --- a/tests/ovn-inc-proc-graph-dump.at
> +++ b/tests/ovn-inc-proc-graph-dump.at
> @@ -150,16 +150,18 @@ digraph "Incremental-Processing-Engine" {
> bfd [[style=filled, shape=box, fillcolor=white, label="bfd"]];
> NB_bfd -> bfd [[label=""]];
> SB_bfd -> bfd [[label=""]];
> + NB_logical_router_static_route [[style=filled, shape=box,
> fillcolor=white, label="NB_logical_router_static_route"]];
> routes [[style=filled, shape=box, fillcolor=white, label="routes"]];
> bfd -> routes [[label=""]];
> northd -> routes [[label="routes_northd_change_handler"]];
> + NB_logical_router_static_route -> routes
> [[label="routes_static_route_change_handler"]];
> route_policies [[style=filled, shape=box, fillcolor=white,
> label="route_policies"]];
> bfd -> route_policies [[label=""]];
> northd -> route_policies
> [[label="route_policies_northd_change_handler"]];
> bfd_sync [[style=filled, shape=box, fillcolor=white, label="bfd_sync"]];
> bfd -> bfd_sync [[label=""]];
> NB_bfd -> bfd_sync [[label=""]];
> - routes -> bfd_sync [[label=""]];
> + routes -> bfd_sync [[label="bfd_sync_routes_change_handler"]];
> route_policies -> bfd_sync [[label=""]];
> northd -> bfd_sync [[label="bfd_sync_northd_change_handler"]];
> SB_learned_route [[style=filled, shape=box, fillcolor=white,
> label="SB_learned_route"]];
> @@ -167,7 +169,7 @@ digraph "Incremental-Processing-Engine" {
> SB_learned_route -> learned_route_sync
> [[label="learned_route_sync_sb_learned_route_change_handler"]];
> northd -> learned_route_sync
> [[label="learned_route_sync_northd_change_handler"]];
> group_ecmp_route [[style=filled, shape=box, fillcolor=white,
> label="group_ecmp_route"]];
> - routes -> group_ecmp_route [[label=""]];
> + routes -> group_ecmp_route
> [[label="group_ecmp_route_routes_change_handler"]];
> learned_route_sync -> group_ecmp_route
> [[label="group_ecmp_route_learned_route_change_handler"]];
> ls_stateful [[style=filled, shape=box, fillcolor=white,
> label="ls_stateful"]];
> northd -> ls_stateful [[label="ls_stateful_northd_handler"]];
> @@ -185,7 +187,7 @@ digraph "Incremental-Processing-Engine" {
> SB_logical_dp_group -> lflow [[label=""]];
> bfd_sync -> lflow [[label=""]];
> route_policies -> lflow [[label=""]];
> - routes -> lflow [[label=""]];
> + routes -> lflow [[label="engine_noop_handler"]];
> group_ecmp_route -> lflow
> [[label="lflow_group_ecmp_route_change_handler"]];
> global_config -> lflow [[label="node_global_config_handler"]];
> sampling_app -> lflow [[label=""]];
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 2edcfc2cb..3c4a98bfb 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -4485,7 +4485,7 @@ check ovn-nbctl --bfd=$uuid lr-route-add r0 100.0.0.0/8
> 192.168.1.2
> wait_column down bfd status logical_port=r0-sw1
> AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.1.2 | grep -q bfd], [0],
> [], [ignore])
>
> -check_engine_stats northd recompute nocompute
> +check_engine_stats northd norecompute compute
> check_engine_stats bfd recompute nocompute
> check_engine_stats routes recompute nocompute
> check_engine_stats lflow recompute nocompute
> @@ -4501,7 +4501,7 @@ check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8
> 192.168.5.2 r0-sw5
> wait_column down bfd status logical_port=r0-sw5
> AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.5.2 | grep -q bfd], [0],
> [], [ignore])
>
> -check_engine_stats northd recompute nocompute
> +check_engine_stats northd norecompute compute
> check_engine_stats bfd recompute nocompute
> check_engine_stats routes recompute nocompute
> check_engine_stats lflow recompute nocompute
> @@ -4513,7 +4513,7 @@ check ovn-nbctl --bfd --policy=src-ip lr-route-add r0
> 192.168.6.1/32 192.168.10.
> wait_column down bfd status logical_port=r0-sw6
> AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.6.1 | grep -q bfd], [0],
> [], [ignore])
>
> -check_engine_stats northd recompute nocompute
> +check_engine_stats northd norecompute compute
> check_engine_stats bfd recompute nocompute
> check_engine_stats route_policies recompute nocompute
> check_engine_stats lflow recompute nocompute
> @@ -4548,7 +4548,7 @@ wait_column down bfd status logical_port=r0-sw8
> bfd_route_policy_uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw8)
> AT_CHECK([ovn-nbctl list logical_router_policy | grep -q
> $bfd_route_policy_uuid])
>
> -check_engine_stats northd recompute nocompute
> +check_engine_stats northd recompute incremental
> check_engine_stats bfd recompute nocompute
> check_engine_stats routes recompute nocompute
> check_engine_stats lflow recompute nocompute
> @@ -17014,12 +17014,12 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> check ovn-nbctl --wait=sb lr-route-add lr0 192.168.0.0/24 10.0.0.10
> -check_engine_compute northd recompute
> -check_engine_compute routes recompute
> +check_engine_compute northd incremental
> +check_engine_compute routes incremental
> check_engine_compute advertised_route_sync recompute
> -check_engine_compute learned_route_sync recompute
> -check_engine_compute group_ecmp_route recompute
> -check_engine_compute lflow recompute
> +check_engine_compute learned_route_sync incremental
> +check_engine_compute group_ecmp_route incremental
> +check_engine_compute lflow incremental
> CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> @@ -21201,3 +21201,118 @@ check_column "$global_svc_mon_mac"
> sb:Service_Monitor src_mac port=2
> OVN_CLEANUP_NORTHD
> AT_CLEANUP
> ])
> +
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([Static Route incremental processing])
> +ovn_start
> +
> +check ovn-nbctl lr-add r0
> +
> +check ovn-nbctl --wait=sb lrp-add r0 r0-lrp1 00:00:00:00:00:01
> 192.168.1.1/24 fe80::1/64
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb lr-route-add r0 10.0.0.0/24 192.168.1.2
> +
> +check_engine_compute northd incremental
> +check_engine_compute routes incremental
> +check_engine_compute bfd_sync incremental
> +check_engine_compute group_ecmp_route incremental
> +check_engine_compute lflow incremental
> +
> +static_route_uuid=`ovn-nbctl --bare --columns _uuid find
> Logical_Router_Static_Route nexthop=192.168.1.2`
> +AT_CHECK([test "$static_route_uuid" != ""])
> +
> +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 && ip4.dst
> == 10.0.0.0/24"')
> +AT_CHECK([test "$sr_lflow_uuid" != ""])
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb set logical_router_static_route $static_route_uuid
> nexthop=192.168.1.3
> +check_engine_compute northd incremental
> +check_engine_compute routes incremental
> +check_engine_compute bfd_sync incremental
> +check_engine_compute group_ecmp_route incremental
> +check_engine_compute lflow incremental
> +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 && ip4.dst
> == 10.0.0.0/24"')
> +AT_CHECK([test "$sr_lflow_uuid" != ""])
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb set logical_router_static_route $static_route_uuid
> ip_prefix=10.0.1.0/24
> +check_engine_compute northd incremental
> +check_engine_compute routes incremental
> +check_engine_compute bfd_sync incremental
> +check_engine_compute group_ecmp_route incremental
> +check_engine_compute lflow incremental
> +sr_lflow_uuid_old=$(fetch_column Logical_flow _uuid match='"reg7 == 0 &&
> ip4.dst == 10.0.0.0/24"')
> +AT_CHECK([test "$sr_lflow_uuid_old" == ""])
> +
> +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 && ip4.dst
> == 10.0.1.0/24"')
> +AT_CHECK([test "$sr_lflow_uuid" != ""])
> +
> +check ovn-nbctl --wait=sb lrp-add r0 r0-lrp2 00:00:00:00:00:02
> 192.168.1.10/24
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb set logical_router_static_route $static_route_uuid
> output_port=r0-lrp2
> +check_engine_compute northd incremental
> +check_engine_compute routes incremental
> +check_engine_compute bfd_sync incremental
> +check_engine_compute group_ecmp_route incremental
> +check_engine_compute lflow incremental
> +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 && ip4.dst
> == 10.0.1.0/24"')
> +AT_CHECK([test "$sr_lflow_uuid" != ""])
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb set logical_router_static_route $static_route_uuid
> policy=src-ip
> +check_engine_compute northd incremental
> +check_engine_compute routes incremental
> +check_engine_compute bfd_sync incremental
> +check_engine_compute group_ecmp_route incremental
> +check_engine_compute lflow incremental
> +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 && ip4.src
> == 10.0.1.0/24"')
> +AT_CHECK([test "$sr_lflow_uuid" != ""])
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb set logical_router_static_route $static_route_uuid
> policy=dst-ip
> +check ovn-nbctl --wait=sb set logical_router_static_route $static_route_uuid
> selection_fields="ip_proto,ip_src,ip_dst"
> +check_engine_compute northd incremental
> +check_engine_compute routes incremental
> +check_engine_compute bfd_sync incremental
> +check_engine_compute group_ecmp_route incremental
> +check_engine_compute lflow incremental
> +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 && ip4.dst
> == 10.0.1.0/24"')
> +AT_CHECK([test "$sr_lflow_uuid" != ""])
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb set logical_router_static_route $static_route_uuid
> options:ecmp_symmetric_reply=true
> +check_engine_compute northd incremental
> +check_engine_compute routes incremental
> +check_engine_compute bfd_sync incremental
> +check_engine_compute group_ecmp_route incremental
> +check_engine_compute lflow incremental
> +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 && ip4.dst
> == 10.0.1.0/24"')
> +AT_CHECK([test "$sr_lflow_uuid" != ""])
> +
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb set logical_router_static_route $static_route_uuid
> nexthop=\"fe80::2\"
> +check_engine_compute northd incremental
> +check_engine_compute routes incremental
> +check_engine_compute bfd_sync incremental
> +check_engine_compute group_ecmp_route incremental
> +check_engine_compute lflow incremental
> +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 && ip4.dst
> == 10.0.1.0/24"')
> +AT_CHECK([test "$sr_lflow_uuid" != ""])
> +
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl remove logical_router r0 static_routes $static_route_uuid
> +check_engine_compute northd incremental
> +check_engine_compute routes incremental
> +check_engine_compute bfd_sync incremental
> +check_engine_compute group_ecmp_route incremental
> +check_engine_compute lflow incremental
> +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 && ip4.src
> == 10.0.1.0/24"')
> +AT_CHECK([test "$sr_lflow_uuid" == ""])
> +
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> +
> +OVN_CLEANUP_NORTHD
> +AT_CLEANUP
> +])
I took care of the minor issues above and pushed the patch to main.
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
