Re: [ovs-dev] [PATCH v6 ovn] controller: add datapath meter capability check
I merged this to master. Thanks Lorenzo and Mark! On 9/17/21 12:25 PM, Lorenzo Bianconi wrote: Dump datapath meter capabilities before configuring meters in ovn-controller Signed-off-by: Lorenzo Bianconi --- 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 #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_s
Re: [ovs-dev] [PATCH v6 ovn] controller: add datapath meter capability check
On 17/09/2021 17:25, Lorenzo Bianconi wrote: > Dump datapath meter capabilities before configuring meters in > ovn-controller > > Signed-off-by: Lorenzo Bianconi > --- > 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 > > #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,
[ovs-dev] [PATCH v6 ovn] controller: add datapath meter capability check
Dump datapath meter capabilities before configuring meters in ovn-controller Signed-off-by: Lorenzo Bianconi --- 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 #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)) { +