diff mbox series

[ovs-dev] northd: Support an option to ignore chassis features.

Message ID 20230906064932.764795-1-hzhou@ovn.org
State Accepted
Headers show
Series [ovs-dev] northd: Support an option to ignore chassis features. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Han Zhou Sept. 6, 2023, 6:49 a.m. UTC
Add option ignore_chassis_features for northd to bypass the support
status of features on each chassis and to directly implement the latest
features.

This is particularly useful for users who follow the suggested upgrade
order that upgrades ovn-controllers before ovn-northd, and want to
safeguard the operation of northd from being adversely affected by a
mismatched configuration of a chassis. This not only avoids feature
degradation but can also avoid risks of any broken feature in backward
compatible mode. An example of such impact is discussed at [0].

[0] https://mail.openvswitch.org/pipermail/ovs-dev/2023-September/407756.html

Signed-off-by: Han Zhou <hzhou@ovn.org>
---
 northd/northd.c | 25 ++++++++++++++++++-------
 ovn-nb.xml      | 23 +++++++++++++++++++++++
 2 files changed, 41 insertions(+), 7 deletions(-)

Comments

Ales Musil Sept. 6, 2023, 8:17 a.m. UTC | #1
On Wed, Sep 6, 2023 at 8:50 AM Han Zhou <hzhou@ovn.org> wrote:
>
> Add option ignore_chassis_features for northd to bypass the support
> status of features on each chassis and to directly implement the latest
> features.
>
> This is particularly useful for users who follow the suggested upgrade
> order that upgrades ovn-controllers before ovn-northd, and want to
> safeguard the operation of northd from being adversely affected by a
> mismatched configuration of a chassis. This not only avoids feature
> degradation but can also avoid risks of any broken feature in backward
> compatible mode. An example of such impact is discussed at [0].
>
> [0] https://mail.openvswitch.org/pipermail/ovs-dev/2023-September/407756.html
>
> Signed-off-by: Han Zhou <hzhou@ovn.org>
> ---
>  northd/northd.c | 25 ++++++++++++++++++-------
>  ovn-nb.xml      | 23 +++++++++++++++++++++++
>  2 files changed, 41 insertions(+), 7 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 3eaa43f07a1f..e7b5b777a63b 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -17058,6 +17058,17 @@ destroy_datapaths_and_ports(struct ovn_datapaths *ls_datapaths,
>      ovn_datapaths_destroy(lr_datapaths);
>  }
>
> +static void
> +northd_enable_all_features(struct northd_data *data)
> +{
> +    data->features = (struct chassis_features) {
> +        .ct_no_masked_label = true,
> +        .mac_binding_timestamp = true,
> +        .ct_lb_related = true,
> +        .fdb_timestamp = true,
> +    };
> +}
> +
>  void
>  northd_init(struct northd_data *data)
>  {
> @@ -17068,12 +17079,7 @@ northd_init(struct northd_data *data)
>      hmap_init(&data->lbs);
>      hmap_init(&data->lb_groups);
>      ovs_list_init(&data->lr_list);
> -    data->features = (struct chassis_features) {
> -        .ct_no_masked_label = true,
> -        .mac_binding_timestamp = true,
> -        .ct_lb_related = true,
> -        .fdb_timestamp = true,
> -    };
> +    northd_enable_all_features(data);
>      data->ovn_internal_version_changed = false;
>      sset_init(&data->svc_monitor_lsps);
>      data->change_tracked = false;
> @@ -17192,7 +17198,12 @@ ovnnb_db_run(struct northd_input *input_data,
>                                                false);
>      use_common_zone = smap_get_bool(&nb->options, "use_common_zone", false);
>
> -    build_chassis_features(input_data->sbrec_chassis_table, &data->features);
> +    if (smap_get_bool(&nb->options, "ignore_chassis_features", false)) {
> +        northd_enable_all_features(data);
> +    } else {
> +        build_chassis_features(input_data->sbrec_chassis_table,
> +                               &data->features);
> +    }
>
>      init_debug_config(nb);
>
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index 9131305ea99e..1de0c30416ce 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -157,6 +157,29 @@
>          </column>
>        </group>
>
> +       <column name="options" key="ignore_chassis_features">
> +        <p>
> +          When set to <code>false</code>, the <code>ovn-northd</code> will
> +          evaluate the features supported by each chassis and will only
> +          activate features that are universally supported by all chassis. This
> +          approach is crucial for maintaining backward compatibility during an
> +          upgrade when the <code>ovn-northd</code> is updated prior to the
> +          <code>ovn-controller</code>. However, if any chassis is poorly
> +          managed and the upgrade is unsuccessful, it will restrict
> +          <code>ovn-northd</code> from activating the new features.
> +        </p>
> +        <p>
> +          Alternatively, setting this option to <code>true</code> instructs
> +          <code>ovn-northd</code> to bypass the support status of features on
> +          each chassis and to directly implement the latest features. This
> +          approach safeguards the operation of <code>ovn-northd</code> from
> +          being adversely affected by a mismatched configuration of a chassis.
> +        </p>
> +        <p>
> +          The default setting for this option is <code>false</code>.
> +        </p>
> +      </column>
> +
>        <column name="options" key="mac_prefix">
>          Configure a given OUI to be used as prefix when L2 address is
>          dynamically assigned, e.g. <code>00:11:22</code>
> --
> 2.38.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

Looks good to me, thanks.

Acked-by: Ales Musil <amusil@redhat.com>
Han Zhou Sept. 7, 2023, 4:08 p.m. UTC | #2
On Wed, Sep 6, 2023 at 1:17 AM Ales Musil <amusil@redhat.com> wrote:
>
> On Wed, Sep 6, 2023 at 8:50 AM Han Zhou <hzhou@ovn.org> wrote:
> >
> > Add option ignore_chassis_features for northd to bypass the support
> > status of features on each chassis and to directly implement the latest
> > features.
> >
> > This is particularly useful for users who follow the suggested upgrade
> > order that upgrades ovn-controllers before ovn-northd, and want to
> > safeguard the operation of northd from being adversely affected by a
> > mismatched configuration of a chassis. This not only avoids feature
> > degradation but can also avoid risks of any broken feature in backward
> > compatible mode. An example of such impact is discussed at [0].
> >
> > [0]
https://mail.openvswitch.org/pipermail/ovs-dev/2023-September/407756.html
> >
> > Signed-off-by: Han Zhou <hzhou@ovn.org>
> > ---
> >  northd/northd.c | 25 ++++++++++++++++++-------
> >  ovn-nb.xml      | 23 +++++++++++++++++++++++
> >  2 files changed, 41 insertions(+), 7 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index 3eaa43f07a1f..e7b5b777a63b 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -17058,6 +17058,17 @@ destroy_datapaths_and_ports(struct
ovn_datapaths *ls_datapaths,
> >      ovn_datapaths_destroy(lr_datapaths);
> >  }
> >
> > +static void
> > +northd_enable_all_features(struct northd_data *data)
> > +{
> > +    data->features = (struct chassis_features) {
> > +        .ct_no_masked_label = true,
> > +        .mac_binding_timestamp = true,
> > +        .ct_lb_related = true,
> > +        .fdb_timestamp = true,
> > +    };
> > +}
> > +
> >  void
> >  northd_init(struct northd_data *data)
> >  {
> > @@ -17068,12 +17079,7 @@ northd_init(struct northd_data *data)
> >      hmap_init(&data->lbs);
> >      hmap_init(&data->lb_groups);
> >      ovs_list_init(&data->lr_list);
> > -    data->features = (struct chassis_features) {
> > -        .ct_no_masked_label = true,
> > -        .mac_binding_timestamp = true,
> > -        .ct_lb_related = true,
> > -        .fdb_timestamp = true,
> > -    };
> > +    northd_enable_all_features(data);
> >      data->ovn_internal_version_changed = false;
> >      sset_init(&data->svc_monitor_lsps);
> >      data->change_tracked = false;
> > @@ -17192,7 +17198,12 @@ ovnnb_db_run(struct northd_input *input_data,
> >                                                false);
> >      use_common_zone = smap_get_bool(&nb->options, "use_common_zone",
false);
> >
> > -    build_chassis_features(input_data->sbrec_chassis_table,
&data->features);
> > +    if (smap_get_bool(&nb->options, "ignore_chassis_features", false))
{
> > +        northd_enable_all_features(data);
> > +    } else {
> > +        build_chassis_features(input_data->sbrec_chassis_table,
> > +                               &data->features);
> > +    }
> >
> >      init_debug_config(nb);
> >
> > diff --git a/ovn-nb.xml b/ovn-nb.xml
> > index 9131305ea99e..1de0c30416ce 100644
> > --- a/ovn-nb.xml
> > +++ b/ovn-nb.xml
> > @@ -157,6 +157,29 @@
> >          </column>
> >        </group>
> >
> > +       <column name="options" key="ignore_chassis_features">
> > +        <p>
> > +          When set to <code>false</code>, the <code>ovn-northd</code>
will
> > +          evaluate the features supported by each chassis and will only
> > +          activate features that are universally supported by all
chassis. This
> > +          approach is crucial for maintaining backward compatibility
during an
> > +          upgrade when the <code>ovn-northd</code> is updated prior to
the
> > +          <code>ovn-controller</code>. However, if any chassis is
poorly
> > +          managed and the upgrade is unsuccessful, it will restrict
> > +          <code>ovn-northd</code> from activating the new features.
> > +        </p>
> > +        <p>
> > +          Alternatively, setting this option to <code>true</code>
instructs
> > +          <code>ovn-northd</code> to bypass the support status of
features on
> > +          each chassis and to directly implement the latest features.
This
> > +          approach safeguards the operation of <code>ovn-northd</code>
from
> > +          being adversely affected by a mismatched configuration of a
chassis.
> > +        </p>
> > +        <p>
> > +          The default setting for this option is <code>false</code>.
> > +        </p>
> > +      </column>
> > +
> >        <column name="options" key="mac_prefix">
> >          Configure a given OUI to be used as prefix when L2 address is
> >          dynamically assigned, e.g. <code>00:11:22</code>
> > --
> > 2.38.1
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
> Looks good to me, thanks.
>
> Acked-by: Ales Musil <amusil@redhat.com>
>

Thanks! Applied to main.

Han

> --
>
> Ales Musil
>
> Senior Software Engineer - OVN Core
>
> Red Hat EMEA
>
> amusil@redhat.com    IM: amusil
>
diff mbox series

Patch

diff --git a/northd/northd.c b/northd/northd.c
index 3eaa43f07a1f..e7b5b777a63b 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -17058,6 +17058,17 @@  destroy_datapaths_and_ports(struct ovn_datapaths *ls_datapaths,
     ovn_datapaths_destroy(lr_datapaths);
 }
 
+static void
+northd_enable_all_features(struct northd_data *data)
+{
+    data->features = (struct chassis_features) {
+        .ct_no_masked_label = true,
+        .mac_binding_timestamp = true,
+        .ct_lb_related = true,
+        .fdb_timestamp = true,
+    };
+}
+
 void
 northd_init(struct northd_data *data)
 {
@@ -17068,12 +17079,7 @@  northd_init(struct northd_data *data)
     hmap_init(&data->lbs);
     hmap_init(&data->lb_groups);
     ovs_list_init(&data->lr_list);
-    data->features = (struct chassis_features) {
-        .ct_no_masked_label = true,
-        .mac_binding_timestamp = true,
-        .ct_lb_related = true,
-        .fdb_timestamp = true,
-    };
+    northd_enable_all_features(data);
     data->ovn_internal_version_changed = false;
     sset_init(&data->svc_monitor_lsps);
     data->change_tracked = false;
@@ -17192,7 +17198,12 @@  ovnnb_db_run(struct northd_input *input_data,
                                               false);
     use_common_zone = smap_get_bool(&nb->options, "use_common_zone", false);
 
-    build_chassis_features(input_data->sbrec_chassis_table, &data->features);
+    if (smap_get_bool(&nb->options, "ignore_chassis_features", false)) {
+        northd_enable_all_features(data);
+    } else {
+        build_chassis_features(input_data->sbrec_chassis_table,
+                               &data->features);
+    }
 
     init_debug_config(nb);
 
diff --git a/ovn-nb.xml b/ovn-nb.xml
index 9131305ea99e..1de0c30416ce 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -157,6 +157,29 @@ 
         </column>
       </group>
 
+       <column name="options" key="ignore_chassis_features">
+        <p>
+          When set to <code>false</code>, the <code>ovn-northd</code> will
+          evaluate the features supported by each chassis and will only
+          activate features that are universally supported by all chassis. This
+          approach is crucial for maintaining backward compatibility during an
+          upgrade when the <code>ovn-northd</code> is updated prior to the
+          <code>ovn-controller</code>. However, if any chassis is poorly
+          managed and the upgrade is unsuccessful, it will restrict
+          <code>ovn-northd</code> from activating the new features.
+        </p>
+        <p>
+          Alternatively, setting this option to <code>true</code> instructs
+          <code>ovn-northd</code> to bypass the support status of features on
+          each chassis and to directly implement the latest features. This
+          approach safeguards the operation of <code>ovn-northd</code> from
+          being adversely affected by a mismatched configuration of a chassis.
+        </p>
+        <p>
+          The default setting for this option is <code>false</code>.
+        </p>
+      </column>
+
       <column name="options" key="mac_prefix">
         Configure a given OUI to be used as prefix when L2 address is
         dynamically assigned, e.g. <code>00:11:22</code>