On Wed, Mar 8, 2023 at 3:44 PM Dumitru Ceara <[email protected]> wrote:

> Chassis in remote AZs are not programmed by the local ovn-northd.  So we
> don't need to take into account their supported feature set when
> building logical flows.
>
> This patch also introduces a new northd unix command,
> "debug/chassis-features-list".  This is used by the newly added self
> test but might also be useful when debugging live issues.
>
> Suggested-by: Numan Siddique <[email protected]>
> Signed-off-by: Dumitru Ceara <[email protected]>
> ---
> V2:
> - Addressed Ales' comment:
>   - explicitly set remote chassis features as unsupported in the test.
> ---
>  northd/inc-proc-northd.c | 26 ++++++++++++++++++++++++++
>  northd/northd.c          |  7 +++++++
>  tests/ovn-northd.at      | 36 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 69 insertions(+)
>
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index 1c8550df5d..b28f551895 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -34,10 +34,13 @@
>  #include "en-lflow.h"
>  #include "en-northd-output.h"
>  #include "en-sync-sb.h"
> +#include "unixctl.h"
>  #include "util.h"
>
>  VLOG_DEFINE_THIS_MODULE(inc_proc_northd);
>
> +static unixctl_cb_func chassis_features_list;
> +
>  #define NB_NODES \
>      NB_NODE(nb_global, "nb_global") \
>      NB_NODE(copp, "copp") \
> @@ -305,6 +308,12 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>      engine_ovsdb_node_add_index(&en_sb_address_set,
>                                  "sbrec_address_set_by_name",
>                                  sbrec_address_set_by_name);
> +
> +    struct northd_data *northd_data =
> +        engine_get_internal_data(&en_northd);
> +    unixctl_command_register("debug/chassis-features-list", "", 0, 0,
> +                             chassis_features_list,
> +                             &northd_data->features);
>  }
>
>  /* Returns true if the incremental processing ended up updating nodes. */
> @@ -355,3 +364,20 @@ void inc_proc_northd_cleanup(void)
>      engine_cleanup();
>      engine_set_context(NULL);
>  }
> +
> +static void
> +chassis_features_list(struct unixctl_conn *conn, int argc OVS_UNUSED,
> +                      const char *argv[] OVS_UNUSED, void *features_)
> +{
> +    struct chassis_features *features = features_;
> +    struct ds ds = DS_EMPTY_INITIALIZER;
> +
> +    ds_put_format(&ds, "ct_no_masked_label:    %s\n",
> +                  features->ct_no_masked_label ? "true" : "false");
> +    ds_put_format(&ds, "ct_lb_related:         %s\n",
> +                  features->ct_lb_related ? "true" : "false");
> +    ds_put_format(&ds, "mac_binding_timestamp: %s\n",
> +                  features->mac_binding_timestamp ? "true" : "false");
> +    unixctl_command_reply(conn, ds_cstr(&ds));
> +    ds_destroy(&ds);
> +}
> diff --git a/northd/northd.c b/northd/northd.c
> index ab10fea6ab..661f91ff7e 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -432,6 +432,13 @@ build_chassis_features(const struct northd_input
> *input_data,
>      const struct sbrec_chassis *chassis;
>
>      SBREC_CHASSIS_TABLE_FOR_EACH (chassis, input_data->sbrec_chassis) {
> +        /* Only consider local AZ chassis.  Remote ones don't install
> +         * flows generated by the local northd.
> +         */
> +        if (smap_get_bool(&chassis->other_config, "is-remote", false)) {
> +            continue;
> +        }
> +
>          bool ct_no_masked_label =
>              smap_get_bool(&chassis->other_config,
>                            OVN_FEATURE_CT_NO_MASKED_LABEL,
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index d0f6893e94..6c901a202b 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -8650,3 +8650,39 @@ AT_CHECK([grep -e "ls_in_acl" -e "ls_out_acl"
> lflows2 | grep "priority=65532"],
>
>  AT_CLEANUP
>  ])
> +
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([Chassis-feature compatibitility - remote chassis])
> +ovn_start
> +
> +AS_BOX([Local chassis])
> +check ovn-sbctl chassis-add hv1 geneve 127.0.0.1 \
> +  -- set chassis hv1 other_config:ct-no-masked-label=true \
> +  -- set chassis hv1 other_config:ovn-ct-lb-related=true \
> +  -- set chassis hv1 other_config:mac-binding-timestamp=true
> +
> +check ovn-nbctl --wait=sb sync
> +
> +AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE
> debug/chassis-features-list], [0], [dnl
> +ct_no_masked_label:    true
> +ct_lb_related:         true
> +mac_binding_timestamp: true
> +])
> +
> +AS_BOX([Remote chassis])
> +check ovn-sbctl chassis-add hv2 geneve 127.0.0.2 \
> +  -- set chassis hv2 other_config:is-remote=true \
> +  -- set chassis hv2 other_config:ct-no-masked-label=false \
> +  -- set chassis hv2 other_config:ovn-ct-lb-related=false \
> +  -- set chassis hv2 other_config:mac-binding-timestamp=false
> +
> +check ovn-nbctl --wait=sb sync
> +
> +AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE
> debug/chassis-features-list], [0], [dnl
> +ct_no_masked_label:    true
> +ct_lb_related:         true
> +mac_binding_timestamp: true
> +])
> +
> +AT_CLEANUP
> +])
> --
> 2.31.1
>
>
Looks good to me, thanks.

Acked-by: Ales Musil <[email protected]>

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

[email protected]    IM: amusil
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to