[ovs-dev] [PATCH v4 3/3] ovn-controller: Use separate thread for packet-in processing.
Guoshuai Li
ligs at dtdream.com
Wed Sep 27 00:08:37 UTC 2017
This is very useful to me.
I found a problem in my use:
In the ovn-controller and the south of the database connection,when the
ovn-controller as a passive service, the SB as a client, such as
configured to
ovs-vsctl set Open_vSwitch . external-ids:ovn-remote=ptcp:6644:0.0.0.0
ovn-sbctl set-connection tcp:10.157.145.211:6644
tcp:10.157.145.212:6644 tcp:10.157.145.213:6644 tcp:10.157.145.214:6644
This configuration is to determine the status of the chassis through the
state of the connection in the OVNSB。
But here the two threads listening conflict:
2017-09-27T00:00:59.387Z|26144|socket_util|ERR|6644:0.0.0.0: bind:
Address already in use
2017-09-27T00:00:59.387Z|26145|reconnect|INFO|ptcp:6644:0.0.0.0:
listening...
2017-09-27T00:00:59.387Z|26146|reconnect|INFO|ptcp:6644:0.0.0.0: listen
attempt failed (Address already in use)
Do you have a good way to fix it?
And this patch also conflict with master...
2017/8/28 12:14, Han Zhou :
> This patch introduces multi-threading for ovn-controller and use
> dedicated thread for packet-in processing as a start. It decouples
> packet-in processing and ovs flow computing, so that packet-in inputs
> won't trigger flow recomputing, and flow computing won't block
> packet-in processing. In large scale environment this largely reduces
> CPU cost and improves performance.
>
> Related effort and discussion:
> https://mail.openvswitch.org/pipermail/ovs-dev/2017-May/331813.html
>
> Signed-off-by: Han Zhou <zhouhan at gmail.com>
> ---
> v3->v4: rebased on master.
>
> ovn/controller/ovn-controller.c | 71 ++++++++++++++++++---------
> ovn/controller/ovn-controller.h | 38 +++++++++++++++
> ovn/controller/pinctrl.c | 105 ++++++++++++++++++++++++++++++++++++++++
> ovn/controller/pinctrl.h | 1 +
> 4 files changed, 193 insertions(+), 22 deletions(-)
>
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
> index 414443f..cb04244 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -56,6 +56,8 @@
> #include "stream.h"
> #include "unixctl.h"
> #include "util.h"
> +#include "latch.h"
> +#include "ovs-thread.h"
>
> VLOG_DEFINE_THIS_MODULE(main);
>
> @@ -66,8 +68,6 @@ static unixctl_cb_func inject_pkt;
> #define DEFAULT_BRIDGE_NAME "br-int"
> #define DEFAULT_PROBE_INTERVAL_MSEC 5000
>
> -static void update_probe_interval(struct controller_ctx *,
> - const char *ovnsb_remote);
> static void parse_options(int argc, char *argv[]);
> OVS_NO_RETURN static void usage(void);
>
> @@ -78,7 +78,7 @@ struct pending_pkt {
> char *flow_s;
> };
>
> -static char *ovs_remote;
> +char *ovs_remote;
>
> struct local_datapath *
> get_local_datapath(const struct hmap *local_datapaths, uint32_t tunnel_key)
> @@ -129,7 +129,7 @@ get_bridge(struct ovsdb_idl *ovs_idl, const char *br_name)
> return NULL;
> }
>
> -static void
> +void
> update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
> const struct sbrec_chassis *chassis,
> const struct sset *local_ifaces,
> @@ -257,7 +257,7 @@ create_br_int(struct controller_ctx *ctx)
> return bridge;
> }
>
> -static const struct ovsrec_bridge *
> +const struct ovsrec_bridge *
> get_br_int(struct controller_ctx *ctx)
> {
> const struct ovsrec_open_vswitch *cfg;
> @@ -269,7 +269,7 @@ get_br_int(struct controller_ctx *ctx)
> return get_bridge(ctx->ovs_idl, br_int_name(cfg));
> }
>
> -static const char *
> +const char *
> get_chassis_id(const struct ovsdb_idl *ovs_idl)
> {
> const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_first(ovs_idl);
> @@ -309,7 +309,7 @@ update_ssl_config(const struct ovsdb_idl *ovs_idl)
>
> /* Retrieves the OVN Southbound remote location from the
> * "external-ids:ovn-remote" key in 'ovs_idl' and returns a copy of it. */
> -static char *
> +char *
> get_ovnsb_remote(struct ovsdb_idl *ovs_idl)
> {
> while (1) {
> @@ -498,6 +498,22 @@ get_nb_cfg(struct ovsdb_idl *idl)
> }
>
> static void
> +ctrl_thread_create(struct ctrl_thread *thread, const char *name,
> + void *(*start)(void *))
> +{
> + latch_init(&thread->exit_latch);
> + thread->thread = ovs_thread_create(name, start, thread);
> +}
> +
> +static void
> +ctrl_thread_exit(struct ctrl_thread *thread)
> +{
> + latch_set(&thread->exit_latch);
> + xpthread_join(thread->thread, NULL);
> + latch_destroy(&thread->exit_latch);
> +}
> +
> +void
> ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl)
> {
> /* We do not monitor all tables by default, so modules must register
> @@ -574,6 +590,22 @@ create_ovnsb_indexes(struct ovsdb_idl *ovnsb_idl)
> OVSDB_INDEX_ASC, NULL);
> }
>
> +void
> +connect_ovnsb(struct ovsdb_idl_loop *ovnsb_idl_loop,
> + struct ovnsb_cursors *cursors,
> + const char *ovnsb_remote)
> +{
> + ovnsb_idl_loop->idl = ovsdb_idl_create(ovnsb_remote,
> + &sbrec_idl_class, true, true);
> +
> + create_ovnsb_indexes(ovnsb_idl_loop->idl);
> + lport_init(cursors, ovnsb_idl_loop->idl);
> +
> + ovsdb_idl_omit_alert(ovnsb_idl_loop->idl, &sbrec_chassis_col_nb_cfg);
> + update_sb_monitors(ovnsb_idl_loop->idl, NULL, NULL, NULL);
> + ovsdb_idl_get_initial_snapshot(ovnsb_idl_loop->idl);
> +}
> +
> int
> main(int argc, char *argv[])
> {
> @@ -605,7 +637,6 @@ main(int argc, char *argv[])
> daemonize_complete();
>
> ofctrl_init(&group_table);
> - pinctrl_init();
> lflow_init();
>
> /* Connect to OVS OVSDB instance. */
> @@ -616,16 +647,9 @@ main(int argc, char *argv[])
>
> /* Connect to OVN SB database and get a snapshot. */
> char *ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl);
> - struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
> - ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class, true, true));
> -
> - create_ovnsb_indexes(ovnsb_idl_loop.idl);
> + struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(NULL);
> struct ovnsb_cursors ovnsb_cursors;
> - lport_init(ovnsb_cursors, ovnsb_idl_loop.idl);
> -
> - ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, &sbrec_chassis_col_nb_cfg);
> - update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL);
> - ovsdb_idl_get_initial_snapshot(ovnsb_idl_loop.idl);
> + connect_ovnsb(&ovnsb_idl_loop, &ovnsb_cursors, ovnsb_remote);
>
> /* Initialize connection tracking zones. */
> struct simap ct_zones = SIMAP_INITIALIZER(&ct_zones);
> @@ -641,6 +665,10 @@ main(int argc, char *argv[])
> unixctl_command_register("inject-pkt", "MICROFLOW", 1, 1, inject_pkt,
> &pending_pkt);
>
> +
> + struct ctrl_thread pinctrl_thread;
> + ctrl_thread_create(&pinctrl_thread, "pinctrl", pinctrl_thread_main);
> +
> /* Main loop. */
> exiting = false;
> while (!exiting) {
> @@ -705,8 +733,6 @@ main(int argc, char *argv[])
> enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int,
> &pending_ct_zones);
>
> - pinctrl_run(&ctx, br_int, chassis, &chassis_index,
> - &local_datapaths, &active_tunnels);
> update_ct_zones(&local_lports, &local_datapaths, &ct_zones,
> ct_zone_bitmap, &pending_ct_zones);
> if (ctx.ovs_idl_txn) {
> @@ -791,7 +817,6 @@ main(int argc, char *argv[])
>
> if (br_int) {
> ofctrl_wait();
> - pinctrl_wait(&ctx);
> }
> ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop);
>
> @@ -840,10 +865,12 @@ main(int argc, char *argv[])
> poll_block();
> }
>
> + /* stop child controller threads */
> + ctrl_thread_exit(&pinctrl_thread);
> +
> unixctl_server_destroy(unixctl);
> lflow_destroy();
> ofctrl_destroy();
> - pinctrl_destroy();
>
> simap_destroy(&ct_zones);
>
> @@ -1000,7 +1027,7 @@ inject_pkt(struct unixctl_conn *conn, int argc OVS_UNUSED,
>
> /* Get the desired SB probe timer from the OVS database and configure it into
> * the SB database. */
> -static void
> +void
> update_probe_interval(struct controller_ctx *ctx, const char *ovnsb_remote)
> {
> const struct ovsrec_open_vswitch *cfg
> diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h
> index f57c557..1234c2b 100644
> --- a/ovn/controller/ovn-controller.h
> +++ b/ovn/controller/ovn-controller.h
> @@ -18,7 +18,9 @@
> #define OVN_CONTROLLER_H 1
>
> #include "simap.h"
> +#include "sset.h"
> #include "ovn/lib/ovn-sb-idl.h"
> +#include "latch.h"
>
> /* Linux supports a maximum of 64K zones, which seems like a fine default. */
> #define MAX_CT_ZONES 65535
> @@ -71,6 +73,13 @@ struct local_datapath {
> size_t n_peer_dps;
> };
>
> +struct ctrl_thread {
> + pthread_t thread;
> +
> + /* Controls thread exit. */
> + struct latch exit_latch;
> +};
> +
> struct local_datapath *get_local_datapath(const struct hmap *,
> uint32_t tunnel_key);
>
> @@ -90,5 +99,34 @@ enum chassis_tunnel_type {
>
> uint32_t get_tunnel_type(const char *name);
>
> +/* Retrieves the OVN Southbound remote location from the
> + * "external-ids:ovn-remote" key in 'ovs_idl' and returns a copy of it. */
> +char *get_ovnsb_remote(struct ovsdb_idl *ovs_idl);
> +
> +void
> +update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
> + const struct sbrec_chassis *chassis,
> + const struct sset *local_ifaces,
> + struct hmap *local_datapaths);
> +
> +/* Get the desired SB probe timer from the OVS database and configure it into
> + * the SB database. */
> +void
> +update_probe_interval(struct controller_ctx *ctx, const char *ovnsb_remote);
> +
> +const struct ovsrec_bridge *
> +get_br_int(struct controller_ctx *ctx);
> +
> +const char *
> +get_chassis_id(const struct ovsdb_idl *ovs_idl);
> +
> +void
> +ctrl_register_ovs_idl(struct ovsdb_idl *ovs_idl);
> +
> +void
> +connect_ovnsb(struct ovsdb_idl_loop *ovnsb_idl_loop,
> + struct ovnsb_cursors *cursors,
> + const char *ovnsb_remote);
>
> +extern char *ovs_remote;
> #endif /* ovn/ovn-controller.h */
> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> index 9412b48..8a30be2 100644
> --- a/ovn/controller/pinctrl.c
> +++ b/ovn/controller/pinctrl.c
> @@ -18,6 +18,7 @@
>
> #include "pinctrl.h"
>
> +#include "bfd.h"
> #include "coverage.h"
> #include "csum.h"
> #include "dirs.h"
> @@ -41,6 +42,7 @@
> #include "ovn/lex.h"
> #include "ovn/lib/acl-log.h"
> #include "ovn/lib/logical-fields.h"
> +#include "ovn/lib/chassis-index.h"
> #include "ovn/lib/ovn-dhcp.h"
> #include "ovn/lib/ovn-util.h"
> #include "poll-loop.h"
> @@ -48,6 +50,8 @@
> #include "socket-util.h"
> #include "timeval.h"
> #include "vswitch-idl.h"
> +#include "latch.h"
> +#include "binding.h"
>
> VLOG_DEFINE_THIS_MODULE(pinctrl);
>
> @@ -84,6 +88,107 @@ static void reload_metadata(struct ofpbuf *ofpacts,
>
> COVERAGE_DEFINE(pinctrl_drop_put_mac_binding);
>
> +void *
> +pinctrl_thread_main(void *arg)
> +{
> + struct ctrl_thread *thread = arg;
> + pinctrl_init();
> +
> + /* Connect to OVS OVSDB instance. */
> + struct ovsdb_idl_loop ovs_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
> + ovsdb_idl_create(ovs_remote, &ovsrec_idl_class, false, true));
> + ctrl_register_ovs_idl(ovs_idl_loop.idl);
> + ovsdb_idl_get_initial_snapshot(ovs_idl_loop.idl);
> +
> + /* Connect to OVN SB database and get a snapshot. */
> + char *ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl);
> + struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(NULL);
> + struct ovnsb_cursors ovnsb_cursors;
> + connect_ovnsb(&ovnsb_idl_loop, &ovnsb_cursors, ovnsb_remote);
> +
> + while (!latch_is_set(&thread->exit_latch)) {
> + /* Below logic is similar as in main loop in ovn-controller.c,
> + * while the purpose here is packet-in processing only */
> + char *new_ovnsb_remote = get_ovnsb_remote(ovs_idl_loop.idl);
> + if (strcmp(ovnsb_remote, new_ovnsb_remote)) {
> + free(ovnsb_remote);
> + ovnsb_remote = new_ovnsb_remote;
> + ovsdb_idl_set_remote(ovnsb_idl_loop.idl, ovnsb_remote, true);
> + } else {
> + free(new_ovnsb_remote);
> + }
> +
> + struct controller_ctx ctx = {
> + .ovs_idl = ovs_idl_loop.idl,
> + .ovs_idl_txn = ovsdb_idl_loop_run(&ovs_idl_loop),
> + .ovnsb_idl = ovnsb_idl_loop.idl,
> + .ovnsb_idl_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop),
> + .ovnsb_cursors = &ovnsb_cursors,
> + };
> +
> + update_probe_interval(&ctx, ovnsb_remote);
> +
> + struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths);
> + struct sset local_lports = SSET_INITIALIZER(&local_lports);
> + struct sset active_tunnels = SSET_INITIALIZER(&active_tunnels);
> + const struct ovsrec_bridge *br_int = get_br_int(&ctx);
> + const char *chassis_id = get_chassis_id(ctx.ovs_idl);
> +
> + struct chassis_index chassis_index;
> +
> + chassis_index_init(&chassis_index, ctx.ovnsb_idl);
> +
> + if (ctx.ovnsb_idl_txn) {
> + const struct sbrec_chassis *chassis = NULL;
> + if (chassis_id) {
> + chassis = get_chassis(ctx.ovnsb_idl, chassis_id);
> + bfd_calculate_active_tunnels(br_int, &active_tunnels);
> + binding_get(&ctx, br_int, chassis,
> + &chassis_index, &active_tunnels, &local_datapaths,
> + &local_lports);
> + }
> +
> + if (br_int && chassis) {
> + pinctrl_run(&ctx, br_int, chassis, &chassis_index,
> + &local_datapaths, &active_tunnels);
> + update_sb_monitors(ctx.ovnsb_idl, chassis,
> + &local_lports, &local_datapaths);
> + }
> + }
> +
> + chassis_index_destroy(&chassis_index);
> + sset_destroy(&local_lports);
> + sset_destroy(&active_tunnels);
> +
> + struct local_datapath *cur_node, *next_node;
> + HMAP_FOR_EACH_SAFE (cur_node, next_node, hmap_node, &local_datapaths) {
> + free(cur_node->peer_dps);
> + hmap_remove(&local_datapaths, &cur_node->hmap_node);
> + free(cur_node);
> + }
> + hmap_destroy(&local_datapaths);
> +
> + if (br_int) {
> + pinctrl_wait(&ctx);
> + }
> + ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop);
> + ovsdb_idl_loop_commit_and_wait(&ovs_idl_loop);
> +
> + latch_wait(&thread->exit_latch);
> + poll_block();
> + }
> +
> + pinctrl_destroy();
> +
> + ovsdb_idl_loop_destroy(&ovs_idl_loop);
> + ovsdb_idl_loop_destroy(&ovnsb_idl_loop);
> +
> + free(ovnsb_remote);
> +
> + VLOG_INFO("pinctrl thread done");
> + return NULL;
> +}
> +
> void
> pinctrl_init(void)
> {
> diff --git a/ovn/controller/pinctrl.h b/ovn/controller/pinctrl.h
> index fc9cca8..15b12a0 100644
> --- a/ovn/controller/pinctrl.h
> +++ b/ovn/controller/pinctrl.h
> @@ -34,6 +34,7 @@ void pinctrl_run(struct controller_ctx *,
> const struct ovsrec_bridge *, const struct sbrec_chassis *,
> const struct chassis_index *, struct hmap *local_datapaths,
> struct sset *active_tunnels);
> +void *pinctrl_thread_main(void *arg);
> void pinctrl_wait(struct controller_ctx *);
> void pinctrl_destroy(void);
>
More information about the dev
mailing list