From 71c00f7492c954b422c5b6c432e1c34b3f3f86e5 Mon Sep 17 00:00:00 2001 From: Ales Musil Date: Thu, 18 Jul 2024 10:52:25 +0200 Subject: [PATCH] features: Make querying of OpenFlow features more versatile. Up until now we were interested only in two OpenFlow features, meters and groups. The current system of querying worked, however it wasn't very versatile, and it would be hard to query more features, make the system more extensible instead. Signed-off-by: Ales Musil --- V6: - Removed Dumitru's ack. V5: - Address Ilya's comment: - Rename OVS_DP_GROUP_SUPPORT to OVS_OF_GROUP_SUPPORT - Added Dumitru's ack --- include/ovn/features.h | 2 + lib/features.c | 269 +++++++++++++++++++++++++++++------------ 2 files changed, 196 insertions(+), 75 deletions(-) diff --git a/include/ovn/features.h b/include/ovn/features.h index d7bceb62c4..97669410af 100644 --- a/include/ovn/features.h +++ b/include/ovn/features.h @@ -38,6 +38,7 @@ enum ovs_feature_support_bits { OVS_DP_METER_SUPPORT_BIT, OVS_CT_TUPLE_FLUSH_BIT, OVS_DP_HASH_L4_SYM_BIT, + OVS_OF_GROUP_SUPPORT_BIT, }; enum ovs_feature_value { @@ -45,6 +46,7 @@ enum ovs_feature_value { 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), + OVS_OF_GROUP_SUPPORT = (1 << OVS_OF_GROUP_SUPPORT_BIT), }; void ovs_feature_support_destroy(void); diff --git a/lib/features.c b/lib/features.c index 607e4bd313..d3591d6410 100644 --- a/lib/features.c +++ b/lib/features.c @@ -28,6 +28,7 @@ #include "openvswitch/ofp-msgs.h" #include "openvswitch/ofp-meter.h" #include "openvswitch/ofp-group.h" +#include "openvswitch/ofp-print.h" #include "openvswitch/ofp-util.h" #include "openvswitch/rconn.h" #include "ovn/features.h" @@ -47,6 +48,18 @@ struct ovs_feature { ovs_feature_parse_func *parse; }; +struct ovs_openflow_feature { + enum ovs_feature_value value; + const char *name; + bool queued; + ovs_be32 xid; + ovs_be32 barrier_xid; + void (*send_request)(struct ovs_openflow_feature *feature); + bool (*handle_response)(struct ovs_openflow_feature *feature, + enum ofptype type, const struct ofp_header *oh); + bool (*handle_barrier)(struct ovs_openflow_feature *feature); +}; + static bool bool_parser(const struct smap *ovs_capabilities, const char *cap_name) { @@ -83,24 +96,172 @@ static struct ovs_feature all_ovs_features[] = { /* A bitmap of OVS features that have been detected as 'supported'. */ static uint32_t supported_ovs_features; -/* Last set of received feature replies. */ -static struct ofputil_meter_features ovs_meter_features_reply; -static struct ofputil_group_features ovs_group_features_reply; /* Currently discovered set of features. */ static struct ofputil_meter_features ovs_meter_features; static struct ofputil_group_features ovs_group_features; -/* Number of features replies still expected to receive for the requests - * we sent already. */ -static uint32_t n_features_reply_expected; - -static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5); +static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 10); /* ovs-vswitchd connection. */ static struct rconn *swconn; static uint32_t conn_seq_no; +static void +log_unexpected_reply(struct ovs_openflow_feature *feature, + const struct ofp_header *oh) +{ + if (VLOG_IS_WARN_ENABLED()) { + char *s = ofp_to_string(oh, ntohs(oh->length), NULL, NULL, 2); + VLOG_WARN_RL(&rl, "OVS Feature: %s, unexpected reply: %s", + feature->name, s); + free(s); + } +} + +static bool +default_barrier_response_handle(struct ovs_openflow_feature *feature) +{ + VLOG_WARN_RL(&rl, "OVS Feature: %s, didn't receive any reply", + feature->name); + return supported_ovs_features & feature->value; +} + +static void +meter_features_send_request(struct ovs_openflow_feature *feature) +{ + struct ofpbuf *msg = ofpraw_alloc(OFPRAW_OFPST13_METER_FEATURES_REQUEST, + rconn_get_version(swconn), 0); + feature->xid = ((struct ofp_header *) msg->data)->xid; + rconn_send(swconn, msg, NULL); +} + +static bool +meter_features_handle_response(struct ovs_openflow_feature *feature, + enum ofptype type, const struct ofp_header *oh) +{ + if (type != OFPTYPE_METER_FEATURES_STATS_REPLY) { + log_unexpected_reply(feature, oh); + return supported_ovs_features & feature->value; + } + + struct ofputil_meter_features features; + ofputil_decode_meter_features(oh, &features); + + if (memcmp(&ovs_meter_features, &features, sizeof features)) { + ovs_meter_features = features; + return ovs_meter_features.max_meters; + } + + return supported_ovs_features & feature->value; +} + +static void +group_features_send_request(struct ovs_openflow_feature *feature) +{ + struct ofpbuf *msg = + ofputil_encode_group_features_request(rconn_get_version(swconn)); + feature->xid = ((struct ofp_header *) msg->data)->xid; + rconn_send(swconn, msg, NULL); +} + +static bool +group_features_handle_response(struct ovs_openflow_feature *feature, + enum ofptype type, const struct ofp_header *oh) +{ + if (type != OFPTYPE_GROUP_FEATURES_STATS_REPLY) { + log_unexpected_reply(feature, oh); + return supported_ovs_features & feature->value; + } + + struct ofputil_group_features features; + ofputil_decode_group_features_reply(oh, &features); + + if (memcmp(&ovs_group_features, &features, sizeof features)) { + ovs_group_features = features; + return ovs_group_features.max_groups[OFPGT11_SELECT]; + } + + return supported_ovs_features & feature->value; +} + +static struct ovs_openflow_feature all_openflow_features[] = { + { + .value = OVS_DP_METER_SUPPORT, + .name = "meter_support", + .send_request = meter_features_send_request, + .handle_response = meter_features_handle_response, + .handle_barrier = default_barrier_response_handle, + }, + { + .value = OVS_OF_GROUP_SUPPORT, + .name = "group_support", + .send_request = group_features_send_request, + .handle_response = group_features_handle_response, + .handle_barrier = default_barrier_response_handle, + } +}; + +static bool +handle_feature_state_update(bool new_state, enum ovs_feature_value value, + const char *name) +{ + bool updated = false; + + bool old_state = supported_ovs_features & value; + 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; +} + +static bool +features_handle_rconn_msg(struct ofpbuf *msg) +{ + const struct ofp_header *oh = msg->data; + + enum ofptype type; + ofptype_decode(&type, oh); + + if (type == OFPTYPE_ECHO_REQUEST) { + rconn_send(swconn, ofputil_encode_echo_reply(oh), NULL); + return false; + } + + for (size_t i = 0; i < ARRAY_SIZE(all_openflow_features); i++) { + struct ovs_openflow_feature *feature = &all_openflow_features[i]; + + bool new_state; + if (feature->queued && feature->xid == oh->xid) { + new_state = feature->handle_response(feature, type, oh); + } else if (feature->queued && feature->barrier_xid == oh->xid) { + new_state = feature->handle_barrier(feature); + } else { + continue; + } + + feature->queued = false; + return handle_feature_state_update(new_state, feature->value, + feature->name); + } + + if (VLOG_IS_DBG_ENABLED()) { + char *s = ofp_to_string(oh, ntohs(oh->length), NULL, NULL, 2); + VLOG_DBG_RL(&rl, "OpenFlow packet ignored: %s", s); + free(s); + } + + return false; +} + static bool ovs_feature_is_valid(enum ovs_feature_value feature) { @@ -109,6 +270,7 @@ ovs_feature_is_valid(enum ovs_feature_value feature) case OVS_DP_METER_SUPPORT: case OVS_CT_TUPLE_FLUSH_SUPPORT: case OVS_DP_HASH_L4_SYM_SUPPORT: + case OVS_OF_GROUP_SUPPORT: return true; default: return false; @@ -126,8 +288,6 @@ ovs_feature_is_supported(enum ovs_feature_value feature) static bool ovs_feature_get_openflow_cap(void) { - struct ofpbuf *msg; - rconn_run(swconn); if (!rconn_is_connected(swconn)) { rconn_run_wait(swconn); @@ -137,67 +297,33 @@ ovs_feature_get_openflow_cap(void) /* send new requests just after reconnect. */ if (conn_seq_no != rconn_get_connection_seqno(swconn)) { - n_features_reply_expected = 0; - - /* Dump OpenFlow switch meter capabilities. */ - msg = ofpraw_alloc(OFPRAW_OFPST13_METER_FEATURES_REQUEST, - rconn_get_version(swconn), 0); - rconn_send(swconn, msg, NULL); - n_features_reply_expected++; - /* Dump OpenFlow switch group capabilities. */ - msg = ofputil_encode_group_features_request(rconn_get_version(swconn)); - rconn_send(swconn, msg, NULL); - n_features_reply_expected++; + for (size_t i = 0; i < ARRAY_SIZE(all_openflow_features); i++) { + struct ovs_openflow_feature *feature = &all_openflow_features[i]; + + feature->queued = true; + feature->send_request(feature); + + struct ofpbuf *msg = + ofputil_encode_barrier_request(rconn_get_version(swconn)); + feature->barrier_xid = ((struct ofp_header *) msg->data)->xid; + rconn_send(swconn, msg, NULL); + } } conn_seq_no = rconn_get_connection_seqno(swconn); bool ret = false; for (int i = 0; i < 50; i++) { - msg = rconn_recv(swconn); + struct ofpbuf *msg = rconn_recv(swconn); if (!msg) { break; } - const struct ofp_header *oh = msg->data; - enum ofptype type; - ofptype_decode(&type, oh); - - if (type == OFPTYPE_METER_FEATURES_STATS_REPLY) { - ofputil_decode_meter_features(oh, &ovs_meter_features_reply); - ovs_assert(n_features_reply_expected); - n_features_reply_expected--; - } else if (type == OFPTYPE_GROUP_FEATURES_STATS_REPLY) { - ofputil_decode_group_features_reply(oh, &ovs_group_features_reply); - ovs_assert(n_features_reply_expected); - n_features_reply_expected--; - } else if (type == OFPTYPE_ECHO_REQUEST) { - rconn_send(swconn, ofputil_encode_echo_reply(oh), NULL); - } + ret |= features_handle_rconn_msg(msg); ofpbuf_delete(msg); } rconn_run_wait(swconn); rconn_recv_wait(swconn); - /* If all feature replies were received, update the set of supported - * features. */ - if (!n_features_reply_expected) { - if (memcmp(&ovs_meter_features, &ovs_meter_features_reply, - sizeof ovs_meter_features_reply)) { - ovs_meter_features = ovs_meter_features_reply; - if (ovs_meter_features.max_meters) { - supported_ovs_features |= OVS_DP_METER_SUPPORT; - } else { - supported_ovs_features &= ~OVS_DP_METER_SUPPORT; - } - ret = true; - } - if (memcmp(&ovs_group_features, &ovs_group_features_reply, - sizeof ovs_group_features_reply)) { - ovs_group_features = ovs_group_features_reply; - ret = true; - } - } - return ret; } @@ -214,7 +340,6 @@ ovs_feature_support_run(const struct smap *ovs_capabilities, const char *conn_target, int probe_interval) { static struct smap empty_caps = SMAP_INITIALIZER(&empty_caps); - bool updated = false; if (!ovs_capabilities) { ovs_capabilities = &empty_caps; @@ -225,24 +350,13 @@ ovs_feature_support_run(const struct smap *ovs_capabilities, } ovn_update_swconn_at(swconn, conn_target, probe_interval, "features"); - if (ovs_feature_get_openflow_cap()) { - updated = true; - } + bool updated = ovs_feature_get_openflow_cap(); for (size_t i = 0; i < ARRAY_SIZE(all_ovs_features); i++) { 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 |= feature->value; - } else { - supported_ovs_features &= ~feature->value; - } - VLOG_INFO_RL(&rl, "OVS Feature: %s, state: %s", feature->name, - new_state ? "supported" : "not supported"); - } + bool new_value = feature->parse(ovs_capabilities, feature->name); + updated |= handle_feature_state_update(new_value, feature->value, + feature->name); } return updated; } @@ -252,8 +366,13 @@ ovs_feature_set_discovered(void) { /* The supported feature set has been discovered if we're connected * to OVS and it replied to all our feature request messages. */ - return swconn && rconn_is_connected(swconn) && - n_features_reply_expected == 0; + bool replied_to_all = false; + for (size_t i = 0; i < ARRAY_SIZE(all_openflow_features); i++) { + struct ovs_openflow_feature *feature = &all_openflow_features[i]; + replied_to_all |= !feature->queued; + } + + return swconn && rconn_is_connected(swconn) && replied_to_all; } /* Returns the number of meters the OVS datapath supports. */