On Fri, Jul 18, 2025 at 6:15 PM Mark Michelson via dev < ovs-dev@openvswitch.org> wrote:
> Reported-at: https://issues.redhat.com/browse/FDP-1522 > Reported-at: https://issues.redhat.com/browse/FDP-1523 > > Signed-off-by: Mark Michelson <mmich...@redhat.com> > --- > northd/en-datapath-logical-router.c | 131 ++++++++++++++++++++++++++-- > northd/en-datapath-logical-router.h | 16 +++- > northd/en-datapath-logical-switch.c | 127 +++++++++++++++++++++++++-- > northd/en-datapath-logical-switch.h | 14 +++ > northd/inc-proc-northd.c | 8 +- > tests/ovn-northd.at | 53 +++++++++++ > 6 files changed, 332 insertions(+), 17 deletions(-) > > diff --git a/northd/en-datapath-logical-router.c > b/northd/en-datapath-logical-router.c > index 8a9adbc4f..a174e8abe 100644 > --- a/northd/en-datapath-logical-router.c > +++ b/northd/en-datapath-logical-router.c > @@ -230,12 +230,30 @@ en_datapath_logical_router_cleanup(void *data) > ovn_unsynced_datapath_map_destroy(map); > } > > +struct ovn_synced_logical_router * > +ovn_synced_logical_router_find(const struct ovn_synced_logical_router_map > *map, > + const struct uuid *nb_uuid) > +{ > + struct ovn_synced_logical_router *lr; > + HMAP_FOR_EACH_WITH_HASH (lr, hmap_node, uuid_hash(nb_uuid), > + &map->synced_routers) { > + if (uuid_equals(&lr->nb->header_.uuid, nb_uuid)) { > + return lr; > + } > + } > + > + return NULL; > +} > + > static void > synced_logical_router_map_init( > struct ovn_synced_logical_router_map *router_map) > { > *router_map = (struct ovn_synced_logical_router_map) { > .synced_routers = HMAP_INITIALIZER(&router_map->synced_routers), > + .new = HMAPX_INITIALIZER(&router_map->new), > + .updated = HMAPX_INITIALIZER(&router_map->updated), > + .deleted = HMAPX_INITIALIZER(&router_map->deleted), > }; > } > > @@ -243,7 +261,17 @@ static void > synced_logical_router_map_destroy( > struct ovn_synced_logical_router_map *router_map) > { > + hmapx_destroy(&router_map->new); > + hmapx_destroy(&router_map->updated); > + > + struct hmapx_node *node; > struct ovn_synced_logical_router *lr; > + HMAPX_FOR_EACH_SAFE (node, &router_map->deleted) { > + lr = node->data; > + free(lr); > + hmapx_delete(&router_map->deleted, node); > + } > + hmapx_destroy(&router_map->deleted); > HMAP_FOR_EACH_POP (lr, hmap_node, &router_map->synced_routers) { > free(lr); > } > @@ -261,6 +289,18 @@ en_datapath_synced_logical_router_init(struct > engine_node *node OVS_UNUSED, > return router_map; > } > > +static struct ovn_synced_logical_router * > +synced_logical_router_alloc(const struct ovn_synced_datapath *sdp) > +{ > + struct ovn_synced_logical_router *lr = xmalloc(sizeof *lr); > + *lr = (struct ovn_synced_logical_router) { > + .nb = CONTAINER_OF(sdp->nb_row, struct nbrec_logical_router, > + header_), > + .sb = sdp->sb_dp, > + }; > + return lr; > +} > + > enum engine_node_state > en_datapath_synced_logical_router_run(struct engine_node *node , void > *data) > { > @@ -276,12 +316,8 @@ en_datapath_synced_logical_router_run(struct > engine_node *node , void *data) > if (sdp->nb_row->table->class_ != &nbrec_table_logical_router) { > continue; > } > - struct ovn_synced_logical_router *lr = xmalloc(sizeof *lr); > - *lr = (struct ovn_synced_logical_router) { > - .nb = CONTAINER_OF(sdp->nb_row, struct nbrec_logical_router, > - header_), > - .sb = sdp->sb_dp, > - }; > + struct ovn_synced_logical_router *lr = > + synced_logical_router_alloc(sdp); > hmap_insert(&router_map->synced_routers, &lr->hmap_node, > uuid_hash(&lr->nb->header_.uuid)); > } > @@ -289,6 +325,89 @@ en_datapath_synced_logical_router_run(struct > engine_node *node , void *data) > return EN_UPDATED; > } > > +void > +en_datapath_synced_logical_router_clear_tracked_data(void *data) > +{ > + struct ovn_synced_logical_router_map *router_map = data; > + > + hmapx_clear(&router_map->new); > + hmapx_clear(&router_map->updated); > + > + struct hmapx_node *node; > + HMAPX_FOR_EACH_SAFE (node, &router_map->deleted) { > + struct ovn_synced_logical_router *lr = node->data; > + free(lr); > + hmapx_delete(&router_map->deleted, node); > + } > +} > + > +enum engine_input_handler_result > +en_datapath_synced_logical_router_datapath_sync_handler( > + struct engine_node *node, void *data) > +{ > + const struct ovn_synced_datapaths *dps = > + engine_get_input_data("datapath_sync", node); > + struct ovn_synced_logical_router_map *router_map = data; > + > + if (hmapx_is_empty(&dps->deleted) && > + hmapx_is_empty(&dps->new) && > + hmapx_is_empty(&dps->updated)) { > + return EN_UNHANDLED; > + } > + > + enum engine_input_handler_result result = EN_HANDLED_UNCHANGED; > + > + struct hmapx_node *hmapx_node; > + struct ovn_synced_datapath *sdp; > + struct ovn_synced_logical_router *lr; > + HMAPX_FOR_EACH (hmapx_node, &dps->new) { > + sdp = hmapx_node->data; > + if (sdp->nb_row->table->class_ != &nbrec_table_logical_router) { > + continue; > + } > + lr = synced_logical_router_alloc(sdp); > + hmap_insert(&router_map->synced_routers, &lr->hmap_node, > + uuid_hash(&lr->nb->header_.uuid)); > + hmapx_add(&router_map->new, lr); > + result = EN_HANDLED_UPDATED; > + } > + HMAPX_FOR_EACH (hmapx_node, &dps->deleted) { > + sdp = hmapx_node->data; > + if (sdp->nb_row->table->class_ != &nbrec_table_logical_router) { > + continue; > + } > + lr = ovn_synced_logical_router_find(router_map, > &sdp->nb_row->uuid); > + if (!lr) { > + return EN_UNHANDLED; > + } > + hmap_remove(&router_map->synced_routers, &lr->hmap_node); > + hmapx_add(&router_map->deleted, lr); > + result = EN_HANDLED_UPDATED; > + } > + HMAPX_FOR_EACH (hmapx_node, &dps->updated) { > + sdp = hmapx_node->data; > + if (sdp->nb_row->table->class_ != &nbrec_table_logical_router) { > + continue; > + } > + lr = ovn_synced_logical_router_find(router_map, > &sdp->nb_row->uuid); > + if (!lr) { > + return EN_UNHANDLED; > + } > + lr->nb = CONTAINER_OF(sdp->nb_row, struct nbrec_logical_router, > + header_); > + lr->sb = sdp->sb_dp; > + hmap_remove(&router_map->synced_routers, &lr->hmap_node); > + free(lr); > + lr = synced_logical_router_alloc(sdp); > + hmap_insert(&router_map->synced_routers, &lr->hmap_node, > + uuid_hash(&lr->nb->header_.uuid)); > + hmapx_add(&router_map->updated, lr); > + result = EN_HANDLED_UPDATED; > + } > + > + return result; > +} > + > void en_datapath_synced_logical_router_cleanup(void *data) > { > struct ovn_synced_logical_router_map *router_map = data; > diff --git a/northd/en-datapath-logical-router.h > b/northd/en-datapath-logical-router.h > index fa2bf798f..555dd41e8 100644 > --- a/northd/en-datapath-logical-router.h > +++ b/northd/en-datapath-logical-router.h > @@ -19,6 +19,7 @@ > > #include "lib/inc-proc-eng.h" > #include "openvswitch/hmap.h" > +#include "lib/hmapx.h" > > void *en_datapath_logical_router_init(struct engine_node *, > struct engine_arg *); > @@ -36,14 +37,27 @@ struct ovn_synced_logical_router { > > struct ovn_synced_logical_router_map { > struct hmap synced_routers; > + > + bool has_tracked_data; > + struct hmapx new; > + struct hmapx updated; > + struct hmapx deleted; > }; > > +struct uuid; > +struct ovn_synced_logical_router * > +ovn_synced_logical_router_find(const struct ovn_synced_logical_router_map > *map, > + const struct uuid *nb_uuid); > + > void *en_datapath_synced_logical_router_init(struct engine_node *, > struct engine_arg *); > > enum engine_node_state en_datapath_synced_logical_router_run( > struct engine_node *, void *data); > - > +void en_datapath_synced_logical_router_clear_tracked_data(void *data); > +enum engine_input_handler_result > +en_datapath_synced_logical_router_datapath_sync_handler( > + struct engine_node *node, void *data); > void en_datapath_synced_logical_router_cleanup(void *data); > > enum engine_input_handler_result > diff --git a/northd/en-datapath-logical-switch.c > b/northd/en-datapath-logical-switch.c > index 83d065dae..decdb4006 100644 > --- a/northd/en-datapath-logical-switch.c > +++ b/northd/en-datapath-logical-switch.c > @@ -198,12 +198,30 @@ en_datapath_logical_switch_cleanup(void *data) > ovn_unsynced_datapath_map_destroy(map); > } > > +struct ovn_synced_logical_switch * > +ovn_synced_logical_switch_find(const struct ovn_synced_logical_switch_map > *map, > + const struct uuid *nb_uuid) > +{ > + struct ovn_synced_logical_switch *lsw; > + HMAP_FOR_EACH_WITH_HASH (lsw, hmap_node, uuid_hash(nb_uuid), > + &map->synced_switches) { > + if (uuid_equals(&lsw->nb->header_.uuid, nb_uuid)) { > + return lsw; > + } > + } > + > + return NULL; > +} > + > static void > synced_logical_switch_map_init( > struct ovn_synced_logical_switch_map *switch_map) > { > *switch_map = (struct ovn_synced_logical_switch_map) { > .synced_switches = HMAP_INITIALIZER(&switch_map->synced_switches), > + .new = HMAPX_INITIALIZER(&switch_map->new), > + .updated = HMAPX_INITIALIZER(&switch_map->updated), > + .deleted = HMAPX_INITIALIZER(&switch_map->deleted), > }; > } > > @@ -211,7 +229,17 @@ static void > synced_logical_switch_map_destroy( > struct ovn_synced_logical_switch_map *switch_map) > { > + hmapx_destroy(&switch_map->new); > + hmapx_destroy(&switch_map->updated); > + > + struct hmapx_node *node; > struct ovn_synced_logical_switch *ls; > + HMAPX_FOR_EACH_SAFE (node, &switch_map->deleted) { > + ls = node->data; > + free(ls); > + hmapx_delete(&switch_map->deleted, node); > + } > + hmapx_destroy(&switch_map->deleted); > HMAP_FOR_EACH_POP (ls, hmap_node, &switch_map->synced_switches) { > free(ls); > } > @@ -229,6 +257,18 @@ en_datapath_synced_logical_switch_init(struct > engine_node *node OVS_UNUSED, > return switch_map; > } > > +static struct ovn_synced_logical_switch * > +synced_logical_switch_alloc(const struct ovn_synced_datapath *sdp) > +{ > + struct ovn_synced_logical_switch *lsw = xmalloc(sizeof *lsw); > + *lsw = (struct ovn_synced_logical_switch) { > + .nb = CONTAINER_OF(sdp->nb_row, struct nbrec_logical_switch, > + header_), > + .sb = sdp->sb_dp, > + }; > + return lsw; > +} > + > enum engine_node_state > en_datapath_synced_logical_switch_run(struct engine_node *node , void > *data) > { > @@ -244,12 +284,8 @@ en_datapath_synced_logical_switch_run(struct > engine_node *node , void *data) > if (sdp->nb_row->table->class_ != &nbrec_table_logical_switch) { > continue; > } > - struct ovn_synced_logical_switch *lsw = xmalloc(sizeof *lsw); > - *lsw = (struct ovn_synced_logical_switch) { > - .nb = CONTAINER_OF(sdp->nb_row, struct nbrec_logical_switch, > - header_), > - .sb = sdp->sb_dp, > - }; > + struct ovn_synced_logical_switch *lsw = > + synced_logical_switch_alloc(sdp); > hmap_insert(&switch_map->synced_switches, &lsw->hmap_node, > uuid_hash(&lsw->nb->header_.uuid)); > } > @@ -257,6 +293,85 @@ en_datapath_synced_logical_switch_run(struct > engine_node *node , void *data) > return EN_UPDATED; > } > > +void > +en_datapath_synced_logical_switch_clear_tracked_data(void *data) > +{ > + struct ovn_synced_logical_switch_map *switch_map = data; > + > + hmapx_clear(&switch_map->new); > + hmapx_clear(&switch_map->updated); > + > + struct hmapx_node *node; > + HMAPX_FOR_EACH_SAFE (node, &switch_map->deleted) { > + struct ovn_synced_logical_router *lr = node->data; > + free(lr); > + hmapx_delete(&switch_map->deleted, node); > + } > +} > + > + > +enum engine_input_handler_result > +en_datapath_synced_logical_switch_datapath_sync_handler( > + struct engine_node *node, void *data) > +{ > + const struct ovn_synced_datapaths *dps = > + engine_get_input_data("datapath_sync", node); > + struct ovn_synced_logical_switch_map *switch_map = data; > + > + if (hmapx_is_empty(&dps->deleted) && > + hmapx_is_empty(&dps->new) && > + hmapx_is_empty(&dps->updated)) { > + return EN_UNHANDLED; > + } > + > + enum engine_input_handler_result result = EN_HANDLED_UNCHANGED; > + > + struct hmapx_node *hmapx_node; > + struct ovn_synced_datapath *sdp; > + struct ovn_synced_logical_switch *lsw; > + HMAPX_FOR_EACH (hmapx_node, &dps->new) { > + sdp = hmapx_node->data; > + if (sdp->nb_row->table->class_ != &nbrec_table_logical_switch) { > + continue; > + } > + lsw = synced_logical_switch_alloc(sdp); > + hmap_insert(&switch_map->synced_switches, &lsw->hmap_node, > + uuid_hash(&lsw->nb->header_.uuid)); > + hmapx_add(&switch_map->new, lsw); > + result = EN_HANDLED_UPDATED; > + } > + HMAPX_FOR_EACH (hmapx_node, &dps->deleted) { > + sdp = hmapx_node->data; > + if (sdp->nb_row->table->class_ != &nbrec_table_logical_switch) { > + continue; > + } > + lsw = ovn_synced_logical_switch_find(switch_map, > &sdp->nb_row->uuid); > + if (!lsw) { > + return EN_UNHANDLED; > + } > + hmap_remove(&switch_map->synced_switches, &lsw->hmap_node); > + hmapx_add(&switch_map->deleted, lsw); > + result = EN_HANDLED_UPDATED; > + } > + HMAPX_FOR_EACH (hmapx_node, &dps->updated) { > + sdp = hmapx_node->data; > + if (sdp->nb_row->table->class_ != &nbrec_table_logical_switch) { > + continue; > + } > + lsw = ovn_synced_logical_switch_find(switch_map, > &sdp->nb_row->uuid); > + if (!lsw) { > + return EN_UNHANDLED; > + } > + lsw->nb = CONTAINER_OF(sdp->nb_row, struct nbrec_logical_switch, > + header_); > + lsw->sb = sdp->sb_dp; > + hmapx_add(&switch_map->updated, lsw); > + result = EN_HANDLED_UPDATED; > + } > + > + return result; > +} > + > void en_datapath_synced_logical_switch_cleanup(void *data) > { > struct ovn_synced_logical_switch_map *switch_map = data; > diff --git a/northd/en-datapath-logical-switch.h > b/northd/en-datapath-logical-switch.h > index 2a5a97b18..f6366827b 100644 > --- a/northd/en-datapath-logical-switch.h > +++ b/northd/en-datapath-logical-switch.h > @@ -19,6 +19,7 @@ > > #include "lib/inc-proc-eng.h" > #include "openvswitch/hmap.h" > +#include "lib/hmapx.h" > > void *en_datapath_logical_switch_init(struct engine_node *, > struct engine_arg *); > @@ -38,13 +39,26 @@ struct ovn_synced_logical_switch { > > struct ovn_synced_logical_switch_map { > struct hmap synced_switches; > + > + struct hmapx new; > + struct hmapx updated; > + struct hmapx deleted; > }; > > +struct uuid; > +struct ovn_synced_logical_switch * > +ovn_synced_logical_switch_find(const struct ovn_synced_logical_switch_map > *map, > + const struct uuid *nb_uuid); > + > void *en_datapath_synced_logical_switch_init(struct engine_node *, > struct engine_arg *); > > enum engine_node_state en_datapath_synced_logical_switch_run( > struct engine_node *, void *data); > +void en_datapath_synced_logical_switch_clear_tracked_data(void *data); > +enum engine_input_handler_result > +en_datapath_synced_logical_switch_datapath_sync_handler( > + struct engine_node *node, void *data); > void en_datapath_synced_logical_switch_cleanup(void *data); > > #endif /* EN_DATAPATH_LOGICAL_SWITCH_H */ > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > index 166c0d814..c126c4fb3 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -185,8 +185,8 @@ static ENGINE_NODE(group_ecmp_route, > CLEAR_TRACKED_DATA); > static ENGINE_NODE(datapath_logical_router, CLEAR_TRACKED_DATA); > static ENGINE_NODE(datapath_logical_switch, CLEAR_TRACKED_DATA); > static ENGINE_NODE(datapath_sync, CLEAR_TRACKED_DATA); > -static ENGINE_NODE(datapath_synced_logical_router); > -static ENGINE_NODE(datapath_synced_logical_switch); > +static ENGINE_NODE(datapath_synced_logical_router, CLEAR_TRACKED_DATA); > +static ENGINE_NODE(datapath_synced_logical_switch, CLEAR_TRACKED_DATA); > > void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > struct ovsdb_idl_loop *sb) > @@ -234,9 +234,9 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > datapath_sync_logical_router_handler); > > engine_add_input(&en_datapath_synced_logical_router, > &en_datapath_sync, > - NULL); > + > en_datapath_synced_logical_router_datapath_sync_handler); > engine_add_input(&en_datapath_synced_logical_switch, > &en_datapath_sync, > - NULL); > + > en_datapath_synced_logical_switch_datapath_sync_handler); > > engine_add_input(&en_northd, &en_nb_mirror, NULL); > engine_add_input(&en_northd, &en_nb_mirror_rule, NULL); > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index d3fd924fb..6b0711c54 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -17846,3 +17846,56 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE > > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([Synced logical switch and router incremental procesesing]) > +ovn_start > + > +# datapath_synced_logical_switch and datapath_synced_logical_router > +# should only recompute if datapath_sync has to recompute. Therefore, > +# andy situation where datapath_sync can run incrementally, the > +# synced datapath nodes should also run incrementally. > + > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl --wait=sb ls-add sw0 > +check_engine_stats datapath_synced_logical_switch norecompute compute > +check_engine_stats datapath_synced_logical_router norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl --wait=sb set logical_switch sw0 > other_config:fdb_age_threshold=5 > +check_engine_stats datapath_synced_logical_switch norecompute compute > +check_engine_stats datapath_synced_logical_router norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl --wait=sb set logical_switch sw0 > other_config:requested-tnl-key=123 > +check_engine_stats datapath_synced_logical_switch norecompute compute > +check_engine_stats datapath_synced_logical_router norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl --wait=sb ls-del sw0 > +check_engine_stats datapath_synced_logical_switch norecompute compute > +check_engine_stats datapath_synced_logical_router norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl --wait=sb lr-add lr0 > +check_engine_stats datapath_synced_logical_switch norecompute compute > +check_engine_stats datapath_synced_logical_router norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl --wait=sb set Logical_Router lr0 options:ct-zone-limit=10 > +check_engine_stats datapath_synced_logical_switch norecompute compute > +check_engine_stats datapath_synced_logical_router norecompute compute > + > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl --wait=sb lr-del lr0 > +check_engine_stats datapath_synced_logical_switch norecompute compute > +check_engine_stats datapath_synced_logical_router norecompute compute > +CHECK_NO_CHANGE_AFTER_RECOMPUTE > + > +AT_CLEANUP > +]) > -- > 2.49.0 > > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > There are a lot of CI failures. Please check them out. Thanks, Ales _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
