> On 7/31/25 10:53 PM, Lorenzo Bianconi 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, Numan,
>
> Not a full review, just one thing I found while debugging the rebase of
> this series on the current main (and with Jacob's series to do LR
> creation I-P).
>
> > northd/en-ls-stateful.c | 30 +++++++++++++++++++++++++-----
> > 1 file changed, 25 insertions(+), 5 deletions(-)
> >
> > diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c
> > index 8d64b4be2..5f4ebcb3c 100644
> > --- a/northd/en-ls-stateful.c
> > +++ b/northd/en-ls-stateful.c
> > @@ -131,12 +131,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 +142,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;
> >
> > @@ -170,6 +176,20 @@ ls_stateful_northd_handler(struct engine_node *node,
> > void *data_)
> > hmapx_add(&data->trk_data.crupdated, ls_stateful_rec);
> > }
> >
> > + 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.crupdated, ls_stateful_rec);
>
> ls_stateful_rec should be destroyed at the end of the engine_run().
> However, because we add it to data->trk_data.crupdated we don't do that.
> We need to have a new data->trk_data.deleted field and add it there
> instead.
>
> Then in en_ls_stateful_clear_tracked_data() we need to walk the new
> data->trk_data.deleted records and fully destroy them.
Hi Dumitru,
thx for the review. I will fix it in v5.
Regards,
Lorenzo
>
> Regards,
> Dumitru
>
>
> > + }
> > + }
> > +
> > if (ls_stateful_has_tracked_data(&data->trk_data)) {
> > return EN_HANDLED_UPDATED;
> > }
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
