On Fri, Aug 27, 2021 at 4:12 PM Mark Michelson <[email protected]> wrote:
>
> This commit reverts the following two commits:
> 56e2cd3a (ovn-controller: Detect OVS datapath capabilities.)
> 58683a42 (ovn-controller: Handle DNAT/no-NAT conntrack tuple
> collisions.)
>
> The former commit causes an incompatibility with OVS databases that do
> not have the datapath table present. This specifically is making
> OpenStack upgrades from OSP 13 to OSP 16.2 fail.
>
> The latter commit causes significantly degraded dataplane performance
> when testing a sense OpenShift cluster. This was pinpointed to be due to
> an extra ct(nat(src)) that this commit added.
>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1992705
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1992012
>
> Signed-off-by: Mark Michelson <[email protected]>

Acked-by: Numan Siddique <[email protected]>

Numan

>
> ---
> v1 -> v2:
>   * Switched from reverting just one commit to reverting two, thereby
>     eliminating two issues.
> ---
>
> Signed-off-by: Mark Michelson <[email protected]>
> ---
>  controller/ovn-controller.c   | 115 +++++---------------
>  include/ovn/actions.h         |   1 -
>  include/ovn/features.h        |  18 ----
>  lib/actions.c                 |  31 ------
>  lib/automake.mk               |   1 -
>  lib/features.c                |  84 ---------------
>  lib/test-ovn-features.c       |  56 ----------
>  tests/automake.mk             |   3 -
>  tests/ovn-controller.at       |  11 +-
>  tests/ovn-features.at         |   8 --
>  tests/ovn.at                  |   2 +-
>  tests/system-common-macros.at |   4 -
>  tests/system-ovn.at           | 190 ----------------------------------
>  tests/testsuite.at            |   1 -
>  14 files changed, 34 insertions(+), 491 deletions(-)
>  delete mode 100644 lib/features.c
>  delete mode 100644 lib/test-ovn-features.c
>  delete mode 100644 tests/ovn-features.at
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 739048cf8..4927187c5 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -48,7 +48,6 @@
>  #include "openvswitch/vconn.h"
>  #include "openvswitch/vlog.h"
>  #include "ovn/actions.h"
> -#include "ovn/features.h"
>  #include "lib/chassis-index.h"
>  #include "lib/extend-table.h"
>  #include "lib/ip-mcast-index.h"
> @@ -91,7 +90,6 @@ static unixctl_cb_func lflow_cache_show_stats_cmd;
>  static unixctl_cb_func debug_delay_nb_cfg_report;
>
>  #define DEFAULT_BRIDGE_NAME "br-int"
> -#define DEFAULT_DATAPATH "system"
>  #define DEFAULT_PROBE_INTERVAL_MSEC 5000
>  #define OFCTRL_DEFAULT_PROBE_INTERVAL_SEC 0
>
> @@ -331,6 +329,10 @@ static const struct ovsrec_bridge *
>  create_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
>                const struct ovsrec_open_vswitch_table *ovs_table)
>  {
> +    if (!ovs_idl_txn) {
> +        return NULL;
> +    }
> +
>      const struct ovsrec_open_vswitch *cfg;
>      cfg = ovsrec_open_vswitch_table_first(ovs_table);
>      if (!cfg) {
> @@ -394,21 +396,6 @@ create_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
>      return bridge;
>  }
>
> -static const struct ovsrec_datapath *
> -create_br_datapath(struct ovsdb_idl_txn *ovs_idl_txn,
> -                   const struct ovsrec_open_vswitch *cfg,
> -                   const char *datapath_type)
> -{
> -    ovsdb_idl_txn_add_comment(ovs_idl_txn,
> -                              "ovn-controller: creating bridge datapath 
> '%s'",
> -                              datapath_type);
> -
> -    struct ovsrec_datapath *dp = ovsrec_datapath_insert(ovs_idl_txn);
> -    ovsrec_open_vswitch_verify_datapaths(cfg);
> -    ovsrec_open_vswitch_update_datapaths_setkey(cfg, datapath_type, dp);
> -    return dp;
> -}
> -
>  static const struct ovsrec_bridge *
>  get_br_int(const struct ovsrec_bridge_table *bridge_table,
>             const struct ovsrec_open_vswitch_table *ovs_table)
> @@ -422,69 +409,33 @@ get_br_int(const struct ovsrec_bridge_table 
> *bridge_table,
>      return get_bridge(bridge_table, br_int_name(cfg));
>  }
>
> -static const struct ovsrec_datapath *
> -get_br_datapath(const struct ovsrec_open_vswitch *cfg,
> -                const char *datapath_type)
> -{
> -    for (size_t i = 0; i < cfg->n_datapaths; i++) {
> -        if (!strcmp(cfg->key_datapaths[i], datapath_type)) {
> -            return cfg->value_datapaths[i];
> -        }
> -    }
> -    return NULL;
> -}
> -
> -static void
> +static const struct ovsrec_bridge *
>  process_br_int(struct ovsdb_idl_txn *ovs_idl_txn,
>                 const struct ovsrec_bridge_table *bridge_table,
> -               const struct ovsrec_open_vswitch_table *ovs_table,
> -               const struct ovsrec_bridge **br_int_,
> -               const struct ovsrec_datapath **br_int_dp_)
> +               const struct ovsrec_open_vswitch_table *ovs_table)
>  {
> -    const struct ovsrec_bridge *br_int = get_br_int(bridge_table, ovs_table);
> -    const struct ovsrec_datapath *br_int_dp = NULL;
> -
> -    ovs_assert(br_int_ && br_int_dp_);
> -    if (ovs_idl_txn) {
> -        if (!br_int) {
> -            br_int = create_br_int(ovs_idl_txn, ovs_table);
> +    const struct ovsrec_bridge *br_int = get_br_int(bridge_table,
> +                                                    ovs_table);
> +    if (!br_int) {
> +        br_int = create_br_int(ovs_idl_txn, ovs_table);
> +    }
> +    if (br_int && ovs_idl_txn) {
> +        const struct ovsrec_open_vswitch *cfg;
> +        cfg = ovsrec_open_vswitch_table_first(ovs_table);
> +        ovs_assert(cfg);
> +        const char *datapath_type = smap_get(&cfg->external_ids,
> +                                             "ovn-bridge-datapath-type");
> +        /* Check for the datapath_type and set it only if it is defined in
> +         * cfg. */
> +        if (datapath_type && strcmp(br_int->datapath_type, datapath_type)) {
> +            ovsrec_bridge_set_datapath_type(br_int, datapath_type);
>          }
> -
> -        if (br_int) {
> -            const struct ovsrec_open_vswitch *cfg =
> -                ovsrec_open_vswitch_table_first(ovs_table);
> -            ovs_assert(cfg);
> -
> -            /* Propagate "ovn-bridge-datapath-type" from OVS table, if any.
> -             * Otherwise use the datapath-type set in br-int, if any.
> -             * Finally, assume "system" datapath if none configured.
> -             */
> -            const char *datapath_type =
> -                smap_get(&cfg->external_ids, "ovn-bridge-datapath-type");
> -
> -            if (!datapath_type) {
> -                if (br_int->datapath_type[0]) {
> -                    datapath_type = br_int->datapath_type;
> -                } else {
> -                    datapath_type = DEFAULT_DATAPATH;
> -                }
> -            }
> -            if (strcmp(br_int->datapath_type, datapath_type)) {
> -                ovsrec_bridge_set_datapath_type(br_int, datapath_type);
> -            }
> -            if (!br_int->fail_mode || strcmp(br_int->fail_mode, "secure")) {
> -                ovsrec_bridge_set_fail_mode(br_int, "secure");
> -                VLOG_WARN("Integration bridge fail-mode changed to 
> 'secure'.");
> -            }
> -            br_int_dp = get_br_datapath(cfg, datapath_type);
> -            if (!br_int_dp) {
> -                br_int_dp = create_br_datapath(ovs_idl_txn, cfg,
> -                                               datapath_type);
> -            }
> +        if (!br_int->fail_mode || strcmp(br_int->fail_mode, "secure")) {
> +            ovsrec_bridge_set_fail_mode(br_int, "secure");
> +            VLOG_WARN("Integration bridge fail-mode changed to 'secure'.");
>          }
>      }
> -    *br_int_ = br_int;
> -    *br_int_dp_ = br_int_dp;
> +    return br_int;
>  }
>
>  static const char *
> @@ -928,7 +879,6 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>      ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_external_ids);
>      ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_other_config);
>      ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_bridges);
> -    ovsdb_idl_add_column(ovs_idl, &ovsrec_open_vswitch_col_datapaths);
>      ovsdb_idl_add_table(ovs_idl, &ovsrec_table_interface);
>      ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_name);
>      ovsdb_idl_track_add_column(ovs_idl, &ovsrec_interface_col_bfd);
> @@ -951,8 +901,6 @@ ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
>      ovsdb_idl_add_column(ovs_idl, &ovsrec_ssl_col_ca_cert);
>      ovsdb_idl_add_column(ovs_idl, &ovsrec_ssl_col_certificate);
>      ovsdb_idl_add_column(ovs_idl, &ovsrec_ssl_col_private_key);
> -    ovsdb_idl_add_table(ovs_idl, &ovsrec_table_datapath);
> -    ovsdb_idl_add_column(ovs_idl, &ovsrec_datapath_col_capabilities);
>      chassis_register_ovs_idl(ovs_idl);
>      encaps_register_ovs_idl(ovs_idl);
>      binding_register_ovs_idl(ovs_idl);
> @@ -3517,10 +3465,8 @@ main(int argc, char *argv[])
>              ovsrec_bridge_table_get(ovs_idl_loop.idl);
>          const struct ovsrec_open_vswitch_table *ovs_table =
>              ovsrec_open_vswitch_table_get(ovs_idl_loop.idl);
> -        const struct ovsrec_bridge *br_int = NULL;
> -        const struct ovsrec_datapath *br_int_dp = NULL;
> -        process_br_int(ovs_idl_txn, bridge_table, ovs_table,
> -                       &br_int, &br_int_dp);
> +        const struct ovsrec_bridge *br_int =
> +            process_br_int(ovs_idl_txn, bridge_table, ovs_table);
>
>          /* Enable ACL matching for double tagged traffic. */
>          if (ovs_idl_txn) {
> @@ -3563,13 +3509,6 @@ main(int argc, char *argv[])
>                                        &chassis_private);
>              }
>
> -            /* If any OVS feature support changed, force a full recompute. */
> -            if (br_int_dp
> -                    && ovs_feature_support_update(&br_int_dp->capabilities)) 
> {
> -                VLOG_INFO("OVS feature set changed, force recompute.");
> -                engine_set_force_recompute(true);
> -            }
> -
>              if (br_int) {
>                  ct_zones_data = engine_get_data(&en_ct_zones);
>                  if (ct_zones_data) {
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index f023a37b9..b2f2f57c6 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -25,7 +25,6 @@
>  #include "openvswitch/hmap.h"
>  #include "openvswitch/uuid.h"
>  #include "util.h"
> -#include "ovn/features.h"
>
>  struct expr;
>  struct lexer;
> diff --git a/include/ovn/features.h b/include/ovn/features.h
> index c35d59b14..10ee46fcd 100644
> --- a/include/ovn/features.h
> +++ b/include/ovn/features.h
> @@ -16,25 +16,7 @@
>  #ifndef OVN_FEATURES_H
>  #define OVN_FEATURES_H 1
>
> -#include <stdbool.h>
> -
> -#include "smap.h"
> -
>  /* ovn-controller supported feature names. */
>  #define OVN_FEATURE_PORT_UP_NOTIF "port-up-notif"
>
> -/* OVS datapath supported features.  Based on availability OVN might generate
> - * different types of openflows.
> - */
> -enum ovs_feature_support_bits {
> -    OVS_CT_ZERO_SNAT_SUPPORT_BIT,
> -};
> -
> -enum ovs_feature_value {
> -    OVS_CT_ZERO_SNAT_SUPPORT = (1 << OVS_CT_ZERO_SNAT_SUPPORT_BIT),
> -};
> -
> -bool ovs_feature_is_supported(enum ovs_feature_value feature);
> -bool ovs_feature_support_update(const struct smap *ovs_capabilities);
> -
>  #endif
> diff --git a/lib/actions.c b/lib/actions.c
> index c572e88ae..f0291afef 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -742,22 +742,6 @@ encode_CT_COMMIT_V1(const struct ovnact_ct_commit_v1 *cc,
>      ct->zone_src.ofs = 0;
>      ct->zone_src.n_bits = 16;
>
> -    /* If the datapath supports all-zero SNAT then use it to avoid tuple
> -     * collisions at commit time between NATed and firewalled-only sessions.
> -     */
> -
> -    if (ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT)) {
> -        size_t nat_offset = ofpacts->size;
> -        ofpbuf_pull(ofpacts, nat_offset);
> -
> -        struct ofpact_nat *nat = ofpact_put_NAT(ofpacts);
> -        nat->flags = 0;
> -        nat->range_af = AF_UNSPEC;
> -        nat->flags |= NX_NAT_F_SRC;
> -        ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset);
> -        ct = ofpacts->header;
> -    }
> -
>      size_t set_field_offset = ofpacts->size;
>      ofpbuf_pull(ofpacts, set_field_offset);
>
> @@ -808,21 +792,6 @@ encode_CT_COMMIT_V2(const struct ovnact_nest *on,
>      ct->zone_src.ofs = 0;
>      ct->zone_src.n_bits = 16;
>
> -    /* If the datapath supports all-zero SNAT then use it to avoid tuple
> -     * collisions at commit time between NATed and firewalled-only sessions.
> -     */
> -    if (ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT)) {
> -        size_t nat_offset = ofpacts->size;
> -        ofpbuf_pull(ofpacts, nat_offset);
> -
> -        struct ofpact_nat *nat = ofpact_put_NAT(ofpacts);
> -        nat->flags = 0;
> -        nat->range_af = AF_UNSPEC;
> -        nat->flags |= NX_NAT_F_SRC;
> -        ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset);
> -        ct = ofpacts->header;
> -    }
> -
>      size_t set_field_offset = ofpacts->size;
>      ofpbuf_pull(ofpacts, set_field_offset);
>
> diff --git a/lib/automake.mk b/lib/automake.mk
> index ddfe33948..f73e1c9aa 100644
> --- a/lib/automake.mk
> +++ b/lib/automake.mk
> @@ -15,7 +15,6 @@ lib_libovn_la_SOURCES = \
>         lib/expr.c \
>         lib/extend-table.h \
>         lib/extend-table.c \
> -       lib/features.c \
>         lib/ovn-parallel-hmap.h \
>         lib/ovn-parallel-hmap.c \
>         lib/ip-mcast-index.c \
> diff --git a/lib/features.c b/lib/features.c
> deleted file mode 100644
> index 87d04ee3f..000000000
> --- a/lib/features.c
> +++ /dev/null
> @@ -1,84 +0,0 @@
> -/* Copyright (c) 2021, Red Hat, Inc.
> - *
> - * Licensed under the Apache License, Version 2.0 (the "License");
> - * you may not use this file except in compliance with the License.
> - * You may obtain a copy of the License at:
> - *
> - *     http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -
> -#include <config.h>
> -#include <stdint.h>
> -#include <stdlib.h>
> -
> -#include "lib/util.h"
> -#include "openvswitch/vlog.h"
> -#include "ovn/features.h"
> -
> -VLOG_DEFINE_THIS_MODULE(features);
> -
> -struct ovs_feature {
> -    enum ovs_feature_value value;
> -    const char *name;
> -};
> -
> -static struct ovs_feature all_ovs_features[] = {
> -    {
> -        .value = OVS_CT_ZERO_SNAT_SUPPORT,
> -        .name = "ct_zero_snat"
> -    },
> -};
> -
> -/* A bitmap of OVS features that have been detected as 'supported'. */
> -static uint32_t supported_ovs_features;
> -
> -static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
> -
> -static bool
> -ovs_feature_is_valid(enum ovs_feature_value feature)
> -{
> -    switch (feature) {
> -    case OVS_CT_ZERO_SNAT_SUPPORT:
> -        return true;
> -    default:
> -        return false;
> -    }
> -}
> -
> -bool
> -ovs_feature_is_supported(enum ovs_feature_value feature)
> -{
> -    ovs_assert(ovs_feature_is_valid(feature));
> -    return supported_ovs_features & feature;
> -}
> -
> -/* Returns 'true' if the set of tracked OVS features has been updated. */
> -bool
> -ovs_feature_support_update(const struct smap *ovs_capabilities)
> -{
> -    bool updated = false;
> -
> -    for (size_t i = 0; i < ARRAY_SIZE(all_ovs_features); i++) {
> -        enum ovs_feature_value value = all_ovs_features[i].value;
> -        const char *name = all_ovs_features[i].name;
> -        bool old_state = supported_ovs_features & value;
> -        bool new_state = smap_get_bool(ovs_capabilities, name, false);
> -        if (new_state != old_state) {
> -            updated = true;
> -            if (new_state) {
> -                supported_ovs_features |= value;
> -            } else {
> -                supported_ovs_features &= ~value;
> -            }
> -            VLOG_INFO_RL(&rl, "OVS Feature: %s, state: %s", name,
> -                         new_state ? "supported" : "not supported");
> -        }
> -    }
> -    return updated;
> -}
> diff --git a/lib/test-ovn-features.c b/lib/test-ovn-features.c
> deleted file mode 100644
> index deb97581e..000000000
> --- a/lib/test-ovn-features.c
> +++ /dev/null
> @@ -1,56 +0,0 @@
> -/* Copyright (c) 2021, Red Hat, Inc.
> - *
> - * Licensed under the Apache License, Version 2.0 (the "License");
> - * you may not use this file except in compliance with the License.
> - * You may obtain a copy of the License at:
> - *
> - *     http://www.apache.org/licenses/LICENSE-2.0
> - *
> - * Unless required by applicable law or agreed to in writing, software
> - * distributed under the License is distributed on an "AS IS" BASIS,
> - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> - * See the License for the specific language governing permissions and
> - * limitations under the License.
> - */
> -
> -#include <config.h>
> -
> -#include "ovn/features.h"
> -#include "tests/ovstest.h"
> -
> -static void
> -test_ovn_features(struct ovs_cmdl_context *ctx OVS_UNUSED)
> -{
> -    ovs_assert(!ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT));
> -
> -    struct smap features = SMAP_INITIALIZER(&features);
> -
> -    smap_add(&features, "ct_zero_snat", "false");
> -    ovs_assert(!ovs_feature_support_update(&features));
> -    ovs_assert(!ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT));
> -
> -    smap_replace(&features, "ct_zero_snat", "true");
> -    ovs_assert(ovs_feature_support_update(&features));
> -    ovs_assert(ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT));
> -
> -    smap_add(&features, "unknown_feature", "true");
> -    ovs_assert(!ovs_feature_support_update(&features));
> -
> -    smap_destroy(&features);
> -}
> -
> -static void
> -test_ovn_features_main(int argc, char *argv[])
> -{
> -    set_program_name(argv[0]);
> -    static const struct ovs_cmdl_command commands[] = {
> -        {"run", NULL, 0, 0, test_ovn_features, OVS_RO},
> -        {NULL, NULL, 0, 0, NULL, OVS_RO},
> -    };
> -    struct ovs_cmdl_context ctx;
> -    ctx.argc = argc - 1;
> -    ctx.argv = argv + 1;
> -    ovs_cmdl_run_command(&ctx, commands);
> -}
> -
> -OVSTEST_REGISTER("test-ovn-features", test_ovn_features_main);
> diff --git a/tests/automake.mk b/tests/automake.mk
> index 5b890d644..60c732aae 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -36,7 +36,6 @@ TESTSUITE_AT = \
>         tests/ovn-performance.at \
>         tests/ovn-ofctrl-seqno.at \
>         tests/ovn-ipam.at \
> -       tests/ovn-features.at \
>         tests/ovn-lflow-cache.at \
>         tests/ovn-ipsec.at
>
> @@ -235,7 +234,6 @@ $(srcdir)/package.m4: $(top_srcdir)/configure.ac
>
>  noinst_PROGRAMS += tests/ovstest
>  tests_ovstest_SOURCES = \
> -       include/ovn/features.h \
>         tests/ovstest.c \
>         tests/ovstest.h \
>         tests/test-utils.c \
> @@ -247,7 +245,6 @@ tests_ovstest_SOURCES = \
>         controller/lflow-cache.h \
>         controller/ofctrl-seqno.c \
>         controller/ofctrl-seqno.h \
> -       lib/test-ovn-features.c \
>         northd/test-ipam.c \
>         northd/ipam.c \
>         northd/ipam.h
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index 4ae218ed6..29ddd742f 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -151,24 +151,23 @@ sysid=$(ovs-vsctl get Open_vSwitch . 
> external_ids:system-id)
>  check_datapath_type () {
>      datapath_type=$1
>      chassis_datapath_type=$(ovn-sbctl get Chassis ${sysid} 
> other_config:datapath-type | sed -e 's/"//g') #"
> -    ovs_datapath_type=$(ovs-vsctl get Bridge br-int datapath-type)
> -    test "${datapath_type}" = "${chassis_datapath_type}" && test 
> "${datapath_type}" = "${ovs_datapath_type}"
> +    test "${datapath_type}" = "${chassis_datapath_type}"
>  }
>
> -OVS_WAIT_UNTIL([check_datapath_type system])
> +OVS_WAIT_UNTIL([check_datapath_type ""])
>
>  ovs-vsctl set Bridge br-int datapath-type=foo
>  OVS_WAIT_UNTIL([check_datapath_type foo])
>
>  # Change "ovn-bridge-mappings" value. It should not change the 
> "datapath-type".
>  ovs-vsctl set Open_vSwitch . external_ids:ovn-bridge-mappings=foo-mapping
> -AT_CHECK([check_datapath_type foo])
> +check_datapath_type foo
>
>  ovs-vsctl set Bridge br-int datapath-type=bar
>  OVS_WAIT_UNTIL([check_datapath_type bar])
>
>  ovs-vsctl set Bridge br-int datapath-type=\"\"
> -OVS_WAIT_UNTIL([check_datapath_type system])
> +OVS_WAIT_UNTIL([check_datapath_type ""])
>
>  # Set the datapath_type in external_ids:ovn-bridge-datapath-type.
>  ovs-vsctl set Open_vSwitch . external_ids:ovn-bridge-datapath-type=foo
> @@ -177,9 +176,11 @@ OVS_WAIT_UNTIL([check_datapath_type foo])
>  # Change the br-int's datapath type to bar.
>  # It should be reset to foo since ovn-bridge-datapath-type is configured.
>  ovs-vsctl set Bridge br-int datapath-type=bar
> +OVS_WAIT_UNTIL([test foo = `ovs-vsctl get Bridge br-int datapath-type`])
>  OVS_WAIT_UNTIL([check_datapath_type foo])
>
>  ovs-vsctl set Open_vSwitch . external_ids:ovn-bridge-datapath-type=foobar
> +OVS_WAIT_UNTIL([test foobar = `ovs-vsctl get Bridge br-int datapath-type`])
>  OVS_WAIT_UNTIL([check_datapath_type foobar])
>
>  expected_iface_types=$(ovs-vsctl get Open_vSwitch . iface_types | tr -d 
> '[[]] ""')
> diff --git a/tests/ovn-features.at b/tests/ovn-features.at
> deleted file mode 100644
> index 7910c57e9..000000000
> --- a/tests/ovn-features.at
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -#
> -# Unit tests for the lib/features.c module.
> -#
> -AT_BANNER([OVN unit tests - features])
> -
> -AT_SETUP([unit test -- OVS feature detection tests])
> -AT_CHECK([ovstest test-ovn-features run], [0], [])
> -AT_CLEANUP
> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
> index 616a87fcf..c23804f6f 100644
> --- a/tests/system-common-macros.at
> +++ b/tests/system-common-macros.at
> @@ -330,7 +330,3 @@ m4_define([OVS_CHECK_IPROUTE_ENCAP],
>  # OVS_CHECK_CT_CLEAR()
>  m4_define([OVS_CHECK_CT_CLEAR],
>      [AT_SKIP_IF([! grep -q "Datapath supports ct_clear action" 
> ovs-vswitchd.log])])
> -
> -# OVS_CHECK_CT_ZERO_SNAT()
> -m4_define([OVS_CHECK_CT_ZERO_SNAT],
> -    [AT_SKIP_IF([! grep -q "Datapath supports ct_zero_snat" 
> ovs-vswitchd.log])]))
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index aadd68634..9487dde49 100644
> --- a/tests/system-ovn.at
> +++ b/tests/system-ovn.at
> @@ -5319,196 +5319,6 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port 
> patch-.*/d
>  AT_CLEANUP
>  ])
>
> -OVN_FOR_EACH_NORTHD([
> -AT_SETUP([load-balancer and firewall tuple conflict IPv4])
> -AT_SKIP_IF([test $HAVE_NC = no])
> -AT_KEYWORDS([ovnlb])
> -
> -CHECK_CONNTRACK()
> -CHECK_CONNTRACK_NAT()
> -ovn_start
> -OVS_TRAFFIC_VSWITCHD_START()
> -OVS_CHECK_CT_ZERO_SNAT()
> -ADD_BR([br-int])
> -
> -# Set external-ids in br-int needed for ovn-controller
> -ovs-vsctl \
> -        -- set Open_vSwitch . external-ids:system-id=hv1 \
> -        -- set Open_vSwitch . 
> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
> -        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
> -        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
> -        -- set bridge br-int fail-mode=secure 
> other-config:disable-in-band=true
> -
> -# Start ovn-controller
> -start_daemon ovn-controller
> -
> -# Logical network:
> -# 1 logical switch connetected to one logical router.
> -# 2 VMs, one used as backend for a load balancer.
> -
> -check ovn-nbctl                                                  \
> -    -- lr-add rtr                                                \
> -    -- lrp-add rtr rtr-ls 00:00:00:00:01:00 42.42.42.1/24        \
> -    -- ls-add ls                                                 \
> -    -- lsp-add ls ls-rtr                                         \
> -    -- lsp-set-addresses ls-rtr 00:00:00:00:01:00                \
> -    -- lsp-set-type ls-rtr router                                \
> -    -- lsp-set-options ls-rtr router-port=rtr-ls                 \
> -    -- lsp-add ls vm1 -- lsp-set-addresses vm1 00:00:00:00:00:01 \
> -    -- lsp-add ls vm2 -- lsp-set-addresses vm2 00:00:00:00:00:02 \
> -    -- lb-add lb-test 66.66.66.66:666 42.42.42.2:4242 tcp        \
> -    -- ls-lb-add ls lb-test
> -
> -ADD_NAMESPACES(vm1)
> -ADD_VETH(vm1, vm1, br-int, "42.42.42.2/24", "00:00:00:00:00:01", 
> "42.42.42.1")
> -
> -ADD_NAMESPACES(vm2)
> -ADD_VETH(vm2, vm2, br-int, "42.42.42.3/24", "00:00:00:00:00:02", 
> "42.42.42.1")
> -
> -# Wait for ovn-controller to catch up.
> -wait_for_ports_up
> -check ovn-nbctl --wait=hv sync
> -
> -# Start IPv4 TCP server on vm1.
> -NETNS_DAEMONIZE([vm1], [nc -k -l 42.42.42.2 4242], [nc-vm1.pid])
> -
> -# Make sure connecting to the VIP works.
> -NS_CHECK_EXEC([vm2], [nc 66.66.66.66 666 -p 2000 -z])
> -
> -# Start IPv4 TCP connection to VIP from vm2.
> -NS_CHECK_EXEC([vm2], [nc 66.66.66.66 666 -p 2001 -z])
> -
> -# Check conntrack.  We expect two entries:
> -# - one in vm1's zone (firewall)
> -# - one in vm2's zone (dnat)
> -AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep 2001 |             \
> -grep "orig=.src=42\.42\.42\.3" |                                    \
> -sed -e 's/port=2001/port=<clnt_s_port>/g'                           \
> -    -e 's/sport=4242,dport=[[0-9]]\+/sport=4242,dport=<rnd_port>/g' \
> -    -e 's/state=[[0-9_A-Z]]*/state=<cleared>/g'                     \
> -    -e 's/zone=[[0-9]]*/zone=<cleared>/' | sort], [0], [dnl
> -tcp,orig=(src=42.42.42.3,dst=42.42.42.2,sport=<clnt_s_port>,dport=4242),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,protoinfo=(state=<cleared>)
> -tcp,orig=(src=42.42.42.3,dst=66.66.66.66,sport=<clnt_s_port>,dport=666),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,labels=0x2,protoinfo=(state=<cleared>)
> -])
> -
> -# Start IPv4 TCP connection to backend IP from vm2 which would require
> -# additional source port translation to avoid a tuple conflict.
> -NS_CHECK_EXEC([vm2], [nc 42.42.42.2 4242 -p 2001 -z])
> -
> -# Check conntrack.  We expect three entries:
> -# - one in vm1's zone (firewall) - reused from the previous connection.
> -# - one in vm2's zone (dnat) - still in TIME_WAIT after the previous 
> connection.
> -# - one in vm2's zone (firewall + additional all-zero SNAT)
> -AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep 2001 |             \
> -grep "orig=.src=42\.42\.42\.3" |                                    \
> -sed -e 's/port=2001/port=<clnt_s_port>/g'                           \
> -    -e 's/sport=4242,dport=[[0-9]]\+/sport=4242,dport=<rnd_port>/g' \
> -    -e 's/state=[[0-9_A-Z]]*/state=<cleared>/g'                     \
> -    -e 's/zone=[[0-9]]*/zone=<cleared>/' | sort], [0], [dnl
> -tcp,orig=(src=42.42.42.3,dst=42.42.42.2,sport=<clnt_s_port>,dport=4242),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,protoinfo=(state=<cleared>)
> -tcp,orig=(src=42.42.42.3,dst=42.42.42.2,sport=<clnt_s_port>,dport=4242),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=<rnd_port>),zone=<cleared>,protoinfo=(state=<cleared>)
> -tcp,orig=(src=42.42.42.3,dst=66.66.66.66,sport=<clnt_s_port>,dport=666),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,labels=0x2,protoinfo=(state=<cleared>)
> -])
> -
> -AT_CLEANUP
> -])
> -
> -OVN_FOR_EACH_NORTHD([
> -AT_SETUP([load-balancer and firewall tuple conflict IPv6])
> -AT_SKIP_IF([test $HAVE_NC = no])
> -AT_KEYWORDS([ovnlb])
> -
> -CHECK_CONNTRACK()
> -CHECK_CONNTRACK_NAT()
> -ovn_start
> -OVS_TRAFFIC_VSWITCHD_START()
> -OVS_CHECK_CT_ZERO_SNAT()
> -ADD_BR([br-int])
> -
> -# Set external-ids in br-int needed for ovn-controller
> -ovs-vsctl \
> -        -- set Open_vSwitch . external-ids:system-id=hv1 \
> -        -- set Open_vSwitch . 
> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
> -        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
> -        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
> -        -- set bridge br-int fail-mode=secure 
> other-config:disable-in-band=true
> -
> -# Start ovn-controller
> -start_daemon ovn-controller
> -
> -# Logical network:
> -# 1 logical switch connetected to one logical router.
> -# 2 VMs, one used as backend for a load balancer.
> -
> -check ovn-nbctl                                                  \
> -    -- lr-add rtr                                                \
> -    -- lrp-add rtr rtr-ls 00:00:00:00:01:00 4242::1/64           \
> -    -- ls-add ls                                                 \
> -    -- lsp-add ls ls-rtr                                         \
> -    -- lsp-set-addresses ls-rtr 00:00:00:00:01:00                \
> -    -- lsp-set-type ls-rtr router                                \
> -    -- lsp-set-options ls-rtr router-port=rtr-ls                 \
> -    -- lsp-add ls vm1 -- lsp-set-addresses vm1 00:00:00:00:00:01 \
> -    -- lsp-add ls vm2 -- lsp-set-addresses vm2 00:00:00:00:00:02 \
> -    -- lb-add lb-test [[6666::1]]:666 [[4242::2]]:4242 tcp       \
> -    -- ls-lb-add ls lb-test
> -
> -ADD_NAMESPACES(vm1)
> -ADD_VETH(vm1, vm1, br-int, "4242::2/64", "00:00:00:00:00:01", "4242::1")
> -OVS_WAIT_UNTIL([test "$(ip netns exec vm1 ip a | grep 4242::2 | grep 
> tentative)" = ""])
> -
> -ADD_NAMESPACES(vm2)
> -ADD_VETH(vm2, vm2, br-int, "4242::3/64", "00:00:00:00:00:02", "4242::1")
> -OVS_WAIT_UNTIL([test "$(ip netns exec vm2 ip a | grep 4242::3 | grep 
> tentative)" = ""])
> -
> -# Wait for ovn-controller to catch up.
> -wait_for_ports_up
> -check ovn-nbctl --wait=hv sync
> -
> -# Start IPv6 TCP server on vm1.
> -NETNS_DAEMONIZE([vm1], [nc -k -l 4242::2 4242], [nc-vm1.pid])
> -
> -# Make sure connecting to the VIP works.
> -NS_CHECK_EXEC([vm2], [nc 6666::1 666 -p 2000 -z])
> -
> -# Start IPv6 TCP connection to VIP from vm2.
> -NS_CHECK_EXEC([vm2], [nc 6666::1 666 -p 2001 -z])
> -
> -# Check conntrack.  We expect two entries:
> -# - one in vm1's zone (firewall)
> -# - one in vm2's zone (dnat)
> -AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep 2001 |             \
> -grep "orig=.src=4242::3" |                                         \
> -sed -e 's/port=2001/port=<clnt_s_port>/g'                           \
> -    -e 's/sport=4242,dport=[[0-9]]\+/sport=4242,dport=<rnd_port>/g' \
> -    -e 's/state=[[0-9_A-Z]]*/state=<cleared>/g'                     \
> -    -e 's/zone=[[0-9]]*/zone=<cleared>/' | sort], [0], [dnl
> -tcp,orig=(src=4242::3,dst=4242::2,sport=<clnt_s_port>,dport=4242),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,protoinfo=(state=<cleared>)
> -tcp,orig=(src=4242::3,dst=6666::1,sport=<clnt_s_port>,dport=666),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,labels=0x2,protoinfo=(state=<cleared>)
> -])
> -
> -# Start IPv6 TCP connection to backend IP from vm2 which would require
> -# additional source port translation to avoid a tuple conflict.
> -NS_CHECK_EXEC([vm2], [nc 4242::2 4242 -p 2001 -z])
> -
> -# Check conntrack.  We expect three entries:
> -# - one in vm1's zone (firewall) - reused from the previous connection.
> -# - one in vm2's zone (dnat) - still in TIME_WAIT after the previous 
> connection.
> -# - one in vm2's zone (firewall + additional all-zero SNAT)
> -AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep 2001 |             \
> -grep "orig=.src=4242::3" |                                          \
> -sed -e 's/port=2001/port=<clnt_s_port>/g'                           \
> -    -e 's/sport=4242,dport=[[0-9]]\+/sport=4242,dport=<rnd_port>/g' \
> -    -e 's/state=[[0-9_A-Z]]*/state=<cleared>/g'                     \
> -    -e 's/zone=[[0-9]]*/zone=<cleared>/' | sort], [0], [dnl
> -tcp,orig=(src=4242::3,dst=4242::2,sport=<clnt_s_port>,dport=4242),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,protoinfo=(state=<cleared>)
> -tcp,orig=(src=4242::3,dst=4242::2,sport=<clnt_s_port>,dport=4242),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<rnd_port>),zone=<cleared>,protoinfo=(state=<cleared>)
> -tcp,orig=(src=4242::3,dst=6666::1,sport=<clnt_s_port>,dport=666),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,labels=0x2,protoinfo=(state=<cleared>)
> -])
> -
> -AT_CLEANUP
> -])
> -
>  # When a lport is released on a chassis, ovn-controller was
>  # not clearing some of the flowss in the table 33 leading
>  # to packet drops if ct() is hit.
> diff --git a/tests/testsuite.at b/tests/testsuite.at
> index b716a1ad9..ddc3f11d6 100644
> --- a/tests/testsuite.at
> +++ b/tests/testsuite.at
> @@ -27,7 +27,6 @@ m4_include([tests/ovn.at])
>  m4_include([tests/ovn-performance.at])
>  m4_include([tests/ovn-northd.at])
>  m4_include([tests/ovn-nbctl.at])
> -m4_include([tests/ovn-features.at])
>  m4_include([tests/ovn-lflow-cache.at])
>  m4_include([tests/ovn-ofctrl-seqno.at])
>  m4_include([tests/ovn-sbctl.at])
> --
> 2.31.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to