On 6/3/24 22:09, Mark Michelson wrote: > Thanks for keeping this fresh, Ales. > > Acked-by: Mark Michelson <[email protected]> >
Thanks, Ales and Mark! I applied this to main with some small changes, mentioned below. > On 5/2/24 12:44, 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 <[email protected]> >> --- >> v4: Rebase on top of current main. >> v3: Rebase on top of current main. >> Fix the copy-paste error in ovn-controller documentation. >> 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 3b5e93dc9..4e15f31c8 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -17,6 +17,12 @@ Post v24.03.0 >> external-ids, the option is no longer needed as it became >> effectively >> "true" for all scenarios. >> - Added DHCPv4 relay support. >> + - 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 *, >> + I removed this unrelated newline. >> +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 85e7966d7..b6404a19d 100644 >> --- a/controller/ovn-controller.8.xml >> +++ b/controller/ovn-controller.8.xml >> @@ -378,6 +378,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-probe-interval</code></dt> >> + <dd> >> + <p> >> + The inactivity probe interval of the connection to the OVN >> management >> + bridge, in milliseconds. I added "It defaults to zero." here. >> + 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 23269af83..0bc55c9a4 100644 >> --- a/controller/ovn-controller.c >> +++ b/controller/ovn-controller.c >> @@ -123,6 +123,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); >> @@ -5074,11 +5079,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. */ >> @@ -5689,6 +5726,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) { >> @@ -5743,7 +5785,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()) { >> @@ -5761,7 +5804,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 >> @@ -5878,7 +5922,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, >> @@ -5932,7 +5976,6 @@ main(int argc, char *argv[]) >> } >> if (mac_cache_data) { >> - statctrl_update(br_int->name); >> statctrl_run(ovnsb_idl_txn, mac_cache_data); >> } >> @@ -6053,13 +6096,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); >> @@ -6192,6 +6228,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 0ee6d8fa8..79efdae0e 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(); >> @@ -3981,30 +3982,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; >> @@ -4020,13 +4004,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); >> @@ -4085,37 +4066,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); >> @@ -4737,7 +4705,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 35a5d8ba0..d7bceb62c 100644 >> --- a/include/ovn/features.h >> +++ b/include/ovn/features.h >> @@ -50,7 +50,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 9f97ae2ca..58e941193 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" >> @@ -1288,3 +1289,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 27cec2aec..e65f5b941 100644 >> --- a/tests/ovn-controller.at >> +++ b/tests/ovn-controller.at >> @@ -2973,3 +2973,48 @@ >> priority=1100,ip,reg15=0x1,metadata=0x1,nw_src=10.0.0.4 >> actions=load:0x1->OXM_OF >> 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]) There's a tiny race condition here as a reconnect can happen during the test. I changed this to: OVS_WAIT_UNTIL([grep -q 'tcp:127.0.0.1:1234: connected' hv1/ovn-controller.log]) >> + >> +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]) And this one to: OVS_WAIT_UNTIL([grep -q 'tcp:127.0.0.1:1235: connected' hv1/ovn-controller.log]) >> + >> +OVN_CLEANUP([hv1]) >> +AT_CLEANUP > Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
