Message ID | 20210827201134.1985796-1-mmichels@redhat.com |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev,v2] Revert features detection and zero-snat patches. | expand |
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 |
On Fri, Aug 27, 2021 at 4:12 PM Mark Michelson <mmichels@redhat.com> 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 <mmichels@redhat.com> Acked-by: Numan Siddique <numans@ovn.org> Numan > > --- > v1 -> v2: > * Switched from reverting just one commit to reverting two, thereby > eliminating two issues. > --- > > Signed-off-by: Mark Michelson <mmichels@redhat.com> > --- > 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
> On Aug 27, 2021, at 4:11 PM, Mark Michelson <mmichels@redhat.com> 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 <mmichels@redhat.com> > > --- > v1 -> v2: > * Switched from reverting just one commit to reverting two, thereby > eliminating two issues. > --- > > Signed-off-by: Mark Michelson <mmichels@redhat.com> > --- Acked-by: Flavio Fernandes <flavio@flaviof.com <mailto:flavio@flaviof.com>> > 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
Thank you both for the quick reviews. I pushed the change to master. On 8/27/21 5:28 PM, Flavio Fernandes wrote: > > >> On Aug 27, 2021, at 4:11 PM, Mark Michelson <mmichels@redhat.com >> <mailto:mmichels@redhat.com>> 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 >> <https://bugzilla.redhat.com/show_bug.cgi?id=1992705> >> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1992012 >> <https://bugzilla.redhat.com/show_bug.cgi?id=1992012> >> >> Signed-off-by: Mark Michelson <mmichels@redhat.com >> <mailto:mmichels@redhat.com>> >> >> --- >> v1 -> v2: >> * Switched from reverting just one commit to reverting two, thereby >> eliminating two issues. >> --- >> >> Signed-off-by: Mark Michelson <mmichels@redhat.com >> <mailto:mmichels@redhat.com>> >> --- > > > Acked-by: Flavio Fernandes <flavio@flaviof.com <mailto:flavio@flaviof.com>> > > >> 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 <http://ovn-controller.at> | 11 +- >> tests/ovn-features.at <http://ovn-features.at> | 8 -- >> tests/ovn.at <http://ovn.at> | 2 +- >> tests/system-common-macros.at <http://system-common-macros.at> | 4 - >> tests/system-ovn.at <http://system-ovn.at> | 190 >> ---------------------------------- >> tests/testsuite.at <http://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 <http://ovn-features.at> >
On Fri, Aug 27, 2021 at 8:06 PM Mark Michelson <mmichels@redhat.com> wrote: > > Thank you both for the quick reviews. I pushed the change to master. Would this require a backport to branch-21.06 ? Numan > > On 8/27/21 5:28 PM, Flavio Fernandes wrote: > > > > > >> On Aug 27, 2021, at 4:11 PM, Mark Michelson <mmichels@redhat.com > >> <mailto:mmichels@redhat.com>> 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 > >> <https://bugzilla.redhat.com/show_bug.cgi?id=1992705> > >> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1992012 > >> <https://bugzilla.redhat.com/show_bug.cgi?id=1992012> > >> > >> Signed-off-by: Mark Michelson <mmichels@redhat.com > >> <mailto:mmichels@redhat.com>> > >> > >> --- > >> v1 -> v2: > >> * Switched from reverting just one commit to reverting two, thereby > >> eliminating two issues. > >> --- > >> > >> Signed-off-by: Mark Michelson <mmichels@redhat.com > >> <mailto:mmichels@redhat.com>> > >> --- > > > > > > Acked-by: Flavio Fernandes <flavio@flaviof.com <mailto:flavio@flaviof.com>> > > > > > >> 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 <http://ovn-controller.at> | 11 +- > >> tests/ovn-features.at <http://ovn-features.at> | 8 -- > >> tests/ovn.at <http://ovn.at> | 2 +- > >> tests/system-common-macros.at <http://system-common-macros.at> | 4 - > >> tests/system-ovn.at <http://system-ovn.at> | 190 > >> ---------------------------------- > >> tests/testsuite.at <http://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 <http://ovn-features.at> > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 8/28/21 2:44 AM, Numan Siddique wrote: > On Fri, Aug 27, 2021 at 8:06 PM Mark Michelson <mmichels@redhat.com> wrote: >> >> Thank you both for the quick reviews. I pushed the change to master. Thanks Mark, Numan, and Flavio for fixing this and sorry for introducing the issues in the first place. > > Would this require a backport to branch-21.06 ? > If it's not too much work this backport might be a good to have, to avoid behavioral differences between released versions. I'll start thinking about a different way of addressing what 58683a42 (ovn-controller: Handle DNAT/no-NAT conntrack tuple collisions.) was trying to fix. > Numan > Regards, Dumitru >> >> On 8/27/21 5:28 PM, Flavio Fernandes wrote: >>> >>> >>>> On Aug 27, 2021, at 4:11 PM, Mark Michelson <mmichels@redhat.com >>>> <mailto:mmichels@redhat.com>> 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 >>>> <https://bugzilla.redhat.com/show_bug.cgi?id=1992705> >>>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1992012 >>>> <https://bugzilla.redhat.com/show_bug.cgi?id=1992012> >>>> >>>> Signed-off-by: Mark Michelson <mmichels@redhat.com >>>> <mailto:mmichels@redhat.com>> >>>> >>>> --- >>>> v1 -> v2: >>>> * Switched from reverting just one commit to reverting two, thereby >>>> eliminating two issues. >>>> --- >>>> >>>> Signed-off-by: Mark Michelson <mmichels@redhat.com >>>> <mailto:mmichels@redhat.com>> >>>> --- >>> >>> >>> Acked-by: Flavio Fernandes <flavio@flaviof.com <mailto:flavio@flaviof.com>> >>> >>> >>>> 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 <http://ovn-controller.at> | 11 +- >>>> tests/ovn-features.at <http://ovn-features.at> | 8 -- >>>> tests/ovn.at <http://ovn.at> | 2 +- >>>> tests/system-common-macros.at <http://system-common-macros.at> | 4 - >>>> tests/system-ovn.at <http://system-ovn.at> | 190 >>>> ---------------------------------- >>>> tests/testsuite.at <http://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 <http://ovn-features.at> >>> >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
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])