On Tue, Mar 10, 2026 at 12:43 PM Lorenzo Bianconi via dev < [email protected]> wrote:
> Introduce logical-flows incremental processing support for > logical_switch creation/deletion. > > Reported-at: https://issues.redhat.com/browse/FDP-2025 > Signed-off-by: Lorenzo Bianconi <[email protected]> > --- > Hi Lorenzo, thank you for the patch, I have one question below. > northd/en-lflow.c | 10 +++-- > northd/en-multicast.c | 3 +- > northd/lflow-mgr.c | 4 ++ > northd/northd.c | 97 +++++++++++++++++++++++++++++++++++-------- > northd/northd.h | 7 +++- > tests/ovn-northd.at | 11 +++-- > 6 files changed, 105 insertions(+), 27 deletions(-) > > diff --git a/northd/en-lflow.c b/northd/en-lflow.c > index d4351edb9..7432d5278 100644 > --- a/northd/en-lflow.c > +++ b/northd/en-lflow.c > @@ -149,9 +149,6 @@ lflow_northd_handler(struct engine_node *node, > return EN_UNHANDLED; > } > > - if (northd_has_lswitches_in_tracked_data(&northd_data->trk_data)) { > - return EN_UNHANDLED; > - } > > const struct engine_context *eng_ctx = engine_get_context(); > struct lflow_data *lflow_data = data; > @@ -166,6 +163,13 @@ lflow_northd_handler(struct engine_node *node, > return EN_UNHANDLED; > } > > + if (!lflow_handle_northd_ls_changes(eng_ctx->ovnsb_idl_txn, > + > &northd_data->trk_data.trk_switches, > + &lflow_input, > + lflow_data->lflow_table)) { > + return EN_UNHANDLED; > + } > + > if (!lflow_handle_northd_port_changes(eng_ctx->ovnsb_idl_txn, > &northd_data->trk_data.trk_lsps, > &lflow_input, > diff --git a/northd/en-multicast.c b/northd/en-multicast.c > index a7dfd71c4..97b12f6a3 100644 > --- a/northd/en-multicast.c > +++ b/northd/en-multicast.c > @@ -153,7 +153,8 @@ multicast_igmp_northd_handler(struct engine_node > *node, void *data OVS_UNUSED) > return EN_UNHANDLED; > } > > - if (hmapx_count(&northd_data->trk_data.trk_switches.deleted)) { > + if (hmapx_count(&northd_data->trk_data.trk_switches.crupdated) || > + hmapx_count(&northd_data->trk_data.trk_switches.deleted)) { > return EN_UNHANDLED; > } > > diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c > index b236c13c6..52320eb78 100644 > --- a/northd/lflow-mgr.c > +++ b/northd/lflow-mgr.c > @@ -780,6 +780,10 @@ lflow_table_add_lflow__(struct lflow_table > *lflow_table, > hmap_insert(&lflow_ref->lflow_ref_nodes, &lrn->ref_node, > hash); > } > > + if (lrn->dpgrp_lflow) { > + dynamic_bitmap_realloc(&lrn->dpgrp_bitmap, dp_bitmap_len); > + } > + > if (!lrn->linked) { > if (lrn->dpgrp_lflow) { > ovs_assert(lrn->dpgrp_bitmap.capacity == dp_bitmap_len); > diff --git a/northd/northd.c b/northd/northd.c > index 0da15629e..a5315e26b 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -5954,10 +5954,13 @@ enum mirror_filter { > > static void > build_mirror_default_lflow(struct ovn_datapath *od, > - struct lflow_table *lflows) > + struct lflow_table *lflows, > + struct lflow_ref *lflow_ref) > { > - ovn_lflow_add(lflows, od, S_SWITCH_IN_MIRROR, 0, "1", "next;", NULL); > - ovn_lflow_add(lflows, od, S_SWITCH_OUT_MIRROR, 0, "1", "next;", NULL); > + ovn_lflow_add(lflows, od, S_SWITCH_IN_MIRROR, 0, "1", "next;", > + lflow_ref); > + ovn_lflow_add(lflows, od, S_SWITCH_OUT_MIRROR, 0, "1", "next;", > + lflow_ref); > } > > static void > @@ -19237,29 +19240,36 @@ build_lswitch_and_lrouter_iterate_by_ls(struct > ovn_datapath *od, > struct lswitch_flow_build_info > *lsi) > { > ovs_assert(od->nbs); > - build_mirror_default_lflow(od, lsi->lflows); > + build_mirror_default_lflow(od, lsi->lflows, od->datapath_lflows); > build_lswitch_lflows_pre_acl_and_acl(od, lsi->lflows, > - lsi->meter_groups, NULL); > - build_network_function(od, lsi->lflows, lsi->ls_port_groups, NULL); > - build_fwd_group_lflows(od, lsi->lflows, NULL); > - build_lswitch_lflows_admission_control(od, lsi->lflows, NULL); > - build_lswitch_learn_fdb_od(od, lsi->lflows, NULL); > - build_lswitch_arp_nd_responder_default(od, lsi->lflows, NULL); > + lsi->meter_groups, > + od->datapath_lflows); > + build_network_function(od, lsi->lflows, lsi->ls_port_groups, > + od->datapath_lflows); > + build_fwd_group_lflows(od, lsi->lflows, od->datapath_lflows); > + build_lswitch_lflows_admission_control(od, lsi->lflows, > + od->datapath_lflows); > + build_lswitch_learn_fdb_od(od, lsi->lflows, od->datapath_lflows); > + build_lswitch_arp_nd_responder_default(od, lsi->lflows, > + od->datapath_lflows); > build_lswitch_dns_lookup_and_response(od, lsi->lflows, > lsi->meter_groups, > - NULL); > - build_lswitch_dhcp_and_dns_defaults(od, lsi->lflows, NULL); > + od->datapath_lflows); > + build_lswitch_dhcp_and_dns_defaults(od, lsi->lflows, > od->datapath_lflows); > build_lswitch_destination_lookup_bmcast(od, lsi->lflows, > &lsi->actions, > - lsi->meter_groups, NULL); > - build_lswitch_output_port_sec_od(od, lsi->lflows, NULL); > + lsi->meter_groups, > + od->datapath_lflows); > + build_lswitch_output_port_sec_od(od, lsi->lflows, > od->datapath_lflows); > /* CT extraction flows are built with stateful flows, but default > rule is > * always needed */ > ovn_lflow_add(lsi->lflows, od, S_SWITCH_IN_CT_EXTRACT, 0, "1", > "next;", > - NULL); > - build_lswitch_lb_affinity_default_flows(od, lsi->lflows, NULL); > + od->datapath_lflows); > + build_lswitch_lb_affinity_default_flows(od, lsi->lflows, > + od->datapath_lflows); > if (od->has_evpn_vni) { > - build_lswitch_lflows_evpn_l2_unknown(od, lsi->lflows, NULL); > + build_lswitch_lflows_evpn_l2_unknown(od, lsi->lflows, > + od->datapath_lflows); > } else { > - build_lswitch_lflows_l2_unknown(od, lsi->lflows, NULL); > + build_lswitch_lflows_l2_unknown(od, lsi->lflows, > od->datapath_lflows); > } > build_mcast_flood_lswitch(od, lsi->lflows, &lsi->actions, NULL); > } > @@ -20036,6 +20046,57 @@ lflow_handle_northd_lr_changes(struct > ovsdb_idl_txn *ovnsb_txn, > return handled; > } > > +bool > +lflow_handle_northd_ls_changes(struct ovsdb_idl_txn *ovnsb_txn, > + struct tracked_dps *tracked_ls, > + struct lflow_input *lflow_input, > + struct lflow_table *lflows) > +{ > + struct hmapx_node *hmapx_node; > + struct ovn_datapath *od; > + > + HMAPX_FOR_EACH (hmapx_node, &tracked_ls->deleted) { > + od = hmapx_node->data; > + if (!lflow_ref_resync_flows( > + od->datapath_lflows, lflows, ovnsb_txn, > lflow_input->dps, > + lflow_input->ovn_internal_version_changed, > + lflow_input->sbrec_logical_flow_table, > + lflow_input->sbrec_logical_dp_group_table)) { > + return false; > + } > + } > + > + struct lswitch_flow_build_info lsi = { > + .meter_groups = lflow_input->meter_groups, > + .ls_port_groups = lflow_input->ls_port_groups, > + .lflows = lflows, > + .match = DS_EMPTY_INITIALIZER, > + .actions = DS_EMPTY_INITIALIZER, > + }; > + > + HMAPX_FOR_EACH (hmapx_node, &tracked_ls->crupdated) { > + od = hmapx_node->data; > + lflow_ref_unlink_lflows(od->datapath_lflows); > + build_lswitch_and_lrouter_iterate_by_ls(od, &lsi); > + } > + > + ds_destroy(&lsi.actions); > + ds_destroy(&lsi.match); > + > + HMAPX_FOR_EACH (hmapx_node, &tracked_ls->crupdated) { > + od = hmapx_node->data; > + if (!lflow_ref_sync_lflows( > + od->datapath_lflows, lflows, ovnsb_txn, > lflow_input->dps, > + lflow_input->ovn_internal_version_changed, > + lflow_input->sbrec_logical_flow_table, > + lflow_input->sbrec_logical_dp_group_table)) { > + return false; > + } > + } > + > + return true; > +} > + > bool > lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn, > struct tracked_ovn_ports *trk_lsps, > diff --git a/northd/northd.h b/northd/northd.h > index 41190110b..6b4c575fa 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -474,8 +474,7 @@ struct ovn_datapath { > /* Map of ovn_port objects belonging to this datapath. > * This map doesn't include derived ports. */ > struct hmap ports; > - /* Reference to the lflows belonging to this datapath currently router > - * only lflows. */ > + /* Reference to the lflows belonging to this datapath. */ > struct lflow_ref *datapath_lflows; > }; > > @@ -949,6 +948,10 @@ bool lflow_handle_northd_lr_changes(struct > ovsdb_idl_txn *ovnsh_txn, > struct tracked_dps *, > struct lflow_input *, > struct lflow_table *lflows); > +bool lflow_handle_northd_ls_changes(struct ovsdb_idl_txn *ovnsb_txn, > + struct tracked_dps *, > + struct lflow_input *, > + struct lflow_table *lflows); > bool lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn, > struct tracked_ovn_ports *, > struct lflow_input *, > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index fd049d096..9d9a8cb84 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -2959,6 +2959,8 @@ OVN_FOR_EACH_NORTHD_NO_HV([ > AT_SETUP([check VXLAN mode disabling]) > ovn_start > > +OVS_CTL_TIMEOUT=120 > + Why is increasing the timeout still necessary? I was under the impression that doing the unlink first and then sync in separate loop should solve the issue. But if that's not the case we need to definitely investigate and fix the slowness before considering this as a solution. > # Create a fake chassis with vxlan encap to implicitly enable VXLAN mode. > check_uuid ovn-sbctl \ > --id=@e create encap chassis_name=hv1 ip="192.168.0.1" type="vxlan" \ > @@ -2994,6 +2996,8 @@ OVN_FOR_EACH_NORTHD_NO_HV([ > AT_SETUP([check datapath tunnel ids exhaustion]) > ovn_start > > +OVS_CTL_TIMEOUT=120 > + > # Create a fake chassis with vxlan encap to lower MAX DP tunnel key to > 2^12 > check_uuid ovn-sbctl \ > --id=@e create encap chassis_name=hv1 ip="192.168.0.1" type="vxlan" \ > @@ -3148,11 +3152,12 @@ check_row_count sb:logical_dp_group 1 > echo "$sw2_sb_uuid" >> sw_sb_uuids > > AT_CHECK_UNQUOTED([ovn-sbctl --bare --columns _uuid,datapaths find > Logical_DP_Group dnl > - | grep -A1 $ls_dpg_uuid | tail -1 | tr ' ' '\n' | > sort], [0], [dnl > + | tail -1 | tr ' ' '\n' | sort], [0], [dnl > $(cat sw_sb_uuids | sort) > ]) > > dnl Add two routers and check that they are in their new separate group. > +ls_dpg_uuid=$(ovn-sbctl --bare --columns _uuid list Logical_DP_Group .) > check ovn-nbctl --wait=sb lr-add lr0 -- lr-add lr1 > lr0_sb_uuid=$(fetch_column datapath_binding _uuid external_ids:name=lr0) > lr1_sb_uuid=$(fetch_column datapath_binding _uuid external_ids:name=lr1) > @@ -14958,7 +14963,7 @@ 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 norecompute compute > -check_engine_stats lflow recompute nocompute > +check_engine_stats lflow norecompute compute > > # For the below engine nodes, en_northd is input. So check > # their engine status. > @@ -14996,7 +15001,7 @@ 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 norecompute compute > -check_engine_stats lflow recompute nocompute > +check_engine_stats lflow norecompute compute > > # For the below engine nodes, en_northd is input. So check > # their engine status. > -- > 2.53.0 > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Regards, Ales _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
