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 > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
