On Fri, Jul 18, 2025 at 6:15 PM Mark Michelson via dev < ovs-dev@openvswitch.org> wrote:
> From: Lorenzo Bianconi <lorenzo.bianc...@redhat.com> > > Introduce Incremental Processing logic for en-datapath-logical-switch > node. > > Reported-at: https://issues.redhat.com/browse/FDP-1519 > Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com> > --- > Hi Mark and Lorenzo, thank you for the patch. I have one minor comment down below. > northd/en-datapath-logical-switch.c | 182 ++++++++++++++++++++-------- > northd/en-datapath-logical-switch.h | 3 + > northd/inc-proc-northd.c | 8 +- > tests/ovn-northd.at | 28 +++++ > 4 files changed, 168 insertions(+), 53 deletions(-) > > diff --git a/northd/en-datapath-logical-switch.c > b/northd/en-datapath-logical-switch.c > index 23fa9725f..83d065dae 100644 > --- a/northd/en-datapath-logical-switch.c > +++ b/northd/en-datapath-logical-switch.c > @@ -38,72 +38,154 @@ en_datapath_logical_switch_init(struct engine_node > *node OVS_UNUSED, > return map; > } > > -enum engine_node_state > -en_datapath_logical_switch_run(struct engine_node *node , void *data) > +static uint32_t > +get_requested_tunnel_key(const struct nbrec_logical_switch *nbs, > + bool vxlan_mode) > +{ > + uint32_t requested_tunnel_key = smap_get_int(&nbs->other_config, > + "requested-tnl-key", 0); > + const char *ts = smap_get(&nbs->other_config, "interconn-ts"); > + > + if (!ts && vxlan_mode && requested_tunnel_key >= 1 << 12) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > + VLOG_WARN_RL(&rl, "Tunnel key %"PRIu32" for datapath %s is " > + "incompatible with VXLAN", requested_tunnel_key, > + nbs->name); > + requested_tunnel_key = 0; > + } > + > + return requested_tunnel_key; > +} > + > +static void > +gather_external_ids(const struct nbrec_logical_switch *nbs, > + struct smap *external_ids) > +{ > + smap_add(external_ids, "name", nbs->name); > + const char *neutron_network = smap_get(&nbs->other_config, > + "neutron:network_name"); > + if (neutron_network && neutron_network[0]) { > + smap_add(external_ids, "name2", neutron_network); > + } > + > + int64_t ct_zone_limit = ovn_smap_get_llong(&nbs->other_config, > + "ct-zone-limit", -1); > + if (ct_zone_limit > 0) { > + smap_add_format(external_ids, "ct-zone-limit", "%"PRId64, > + ct_zone_limit); > + } > + > + const char *ts = smap_get(&nbs->other_config, "interconn-ts"); > + if (ts) { > + smap_add(external_ids, "interconn-ts", ts); > + } > + > + uint32_t age_threshold = smap_get_uint(&nbs->other_config, > + "fdb_age_threshold", 0); > + if (age_threshold) { > + smap_add_format(external_ids, "fdb_age_threshold", > + "%u", age_threshold); > + } > + > + /* For backwards-compatibility, also store the NB UUID in > + * external-ids:logical-switch. This is useful if ovn-controller > + * has not updated and expects this to be where to find the > + * UUID. > + */ > + smap_add_format(external_ids, "logical-switch", UUID_FMT, > + UUID_ARGS(&nbs->header_.uuid)); > +} > + > +static struct ovn_unsynced_datapath * > +datapath_unsynced_new_logical_switch_handler( > + const struct nbrec_logical_switch *nbs, > + const struct ed_type_global_config *global_config, > + struct ovn_unsynced_datapath_map *map) > +{ > + uint32_t requested_tunnel_key = get_requested_tunnel_key( > + nbs, global_config->vxlan_mode); > + > + struct ovn_unsynced_datapath *udp = > + ovn_unsynced_datapath_alloc(nbs->name, DP_SWITCH, > + requested_tunnel_key, &nbs->header_); > + > + gather_external_ids(nbs, &udp->external_ids); > + hmap_insert(&map->dps, &udp->hmap_node, > uuid_hash(&nbs->header_.uuid)); > + return udp; > +} > + > +enum engine_input_handler_result > +datapath_logical_switch_handler(struct engine_node *node, void *data) > { > const struct nbrec_logical_switch_table *nb_ls_table = > EN_OVSDB_GET(engine_get_input("NB_logical_switch", node)); > const struct ed_type_global_config *global_config = > engine_get_input_data("global_config", node); > - > + enum engine_input_handler_result ret = EN_HANDLED_UNCHANGED; > struct ovn_unsynced_datapath_map *map = data; > > - ovn_unsynced_datapath_map_destroy(map); > - ovn_unsynced_datapath_map_init(map, DP_SWITCH); > - > const struct nbrec_logical_switch *nbs; > - NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH (nbs, nb_ls_table) { > - uint32_t requested_tunnel_key = smap_get_int(&nbs->other_config, > - "requested-tnl-key", > 0); > - const char *ts = smap_get(&nbs->other_config, "interconn-ts"); > - > - if (!ts && global_config->vxlan_mode && > - requested_tunnel_key >= 1 << 12) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); > - VLOG_WARN_RL(&rl, "Tunnel key %"PRIu32" for datapath %s is " > - "incompatible with VXLAN", requested_tunnel_key, > - nbs->name); > - requested_tunnel_key = 0; > - } > + NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH_TRACKED (nbs, nb_ls_table) { > + struct ovn_unsynced_datapath *udp = > + ovn_unsynced_datapath_find(map, &nbs->header_.uuid); > > - struct ovn_unsynced_datapath *dp > - = ovn_unsynced_datapath_alloc(nbs->name, DP_SWITCH, > - requested_tunnel_key, > &nbs->header_); > + if (nbrec_logical_switch_is_new(nbs)) { > + if (udp) { > + return EN_UNHANDLED; > + } > + udp = datapath_unsynced_new_logical_switch_handler(nbs, > + > global_config, > + map); > + hmapx_add(&map->new, udp); > + ret = EN_HANDLED_UPDATED; > + } else if (nbrec_logical_switch_is_deleted(nbs)) { > + if (!udp) { > + return EN_UNHANDLED; > + } > + hmap_remove(&map->dps, &udp->hmap_node); > + hmapx_add(&map->deleted, udp); > + ret = EN_HANDLED_UPDATED; > + } else { > + if (!udp) { > + return EN_UNHANDLED; > + } > > - smap_add(&dp->external_ids, "name", dp->name); > - const char *neutron_network = smap_get(&nbs->other_config, > - "neutron:network_name"); > - if (neutron_network && neutron_network[0]) { > - smap_add(&dp->external_ids, "name2", neutron_network); > + udp->requested_tunnel_key = get_requested_tunnel_key( > + nbs, global_config->vxlan_mode); > + smap_destroy(&udp->external_ids); > + smap_init(&udp->external_ids); > + gather_external_ids(nbs, &udp->external_ids); > + hmapx_add(&map->updated, udp); > + ret = EN_HANDLED_UPDATED; > } > + } > > - int64_t ct_zone_limit = ovn_smap_get_llong(&nbs->other_config, > - "ct-zone-limit", -1); > - if (ct_zone_limit > 0) { > - smap_add_format(&dp->external_ids, "ct-zone-limit", "%"PRId64, > - ct_zone_limit); > - } > nit: You could check if all hmapx have anything in them and return based on that. + return ret; > +} > > - if (ts) { > - smap_add(&dp->external_ids, "interconn-ts", ts); > - } > +void > +en_datapath_logical_switch_clear_tracked_data(void *data) > +{ > + struct ovn_unsynced_datapath_map *map = data; > + ovn_unsynced_datapath_map_clear_tracked_data(map); > +} > > - uint32_t age_threshold = smap_get_uint(&nbs->other_config, > - "fdb_age_threshold", 0); > - if (age_threshold) { > - smap_add_format(&dp->external_ids, "fdb_age_threshold", > - "%u", age_threshold); > - } > +enum engine_node_state > +en_datapath_logical_switch_run(struct engine_node *node , void *data) > +{ > + const struct nbrec_logical_switch_table *nb_ls_table = > + EN_OVSDB_GET(engine_get_input("NB_logical_switch", node)); > + const struct ed_type_global_config *global_config = > + engine_get_input_data("global_config", node); > > - /* For backwards-compatibility, also store the NB UUID in > - * external-ids:logical-switch. This is useful if ovn-controller > - * has not updated and expects this to be where to find the > - * UUID. > - */ > - smap_add_format(&dp->external_ids, "logical-switch", UUID_FMT, > - UUID_ARGS(&nbs->header_.uuid)); > + struct ovn_unsynced_datapath_map *map = data; > + > + ovn_unsynced_datapath_map_destroy(map); > + ovn_unsynced_datapath_map_init(map, DP_SWITCH); > > - hmap_insert(&map->dps, &dp->hmap_node, > uuid_hash(&nbs->header_.uuid)); > + const struct nbrec_logical_switch *nbs; > + NBREC_LOGICAL_SWITCH_TABLE_FOR_EACH (nbs, nb_ls_table) { > + datapath_unsynced_new_logical_switch_handler(nbs, global_config, > map); > } > > return EN_UPDATED; > diff --git a/northd/en-datapath-logical-switch.h > b/northd/en-datapath-logical-switch.h > index 1bd5fea83..2a5a97b18 100644 > --- a/northd/en-datapath-logical-switch.h > +++ b/northd/en-datapath-logical-switch.h > @@ -23,9 +23,12 @@ > void *en_datapath_logical_switch_init(struct engine_node *, > struct engine_arg *); > > +enum engine_input_handler_result > +datapath_logical_switch_handler(struct engine_node *, void *data); > enum engine_node_state en_datapath_logical_switch_run(struct engine_node > *, > void *data); > void en_datapath_logical_switch_cleanup(void *data); > +void en_datapath_logical_switch_clear_tracked_data(void *data); > > struct ovn_synced_logical_switch { > struct hmap_node hmap_node; > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > index bf1a09c08..0dfbf00b5 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -183,7 +183,7 @@ static ENGINE_NODE(learned_route_sync, > CLEAR_TRACKED_DATA); > static ENGINE_NODE(dynamic_routes); > static ENGINE_NODE(group_ecmp_route, CLEAR_TRACKED_DATA); > static ENGINE_NODE(datapath_logical_router, CLEAR_TRACKED_DATA); > -static ENGINE_NODE(datapath_logical_switch); > +static ENGINE_NODE(datapath_logical_switch, CLEAR_TRACKED_DATA); > static ENGINE_NODE(datapath_sync); > static ENGINE_NODE(datapath_synced_logical_router); > static ENGINE_NODE(datapath_synced_logical_switch); > @@ -217,8 +217,10 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > engine_add_input(&en_acl_id, &en_nb_acl, NULL); > engine_add_input(&en_acl_id, &en_sb_acl_id, NULL); > > - engine_add_input(&en_datapath_logical_switch, &en_nb_logical_switch, > NULL); > - engine_add_input(&en_datapath_logical_switch, &en_global_config, > NULL); > + engine_add_input(&en_datapath_logical_switch, &en_nb_logical_switch, > + datapath_logical_switch_handler); > + engine_add_input(&en_datapath_logical_switch, &en_global_config, > + datapath_logical_switch_handler); > > engine_add_input(&en_datapath_logical_router, &en_nb_logical_router, > en_datapath_logical_router_logical_router_handler); > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 64506e79e..76b59408c 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -17778,3 +17778,31 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE > > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([Datapath Logical switch incremental processing]) > +ovn_start > + > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats > +check ovn-nbctl --wait=sb ls-add sw0 > +check_engine_stats datapath_logical_switch 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_logical_switch 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_logical_switch 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_logical_switch 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 > > With that addressed: Acked-by: Ales Musil <amu...@redhat.com> Thanks, Ales _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
