On Tue, Apr 14, 2026 at 3:41 PM Dumitru Ceara <[email protected]> wrote:
> northd_handle_ls_changes() failed to initialize lb_with_stateless_mode
> and has_evpn_vni when a new logical switch was created incrementally.
> These fields are derived from NB other_config and were only initialized
> in the full recompute path (build_datapaths()).
>
> Extract init_ls_other_config() and call it from both paths so that
> creating a logical switch with dynamic-routing-vni or
> enable-stateless-acl-with-lb set in the same transaction produces
> correct flows.
>
> Fixes: 06e2c1bf0ce7 ("northd: I-P for logical switch creation/deletion in
> en_northd.")
> Assisted-by: Claude, with model: claude-opus-4-6
> Signed-off-by: Dumitru Ceara <[email protected]>
> ---
> northd/northd.c | 23 ++++++++++-------
> tests/ovn-northd.at | 60 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 74 insertions(+), 9 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 0d133c62c2..6a8d0c311a 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -923,6 +923,18 @@ ods_assign_array_index(struct ovn_datapaths
> *datapaths,
> od->datapaths = datapaths;
> }
>
> +static void
> +init_ls_other_config(struct ovn_datapath *od)
> +{
> + od->lb_with_stateless_mode =
> + smap_get_bool(&od->nbs->other_config,
> + "enable-stateless-acl-with-lb", false);
> +
> + int64_t vni = ovn_smap_get_llong(&od->nbs->other_config,
> + "dynamic-routing-vni", -1);
> + od->has_evpn_vni = ovn_is_valid_vni(vni);
> +}
> +
> /* Initializes 'ls_datapaths' to contain a "struct ovn_datapath" for every
> * logical switch, and initializes 'lr_datapaths' to contain a
> * "struct ovn_datapath" for every logical router.
> @@ -940,15 +952,7 @@ build_datapaths(const struct
> ovn_synced_logical_switch_map *ls_map,
> &ls->nb->header_.uuid,
> ls->nb, NULL, ls->sdp);
> init_ipam_info_for_datapath(od);
> - if (smap_get_bool(&od->nbs->other_config,
> - "enable-stateless-acl-with-lb",
> - false)) {
> - od->lb_with_stateless_mode = true;
> - }
> -
> - int64_t vni = ovn_smap_get_llong(&od->nbs->other_config,
> - "dynamic-routing-vni", -1);
> - od->has_evpn_vni = ovn_is_valid_vni(vni);
> + init_ls_other_config(od);
> }
>
> struct ovn_synced_logical_router *lr;
> @@ -5133,6 +5137,7 @@ northd_handle_ls_changes(struct ovsdb_idl_txn
> *ovnsb_idl_txn,
>
> ods_assign_array_index(&nd->ls_datapaths, od);
> init_ipam_info_for_datapath(od);
> + init_ls_other_config(od);
> init_mcast_info_for_datapath(od);
>
> /* Create SB:IP_Multicast for the logical switch. */
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 7c2f53da69..624e08c1df 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -18897,6 +18897,66 @@ OVN_CLEANUP_NORTHD
> AT_CLEANUP
> ])
>
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([LS incremental processing - other_config fields])
> +AT_KEYWORDS([ovn, incremental])
> +ovn_start
> +
> +dnl Test that has_evpn_vni and lb_with_stateless_mode are properly
> +dnl initialized when a logical switch is created incrementally (i.e.,
> +dnl created with other_config set in the same transaction).
> +
> +AS_BOX([has_evpn_vni initialized during incremental LS creation])
> +dnl Create the switch with dynamic-routing-vni in the SAME transaction
> +dnl to exercise the incremental code path in northd_handle_ls_changes().
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb \
> + -- ls-add ls-evpn \
> + -- set logical_switch ls-evpn other_config:dynamic-routing-vni=10 \
> + -- lsp-add ls-evpn lsp0 \
> + -- lsp-set-addresses lsp0 "00:00:00:00:00:01 10.0.0.1"
> +
> +dnl The northd engine node must not recompute; this proves the
> +dnl incremental code path (northd_handle_ls_changes) was used.
> +check_engine_stats northd norecompute compute
> +
> +ovn-sbctl dump-flows ls-evpn > lflows
> +
> +dnl When has_evpn_vni is true, the l2_lkup flow should include
> +dnl get_remote_fdb in addition to get_fdb.
> +AT_CHECK([grep 'ls_in_l2_lkup' lflows | grep "get_fdb" |
> ovn_strip_lflows], [0], [dnl
> + table=??(ls_in_l2_lkup ), priority=0 , match=(1),
> action=(outport = get_fdb(eth.dst); remote_outport =
> get_remote_fdb(eth.dst); next;)
> +])
> +
> +AS_BOX([lb_with_stateless_mode initialized during incremental LS
> creation])
> +dnl Create the switch with enable-stateless-acl-with-lb in the SAME
> +dnl transaction to exercise the incremental code path.
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb \
> + -- ls-add ls-stateless \
> + -- set logical_switch ls-stateless
> other_config:enable-stateless-acl-with-lb=true \
> + -- lsp-add ls-stateless lsp1 \
> + -- lsp-set-addresses lsp1 "00:00:00:00:00:02 10.0.0.2"
> +
> +dnl The northd engine node must not recompute; this proves the
> +dnl incremental code path (northd_handle_ls_changes) was used.
> +check_engine_stats northd norecompute compute
> +
> +check ovn-nbctl lb-add lb1 10.0.0.100:80 10.0.0.2:80
> +check ovn-nbctl ls-lb-add ls-stateless lb1
> +check ovn-nbctl --wait=sb acl-add ls-stateless from-lport 100 "ip"
> allow-related
> +
> +ovn-sbctl dump-flows ls-stateless > lflows
> +
> +dnl With lb_with_stateless_mode, the priority 65532 drop rule in acl_eval
> +dnl should NOT contain ct.inv. It should match only
> +dnl (ct.est && ct.rpl && ct_mark.blocked == 1).
> +AT_CHECK([grep 'ls_in_acl_eval' lflows | grep 'priority=65532' | grep -q
> 'ct.inv'], [1])
> +
> +OVN_CLEANUP_NORTHD
> +AT_CLEANUP
> +])
> +
> OVN_FOR_EACH_NORTHD_NO_HV([
> AT_SETUP([Check network function])
> ovn_start
> --
> 2.53.0
>
>
Looks good to me, thanks.
Acked-by: Ales Musil <[email protected]>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
