On 9/9/25 9:09 AM, Ales Musil wrote:
> The commit mentioned below removed bunch of features, however in
> order for ovn-controller to work with older northd we need to
> report them. Those features can be removed once we release the
> next LTS. As the upgrade beyond the next LTS is not supported.
>
> Fixes: 92ba14e0d34e ("features: Consolidate chassis features usage, reporting
> and testing.")
> Signed-off-by: Ales Musil <[email protected]>
> ---
Hi Ales,
Ah, sorry, I should've been more thorough when checking the 24.03 code,
I missed that these were still used there. Luckily CI caught it:
https://github.com/ovn-org/ovn/actions/runs/17567481613/job/49911260927#step:19:43
> controller/chassis.c | 6 ++++++
> include/ovn/features.h | 6 ++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/controller/chassis.c b/controller/chassis.c
> index 118e60748..60c01eb24 100644
> --- a/controller/chassis.c
> +++ b/controller/chassis.c
> @@ -408,7 +408,10 @@ chassis_build_other_config(const struct ovs_chassis_cfg
> *ovs_cfg,
> smap_replace(config, "ovn-evpn-local-ip", ovs_cfg->evpn_local_ip);
> smap_replace(config, "is-interconn",
> ovs_cfg->is_interconn ? "true" : "false");
> + smap_replace(config, OVN_FEATURE_PORT_UP_NOTIF, "true");
> + smap_replace(config, OVN_FEATURE_CT_NO_MASKED_LABEL, "true");
> smap_replace(config, OVN_FEATURE_MAC_BINDING_TIMESTAMP, "true");
> + smap_replace(config, OVN_FEATURE_CT_LB_RELATED, "true");
> smap_replace(config, OVN_FEATURE_FDB_TIMESTAMP, "true");
> smap_replace(config, OVN_FEATURE_LS_DPG_COLUMN, "true");
> smap_replace(config, OVN_FEATURE_CT_COMMIT_NAT_V2, "true");
> @@ -742,7 +745,10 @@ update_supported_sset(struct sset *supported)
> /* Internal options. */
> sset_add(supported, "is-vtep");
> sset_add(supported, "is-remote");
> + sset_add(supported, OVN_FEATURE_PORT_UP_NOTIF);
> + sset_add(supported, OVN_FEATURE_CT_NO_MASKED_LABEL);
> sset_add(supported, OVN_FEATURE_MAC_BINDING_TIMESTAMP);
> + sset_add(supported, OVN_FEATURE_CT_LB_RELATED);
> sset_add(supported, OVN_FEATURE_FDB_TIMESTAMP);
> sset_add(supported, OVN_FEATURE_LS_DPG_COLUMN);
> sset_add(supported, OVN_FEATURE_CT_COMMIT_NAT_V2);
I think unfortunately this is not enough though. We should also add
back the part that reconciles Sb.Chassis.other_config for these
features. Otherwise, if the SB record is changed externally
ovn-controller won't re-enable the feature, causing old northds
(24.03) to fall back the old way of doing things even if
ovn-controller has been upgraded already.
That's what we do for all feature flags (non-deprecated too).
Something like:
diff --git a/controller/chassis.c b/controller/chassis.c
index 60c01eb245..07bef1c56d 100644
--- a/controller/chassis.c
+++ b/controller/chassis.c
@@ -538,12 +538,29 @@ chassis_other_config_changed(const struct ovs_chassis_cfg
*ovs_cfg,
return true;
}
+ if (!smap_get_bool(&chassis_rec->other_config, OVN_FEATURE_PORT_UP_NOTIF,
+ false)) {
+ return true;
+ }
+
+ if (!smap_get_bool(&chassis_rec->other_config,
+ OVN_FEATURE_CT_NO_MASKED_LABEL,
+ false)) {
+ return true;
+ }
+
if (!smap_get_bool(&chassis_rec->other_config,
OVN_FEATURE_MAC_BINDING_TIMESTAMP,
false)) {
return true;
}
+ if (!smap_get_bool(&chassis_rec->other_config,
+ OVN_FEATURE_CT_LB_RELATED,
+ false)) {
+ return true;
+ }
+
if (!smap_get_bool(&chassis_rec->other_config,
OVN_FEATURE_FDB_TIMESTAMP,
false)) {
diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
index 17ec3c5d95..0b00906ae7 100644
--- a/tests/ovn-controller.at
+++ b/tests/ovn-controller.at
@@ -642,7 +642,9 @@ ovn_attach n1 br-phys 192.168.0.1
# Wait for ovn-controller to register in the SB.
wait_row_count Chassis 1 name=hv1 other_config:mac-binding-timestamp="true"
-check_row_count Chassis 1 name=hv1 other_config:fdb-timestamp="true"
+check_row_count Chassis 1 name=hv1 other_config:port-up-notif="true"
+check_row_count Chassis 1 name=hv1 other_config:ct-no-masked-label="true"
+check_row_count Chassis 1 name=hv1 other_config:ovn-ct-lb-related="true"
check_row_count Chassis 1 name=hv1 other_config:ls-dpg-column="true"
check_row_count Chassis 1 name=hv1 other_config:ct-commit-nat-v2="true"
check_row_count Chassis 1 name=hv1 other_config:ct-commit-to-zone="true"
---
With that addressed, and assuming CI is green:
Acked-by: Dumitru Ceara <[email protected]>
Regards,
Dumitru
> diff --git a/include/ovn/features.h b/include/ovn/features.h
> index d5828a7e5..3caeaea25 100644
> --- a/include/ovn/features.h
> +++ b/include/ovn/features.h
> @@ -31,6 +31,12 @@
> #define OVN_FEATURE_CT_LABEL_FLUSH "ct-label-flush"
> #define OVN_FEATURE_CT_STATE_SAVE "ct-state-save"
>
> +/* DEPRACATED: The following features can be removed
> + * after the next LTS version release. */
> +#define OVN_FEATURE_PORT_UP_NOTIF "port-up-notif"
> +#define OVN_FEATURE_CT_NO_MASKED_LABEL "ct-no-masked-label"
> +#define OVN_FEATURE_CT_LB_RELATED "ovn-ct-lb-related"
> +
> /* OVS datapath supported features. Based on availability OVN might generate
> * different types of openflows.
> */
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev