Dump datapath meter capabilities before configuring meters in
ovn-controller
Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com>
---
Changes since v5:
- send new command only on reconnection
Changes since v4:
- move rconn setup in ovs_feature_support_update and rename it in
ovs_feature_support_run
- drop ovs_feature_support_init()
- rename ovs_feature_support_deinit in ovs_feature_support_destroy
Changes since v3:
- add missing rconn_run_wait() and rconn_recv_wait() at the end of swconn for
loop
- cosmetics
Changes since v2:
- move meter capability logic in lib/features.c
Changes since v1:
- move rconn in ovn-controller to avoid concurrency issues
---
controller/ovn-controller.c | 7 +--
include/ovn/features.h | 6 ++-
lib/actions.c | 3 ++
lib/automake.mk | 1 +
lib/features.c | 102 +++++++++++++++++++++++++++++++++++-
lib/test-ovn-features.c | 6 +--
tests/ovn.at | 4 +-
7 files changed, 119 insertions(+), 10 deletions(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index d98ebbbfa..aa7941eeb 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -3574,9 +3574,9 @@ main(int argc, char *argv[])
* 'br_int_dp' is valid only if an OVS transaction is possible.
*/
if (ovs_idl_txn
- && ovs_feature_support_update(br_int_dp
- ? &br_int_dp->capabilities
- : NULL)) {
+ && ovs_feature_support_run(br_int_dp ?
+ &br_int_dp->capabilities : NULL,
+ br_int ? br_int->name : NULL)) {
VLOG_INFO("OVS feature set changed, force recompute.");
engine_set_force_recompute(true);
}
@@ -3903,6 +3903,7 @@ loop_done:
ovsdb_idl_loop_destroy(&ovs_idl_loop);
ovsdb_idl_loop_destroy(&ovnsb_idl_loop);
+ ovs_feature_support_destroy();
free(ovs_remote);
service_stop();
diff --git a/include/ovn/features.h b/include/ovn/features.h
index c35d59b14..d12a8eb0d 100644
--- a/include/ovn/features.h
+++ b/include/ovn/features.h
@@ -28,13 +28,17 @@
*/
enum ovs_feature_support_bits {
OVS_CT_ZERO_SNAT_SUPPORT_BIT,
+ OVS_DP_METER_SUPPORT_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),
};
+void ovs_feature_support_destroy(void);
bool ovs_feature_is_supported(enum ovs_feature_value feature);
-bool ovs_feature_support_update(const struct smap *ovs_capabilities);
+bool ovs_feature_support_run(const struct smap *ovs_capabilities,
+ const char *br_name);
#endif
diff --git a/lib/actions.c b/lib/actions.c
index c572e88ae..7cf6be308 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -87,6 +87,9 @@ encode_start_controller_op(enum action_opcode opcode, bool
pause,
oc->max_len = UINT16_MAX;
oc->reason = OFPR_ACTION;
oc->pause = pause;
+ if (!ovs_feature_is_supported(OVS_DP_METER_SUPPORT)) {
+ meter_id = NX_CTLR_NO_METER;
+ }
oc->meter_id = meter_id;
struct action_header ah = { .opcode = htonl(opcode) };
diff --git a/lib/automake.mk b/lib/automake.mk
index ddfe33948..9f9f447d5 100644
--- a/lib/automake.mk
+++ b/lib/automake.mk
@@ -2,6 +2,7 @@ lib_LTLIBRARIES += lib/libovn.la
lib_libovn_la_LDFLAGS = \
$(OVS_LTINFO) \
-Wl,--version-script=$(top_builddir)/lib/libovn.sym \
+ $(OVS_LIBDIR)/libopenvswitch.la \
$(AM_LDFLAGS)
lib_libovn_la_SOURCES = \
lib/acl-log.c \
diff --git a/lib/features.c b/lib/features.c
index fddf4c450..f15ec42bb 100644
--- a/lib/features.c
+++ b/lib/features.c
@@ -18,7 +18,14 @@
#include <stdlib.h>
#include "lib/util.h"
+#include "lib/dirs.h"
+#include "socket-util.h"
+#include "lib/vswitch-idl.h"
#include "openvswitch/vlog.h"
+#include "openvswitch/ofpbuf.h"
+#include "openvswitch/rconn.h"
+#include "openvswitch/ofp-msgs.h"
+#include "openvswitch/ofp-meter.h"
#include "ovn/features.h"
VLOG_DEFINE_THIS_MODULE(features);
@@ -40,11 +47,16 @@ static uint32_t supported_ovs_features;
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
+/* ovs-vswitchd connection. */
+static struct rconn *swconn;
+static uint32_t conn_seq_no;
+
static bool
ovs_feature_is_valid(enum ovs_feature_value feature)
{
switch (feature) {
case OVS_CT_ZERO_SNAT_SUPPORT:
+ case OVS_DP_METER_SUPPORT:
return true;
default:
return false;
@@ -58,9 +70,93 @@ ovs_feature_is_supported(enum ovs_feature_value feature)
return supported_ovs_features & feature;
}
+static void
+ovs_feature_rconn_setup(const char *br_name)
+{
+ if (!swconn) {
+ swconn = rconn_create(5, 0, DSCP_DEFAULT, 1 << OFP15_VERSION);
+ }
+
+ if (!rconn_is_connected(swconn)) {
+ char *target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_name);
+ if (strcmp(target, rconn_get_target(swconn))) {
+ VLOG_INFO("%s: connecting to switch", target);
+ rconn_connect(swconn, target, target);
+ }
+ free(target);
+ }
+}
+
+static bool
+ovs_feature_get_openflow_cap(const char *br_name)
+{
+ if (!br_name) {
+ return false;
+ }
+
+ ovs_feature_rconn_setup(br_name);
+
+ rconn_run(swconn);
+ if (!rconn_is_connected(swconn)) {
+ return false;
+ }
+
+ /* send new requests just after reconnect. */
+ if (conn_seq_no == rconn_get_connection_seqno(swconn)) {
+ return false;
+ }
+
+ bool ret = false;
+ /* dump datapath meter capabilities. */
+ struct ofpbuf *msg = ofpraw_alloc(OFPRAW_OFPST13_METER_FEATURES_REQUEST,
+ rconn_get_version(swconn), 0);
+ rconn_send(swconn, msg, NULL);
+ for (int i = 0; i < 50; i++) {
+ 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) {
+ struct ofputil_meter_features mf;
+ ofputil_decode_meter_features(oh, &mf);
+
+ bool old_state = supported_ovs_features & OVS_DP_METER_SUPPORT;
+ bool new_state = mf.max_meters > 0;
+
+ if (old_state != new_state) {
+ ret = true;
+ if (new_state) {
+ supported_ovs_features |= OVS_DP_METER_SUPPORT;
+ } else {
+ supported_ovs_features &= ~OVS_DP_METER_SUPPORT;
+ }
+ }
+ conn_seq_no = rconn_get_connection_seqno(swconn);
+ }
+ ofpbuf_delete(msg);
+ }
+ rconn_run_wait(swconn);
+ rconn_recv_wait(swconn);
+
+ return ret;
+}
+
+void
+ovs_feature_support_destroy(void)
+{
+ rconn_destroy(swconn);
+ swconn = NULL;
+}
+
/* Returns 'true' if the set of tracked OVS features has been updated. */
bool
-ovs_feature_support_update(const struct smap *ovs_capabilities)
+ovs_feature_support_run(const struct smap *ovs_capabilities,
+ const char *br_name)
{
static struct smap empty_caps = SMAP_INITIALIZER(&empty_caps);
bool updated = false;
@@ -69,6 +165,10 @@ ovs_feature_support_update(const struct smap
*ovs_capabilities)
ovs_capabilities = &empty_caps;
}
+ if (ovs_feature_get_openflow_cap(br_name)) {
+ updated = true;
+ }
+
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;
diff --git a/lib/test-ovn-features.c b/lib/test-ovn-features.c
index deb97581e..aabc547e6 100644
--- a/lib/test-ovn-features.c
+++ b/lib/test-ovn-features.c
@@ -26,15 +26,15 @@ test_ovn_features(struct ovs_cmdl_context *ctx OVS_UNUSED)
struct smap features = SMAP_INITIALIZER(&features);
smap_add(&features, "ct_zero_snat", "false");
- ovs_assert(!ovs_feature_support_update(&features));
+ ovs_assert(!ovs_feature_support_run(&features, NULL));
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_support_run(&features, NULL));
ovs_assert(ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT));
smap_add(&features, "unknown_feature", "true");
- ovs_assert(!ovs_feature_support_update(&features));
+ ovs_assert(!ovs_feature_support_run(&features, NULL));
smap_destroy(&features);
}
diff --git a/tests/ovn.at b/tests/ovn.at
index 30625ec37..9e55c0d57 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1529,9 +1529,9 @@ log(verdict=allow, severity=warning);
log(name="test1", verdict=drop, severity=info);
encodes as
controller(userdata=00.00.00.07.00.00.00.00.01.06.74.65.73.74.31)
log(verdict=drop, severity=info, meter="meter1");
- encodes as controller(userdata=00.00.00.07.00.00.00.00.01.06,meter_id=4)
+ encodes as controller(userdata=00.00.00.07.00.00.00.00.01.06)
log(name="test1", verdict=drop, severity=info, meter="meter1");
- encodes as
controller(userdata=00.00.00.07.00.00.00.00.01.06.74.65.73.74.31,meter_id=4)
+ encodes as
controller(userdata=00.00.00.07.00.00.00.00.01.06.74.65.73.74.31)
log(verdict=drop);
formats as log(verdict=drop, severity=info);
encodes as controller(userdata=00.00.00.07.00.00.00.00.01.06)
