On Wed, Aug 20, 2025 at 06:55:26PM +0200, Lorenzo Bianconi via dev wrote:
> From: Numan Siddique <num...@ovn.org>
>
> en_northd engine nodes provides the created or updated logical switches
> in its tracked data and en_ls_stateful node handles these changes.
>
> Acked-by: Mark Michelson <mmich...@redhat.com>
> Signed-off-by: Numan Siddique <num...@ovn.org>
> ---
> northd/en-ls-stateful.c | 49 +++++++++++++++++++++++++++++++++++------
> northd/en-ls-stateful.h | 4 +++-
> northd/northd.c | 11 +++++++++
> tests/ovn-northd.at | 4 ++--
> 4 files changed, 58 insertions(+), 10 deletions(-)
>
> diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c
> index 1711d38e7..a9a685504 100644
> --- a/northd/en-ls-stateful.c
> +++ b/northd/en-ls-stateful.c
> @@ -88,6 +88,7 @@ en_ls_stateful_init(struct engine_node *node OVS_UNUSED,
> struct ed_type_ls_stateful *data = xzalloc(sizeof *data);
> ls_stateful_table_init(&data->table);
> hmapx_init(&data->trk_data.crupdated);
> + hmapx_init(&data->trk_data.deleted);
> return data;
> }
>
> @@ -97,6 +98,13 @@ en_ls_stateful_cleanup(void *data_)
> struct ed_type_ls_stateful *data = data_;
> ls_stateful_table_destroy(&data->table);
> hmapx_destroy(&data->trk_data.crupdated);
> +
> + struct hmapx_node *n;
> + HMAPX_FOR_EACH_SAFE (n, &data->trk_data.deleted) {
> + ls_stateful_record_destroy(n->data);
> + hmapx_delete(&data->trk_data.deleted, n);
> + }
> + hmapx_destroy(&data->trk_data.deleted);
> }
>
> void
> @@ -104,6 +112,13 @@ en_ls_stateful_clear_tracked_data(void *data_)
> {
> struct ed_type_ls_stateful *data = data_;
> hmapx_clear(&data->trk_data.crupdated);
> +
> + struct hmapx_node *n;
> + HMAPX_FOR_EACH_SAFE (n, &data->trk_data.deleted) {
> + ls_stateful_record_destroy(n->data);
> + hmapx_delete(&data->trk_data.deleted, n);
> + }
> + hmapx_clear(&data->trk_data.deleted);
> }
>
> enum engine_node_state
> @@ -131,12 +146,9 @@ ls_stateful_northd_handler(struct engine_node *node,
> void *data_)
> return EN_UNHANDLED;
> }
>
> - if (northd_has_lswitches_in_tracked_data(&northd_data->trk_data)) {
> - return EN_UNHANDLED;
> - }
> -
> if (!northd_has_ls_lbs_in_tracked_data(&northd_data->trk_data) &&
> - !northd_has_ls_acls_in_tracked_data(&northd_data->trk_data)) {
> + !northd_has_ls_acls_in_tracked_data(&northd_data->trk_data) &&
> + !northd_has_lswitches_in_tracked_data(&northd_data->trk_data)) {
> return EN_HANDLED_UNCHANGED;
> }
>
> @@ -145,6 +157,15 @@ ls_stateful_northd_handler(struct engine_node *node,
> void *data_)
> struct ed_type_ls_stateful *data = data_;
> struct hmapx_node *hmapx_node;
>
> + HMAPX_FOR_EACH (hmapx_node, &nd_changes->trk_switches.crupdated) {
> + const struct ovn_datapath *od = hmapx_node->data;
> +
> + if (!ls_stateful_table_find_(&data->table, od->nbs)) {
> + ls_stateful_record_create(&data->table, od,
> + input_data.ls_port_groups);
> + }
> + }
> +
> HMAPX_FOR_EACH (hmapx_node, &nd_changes->ls_with_changed_lbs) {
> const struct ovn_datapath *od = hmapx_node->data;
>
> @@ -172,6 +193,20 @@ ls_stateful_northd_handler(struct engine_node *node,
> void *data_)
> }
> }
>
> + HMAPX_FOR_EACH (hmapx_node, &nd_changes->trk_switches.deleted) {
> + const struct ovn_datapath *od = hmapx_node->data;
> + struct ls_stateful_record *ls_stateful_rec =
> + ls_stateful_table_find_(&data->table, od->nbs);
> +
> + if (ls_stateful_rec &&
> + !ovn_datapath_find(&northd_data->ls_datapaths.datapaths,
> + &od->nbs->header_.uuid)) {
> + hmap_remove(&data->table.entries, &ls_stateful_rec->key_node);
> + /* Add the ls_stateful_rec to the tracking data. */
> + hmapx_add(&data->trk_data.deleted, ls_stateful_rec);
> + }
> + }
> +
> if (ls_stateful_has_tracked_data(&data->trk_data)) {
> return EN_HANDLED_UPDATED;
> }
> @@ -191,11 +226,11 @@ ls_stateful_port_group_handler(struct engine_node
> *node, void *data_)
> const struct nbrec_logical_switch *nbs = hmap_node->data;
> struct ls_stateful_record *ls_stateful_rec =
> ls_stateful_table_find_(&data->table, nbs);
> - ovs_assert(ls_stateful_rec);
> /* Ensure that only one handler per engine run calls
> * ls_stateful_record_set_acls on the same ls_stateful_rec by
> * calling it only when the ls_stateful_rec is added to the hmapx.*/
> - if (hmapx_add(&data->trk_data.crupdated, ls_stateful_rec)) {
> + if (ls_stateful_rec && hmapx_add(&data->trk_data.crupdated,
> + ls_stateful_rec)) {
> ls_stateful_record_set_acls(ls_stateful_rec,
> nbs,
> &pg_data->ls_port_groups);
> diff --git a/northd/en-ls-stateful.h b/northd/en-ls-stateful.h
> index 217adf696..ffc0f4269 100644
> --- a/northd/en-ls-stateful.h
> +++ b/northd/en-ls-stateful.h
> @@ -97,6 +97,7 @@ struct ls_stateful_table {
> struct ls_stateful_tracked_data {
> /* Created or updated logical switch with LB and ACL data. */
> struct hmapx crupdated; /* Stores 'struct ls_stateful_record'. */
> + struct hmapx deleted;
> };
>
> struct ed_type_ls_stateful {
> @@ -121,7 +122,8 @@ const struct ls_stateful_record *ls_stateful_table_find(
>
> static inline bool
> ls_stateful_has_tracked_data(struct ls_stateful_tracked_data *trk_data) {
> - return !hmapx_is_empty(&trk_data->crupdated);
> + return !hmapx_is_empty(&trk_data->crupdated) ||
> + !hmapx_is_empty(&trk_data->deleted);
> }
>
> #endif /* EN_LS_STATEFUL_H */
> diff --git a/northd/northd.c b/northd/northd.c
> index ace5c2301..05b1a2745 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -18751,6 +18751,17 @@ lflow_handle_ls_stateful_changes(struct
> ovsdb_idl_txn *ovnsb_txn,
> }
> }
>
> + HMAPX_FOR_EACH (hmapx_node, &trk_data->deleted) {
> + struct ls_stateful_record *ls_stateful_rec = hmapx_node->data;
> + const struct ovn_datapath *od =
> + ovn_datapaths_find_by_index(lflow_input->ls_datapaths,
> + ls_stateful_rec->ls_index);
> + ovs_assert(od->nbs && uuid_equals(&od->nbs->header_.uuid,
> + &ls_stateful_rec->nbs_uuid));
> +
> + lflow_ref_unlink_lflows(ls_stateful_rec->lflow_ref);
> + }
> +
> return true;
> }
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 9e0e80418..c6cc4eeb9 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -14552,7 +14552,7 @@ 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 northd norecompute compute
> -check_engine_stats ls_stateful recompute nocompute
> +check_engine_stats ls_stateful norecompute compute
> check_engine_stats lflow recompute nocompute
>
> # For the below engine nodes, en_northd is input. So check
> @@ -14590,7 +14590,7 @@ check ovn-nbctl --wait=sb lsp-add sw0 lsp0
> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> check ovn-nbctl --wait=sb ls-del sw0
> check_engine_stats northd norecompute compute
> -check_engine_stats ls_stateful recompute nocompute
> +check_engine_stats ls_stateful norecompute compute
> check_engine_stats lflow recompute nocompute
>
> # For the below engine nodes, en_northd is input. So check
> --
> 2.50.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Looks good to me
Acked-by: Mairtin O'Loingsigh <moloi...@redhat.com>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
