On Thu, Apr 4, 2024 at 9:56 PM Mark Michelson <mmich...@redhat.com> wrote:
> Hi Ales, > > The patch looks good to me except for one problem. > > On 4/3/24 04:53, Ales Musil wrote: > > The br-int connection is hardcoded to use unix socket, which requires > > for the socket to be visible for ovn-controller. This is achievable in > > container by mounting the socket, but in turn the container requires > > additional privileges. > > > > Add option to vswitchd external-ids that allows to specify remote > > target for management bridge. This gives the user possibility to > > connect to management bridge in different manner than unix socket, > > defaulting to the unix socket when not specified. In addition, there > > is an option to specify inactivity probe for this connection, disabled > > by default. > > > > Reported-at: https://issues.redhat.com/browse/FDP-243 > > Signed-off-by: Ales Musil <amu...@redhat.com> > > --- > > v2: Rebase on top of current main. > > Make the probe interval accept milliseconds to be aligned with > other probe intervals. > > Use external-ids instead of options for the ovn-controller. > > --- > > NEWS | 6 +++ > > controller/ofctrl.c | 10 +---- > > controller/ofctrl.h | 5 ++- > > controller/ovn-controller.8.xml | 15 ++++++++ > > controller/ovn-controller.c | 59 +++++++++++++++++++++++------ > > controller/pinctrl.c | 56 ++++++---------------------- > > controller/pinctrl.h | 6 ++- > > controller/statctrl.c | 66 ++++++--------------------------- > > controller/statctrl.h | 3 +- > > include/ovn/features.h | 2 +- > > lib/features.c | 35 +++++------------ > > lib/ovn-util.c | 26 +++++++++++++ > > lib/ovn-util.h | 4 ++ > > lib/test-ovn-features.c | 6 +-- > > tests/ovn-controller.at | 45 ++++++++++++++++++++++ > > 15 files changed, 193 insertions(+), 151 deletions(-) > > > > diff --git a/NEWS b/NEWS > > index 141f1831c..4663780a8 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -13,6 +13,12 @@ Post v24.03.0 > > "lflow-stage-to-oftable STAGE_NAME" that converts stage name into > OpenFlow > > table id. > > - Rename the ovs-sandbox script to ovn-sandbox. > > + - Add "ovn-bridge-remote" config option to vswitchd external-ids, > > + that allows to specify connection method to management bridge for > > + ovn-controller, defaulting to the unix socket. > > + - Add "ovn-bridge-remote-probe-interval" config option to vswitchd > > + external-ids, that sets probe interval for integration bridge > connection, > > + disabled by default. > > > > OVN v24.03.0 - 01 Mar 2024 > > -------------------------- > > diff --git a/controller/ofctrl.c b/controller/ofctrl.c > > index 6a2564604..9d181a782 100644 > > --- a/controller/ofctrl.c > > +++ b/controller/ofctrl.c > > @@ -771,19 +771,13 @@ ofctrl_get_mf_field_id(void) > > * Returns 'true' if an OpenFlow reconnect happened; 'false' otherwise. > > */ > > bool > > -ofctrl_run(const struct ovsrec_bridge *br_int, > > +ofctrl_run(const char *conn_target, int probe_interval, > > const struct ovsrec_open_vswitch_table *ovs_table, > > struct shash *pending_ct_zones) > > { > > - char *target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), > br_int->name); > > bool reconnected = false; > > > > - if (strcmp(target, rconn_get_target(swconn))) { > > - VLOG_INFO("%s: connecting to switch", target); > > - rconn_connect(swconn, target, target); > > - } > > - free(target); > > - > > + ovn_update_swconn_at(swconn, conn_target, probe_interval, "ofctrl"); > > rconn_run(swconn); > > > > if (!rconn_is_connected(swconn) || !pending_ct_zones) { > > diff --git a/controller/ofctrl.h b/controller/ofctrl.h > > index 502c73da6..7df0a24ea 100644 > > --- a/controller/ofctrl.h > > +++ b/controller/ofctrl.h > > @@ -50,8 +50,9 @@ struct ovn_desired_flow_table { > > /* Interface for OVN main loop. */ > > void ofctrl_init(struct ovn_extend_table *group_table, > > struct ovn_extend_table *meter_table); > > -bool ofctrl_run(const struct ovsrec_bridge *br_int, > > - const struct ovsrec_open_vswitch_table *, > > + > > +bool ofctrl_run(const char *conn_target, int probe_interval, > > + const struct ovsrec_open_vswitch_table *ovs_table, > > struct shash *pending_ct_zones); > > enum mf_field_id ofctrl_get_mf_field_id(void); > > void ofctrl_put(struct ovn_desired_flow_table *lflow_table, > > diff --git a/controller/ovn-controller.8.xml > b/controller/ovn-controller.8.xml > > index 5ebef048d..7ae98e4b0 100644 > > --- a/controller/ovn-controller.8.xml > > +++ b/controller/ovn-controller.8.xml > > @@ -388,6 +388,21 @@ > > cap for the exponential backoff used by > <code>ovn-controller</code> > > to send GARPs packets. > > </dd> > > + <dt><code>external_ids:ovn-bridge-remote</code></dt> > > + <dd> > > + <p> > > + Connection to the OVN management bridge in OvS. It defaults to > > + <code>unix:<var>br-int</var>.mgmt</code> when not specified. > > + </p> > > + </dd> > > + <dt><code>external_ids:ovn-bridge-remote</code></dt> > > The name of this option is "ovn-bridge-remote-probe-interval" > Oops copy-pasta I'll fix it in v3. > > > + <dd> > > + <p> > > + The inactivity probe interval of the connection to the OVN > management > > + bridge, in milliseconds. > > + If the value is zero, it disables the inactivity probe. > > + </p> > > + </dd> > > </dl> > > > > <p> > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > > index c9ff5967a..134911a29 100644 > > --- a/controller/ovn-controller.c > > +++ b/controller/ovn-controller.c > > @@ -122,6 +122,11 @@ static unixctl_cb_func debug_ignore_startup_delay; > > #define OVS_NB_CFG_TS_NAME "ovn-nb-cfg-ts" > > #define OVS_STARTUP_TS_NAME "ovn-startup-ts" > > > > +struct br_int_remote { > > + char *target; > > + int probe_interval; > > +}; > > + > > static char *parse_options(int argc, char *argv[]); > > OVS_NO_RETURN static void usage(void); > > > > @@ -5052,11 +5057,43 @@ check_northd_version(struct ovsdb_idl *ovs_idl, > struct ovsdb_idl *ovnsb_idl, > > return true; > > } > > > > +static void > > +br_int_remote_update(struct br_int_remote *remote, > > + const struct ovsrec_bridge *br_int, > > + const struct ovsrec_open_vswitch_table *ovs_table) > > +{ > > + if (!br_int) { > > + return; > > + } > > + > > + const struct ovsrec_open_vswitch *cfg = > > + ovsrec_open_vswitch_table_first(ovs_table); > > + > > + const char *ext_target = > > + smap_get(&cfg->external_ids, "ovn-bridge-remote"); > > + char *target = ext_target > > + ? xstrdup(ext_target) > > + : xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_int->name); > > + > > + if (!remote->target || strcmp(remote->target, target)) { > > + free(remote->target); > > + remote->target = target; > > + } else { > > + free(target); > > + } > > + > > + unsigned long long probe_interval = > > + smap_get_ullong(&cfg->external_ids, > > + "ovn-bridge-remote-probe-interval", 0); > > + remote->probe_interval = MIN(probe_interval / 1000, INT_MAX); > > +} > > + > > int > > main(int argc, char *argv[]) > > { > > struct unixctl_server *unixctl; > > struct ovn_exit_args exit_args = {0}; > > + struct br_int_remote br_int_remote = {0}; > > int retval; > > > > /* Read from system-id-override file once on startup. */ > > @@ -5666,6 +5703,11 @@ main(int argc, char *argv[]) > > > ovsrec_server_has_datapath_table(ovs_idl_loop.idl) > > ? &br_int_dp > > : NULL); > > + br_int_remote_update(&br_int_remote, br_int, ovs_table); > > + statctrl_update_swconn(br_int_remote.target, > > + br_int_remote.probe_interval); > > + pinctrl_update_swconn(br_int_remote.target, > > + br_int_remote.probe_interval); > > > > /* Enable ACL matching for double tagged traffic. */ > > if (ovs_idl_txn) { > > @@ -5720,7 +5762,8 @@ main(int argc, char *argv[]) > > if (ovs_idl_txn > > && ovs_feature_support_run(br_int_dp ? > > &br_int_dp->capabilities : > NULL, > > - br_int ? br_int->name : > NULL)) { > > + br_int_remote.target, > > + > br_int_remote.probe_interval)) { > > VLOG_INFO("OVS feature set changed, force recompute."); > > engine_set_force_recompute(true); > > if (ovs_feature_set_discovered()) { > > @@ -5738,7 +5781,8 @@ main(int argc, char *argv[]) > > > > if (br_int) { > > ct_zones_data = engine_get_data(&en_ct_zones); > > - if (ofctrl_run(br_int, ovs_table, > > + if (ofctrl_run(br_int_remote.target, > > + br_int_remote.probe_interval, ovs_table, > > ct_zones_data ? &ct_zones_data->pending > > : NULL)) { > > static struct vlog_rate_limit rl > > @@ -5855,7 +5899,7 @@ main(int argc, char *argv[]) > > } > > stopwatch_start(PINCTRL_RUN_STOPWATCH_NAME, > > time_msec()); > > - pinctrl_update(ovnsb_idl_loop.idl, > br_int->name); > > + pinctrl_update(ovnsb_idl_loop.idl); > > pinctrl_run(ovnsb_idl_txn, > > sbrec_datapath_binding_by_key, > > sbrec_port_binding_by_datapath, > > @@ -5909,7 +5953,6 @@ main(int argc, char *argv[]) > > } > > > > if (mac_cache_data) { > > - statctrl_update(br_int->name); > > statctrl_run(ovnsb_idl_txn, mac_cache_data); > > } > > > > @@ -6030,13 +6073,6 @@ main(int argc, char *argv[]) > > binding_wait(); > > } > > > > - if (!northd_version_match && br_int) { > > - /* Set the integration bridge name to pinctrl so that the > pinctrl > > - * thread can handle any packet-ins when we are not > processing > > - * any DB updates due to version mismatch. */ > > - pinctrl_set_br_int_name(br_int->name); > > - } > > - > > unixctl_server_run(unixctl); > > > > unixctl_server_wait(unixctl); > > @@ -6169,6 +6205,7 @@ loop_done: > > ovsdb_idl_loop_destroy(&ovnsb_idl_loop); > > > > ovs_feature_support_destroy(); > > + free(br_int_remote.target); > > free(ovs_remote); > > free(file_system_id); > > free(cli_system_id); > > diff --git a/controller/pinctrl.c b/controller/pinctrl.c > > index 9a4deb80a..f0a7dce7a 100644 > > --- a/controller/pinctrl.c > > +++ b/controller/pinctrl.c > > @@ -173,7 +173,8 @@ static bool garp_rarp_continuous; > > static void *pinctrl_handler(void *arg); > > > > struct pinctrl { > > - char *br_int_name; > > + /* OpenFlow connection to the switch. */ > > + struct rconn *swconn; > > pthread_t pinctrl_thread; > > /* Latch to destroy the 'pinctrl_thread' */ > > struct latch pinctrl_thread_exit; > > @@ -563,7 +564,7 @@ pinctrl_init(void) > > init_svc_monitors(); > > bfd_monitor_init(); > > init_fdb_entries(); > > - pinctrl.br_int_name = NULL; > > + pinctrl.swconn = rconn_create(0, 0, DSCP_DEFAULT, 1 << > OFP15_VERSION); > > pinctrl.mac_binding_can_timestamp = false; > > pinctrl_handler_seq = seq_create(); > > pinctrl_main_seq = seq_create(); > > @@ -3523,30 +3524,13 @@ notify_pinctrl_main(void) > > seq_change(pinctrl_main_seq); > > } > > > > -static void > > -pinctrl_rconn_setup(struct rconn *swconn, const char *br_int_name) > > - OVS_REQUIRES(pinctrl_mutex) > > -{ > > - if (br_int_name) { > > - char *target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), > br_int_name); > > - > > - if (strcmp(target, rconn_get_target(swconn))) { > > - VLOG_INFO("%s: connecting to switch", target); > > - rconn_connect(swconn, target, target); > > - } > > - free(target); > > - } else { > > - rconn_disconnect(swconn); > > - } > > -} > > - > > /* pinctrl_handler pthread function. */ > > static void * > > pinctrl_handler(void *arg_) > > { > > struct pinctrl *pctrl = arg_; > > /* OpenFlow connection to the switch. */ > > - struct rconn *swconn; > > + struct rconn *swconn = pctrl->swconn; > > /* Last seen sequence number for 'swconn'. When this differs from > > * rconn_get_connection_seqno(rconn), 'swconn' has reconnected. */ > > unsigned int conn_seq_no = 0; > > @@ -3562,13 +3546,10 @@ pinctrl_handler(void *arg_) > > static long long int svc_monitors_next_run_time = LLONG_MAX; > > static long long int send_prefixd_time = LLONG_MAX; > > > > - swconn = rconn_create(0, 0, DSCP_DEFAULT, 1 << OFP15_VERSION); > > - > > while (!latch_is_set(&pctrl->pinctrl_thread_exit)) { > > long long int bfd_time = LLONG_MAX; > > > > ovs_mutex_lock(&pinctrl_mutex); > > - pinctrl_rconn_setup(swconn, pctrl->br_int_name); > > ip_mcast_snoop_run(); > > ovs_mutex_unlock(&pinctrl_mutex); > > > > @@ -3626,37 +3607,24 @@ pinctrl_handler(void *arg_) > > poll_block(); > > } > > > > - rconn_destroy(swconn); > > return NULL; > > } > > > > -static void > > -pinctrl_set_br_int_name_(const char *br_int_name) > > - OVS_REQUIRES(pinctrl_mutex) > > +void > > +pinctrl_update_swconn(const char *target, int probe_interval) > > { > > - if (br_int_name && (!pinctrl.br_int_name || > strcmp(pinctrl.br_int_name, > > - br_int_name))) { > > - free(pinctrl.br_int_name); > > - pinctrl.br_int_name = xstrdup(br_int_name); > > - /* Notify pinctrl_handler that integration bridge is > > - * set/changed. */ > > + if (ovn_update_swconn_at(pinctrl.swconn, target, > > + probe_interval, "pinctrl")) { > > + /* Notify pinctrl_handler that integration bridge > > + * target is set/changed. */ > > notify_pinctrl_handler(); > > } > > } > > > > void > > -pinctrl_set_br_int_name(const char *br_int_name) > > -{ > > - ovs_mutex_lock(&pinctrl_mutex); > > - pinctrl_set_br_int_name_(br_int_name); > > - ovs_mutex_unlock(&pinctrl_mutex); > > -} > > - > > -void > > -pinctrl_update(const struct ovsdb_idl *idl, const char *br_int_name) > > +pinctrl_update(const struct ovsdb_idl *idl) > > { > > ovs_mutex_lock(&pinctrl_mutex); > > - pinctrl_set_br_int_name_(br_int_name); > > > > bool can_mb_timestamp = > > sbrec_server_has_mac_binding_table_col_timestamp(idl); > > @@ -4278,7 +4246,7 @@ pinctrl_destroy(void) > > latch_set(&pinctrl.pinctrl_thread_exit); > > pthread_join(pinctrl.pinctrl_thread, NULL); > > latch_destroy(&pinctrl.pinctrl_thread_exit); > > - free(pinctrl.br_int_name); > > + rconn_destroy(pinctrl.swconn); > > destroy_send_garps_rarps(); > > destroy_ipv6_ras(); > > destroy_ipv6_prefixd(); > > diff --git a/controller/pinctrl.h b/controller/pinctrl.h > > index 23343f097..3462b670c 100644 > > --- a/controller/pinctrl.h > > +++ b/controller/pinctrl.h > > @@ -62,8 +62,10 @@ void pinctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > > const struct ovsrec_open_vswitch_table *ovs_table); > > void pinctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn); > > void pinctrl_destroy(void); > > -void pinctrl_set_br_int_name(const char *br_int_name); > > -void pinctrl_update(const struct ovsdb_idl *idl, const char > *br_int_name); > > + > > +void pinctrl_update_swconn(const char *target, int probe_interval); > > + > > +void pinctrl_update(const struct ovsdb_idl *idl); > > > > struct activated_port { > > uint32_t dp_key; > > diff --git a/controller/statctrl.c b/controller/statctrl.c > > index 8cce97df8..6e6d7d799 100644 > > --- a/controller/statctrl.c > > +++ b/controller/statctrl.c > > @@ -80,7 +80,8 @@ struct stats_node { > > }; > > > > struct statctrl_ctx { > > - char *br_int; > > + /* OpenFlow connection to the switch. */ > > + struct rconn *swconn; > > > > pthread_t thread; > > struct latch exit_latch; > > @@ -95,8 +96,6 @@ static struct statctrl_ctx statctrl_ctx; > > static struct ovs_mutex mutex; > > > > static void *statctrl_thread_handler(void *arg); > > -static void statctrl_rconn_setup(struct rconn *swconn, char > *conn_target) > > - OVS_REQUIRES(mutex); > > static void statctrl_handle_rconn_msg(struct rconn *swconn, > > struct statctrl_ctx *ctx, > > struct ofpbuf *msg); > > @@ -109,8 +108,6 @@ static void statctrl_send_request(struct rconn > *swconn, > > struct statctrl_ctx *ctx) > > OVS_REQUIRES(mutex); > > static void statctrl_notify_main_thread(struct statctrl_ctx *ctx); > > -static void statctrl_set_conn_target(const char *br_int_name) > > - OVS_REQUIRES(mutex); > > static void statctrl_wait_next_request(struct statctrl_ctx *ctx) > > OVS_REQUIRES(mutex); > > static bool statctrl_update_next_request_timestamp(struct stats_node > *node, > > @@ -121,7 +118,7 @@ static bool > statctrl_update_next_request_timestamp(struct stats_node *node, > > void > > statctrl_init(void) > > { > > - statctrl_ctx.br_int = NULL; > > + statctrl_ctx.swconn = rconn_create(0, 0, DSCP_DEFAULT, 1 << > OFP15_VERSION); > > latch_init(&statctrl_ctx.exit_latch); > > ovs_mutex_init(&mutex); > > statctrl_ctx.thread_seq = seq_create(); > > @@ -185,11 +182,14 @@ statctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > > } > > > > void > > -statctrl_update(const char *br_int_name) > > +statctrl_update_swconn(const char *target, int probe_interval) > > { > > - ovs_mutex_lock(&mutex); > > - statctrl_set_conn_target(br_int_name); > > - ovs_mutex_unlock(&mutex); > > + if (ovn_update_swconn_at(statctrl_ctx.swconn, target, > > + probe_interval, "statctrl")) { > > + /* Notify statctrl thread that integration bridge > > + * target is set/changed. */ > > + seq_change(statctrl_ctx.thread_seq); > > + } > > } > > > > void > > @@ -217,7 +217,7 @@ statctrl_destroy(void) > > latch_set(&statctrl_ctx.exit_latch); > > pthread_join(statctrl_ctx.thread, NULL); > > latch_destroy(&statctrl_ctx.exit_latch); > > - free(statctrl_ctx.br_int); > > + rconn_destroy(statctrl_ctx.swconn); > > seq_destroy(statctrl_ctx.thread_seq); > > seq_destroy(statctrl_ctx.main_seq); > > > > @@ -233,14 +233,9 @@ statctrl_thread_handler(void *arg) > > struct statctrl_ctx *ctx = arg; > > > > /* OpenFlow connection to the switch. */ > > - struct rconn *swconn = rconn_create(0, 0, DSCP_DEFAULT, > > - 1 << OFP15_VERSION); > > + struct rconn *swconn = ctx->swconn; > > > > while (!latch_is_set(&ctx->exit_latch)) { > > - ovs_mutex_lock(&mutex); > > - statctrl_rconn_setup(swconn, ctx->br_int); > > - ovs_mutex_unlock(&mutex); > > - > > rconn_run(swconn); > > uint64_t new_seq = seq_read(ctx->thread_seq); > > > > @@ -273,29 +268,9 @@ statctrl_thread_handler(void *arg) > > poll_block(); > > } > > > > - rconn_destroy(swconn); > > return NULL; > > } > > > > -static void > > -statctrl_rconn_setup(struct rconn *swconn, char *br_int) > > - OVS_REQUIRES(mutex) > > -{ > > - if (!br_int) { > > - rconn_disconnect(swconn); > > - return; > > - } > > - > > - char *conn_target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), > br_int); > > - > > - if (strcmp(conn_target, rconn_get_target(swconn))) { > > - VLOG_INFO("%s: connecting to switch", conn_target); > > - rconn_connect(swconn, conn_target, conn_target); > > - } > > - > > - free(conn_target); > > -} > > - > > static void > > statctrl_handle_rconn_msg(struct rconn *swconn, struct statctrl_ctx > *ctx, > > struct ofpbuf *msg) > > @@ -400,23 +375,6 @@ statctrl_notify_main_thread(struct statctrl_ctx > *ctx) > > } > > } > > > > -static void > > -statctrl_set_conn_target(const char *br_int_name) > > - OVS_REQUIRES(mutex) > > -{ > > - if (!br_int_name) { > > - return; > > - } > > - > > - > > - if (!statctrl_ctx.br_int || strcmp(statctrl_ctx.br_int, > br_int_name)) { > > - free(statctrl_ctx.br_int); > > - statctrl_ctx.br_int = xstrdup(br_int_name); > > - /* Notify statctrl thread that integration bridge is > set/changed. */ > > - seq_change(statctrl_ctx.thread_seq); > > - } > > -} > > - > > static void > > statctrl_wait_next_request(struct statctrl_ctx *ctx) > > OVS_REQUIRES(mutex) > > diff --git a/controller/statctrl.h b/controller/statctrl.h > > index c5cede353..026ff387f 100644 > > --- a/controller/statctrl.h > > +++ b/controller/statctrl.h > > @@ -21,7 +21,8 @@ > > void statctrl_init(void); > > void statctrl_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > > struct mac_cache_data *mac_cache_data); > > -void statctrl_update(const char *br_int_name); > > + > > +void statctrl_update_swconn(const char *target, int probe_interval); > > void statctrl_wait(struct ovsdb_idl_txn *ovnsb_idl_txn); > > void statctrl_destroy(void); > > > > diff --git a/include/ovn/features.h b/include/ovn/features.h > > index 08f1d8288..9e7c88b2a 100644 > > --- a/include/ovn/features.h > > +++ b/include/ovn/features.h > > @@ -49,7 +49,7 @@ enum ovs_feature_value { > > void ovs_feature_support_destroy(void); > > bool ovs_feature_is_supported(enum ovs_feature_value feature); > > bool ovs_feature_support_run(const struct smap *ovs_capabilities, > > - const char *br_name); > > + const char *conn_target, int > probe_interval); > > bool ovs_feature_set_discovered(void); > > uint32_t ovs_feature_max_meters_get(void); > > uint32_t ovs_feature_max_select_groups_get(void); > > diff --git a/lib/features.c b/lib/features.c > > index b04437235..607e4bd31 100644 > > --- a/lib/features.c > > +++ b/lib/features.c > > @@ -29,8 +29,10 @@ > > #include "openvswitch/ofp-meter.h" > > #include "openvswitch/ofp-group.h" > > #include "openvswitch/ofp-util.h" > > +#include "openvswitch/rconn.h" > > #include "ovn/features.h" > > #include "controller/ofctrl.h" > > +#include "ovn-util.h" > > > > VLOG_DEFINE_THIS_MODULE(features); > > > > @@ -120,34 +122,12 @@ 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(0, 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) > > +ovs_feature_get_openflow_cap(void) > > { > > struct ofpbuf *msg; > > > > - if (!br_name) { > > - return false; > > - } > > - > > - ovs_feature_rconn_setup(br_name); > > - > > rconn_run(swconn); > > if (!rconn_is_connected(swconn)) { > > rconn_run_wait(swconn); > > @@ -231,7 +211,7 @@ ovs_feature_support_destroy(void) > > /* Returns 'true' if the set of tracked OVS features has been updated. > */ > > bool > > ovs_feature_support_run(const struct smap *ovs_capabilities, > > - const char *br_name) > > + const char *conn_target, int probe_interval) > > { > > static struct smap empty_caps = SMAP_INITIALIZER(&empty_caps); > > bool updated = false; > > @@ -240,7 +220,12 @@ ovs_feature_support_run(const struct smap > *ovs_capabilities, > > ovs_capabilities = &empty_caps; > > } > > > > - if (ovs_feature_get_openflow_cap(br_name)) { > > + if (!swconn) { > > + swconn = rconn_create(0, 0, DSCP_DEFAULT, 1 << OFP15_VERSION); > > + } > > + ovn_update_swconn_at(swconn, conn_target, probe_interval, > "features"); > > + > > + if (ovs_feature_get_openflow_cap()) { > > updated = true; > > } > > > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > > index ee5cbcdc3..4f50606c0 100644 > > --- a/lib/ovn-util.c > > +++ b/lib/ovn-util.c > > @@ -22,6 +22,7 @@ > > #include "daemon.h" > > #include "include/ovn/actions.h" > > #include "openvswitch/ofp-parse.h" > > +#include "openvswitch/rconn.h" > > #include "openvswitch/vlog.h" > > #include "lib/vswitch-idl.h" > > #include "ovn-dirs.h" > > @@ -1284,3 +1285,28 @@ ovn_exit_args_finish(struct ovn_exit_args > *exit_args) > > } > > free(exit_args->conns); > > } > > + > > +bool > > +ovn_update_swconn_at(struct rconn *swconn, const char *target, > > + int probe_interval, const char *where) > > +{ > > + if (!target) { > > + rconn_disconnect(swconn); > > + return true; > > + } > > + > > + bool notify = false; > > + > > + if (strcmp(target, rconn_get_target(swconn))) { > > + VLOG_INFO("%s: connecting to switch: \"%s\"", where, target); > > + rconn_connect(swconn, target, target); > > + notify = true; > > + } > > + > > + if (probe_interval != rconn_get_probe_interval(swconn)) { > > + rconn_set_probe_interval(swconn, probe_interval); > > + notify = true; > > + } > > + > > + return notify; > > +} > > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > > index 042e6bf82..f75b821b6 100644 > > --- a/lib/ovn-util.h > > +++ b/lib/ovn-util.h > > @@ -48,6 +48,7 @@ struct smap; > > struct svec; > > struct uuid; > > struct unixctl_conn; > > +struct rconn; > > > > struct ipv4_netaddr { > > ovs_be32 addr; /* 192.168.10.123 */ > > @@ -477,4 +478,7 @@ void ovn_exit_command_callback(struct unixctl_conn > *conn, int argc, > > const char *argv[], void *exit_args_); > > void ovn_exit_args_finish(struct ovn_exit_args *exit_args); > > > > +bool ovn_update_swconn_at(struct rconn *swconn, const char *target, > > + int probe_interval, const char *where); > > + > > #endif /* OVN_UTIL_H */ > > diff --git a/lib/test-ovn-features.c b/lib/test-ovn-features.c > > index aabc547e6..cddeae779 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_run(&features, NULL)); > > + ovs_assert(!ovs_feature_support_run(&features, NULL, 0)); > > ovs_assert(!ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT)); > > > > smap_replace(&features, "ct_zero_snat", "true"); > > - ovs_assert(ovs_feature_support_run(&features, NULL)); > > + ovs_assert(ovs_feature_support_run(&features, NULL, 0)); > > ovs_assert(ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT)); > > > > smap_add(&features, "unknown_feature", "true"); > > - ovs_assert(!ovs_feature_support_run(&features, NULL)); > > + ovs_assert(!ovs_feature_support_run(&features, NULL, 0)); > > > > smap_destroy(&features); > > } > > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > > index 3202f0bef..df84f5bdf 100644 > > --- a/tests/ovn-controller.at > > +++ b/tests/ovn-controller.at > > @@ -2879,3 +2879,48 @@ AT_CHECK([test x"$port_uuid"=$(ovs-vsctl get port > $fakech_tunnel _uuid)]) > > OVN_CLEANUP([hv1]) > > AT_CLEANUP > > ]) > > + > > +AT_SETUP([ovn-controller - br-int remote]) > > +AT_KEYWORDS([ovn]) > > +ovn_start > > + > > +net_add n1 > > +sim_add hv1 > > +ovs-vsctl add-br br-phys > > +ovn_attach n1 br-phys 192.168.0.20 > > +ovs-vsctl -- add-port br-int vif1 -- \ > > + set interface vif1 external-ids:iface-id=vif1 > > + > > +# Set the br-int remote and wait for the connection > > +check ovs-vsctl set-controller br-int ptcp:1234 > > +check ovs-vsctl -- \ > > + set open . external-ids:ovn-bridge-remote="tcp:127.0.0.1:1234" -- \ > > + set open . external-ids:ovn-bridge-remote-probe-interval=5000 > > + > > +OVS_WAIT_UNTIL([test $(grep -c 'connecting to switch: "tcp: > 127.0.0.1:1234"' hv1/ovn-controller.log) = 4]) > > +OVS_WAIT_UNTIL([test $(grep -c 'tcp:127.0.0.1:1234: connected' > hv1/ovn-controller.log) = 4]) > > + > > +check ovn-nbctl ls-add sw0 > > + > > +check ovn-nbctl lsp-add sw0 vif1 > > +check ovn-nbctl lsp-set-addresses vif1 "00:00:00:00:00:01 192.168.0.10 > 1000::10" > > + > > +wait_for_ports_up > > +check ovn-nbctl --wait=hv sync > > + > > +check ovs-vsctl -- \ > > + remove open . external-ids ovn-bridge-remote -- \ > > + remove open . external-ids ovn-bridge-remote-probe-interval > > + > > +check ovs-vsctl del-controller br-int > > + > > +# Set different br-int remote and wait for the connection > > +check ovs-vsctl set-controller br-int ptcp:1235 > > +check ovs-vsctl -- \ > > + set open . external-ids:ovn-bridge-remote="tcp:127.0.0.1:1235" > > + > > +OVS_WAIT_UNTIL([test $(grep -c 'connecting to switch: "tcp: > 127.0.0.1:1235"' hv1/ovn-controller.log) = 4]) > > +OVS_WAIT_UNTIL([test $(grep -c 'tcp:127.0.0.1:1235: connected' > hv1/ovn-controller.log) = 4]) > > + > > +OVN_CLEANUP([hv1]) > > +AT_CLEANUP > > Thanks, Ales -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> amu...@redhat.com <https://red.ht/sig> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev