On Thu, Dec 11, 2025 at 7:14 PM Mark Michelson <[email protected]> wrote:
> Thanks Ales and Lorenzo! > > Acked-by: Mark Michelson <[email protected]> > > On Tue, Nov 25, 2025 at 7:59 AM Ales Musil via dev > <[email protected]> wrote: > > > > There were two issues with the handler: > > 1) The handler was handling deleted ls_stateful, but in a wrong way. > > It was trying to find ovn_datapath struct which was already > > removed by northd. > > > > 2) The handler tried to sync the flows one by one which was causing > > performance issues as the dp groups were created over and over > > again. > > > > None of the above was an issue because we would recompute the whole > > lflow nodes in the both cases above. This was noticed with the > > attempt to introduce incremental processing for LS addition. > > > > To fix the issues above make sure we don't check the ovn_datapath for > > deleted ls_stateful struct. Just resync the logical flows. Also make > > a separate loop that will go over the ls_stateful updated structs > > once again just to sync after all datapath groups are actually up to > > date. > > > > Finally make sure we propagate the updated correctly for newly > > created ls_stateful structs. > > > > Fixes: 00cda5a47bd4 ("northd: Add ls_stateful handler for lflow engine > node.") > > Fixes: 48e1736f89c6 ("northd: I-P for logical switch creation/deletion > in en_ls_stateful engine node.") > > Co-authored-by: Lorenzo Bianconi <[email protected]> > > Signed-off-by: Lorenzo Bianconi <[email protected]> > > Signed-off-by: Ales Musil <[email protected]> > > --- > > northd/en-ls-stateful.c | 6 ++++-- > > northd/northd.c | 21 +++++++++++++++------ > > 2 files changed, 19 insertions(+), 8 deletions(-) > > > > diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c > > index a9a685504..0dea24aee 100644 > > --- a/northd/en-ls-stateful.c > > +++ b/northd/en-ls-stateful.c > > @@ -161,8 +161,10 @@ ls_stateful_northd_handler(struct engine_node > *node, void *data_) > > 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); > > + struct ls_stateful_record *ls_stateful_rec = > > + ls_stateful_record_create(&data->table, od, > > + input_data.ls_port_groups); > > + hmapx_add(&data->trk_data.crupdated, ls_stateful_rec); > > } > > } > > > > diff --git a/northd/northd.c b/northd/northd.c > > index 011f449ec..f5cc60e84 100644 > > --- a/northd/northd.c > > +++ b/northd/northd.c > > @@ -19694,7 +19694,13 @@ lflow_handle_ls_stateful_changes(struct > ovsdb_idl_txn *ovnsb_txn, > > build_network_function(od, lflows, > > lflow_input->ls_port_groups, > > ls_stateful_rec->lflow_ref); > > + } > > > > + /* We need to make sure that all datapath groups are allocated > before > > + * trying to sync logical flows. Otherwise, we would need to > recompute > > + * those datapath groups within those flows over and over again. */ > > + HMAPX_FOR_EACH (hmapx_node, &trk_data->crupdated) { > > + struct ls_stateful_record *ls_stateful_rec = hmapx_node->data; > > /* Sync the new flows to SB. */ > > bool handled = lflow_ref_sync_lflows( > > ls_stateful_rec->lflow_ref, lflows, ovnsb_txn, > > @@ -19710,13 +19716,16 @@ 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); > > + if (!lflow_ref_resync_flows( > > + ls_stateful_rec->lflow_ref, lflows, ovnsb_txn, > > + lflow_input->ls_datapaths, > > + lflow_input->lr_datapaths, > > + lflow_input->ovn_internal_version_changed, > > + lflow_input->sbrec_logical_flow_table, > > + lflow_input->sbrec_logical_dp_group_table)) { > > + return false; > > + } > > } > > > > return true; > > -- > > 2.51.1 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > Thank you Mark, I went ahead and merged this into main, the backport is not needed as this is more a preparation for I-P and it won't be exercised until we process lflows for added LS incrementally. Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
