On Aug 27, Ales Musil wrote:
> On Wed, Aug 20, 2025 at 6:56 PM Lorenzo Bianconi via dev <
> ovs-dev@openvswitch.org> 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>
> > ---
> >
>
> Hi Lorenzo,
>
> this commit message needs an adjustment. It doesn't mention that
> deletion is handled as well.
ack, I will fix it in v6.
Regards,
Lorenzo
>
>
>
> > 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
> >
> >
> Thanks,
> Ales
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
