From 1843e3aaa4f9b09327ec7b2a6043d964374f35d0 Mon Sep 17 00:00:00 2001 From: Dumitru Ceara Date: Thu, 13 Jul 2023 16:38:10 +0200 Subject: [PATCH] ovn-controller: Detect and use L4_SYM dp-hash if available. Regular dp-hash is not a canonical L4 hash (at least with the netlink datapath). If the datapath supports l4 symmetrical dp-hash use that one instead. Reported-at: https://github.com/ovn-org/ovn/issues/112 Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2188679 Signed-off-by: Dumitru Ceara Acked-by: Ales Musil (cherry picked from commit 596ea7acbe687fdf780389e664ffef98f3806b53) --- include/ovn/features.h | 2 ++ lib/actions.c | 6 ++++++ lib/features.c | 49 +++++++++++++++++++++++++++++++++--------- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/include/ovn/features.h b/include/ovn/features.h index a37daa4c8f..9849445697 100644 --- a/include/ovn/features.h +++ b/include/ovn/features.h @@ -33,12 +33,14 @@ enum ovs_feature_support_bits { OVS_CT_ZERO_SNAT_SUPPORT_BIT, OVS_DP_METER_SUPPORT_BIT, OVS_CT_TUPLE_FLUSH_BIT, + OVS_DP_HASH_L4_SYM_BIT, }; enum ovs_feature_value { OVS_CT_ZERO_SNAT_SUPPORT = (1 << OVS_CT_ZERO_SNAT_SUPPORT_BIT), OVS_DP_METER_SUPPORT = (1 << OVS_DP_METER_SUPPORT_BIT), OVS_CT_TUPLE_FLUSH_SUPPORT = (1 << OVS_CT_TUPLE_FLUSH_BIT), + OVS_DP_HASH_L4_SYM_SUPPORT = (1 << OVS_DP_HASH_L4_SYM_BIT), }; void ovs_feature_support_destroy(void); diff --git a/lib/actions.c b/lib/actions.c index ec27223f96..c7a07db8e2 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -1625,6 +1625,12 @@ encode_SELECT(const struct ovnact_select *select, struct ds ds = DS_EMPTY_INITIALIZER; ds_put_format(&ds, "type=select,selection_method=dp_hash"); + if (ovs_feature_is_supported(OVS_DP_HASH_L4_SYM_SUPPORT)) { + /* Select dp-hash l4_symmetric by setting the upper 32bits of + * selection_method_param to value 1 (1 << 32): */ + ds_put_cstr(&ds, ",selection_method_param=0x100000000"); + } + struct mf_subfield sf = expr_resolve_field(&select->res_field); for (size_t bucket_id = 0; bucket_id < select->n_dsts; bucket_id++) { diff --git a/lib/features.c b/lib/features.c index 97c9c86f00..d24e8f6c5c 100644 --- a/lib/features.c +++ b/lib/features.c @@ -21,6 +21,7 @@ #include "lib/dirs.h" #include "socket-util.h" #include "lib/vswitch-idl.h" +#include "odp-netlink.h" #include "openvswitch/vlog.h" #include "openvswitch/ofpbuf.h" #include "openvswitch/rconn.h" @@ -33,20 +34,48 @@ VLOG_DEFINE_THIS_MODULE(features); #define FEATURES_DEFAULT_PROBE_INTERVAL_SEC 5 +/* Parses 'cap_name' from 'ovs_capabilities' and returns whether the + * type of capability is supported or not. */ +typedef bool ovs_feature_parse_func(const struct smap *ovs_capabilities, + const char *cap_name); + struct ovs_feature { enum ovs_feature_value value; const char *name; + ovs_feature_parse_func *parse; }; +static bool +bool_parser(const struct smap *ovs_capabilities, const char *cap_name) +{ + return smap_get_bool(ovs_capabilities, cap_name, false); +} + +static bool +dp_hash_l4_sym_support_parser(const struct smap *ovs_capabilities, + const char *cap_name OVS_UNUSED) +{ + int max_hash_alg = smap_get_int(ovs_capabilities, "max_hash_alg", 0); + + return max_hash_alg == OVS_HASH_ALG_SYM_L4; +} + static struct ovs_feature all_ovs_features[] = { { .value = OVS_CT_ZERO_SNAT_SUPPORT, - .name = "ct_zero_snat" + .name = "ct_zero_snat", + .parse = bool_parser, }, { .value = OVS_CT_TUPLE_FLUSH_SUPPORT, - .name = "ct_flush" - } + .name = "ct_flush", + .parse = bool_parser, + }, + { + .value = OVS_DP_HASH_L4_SYM_SUPPORT, + .name = "dp_hash_l4_sym_support", + .parse = dp_hash_l4_sym_support_parser, + }, }; /* A bitmap of OVS features that have been detected as 'supported'. */ @@ -65,6 +94,7 @@ ovs_feature_is_valid(enum ovs_feature_value feature) case OVS_CT_ZERO_SNAT_SUPPORT: case OVS_DP_METER_SUPPORT: case OVS_CT_TUPLE_FLUSH_SUPPORT: + case OVS_DP_HASH_L4_SYM_SUPPORT: return true; default: return false; @@ -183,18 +213,17 @@ ovs_feature_support_run(const struct smap *ovs_capabilities, } 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); + struct ovs_feature *feature = &all_ovs_features[i]; + bool old_state = supported_ovs_features & feature->value; + bool new_state = feature->parse(ovs_capabilities, feature->name); if (new_state != old_state) { updated = true; if (new_state) { - supported_ovs_features |= value; + supported_ovs_features |= feature->value; } else { - supported_ovs_features &= ~value; + supported_ovs_features &= ~feature->value; } - VLOG_INFO_RL(&rl, "OVS Feature: %s, state: %s", name, + VLOG_INFO_RL(&rl, "OVS Feature: %s, state: %s", feature->name, new_state ? "supported" : "not supported"); } }