From patchwork Thu Jul 13 22:54:01 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 788013 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3x7rjY6Csnz9s7m for ; Fri, 14 Jul 2017 08:54:25 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="WtDe2AVP"; dkim-atps=neutral Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 3CC10CB5; Thu, 13 Jul 2017 22:54:22 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id BDFBCAF4 for ; Thu, 13 Jul 2017 22:54:20 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pg0-f67.google.com (mail-pg0-f67.google.com [74.125.83.67]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id DA23B142 for ; Thu, 13 Jul 2017 22:54:19 +0000 (UTC) Received: by mail-pg0-f67.google.com with SMTP id j186so8399915pge.1 for ; Thu, 13 Jul 2017 15:54:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=nwoW5EP1OVPlnAIQDVrT/YnGxlVIijrUYQDOIilo6fY=; b=WtDe2AVPAKdNf3u1AWqf9LZBgGU9gvmzpzWSoj9QzQh11/WZJAMuhczlEw5Mbu4fCu 7uaD2GNvfogOyMlBxaNyqPZ8+DILFBz0J0UPrI0eTR8buqsEy6bq+txRUlXK50jbmNCc 8m2027ZNDwZOQlfoc9m46MXc267AIFdi6KV4XXALou9tflr9k63zVSRojDyCUxPb8KNN 8pSKeNOFb2lvObu3T4n7HiwabP6IXPRijIkUnhwlWheBtb+Ocvmrf3R9H8L+lnG3HKIv SUwK0onNy2RcEMIb6Hd1K++fOFCCL02g2Hb3PB5hwnbXiHRrLuI+LtoSaAqE8c66ueJO T++Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=nwoW5EP1OVPlnAIQDVrT/YnGxlVIijrUYQDOIilo6fY=; b=lKqSsJC7yRpexAXK+BUU/z4uWB+ZZ8K/ZENoYJjTYJK0a6MV/cBQJaByoTIphYctSW F04yCmKIOtj0iDzl22SYCuO7ywVDFy5D7P7nqS/kbEY2rRWsQnywePIw6SPM/V1CNOwm RsRlwRzILIPgsspT0Yaj/VAMo+0GlhhdVM5uuEUGBDPCn2iwkG5nCm5FoMRvzuD0uBEb x8vhlkWoLdBKGzkgq2OxNV+ovvpikypdvLkVRbagEasMf4d79zVU63ZopoIJxZZBxum4 xa4Rgqi42zCcGt6GaYNg2/AbKOMMVhOKFEJno3xWyIaHpedLifXGfOEwyDqm+vNptkos kH7A== X-Gm-Message-State: AIVw111j4VAbGoqj0gY9qpRr4gDpZWJSB/DthfJWD8uZ7ak4Vge/XOBG muswZsu5jYpoV6cr X-Received: by 10.99.149.83 with SMTP id t19mr11545543pgn.247.1499986458867; Thu, 13 Jul 2017 15:54:18 -0700 (PDT) Received: from localhost.localdomain.localdomain (c-73-162-150-77.hsd1.ca.comcast.net. [73.162.150.77]) by smtp.gmail.com with ESMTPSA id p15sm15320968pfi.99.2017.07.13.15.54.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 13 Jul 2017 15:54:18 -0700 (PDT) From: Han Zhou To: dev@openvswitch.org Date: Thu, 13 Jul 2017 15:54:01 -0700 Message-Id: <1499986442-360-1-git-send-email-zhouhan@gmail.com> X-Mailer: git-send-email 2.1.0 X-Spam-Status: No, score=-1.5 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH v3 1/2] ovn-controller: readonly mode binding_run and get_br_int X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org This change is to prepare for the future change for multi-threading. Both binding_run() and get_br_int() are needed by pinctrl thread, but we don't want to update SB DB or create bridges in that scenario, so need "readonly" mode for these functions. Signed-off-by: Han Zhou Co-authored-by: Ben Pfaff Signed-off-by: Ben Pfaff --- v2->v3: separate code that just reads data from code that might write to it. ovn/controller/binding.c | 247 +++++++++++++++++++++++----------------- ovn/controller/binding.h | 4 + ovn/controller/ovn-controller.c | 30 +++-- 3 files changed, 167 insertions(+), 114 deletions(-) diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c index 11145dd..59d06d3 100644 --- a/ovn/controller/binding.c +++ b/ovn/controller/binding.c @@ -67,36 +67,36 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl) static void get_local_iface_ids(const struct ovsrec_bridge *br_int, struct shash *lport_to_iface, - struct sset *local_lports, - struct sset *egress_ifaces) + struct sset *local_lports) { - int i; - - for (i = 0; i < br_int->n_ports; i++) { - const struct ovsrec_port *port_rec = br_int->ports[i]; - const char *iface_id; - int j; - - if (!strcmp(port_rec->name, br_int->name)) { - continue; - } - - for (j = 0; j < port_rec->n_interfaces; j++) { - const struct ovsrec_interface *iface_rec; - - iface_rec = port_rec->interfaces[j]; - iface_id = smap_get(&iface_rec->external_ids, "iface-id"); - int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0; + for (int i = 0; i < br_int->n_ports; i++) { + const struct ovsrec_port *port = br_int->ports[i]; + for (int j = 0; j < port->n_interfaces; j++) { + const struct ovsrec_interface *iface + = port->interfaces[j]; + const char *iface_id = smap_get(&iface->external_ids, "iface-id"); + int64_t ofport = iface->n_ofport ? *iface->ofport : 0; if (iface_id && ofport > 0) { - shash_add(lport_to_iface, iface_id, iface_rec); + shash_add(lport_to_iface, iface_id, iface); sset_add(local_lports, iface_id); } + } + } +} + +static void +get_egress_ifaces(const struct ovsrec_bridge *br_int, + struct sset *egress_ifaces) +{ + for (int i = 0; i < br_int->n_ports; i++) { + const struct ovsrec_port *port = br_int->ports[i]; + for (int j = 0; j < port->n_interfaces; j++) { + const struct ovsrec_interface *iface = port->interfaces[j]; - /* Check if this is a tunnel interface. */ - if (smap_get(&iface_rec->options, "remote_ip")) { + if (smap_get(&iface->options, "remote_ip")) { const char *tunnel_iface - = smap_get(&iface_rec->status, "tunnel_egress_iface"); + = smap_get(&iface->status, "tunnel_egress_iface"); if (tunnel_iface) { sset_add(egress_ifaces, tunnel_iface); } @@ -353,7 +353,19 @@ setup_qos(const char *egress_iface, struct hmap *queue_map) netdev_close(netdev_phy); } -static void +static bool +is_specially_bound(const struct sbrec_port_binding *binding_rec, + const struct sbrec_chassis *chassis_rec, + const char *type, const char *option) +{ + return (!strcmp(binding_rec->type, type) + && !strcmp(smap_get_def(&binding_rec->options, option, ""), + chassis_rec->name)); +} + +/* Returns true if this chassis owns 'binding_rec', that is, it should set + * 'binding_rec->chassis' to point to 'chassis_rec'. */ +static bool consider_local_datapath(struct controller_ctx *ctx, const struct ldatapath_index *ldatapaths, const struct lport_index *lports, @@ -367,7 +379,6 @@ consider_local_datapath(struct controller_ctx *ctx, const struct ovsrec_interface *iface_rec = shash_find_data(lport_to_iface, binding_rec->logical_port); - bool our_chassis = false; if (iface_rec || (binding_rec->parent_port && binding_rec->parent_port[0] && sset_contains(local_lports, binding_rec->parent_port))) { @@ -380,65 +391,61 @@ consider_local_datapath(struct controller_ctx *ctx, if (iface_rec && qos_map && ctx->ovs_idl_txn) { get_qos_params(binding_rec, qos_map); } + /* This port is in our chassis unless it is a localport. */ - if (strcmp(binding_rec->type, "localport")) { - our_chassis = true; - } - } else if (!strcmp(binding_rec->type, "l2gateway")) { - const char *chassis_id = smap_get(&binding_rec->options, - "l2gateway-chassis"); - our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name); - if (our_chassis) { - sset_add(local_lports, binding_rec->logical_port); - add_local_datapath(ldatapaths, lports, binding_rec->datapath, - false, local_datapaths); - } - } else if (!strcmp(binding_rec->type, "chassisredirect")) { - const char *chassis_id = smap_get(&binding_rec->options, - "redirect-chassis"); - our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name); - if (our_chassis) { - add_local_datapath(ldatapaths, lports, binding_rec->datapath, - false, local_datapaths); - } - } else if (!strcmp(binding_rec->type, "l3gateway")) { - const char *chassis_id = smap_get(&binding_rec->options, - "l3gateway-chassis"); - our_chassis = chassis_id && !strcmp(chassis_id, chassis_rec->name); - if (our_chassis) { - add_local_datapath(ldatapaths, lports, binding_rec->datapath, - true, local_datapaths); - } + return strcmp(binding_rec->type, "localport"); + } else if (is_specially_bound(binding_rec, chassis_rec, + "l2gateway", "l2gateway-chassis")) { + sset_add(local_lports, binding_rec->logical_port); + add_local_datapath(ldatapaths, lports, binding_rec->datapath, + false, local_datapaths); + return true; + } else if (is_specially_bound(binding_rec, chassis_rec, + "chassisredirect", "redirect-chassis")) { + add_local_datapath(ldatapaths, lports, binding_rec->datapath, + false, local_datapaths); + return true; + } else if (is_specially_bound(binding_rec, chassis_rec, + "l3gateway", "l3gateway-chassis")) { + add_local_datapath(ldatapaths, lports, binding_rec->datapath, + true, local_datapaths); + return true; } else if (!strcmp(binding_rec->type, "localnet")) { /* Add all localnet ports to local_lports so that we allocate ct zones * for them. */ sset_add(local_lports, binding_rec->logical_port); - our_chassis = false; - } - - if (ctx->ovnsb_idl_txn) { - if (our_chassis) { - if (binding_rec->chassis != chassis_rec) { - if (binding_rec->chassis) { - VLOG_INFO("Changing chassis for lport %s from %s to %s.", - binding_rec->logical_port, - binding_rec->chassis->name, - chassis_rec->name); - } else { - VLOG_INFO("Claiming lport %s for this chassis.", - binding_rec->logical_port); - } - for (int i = 0; i < binding_rec->n_mac; i++) { - VLOG_INFO("%s: Claiming %s", - binding_rec->logical_port, binding_rec->mac[i]); - } - sbrec_port_binding_set_chassis(binding_rec, chassis_rec); + return false; + } else { + return false; + } +} + +static void +update_binding_ownership(const struct sbrec_chassis *chassis_rec, + const struct sbrec_port_binding *binding_rec, + bool should_own) +{ + if (should_own) { + if (binding_rec->chassis != chassis_rec) { + if (binding_rec->chassis) { + VLOG_INFO("Changing chassis for lport %s from %s to %s.", + binding_rec->logical_port, + binding_rec->chassis->name, + chassis_rec->name); + } else { + VLOG_INFO("Claiming lport %s for this chassis.", + binding_rec->logical_port); } - } else if (binding_rec->chassis == chassis_rec) { - VLOG_INFO("Releasing lport %s from this chassis.", - binding_rec->logical_port); - sbrec_port_binding_set_chassis(binding_rec, NULL); + for (int i = 0; i < binding_rec->n_mac; i++) { + VLOG_INFO("%s: Claiming %s", + binding_rec->logical_port, binding_rec->mac[i]); + } + sbrec_port_binding_set_chassis(binding_rec, chassis_rec); } + } else if (binding_rec->chassis == chassis_rec) { + VLOG_INFO("Releasing lport %s from this chassis.", + binding_rec->logical_port); + sbrec_port_binding_set_chassis(binding_rec, NULL); } } @@ -466,38 +473,30 @@ consider_localnet_port(const struct sbrec_port_binding *binding_rec, ld->localnet_port = binding_rec; } -void -binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, - const struct sbrec_chassis *chassis_rec, - const struct ldatapath_index *ldatapaths, - const struct lport_index *lports, struct hmap *local_datapaths, - struct sset *local_lports) +static void +binding_run__(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, + const struct sbrec_chassis *chassis_rec, + const struct ldatapath_index *ldatapaths, + const struct lport_index *lports, struct hmap *qos_map, + struct hmap *local_datapaths, + struct sset *local_lports, bool update_sb) { - if (!chassis_rec) { - return; - } - const struct sbrec_port_binding *binding_rec; struct shash lport_to_iface = SHASH_INITIALIZER(&lport_to_iface); - struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces); - struct hmap qos_map; - - hmap_init(&qos_map); if (br_int) { - get_local_iface_ids(br_int, &lport_to_iface, local_lports, - &egress_ifaces); + get_local_iface_ids(br_int, &lport_to_iface, local_lports); } /* Run through each binding record to see if it is resident on this * chassis and update the binding accordingly. This includes both * directly connected logical ports and children of those ports. */ SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) { - consider_local_datapath(ctx, ldatapaths, lports, - chassis_rec, binding_rec, - sset_is_empty(&egress_ifaces) ? NULL : - &qos_map, local_datapaths, &lport_to_iface, - local_lports); - + bool should_own = consider_local_datapath( + ctx, ldatapaths, lports, chassis_rec, binding_rec, + qos_map, local_datapaths, &lport_to_iface, local_lports); + if (ctx->ovnsb_idl_txn && update_sb) { + update_binding_ownership(chassis_rec, binding_rec, should_own); + } } /* Run through each binding record to see if it is a localnet port @@ -509,6 +508,34 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, } } + shash_destroy(&lport_to_iface); +} + +/* Initializes 'local_datapaths' and 'local_lports' to the sets of logical + * datapaths and logical ports, respectively, that are relevant to this + * machine. Updates Port_Binding records 'chassis' columns to point to + * 'chassis_rec' where appropriate. Sets up QoS appropriately on tunnel egress + * interfaces. */ +void +binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, + const struct sbrec_chassis *chassis_rec, + const struct ldatapath_index *ldatapaths, + const struct lport_index *lports, struct hmap *local_datapaths, + struct sset *local_lports) +{ + if (!chassis_rec) { + return; + } + + struct sset egress_ifaces = SSET_INITIALIZER(&egress_ifaces); + if (br_int) { + get_egress_ifaces(br_int, &egress_ifaces); + } + + struct hmap qos_map = HMAP_INITIALIZER(&qos_map); + binding_run__(ctx, br_int, chassis_rec, ldatapaths, lports, + sset_is_empty(&egress_ifaces) ? NULL : &qos_map, + local_datapaths, local_lports, true); if (!sset_is_empty(&egress_ifaces) && set_noop_qos(ctx, &egress_ifaces)) { const char *entry; @@ -516,10 +543,26 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, setup_qos(entry, &qos_map); } } - - shash_destroy(&lport_to_iface); - sset_destroy(&egress_ifaces); hmap_destroy(&qos_map); + sset_destroy(&egress_ifaces); +} + +/* Initializes 'local_datapaths' and 'local_lports' to the sets of logical + * datapaths and logical ports, respectively, that are relevant to this + * machine. */ +void +binding_get(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, + const struct sbrec_chassis *chassis_rec, + const struct ldatapath_index *ldatapaths, + const struct lport_index *lports, struct hmap *local_datapaths, + struct sset *local_lports) +{ + if (!chassis_rec) { + return; + } + + binding_run__(ctx, br_int, chassis_rec, ldatapaths, lports, NULL, + local_datapaths, local_lports, false); } /* Returns true if the database is all cleaned up, false if more work is diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h index 3bfa7d1..98a45a8 100644 --- a/ovn/controller/binding.h +++ b/ovn/controller/binding.h @@ -33,6 +33,10 @@ void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int, const struct sbrec_chassis *, const struct ldatapath_index *, const struct lport_index *, struct hmap *local_datapaths, struct sset *all_lports); +void binding_get(struct controller_ctx *, const struct ovsrec_bridge *br_int, + const struct sbrec_chassis *, const struct ldatapath_index *, + const struct lport_index *, struct hmap *local_datapaths, + struct sset *all_lports); bool binding_cleanup(struct controller_ctx *, const struct sbrec_chassis *); #endif /* ovn/binding.h */ diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 45a670b..cc01fe9 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -201,15 +201,26 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, ovsdb_idl_condition_destroy(&dns); } +static const char * +br_int_name(const struct ovsrec_open_vswitch *cfg) +{ + return smap_get_def(&cfg->external_ids, "ovn-bridge", DEFAULT_BRIDGE_NAME); +} + static const struct ovsrec_bridge * -create_br_int(struct controller_ctx *ctx, - const struct ovsrec_open_vswitch *cfg, - const char *bridge_name) +create_br_int(struct controller_ctx *ctx) { if (!ctx->ovs_idl_txn) { return NULL; } + const struct ovsrec_open_vswitch *cfg; + cfg = ovsrec_open_vswitch_first(ctx->ovs_idl); + if (!cfg) { + return NULL; + } + const char *bridge_name = br_int_name(cfg); + ovsdb_idl_txn_add_comment(ctx->ovs_idl_txn, "ovn-controller: creating integration bridge '%s'", bridge_name); @@ -252,15 +263,7 @@ get_br_int(struct controller_ctx *ctx) return NULL; } - const char *br_int_name = smap_get_def(&cfg->external_ids, "ovn-bridge", - DEFAULT_BRIDGE_NAME); - - const struct ovsrec_bridge *br; - br = get_bridge(ctx->ovs_idl, br_int_name); - if (!br) { - return create_br_int(ctx, cfg, br_int_name); - } - return br; + return get_bridge(ctx->ovs_idl, br_int_name(cfg)); } static const char * @@ -622,6 +625,9 @@ main(int argc, char *argv[]) struct sset local_lports = SSET_INITIALIZER(&local_lports); const struct ovsrec_bridge *br_int = get_br_int(&ctx); + if (!br_int) { + br_int = create_br_int(&ctx); + } const char *chassis_id = get_chassis_id(ctx.ovs_idl); struct ldatapath_index ldatapaths;