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

Reply via email to