On 6/1/26 3:50 PM, Lucas Vargas Dias wrote: > Hi Dumitru, > Thanks for the review. >
Hi Lucas, No problem! > Em qui., 21 de mai. de 2026 às 12:19, Dumitru Ceara <[email protected]> > escreveu: > >> On 4/21/26 11:12 PM, Lucas Vargas Dias via dev wrote: >>> Create a handler for deleted and updated static routes, >>> and change the handler from engine route which check if >>> it has a new static route created. >>> 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: 4ms >>> ovn-northd completion: 21ms >>> >>> 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: 2ms >>> ovn-northd completion: 32ms >>> >>> Signed-off-by: Lucas Vargas Dias <[email protected]> >>> --- >> >> Hi Lucas, >> >> Thanks for the patch! >> >>> northd/en-group-ecmp-route.c | 57 ++++++++++ >>> northd/en-group-ecmp-route.h | 4 + >>> northd/en-lflow.c | 20 ++++ >>> northd/en-lflow.h | 2 + >>> northd/en-northd.c | 186 +++++++++++++++++++++++++++---- >>> northd/en-northd.h | 5 +- >>> northd/inc-proc-northd.c | 13 ++- >>> northd/northd.c | 107 +++++++++++++----- >>> northd/northd.h | 38 ++++++- >>> tests/ovn-inc-proc-graph-dump.at | 6 +- >>> tests/ovn-northd.at | 98 +++++++++++++--- >>> 11 files changed, 466 insertions(+), 70 deletions(-) >>> >>> diff --git a/northd/en-group-ecmp-route.c b/northd/en-group-ecmp-route.c >>> index c4c93fd84..fcc76b076 100644 >>> --- a/northd/en-group-ecmp-route.c >>> +++ b/northd/en-group-ecmp-route.c >>> @@ -519,3 +519,60 @@ >> group_ecmp_route_learned_route_change_handler(struct engine_node *eng_node, >>> } >>> return EN_HANDLED_UNCHANGED; >>> } >>> + >>> +enum engine_input_handler_result >>> +group_ecmp_static_route_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) { >>> + data->tracked = false; >>> + return EN_UNHANDLED; >>> + } >>> + >>> + 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; >>> + } >>> + >>> + if (pr->is_in_parsed_routes) { >>> + hmap_remove(&routes_data->parsed_routes, &pr->key_node); >> >> We're changing input node data here, we should not do that. A change >> handler for an I-P node should only change that node's data and not its >> input node's data. >> >> E.g., in this loop we're in, what if the first route was successfully >> removed but for the second one we hit !handle_deleted_route(data, pr, >> &updated_routes)? In that case, we already altered route_data and we >> fail incremental processing. Then en_group_ecmp_route_run() will have >> to run (recompute) but it's input data is not correct anymore. >> >> > I agree > > >>> + } >>> + parsed_route_free(pr); >>> + } >>> + >>> + 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)) { >>> + data->tracked = true; >>> + 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..246ca06bf 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_static_route_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-lflow.c b/northd/en-lflow.c >>> index d4351edb9..22cd8fe91 100644 >>> --- a/northd/en-lflow.c >>> +++ b/northd/en-lflow.c >>> @@ -297,6 +297,21 @@ lflow_multicast_igmp_handler(struct engine_node >> *node, void *data) >>> return EN_HANDLED_UPDATED; >>> } >>> >>> +enum engine_input_handler_result >>> +lflow_group_route_change_handler(struct engine_node *node, >>> + void *data OVS_UNUSED) >> >> Nit: indentation >> >> > > I agree > > >>> +{ >>> + struct routes_data *route_data = >>> + engine_get_input_data("routes", node); >>> + >>> + /* If we do not have tracked data we need to recompute. */ >>> + if (!route_data->tracked) { >>> + return EN_UNHANDLED; >>> + } >>> + >>> + return EN_HANDLED_UNCHANGED; >>> +} >>> + >>> enum engine_input_handler_result >>> lflow_group_ecmp_route_change_handler(struct engine_node *node, >>> void *data OVS_UNUSED) >>> @@ -346,6 +361,11 @@ lflow_group_ecmp_route_change_handler(struct >> engine_node *node, >>> route_node->od, lflow_data->lflow_table, >>> route_node, lflow_input.bfd_ports); >>> >>> + build_arp_request_flows_for_lrouter(route_node->od, >>> + lflow_data->lflow_table, >>> + lflow_input.meter_groups, >>> + route_node->lflow_ref); >>> + >>> bool handled = lflow_ref_sync_lflows( >>> route_node->lflow_ref, lflow_data->lflow_table, >>> eng_ctx->ovnsb_idl_txn, lflow_input.dps, >>> diff --git a/northd/en-lflow.h b/northd/en-lflow.h >>> index d2a92e49f..aa320615f 100644 >>> --- a/northd/en-lflow.h >>> +++ b/northd/en-lflow.h >>> @@ -31,5 +31,7 @@ lflow_multicast_igmp_handler(struct engine_node *node, >> void *data); >>> enum engine_input_handler_result >>> lflow_group_ecmp_route_change_handler(struct engine_node *node, void >> *data); >>> enum engine_input_handler_result >>> +lflow_group_route_change_handler(struct engine_node *node, void *data); >>> +enum engine_input_handler_result >>> lflow_ic_learned_svc_mons_handler(struct engine_node *node, void *data); >>> #endif /* EN_LFLOW_H */ >>> diff --git a/northd/en-northd.c b/northd/en-northd.c >>> index c34818dba..c05939a1d 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,32 +330,174 @@ 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) >> >> Why do we need this? Isn't all incremental processing already covered >> by the routes_static_route_change_handler() handler for >> NB.Logical_Router_Static_Route updates? >> >> > You're right, I'll add a new version changing the creation > for routes_static_route_change_handler. > > > > > >>> { >>> struct northd_data *northd_data = engine_get_input_data("northd", >> node); >>> if (!northd_has_tracked_data(&northd_data->trk_data)) { >>> return EN_UNHANDLED; >>> } >>> >>> - /* This node uses the below data from the en_northd engine node. >>> - * See (lr_stateful_get_input_data()) >>> - * 1. northd_data->lr_datapaths >>> - * 2. northd_data->lr_ports >>> - * This data gets updated when a logical router or logical >> router port >>> - * is created or deleted. >>> - * Northd engine node presently falls back to full recompute >> when >>> - * this happens and so does this 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. >>> - */ >>> + if (!northd_has_lr_route_in_tracked_data(&northd_data->trk_data)) { >>> + return EN_HANDLED_UNCHANGED; >>> + } >>> + >>> + struct bfd_data *bfd_data = engine_get_input_data("bfd", node); >>> + struct routes_data *routes_data = data; >>> + struct hmapx_node *hmapx_node; >>> + struct ovn_datapath *od; >>> + HMAPX_FOR_EACH (hmapx_node, &northd_data->trk_data.trk_lrs_routes) { >>> + od = hmapx_node->data; >>> + struct parsed_route *pr; >>> + >>> + for (int i = 0; i < od->nbr->n_static_routes; i++) { >>> + struct nbrec_logical_router_static_route *static_route = >>> + od->nbr->static_routes[i]; >>> + pr = parsed_route_lookup_by_source(ROUTE_SOURCE_STATIC, >>> + &static_route->header_, >>> + >> &routes_data->parsed_routes); >>> + if (pr) { >>> + pr->stale = false; >>> + continue; >>> + } >>> + pr = parsed_routes_add_static(od, &northd_data->lr_ports, >>> + static_route, >>> + &bfd_data->bfd_connections, >>> + &routes_data->parsed_routes, >>> + &routes_data->route_tables, >>> + >> &routes_data->bfd_active_connections); >>> + if (!pr) { >>> + continue; >>> + } >>> + hmapx_add(&routes_data->trk_data.trk_crupdated_parsed_route, >>> + pr); >>> + } >>> + } >>> + >>> + if >> (!hmapx_is_empty(&routes_data->trk_data.trk_crupdated_parsed_route)) { >>> + routes_data->tracked = true; >>> + return EN_HANDLED_UPDATED; >>> + } >>> + >>> return EN_HANDLED_UNCHANGED; >>> } >>> +enum engine_input_handler_result >>> +routes_static_route_change_handler(struct engine_node *node, >>> + void *data) >>> +{ >>> + struct routes_data *routes_data = data; >>> + struct hmapx created_trk_parsed_route = >>> + HMAPX_INITIALIZER(&created_trk_parsed_route); >>> + 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); >>> + >>> + 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) { >>> + >>> + bool is_deleted = nbrec_logical_router_static_route_is_deleted( >>> + >> changed_static_route); >>> + bool is_new = nbrec_logical_router_static_route_is_new( >>> + >> changed_static_route); >>> + >>> + if (is_new && is_deleted) { >>> + continue; >>> + } >>> + >>> + if (is_new) { >>> + hmapx_add(&created_trk_parsed_route, &changed_static_route); >> >> This seems wrong. Did you mean to store the value of >> "changed_static_route" instead? >> > > Also, why do we need a hmapx? We only need a bool to tell us if there >> were any new routes, or am I reading the code wrong? >> >> > I'll add the created/update routes in hmax trk_crupdated_parsed_route. > > >>> + continue; >>> + } >>> + >>> + if (is_deleted) { >>> + struct parsed_route *pr = parsed_route_lookup_by_source( >>> + ROUTE_SOURCE_STATIC, >>> + >> &changed_static_route->header_, >>> + >> &routes_data->parsed_routes); >>> + if (!pr) { >>> + pr = >> parsed_route_lookup_by_source(ROUTE_SOURCE_IC_DYNAMIC, >>> + >> &changed_static_route->header_, >>> + >> &routes_data->parsed_routes); >>> + } >>> + if (pr) { >>> + pr->stale = true; >>> + >> hmapx_add(&routes_data->trk_data.trk_deleted_parsed_route, pr); >>> + } >>> + continue; >>> + } >>> + >>> + if (nbrec_logical_router_static_route_is_updated( >>> + changed_static_route, >>> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_NEXTHOP) >>> + || nbrec_logical_router_static_route_is_updated( >>> + changed_static_route, >>> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_IP_PREFIX) >>> + || nbrec_logical_router_static_route_is_updated( >>> + changed_static_route, >>> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_OUTPUT_PORT) >>> + || nbrec_logical_router_static_route_is_updated( >>> + changed_static_route, >>> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_POLICY) >>> + || nbrec_logical_router_static_route_is_updated( >>> + changed_static_route, >>> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_ROUTE_TABLE) >>> + || nbrec_logical_router_static_route_is_updated( >>> + changed_static_route, >>> + >> NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_SELECTION_FIELDS) >>> + || nbrec_logical_router_static_route_is_updated( >>> + changed_static_route, >>> + NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_OPTIONS)) { >> >> What about NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_BFD? >> > > I miss. > > >> >>> + struct parsed_route *pr = parsed_route_lookup_by_source( >>> + ROUTE_SOURCE_STATIC, >>> + >> &changed_static_route->header_, >>> + >> &routes_data->parsed_routes); >>> + if (!pr) { >>> + pr = parsed_route_lookup_by_source( >>> + ROUTE_SOURCE_IC_DYNAMIC, >>> + >> &changed_static_route->header_, >>> + >> &routes_data->parsed_routes); >>> + } >>> + >>> + if (!pr || !pr->od || !northd_data || !bfd_data) { >> >> northd_data and bfd_data can never be NULL here, I think. >> >>> + continue; >>> + } >>> + struct parsed_route *old_pr = pr; >>> + hmap_remove(&routes_data->parsed_routes, &old_pr->key_node); >>> + pr = parsed_routes_add_static(old_pr->od, >> &northd_data->lr_ports, >>> + changed_static_route, >>> + &bfd_data->bfd_connections, >>> + &routes_data->parsed_routes, >>> + &routes_data->route_tables, >>> + >> &routes_data->bfd_active_connections); >>> + old_pr->is_in_parsed_routes = false; >>> + if (!pr) { >> >> Do we leak old_pr here? >> >> I'll change the logic to save old_pr before try to add the new pr. > > >>> + continue; >>> + } >>> + >>> + hmapx_add(&routes_data->trk_data.trk_crupdated_parsed_route, >>> + pr); >> >> Nit: indentation. >> >>> + hmapx_add(&routes_data->trk_data.trk_deleted_parsed_route, >>> + old_pr); >>> + } >>> + } >>> + if >> (!hmapx_is_empty(&routes_data->trk_data.trk_crupdated_parsed_route) || >>> + >> !hmapx_is_empty(&routes_data->trk_data.trk_deleted_parsed_route)) { >>> + hmapx_destroy(&created_trk_parsed_route); >>> + routes_data->tracked = true; >>> + return EN_HANDLED_UPDATED; >>> + } >> >> I'm a bit confused about how this is supposed to work. Here if we have >> tracked "updated" or "deleted" routes we return EN_HANDLED_UPDATED. >> >> But we could also have new routes that were added in this iteration. >> >> How do we handle those? Also, how often does a static route get >> updated? Usually they're added/deleted. >> > > I add the create here also. In my scenario, it's not updated often. > Do you think it's better to remove the update here? > That's my impression too, that static routes don't get updated usually. They are added/deleted if needed. Maybe it's fine to remove the update part. > >> >>> + >>> + if (!hmapx_is_empty(&created_trk_parsed_route)) { >>> + hmapx_destroy(&created_trk_parsed_route); >>> + return EN_HANDLED_UPDATED; >> >> Same question here about how we handle new routes. >> >>> + } >>> + >>> + hmapx_destroy(&created_trk_parsed_route); >>> + return EN_UNHANDLED; >> >> If we end up here doesn't it actually mean that there are no interesting >> changes? Why would we return EN_UNHANDLED (and trigger a recompute)? >> >> > I see the test "check BFD config propagation to SBDB" fails when adding a > new route with bfd. > parsed_routes_add_static return NULL due to admin_down. > So, I think it's better to trigger recompute. I'll adjust to trigger full > recompute if parsed_routes_add_static > returns NULL. > >> +} >>> >>> enum engine_node_state >>> en_routes_run(struct engine_node *node, void *data) >>> @@ -590,6 +733,11 @@ 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..5247f3e11 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); >>> +routes_northd_change_handler(struct engine_node *node, void *data); >>> +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); >>> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c >>> index ece388ce7..52f5dc57f 100644 >>> --- a/northd/inc-proc-northd.c >>> +++ b/northd/inc-proc-northd.c >>> @@ -76,7 +76,9 @@ 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, >>> @@ -179,7 +181,7 @@ static ENGINE_NODE(lr_stateful, CLEAR_TRACKED_DATA); >>> static ENGINE_NODE(ls_stateful, CLEAR_TRACKED_DATA); >>> static ENGINE_NODE(ls_arp, 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); >>> @@ -341,6 +343,8 @@ 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); >>> @@ -380,7 +384,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_static_route_change_handler); >>> engine_add_input(&en_group_ecmp_route, &en_learned_route_sync, >>> group_ecmp_route_learned_route_change_handler); >>> >>> @@ -399,7 +404,7 @@ 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); >>> + engine_add_input(&en_lflow, &en_routes, >> lflow_group_route_change_handler); >>> /* XXX: The incremental processing only supports changes to learned >> routes. >>> * All other changes trigger a full recompute. */ >>> engine_add_input(&en_lflow, &en_group_ecmp_route, >>> diff --git a/northd/northd.c b/northd/northd.c >>> index 4fd4b9de9..ea48bc442 100644 >>> --- a/northd/northd.c >>> +++ b/northd/northd.c >>> @@ -4517,6 +4517,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); >>> @@ -4540,6 +4541,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); >>> @@ -4558,6 +4560,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); >>> @@ -5379,7 +5382,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; >>> @@ -5404,12 +5408,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; >>> } >>> >>> @@ -5435,6 +5434,13 @@ is_lr_nats_changed(const struct >> nbrec_logical_router *nbr) { >>> || is_lr_nats_seqno_changed(nbr)); >>> } >>> >>> +static bool >>> +is_lr_static_routes_changed(const struct nbrec_logical_router *nbr) { >>> + return nbrec_logical_router_is_updated(nbr, >>> + >> NBREC_LOGICAL_ROUTER_COL_STATIC_ROUTES); >>> +} >>> + >>> + >>> /* Return true if changes are handled incrementally, false otherwise. >>> * >>> * Note: Changes to load balancer and load balancer groups associated >> with >>> @@ -5503,6 +5509,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) { >>> @@ -5543,6 +5565,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; >>> @@ -12189,6 +12214,7 @@ parsed_route_init(const struct ovn_datapath *od, >>> new_pr->route_table_id = route_table_id; >>> new_pr->is_src_route = is_src_route; >>> new_pr->od = od; >>> + new_pr->is_in_parsed_routes = false; >>> new_pr->ecmp_symmetric_reply = ecmp_symmetric_reply; >>> new_pr->is_discard_route = is_discard_route; >>> new_pr->lrp_addr_s = nullable_xstrdup(lrp_addr_s); >>> @@ -12296,6 +12322,7 @@ 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); >>> + new_pr->is_in_parsed_routes = true; >>> return new_pr; >>> } else { >>> pr->stale = false; >>> @@ -12304,7 +12331,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, >>> @@ -12325,8 +12352,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); >>> @@ -12334,7 +12362,7 @@ parsed_routes_add_static(const struct >> ovn_datapath *od, >>> UUID_FMT, route->nexthop, >>> UUID_ARGS(&route->header_.uuid)); >>> free(nexthop); >>> - return; >>> + return NULL; >>> } >>> } >>> >>> @@ -12346,7 +12374,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. */ >>> @@ -12358,7 +12386,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; >>> @@ -12368,7 +12396,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. */ >>> @@ -12385,10 +12413,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; >>> + free(nexthop); >>> + return NULL; >> >> Nit: indentation. >> >>> } >>> } >>> >>> @@ -12427,11 +12454,15 @@ parsed_routes_add_static(const struct >> ovn_datapath *od, >>> source = ROUTE_SOURCE_STATIC; >>> } >>> >>> - parsed_route_add(od, nexthop, &prefix, plen, is_discard_route, >> lrp_addr_s, >>> - out_port, route_table_id, is_src_route, >>> - ecmp_symmetric_reply, &ecmp_selection_fields, >> source, >>> - &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, >>> + &ecmp_selection_fields, >> source, >>> + &route->header_, NULL, >> routes); >>> sset_destroy(&ecmp_selection_fields); >>> + return pr; >>> } >>> >>> static void >>> @@ -16309,13 +16340,14 @@ build_lr_gateway_redirect_flows_for_nats( >>> * In the common case where the Ethernet destination has been resolved, >>> * this table outputs the packet (priority 0). Otherwise, it composes >>> * and sends an ARP/IPv6 NA request (priority 100). */ >>> -static void >>> +void >>> build_arp_request_flows_for_lrouter( >>> - struct ovn_datapath *od, struct lflow_table *lflows, >>> - struct ds *match, struct ds *actions, >>> + const struct ovn_datapath *od, struct lflow_table *lflows, >>> const struct shash *meter_groups, >>> struct lflow_ref *lflow_ref) >>> { >>> + struct ds match = DS_EMPTY_INITIALIZER; >> >> Nit: indentation. >> >> > I agree > > >>> + struct ds actions = DS_EMPTY_INITIALIZER; >>> ovs_assert(od->nbr); >>> for (int i = 0; i < od->nbr->n_static_routes; i++) { >>> const struct nbrec_logical_router_static_route *route; >>> @@ -16329,8 +16361,8 @@ build_arp_request_flows_for_lrouter( >>> continue; >>> } >>> >>> - ds_clear(match); >>> - ds_put_format(match, "eth.dst == 00:00:00:00:00:00 && " >>> + ds_clear(&match); >>> + ds_put_format(&match, "eth.dst == 00:00:00:00:00:00 && " >>> REGBIT_NEXTHOP_IS_IPV4" == 0 && " >>> REG_NEXT_HOP_IPV6 " == %s", >>> route->nexthop); >>> @@ -16342,8 +16374,8 @@ build_arp_request_flows_for_lrouter( >>> char sn_addr_s[INET6_ADDRSTRLEN + 1]; >>> ipv6_string_mapped(sn_addr_s, &sn_addr); >>> >>> - ds_clear(actions); >>> - ds_put_format(actions, >>> + ds_clear(&actions); >>> + ds_put_format(&actions, >>> "nd_ns { " >>> "eth.dst = "ETH_ADDR_FMT"; " >>> "ip6.dst = %s; " >>> @@ -16353,7 +16385,7 @@ build_arp_request_flows_for_lrouter( >>> route->nexthop); >>> >>> ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_REQUEST, 200, >>> - ds_cstr(match), ds_cstr(actions), lflow_ref, >>> + ds_cstr(&match), ds_cstr(&actions), lflow_ref, >>> WITH_CTRL_METER(copp_meter_get(COPP_ND_NS_RESOLVE, >>> od->nbr->copp, >>> meter_groups)), >>> @@ -16385,6 +16417,8 @@ build_arp_request_flows_for_lrouter( >>> >> meter_groups))); >>> ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_REQUEST, 0, "1", "next;", >>> lflow_ref); >>> + ds_destroy(&match); >>> + ds_destroy(&actions); >>> } >>> >>> static void >>> @@ -19508,8 +19542,7 @@ build_lswitch_and_lrouter_iterate_by_lr(struct >> ovn_datapath *od, >>> build_gateway_redirect_flows_for_lrouter(od, lsi->lflows, >> &lsi->match, >>> &lsi->actions, >>> od->datapath_lflows); >>> - build_arp_request_flows_for_lrouter(od, lsi->lflows, &lsi->match, >>> - &lsi->actions, >>> + build_arp_request_flows_for_lrouter(od, lsi->lflows, >>> lsi->meter_groups, >>> od->datapath_lflows); >>> build_ecmp_stateful_egr_flows_for_lrouter(od, lsi->lflows, >>> @@ -21067,6 +21100,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; >>> + hmapx_init(&data->trk_data.trk_deleted_parsed_route); >>> + hmapx_init(&data->trk_data.trk_crupdated_parsed_route); >>> } >>> >>> void >>> @@ -21197,6 +21233,17 @@ routes_destroy(struct routes_data *data) >>> >>> simap_destroy(&data->route_tables); >>> __bfd_destroy(&data->bfd_active_connections); >>> + data->tracked = false; >>> + 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); >>> + hmapx_clear(&data->trk_data.trk_deleted_parsed_route); >>> } >>> >>> void >>> diff --git a/northd/northd.h b/northd/northd.h >>> index a9070d6f6..5b3461840 100644 >>> --- a/northd/northd.h >>> +++ b/northd/northd.h >>> @@ -160,6 +160,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. >>> @@ -177,6 +178,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; >>> @@ -217,10 +222,23 @@ struct route_policy { >>> uint32_t jump_chain_id; >>> }; >>> >>> +struct route_tracked_data { >>> + /* Contains references to group_ecmp_route_node. Each of the >> referenced >>> + * datapaths contains at least one route. */ >>> + struct hmapx trk_crupdated_parsed_route; >>> + >>> + /* Contains references to group_ecmp_route_node. Each of the >> referenced >>> + * datapath previously had some routes. The datapath now no longer >>> + * contains any 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; >>> + bool tracked; >>> + struct route_tracked_data trk_data; >>> }; >>> >>> struct route_policies_data { >>> @@ -855,6 +873,7 @@ struct parsed_route { >>> char *lrp_addr_s; >>> const struct ovn_port *out_port; >>> const struct ovn_port *tracked_port; /* May be NULL. */ >>> + bool is_in_parsed_routes; >>> }; >>> >>> struct parsed_route *parsed_route_clone(const struct parsed_route *); >>> @@ -881,6 +900,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; >>> @@ -925,7 +952,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 *); >>> >>> @@ -951,6 +978,10 @@ void build_route_data_flows_for_lrouter( >>> const struct ovn_datapath *od, struct lflow_table *lflows, >>> const struct group_ecmp_datapath *route_node, >>> const struct sset *bfd_ports); >>> +void build_arp_request_flows_for_lrouter( >>> + const struct ovn_datapath *od, struct lflow_table *lflows, >>> + const struct shash *meter_groups, >>> + struct lflow_ref *lflow_ref); >>> >>> bool lflow_handle_northd_lr_changes(struct ovsdb_idl_txn *ovnsh_txn, >>> struct tracked_dps *, >>> @@ -1041,6 +1072,11 @@ northd_has_lr_nats_in_tracked_data(struct >> northd_tracked_data *trk_nd_changes) >>> { >>> return trk_nd_changes->type & NORTHD_TRACKED_LR_NATS; >>> } >>> +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 178310978..fd05c20dc 100644 >>> --- a/tests/ovn-inc-proc-graph-dump.at >>> +++ b/tests/ovn-inc-proc-graph-dump.at >>> @@ -151,9 +151,11 @@ 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"]]; >>> @@ -168,7 +170,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_static_route_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"]]; >>> @@ -189,7 +191,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="lflow_group_route_change_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 1d7bd6c28..eacccaf20 100644 >>> --- a/tests/ovn-northd.at >>> +++ b/tests/ovn-northd.at >>> @@ -4272,9 +4272,9 @@ 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 routes recompute incremental >>> check_engine_stats lflow recompute nocompute >>> check_engine_stats northd_output norecompute compute >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE >>> @@ -4288,9 +4288,9 @@ 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 routes recompute incremental >>> check_engine_stats lflow recompute nocompute >>> check_engine_stats northd_output norecompute compute >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE >>> @@ -4300,7 +4300,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 >>> @@ -4335,10 +4335,10 @@ 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 >>> +check_engine_stats routes recompute incremental >>> +check_engine_stats lflow recompute incremental >>> check_engine_stats northd_output norecompute compute >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE >>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats >>> @@ -16437,12 +16437,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 >>> @@ -20672,3 +20672,75 @@ 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]) >> >> In general for this test, should we also check the actual contents of >> the SB database after each command? We only check the I-P engine stats. >> > > We can check. > > >>> +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 >>> + >>> +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 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` >>> + >>> +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 group_ecmp_route incremental >>> +check_engine_compute lflow incremental >>> + >>> +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 group_ecmp_route incremental >>> +check_engine_compute lflow incremental >>> + >>> +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 group_ecmp_route incremental >>> +check_engine_compute lflow incremental >>> + >>> +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 group_ecmp_route incremental >>> +check_engine_compute lflow incremental >>> + >>> +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 selection_fields="ip_proto,ip_src,ip_dst" >>> +check_engine_compute northd incremental >>> +check_engine_compute routes incremental >>> +check_engine_compute group_ecmp_route incremental >>> +check_engine_compute lflow incremental >>> + >>> +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 group_ecmp_route incremental >>> +check_engine_compute lflow incremental >>> + >>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats >>> +check ovn-nbctl remove logical_router r0 static_routes >> $static_route_uuid >> >> Missing --wait=sb. >> >> I agree > > > Regards, > Lucas > > Looking forward to v3, thanks! Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
