From patchwork Fri Jul 29 15:18:49 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell Bryant X-Patchwork-Id: 654138 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (archives.nicira.com [96.126.127.54]) by ozlabs.org (Postfix) with ESMTP id 3s1C7C6Rtzz9sDG for ; Sat, 30 Jul 2016 01:19:03 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id DFC4F114EA; Fri, 29 Jul 2016 08:19:01 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx3v3.cudamail.com (mx3.cudamail.com [64.34.241.5]) by archives.nicira.com (Postfix) with ESMTPS id 671AD11499 for ; Fri, 29 Jul 2016 08:19:00 -0700 (PDT) Received: from bar6.cudamail.com (localhost [127.0.0.1]) by mx3v3.cudamail.com (Postfix) with ESMTPS id F3A011620A0 for ; Fri, 29 Jul 2016 09:18:59 -0600 (MDT) X-ASG-Debug-ID: 1469805537-0b323747733fcf70001-byXFYA Received: from mx1-pf2.cudamail.com ([192.168.24.2]) by bar6.cudamail.com with ESMTP id E3wDqEVdP9u4FDvH (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 29 Jul 2016 09:18:57 -0600 (MDT) X-Barracuda-Envelope-From: russell@ovn.org X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.2 Received: from unknown (HELO mx1.redhat.com) (209.132.183.28) by mx1-pf2.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 29 Jul 2016 15:18:57 -0000 Received-SPF: neutral (mx1-pf2.cudamail.com: 209.132.183.28 is neither permitted nor denied by SPF record at ovn.org) X-Barracuda-Apparent-Source-IP: 209.132.183.28 X-Barracuda-RBL-IP: 209.132.183.28 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 42C83285A2; Fri, 29 Jul 2016 15:18:55 +0000 (UTC) Received: from x1c.redhat.com (ovpn-112-12.phx2.redhat.com [10.3.112.12]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u6TFIqqP009100; Fri, 29 Jul 2016 11:18:53 -0400 X-CudaMail-Envelope-Sender: russell@ovn.org From: Russell Bryant To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-E2-728026518 X-CudaMail-DTE: 072916 X-CudaMail-Originating-IP: 209.132.183.28 Date: Fri, 29 Jul 2016 11:18:49 -0400 X-ASG-Orig-Subj: [##CM-E2-728026518##][PATCH v2] ovn-controller: Restore ct zone assignment. Message-Id: <1469805529-10267-1-git-send-email-russell@ovn.org> In-Reply-To: <1469530004-22939-1-git-send-email-bschanmu@redhat.com> References: <1469530004-22939-1-git-send-email-bschanmu@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Fri, 29 Jul 2016 15:18:55 +0000 (UTC) X-Barracuda-Connect: UNKNOWN[192.168.24.2] X-Barracuda-Start-Time: 1469805537 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://web.cudamail.com:443/cgi-mod/mark.cgi X-ASG-Whitelist: Header =?UTF-8?B?eFwtY3VkYW1haWxcLXdoaXRlbGlzdFwtdG8=?= X-Virus-Scanned: by bsmtpd at cudamail.com X-Barracuda-BRTS-Status: 1 Subject: [ovs-dev] [PATCH v2] ovn-controller: Restore ct zone assignment. X-BeenThere: dev@openvswitch.org X-Mailman-Version: 2.1.16 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dev-bounces@openvswitch.org Sender: "dev" From: Babu Shanmugam Recent commits reorganizing bindings handling and also moving ct zone assignment to ovn-controller.c caused ct zone assignment to no longer work. The code relies on an "all_lports" sset that should contain all logical ports that we should be assigning ct zones for. Prior to this change, all_lports was always empty. Signed-off-by: Babu Shanmugam Co-authored-by: Russell Bryant Signed-off-by: Russell Bryant Acked-by: Ryan Moats --- ovn/controller/binding.c | 46 +++++++++++++++++++++++++++++++++++------ ovn/controller/binding.h | 3 ++- ovn/controller/ovn-controller.c | 13 +++++++----- 3 files changed, 50 insertions(+), 12 deletions(-) v1->v2: - Ensure all_lports also includes sub-ports, gateway ports, and localnet ports, as well as ports for local interfaces. diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c index 41165bc..3073727 100644 --- a/ovn/controller/binding.c +++ b/ovn/controller/binding.c @@ -66,7 +66,8 @@ binding_register_ovs_idl(struct ovsdb_idl *ovs_idl) static bool get_local_iface_ids(const struct ovsrec_bridge *br_int, - struct shash *lport_to_iface) + struct shash *lport_to_iface, + struct sset *all_lports) { int i; bool changed = false; @@ -94,6 +95,7 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, shash_add(lport_to_iface, iface_id, iface_rec); if (!sset_find_and_delete(&old_local_ids, iface_id)) { sset_add(&local_ids, iface_id); + sset_add(all_lports, iface_id); changed = true; } } @@ -107,6 +109,7 @@ get_local_iface_ids(const struct ovsrec_bridge *br_int, const char *cur_id; SSET_FOR_EACH(cur_id, &old_local_ids) { sset_find_and_delete(&local_ids, cur_id); + sset_find_and_delete(all_lports, cur_id); } } @@ -174,7 +177,8 @@ consider_local_datapath(struct controller_ctx *ctx, const struct sbrec_chassis *chassis_rec, const struct sbrec_port_binding *binding_rec, struct hmap *local_datapaths, - struct shash *lport_to_iface) + struct shash *lport_to_iface, + struct sset *all_lports) { const struct ovsrec_interface *iface_rec = shash_find_data(lport_to_iface, binding_rec->logical_port); @@ -200,6 +204,9 @@ consider_local_datapath(struct controller_ctx *ctx, binding_rec->logical_port); } sbrec_port_binding_set_chassis(binding_rec, chassis_rec); + if (binding_rec->parent_port && binding_rec->parent_port[0]) { + sset_add(all_lports, binding_rec->logical_port); + } } } else if (!strcmp(binding_rec->type, "l2gateway")) { const char *chassis_id = smap_get(&binding_rec->options, @@ -209,6 +216,7 @@ consider_local_datapath(struct controller_ctx *ctx, VLOG_INFO("Releasing l2gateway port %s from this chassis.", binding_rec->logical_port); sbrec_port_binding_set_chassis(binding_rec, NULL); + sset_find_and_delete(all_lports, binding_rec->logical_port); } return; } @@ -221,6 +229,7 @@ consider_local_datapath(struct controller_ctx *ctx, VLOG_INFO("Claiming l2gateway port %s for this chassis.", binding_rec->logical_port); sbrec_port_binding_set_chassis(binding_rec, chassis_rec); + sset_add(all_lports, binding_rec->logical_port); add_local_datapath(local_datapaths, binding_rec); } } else if (chassis_rec && binding_rec->chassis == chassis_rec @@ -229,13 +238,20 @@ consider_local_datapath(struct controller_ctx *ctx, VLOG_INFO("Releasing lport %s from this chassis.", binding_rec->logical_port); sbrec_port_binding_set_chassis(binding_rec, NULL); + sset_find_and_delete(all_lports, binding_rec->logical_port); } + } else if (!binding_rec->chassis + && !strcmp(binding_rec->type, "localnet")) { + /* Add all localnet ports to all_lports so that we allocate ct zones + * for them. */ + sset_add(all_lports, binding_rec->logical_port); } } void binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, - const char *chassis_id, struct hmap *local_datapaths) + const char *chassis_id, struct hmap *local_datapaths, + struct sset *all_lports) { const struct sbrec_chassis *chassis_rec; const struct sbrec_port_binding *binding_rec; @@ -247,7 +263,8 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, } if (br_int) { - if (ctx->ovnsb_idl_txn && get_local_iface_ids(br_int, &lport_to_iface)) { + if (ctx->ovnsb_idl_txn && get_local_iface_ids(br_int, &lport_to_iface, + all_lports)) { process_full_binding = true; } } else { @@ -260,11 +277,19 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, * chassis and update the binding accordingly. This includes both * directly connected logical ports and children of those ports. */ if (process_full_binding) { + /* Detect any entries in all_lports that have been deleted. + * In particular, this will catch localnet ports that we + * put in all_lports. */ + struct sset removed_lports = SSET_INITIALIZER(&removed_lports); + sset_clone(&removed_lports, all_lports); + struct hmap keep_local_datapath_by_uuid = HMAP_INITIALIZER(&keep_local_datapath_by_uuid); SBREC_PORT_BINDING_FOR_EACH(binding_rec, ctx->ovnsb_idl) { + sset_find_and_delete(&removed_lports, binding_rec->logical_port); consider_local_datapath(ctx, chassis_rec, binding_rec, - local_datapaths, &lport_to_iface); + local_datapaths, &lport_to_iface, + all_lports); struct local_datapath *ld = xzalloc(sizeof *ld); memcpy(&ld->uuid, &binding_rec->header_.uuid, sizeof ld->uuid); hmap_insert(&keep_local_datapath_by_uuid, &ld->uuid_hmap_node, @@ -278,6 +303,14 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, } } hmap_destroy(&keep_local_datapath_by_uuid); + + /* Any remaining entries in removed_lports are logical ports that + * have been deleted and should also be removed from all_ports. */ + const char *cur_id; + SSET_FOR_EACH(cur_id, &removed_lports) { + sset_find_and_delete(all_lports, cur_id); + } + process_full_binding = false; } else { SBREC_PORT_BINDING_FOR_EACH_TRACKED(binding_rec, ctx->ovnsb_idl) { @@ -292,7 +325,8 @@ binding_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int, } } else { consider_local_datapath(ctx, chassis_rec, binding_rec, - local_datapaths, &lport_to_iface); + local_datapaths, &lport_to_iface, + all_lports); } } } diff --git a/ovn/controller/binding.h b/ovn/controller/binding.h index 8753d44..fbd16c8 100644 --- a/ovn/controller/binding.h +++ b/ovn/controller/binding.h @@ -29,7 +29,8 @@ struct sset; void binding_register_ovs_idl(struct ovsdb_idl *); void binding_reset_processing(void); void binding_run(struct controller_ctx *, const struct ovsrec_bridge *br_int, - const char *chassis_id, struct hmap *local_datapaths); + const char *chassis_id, struct hmap *local_datapaths, + struct sset *all_lports); bool binding_cleanup(struct controller_ctx *, const char *chassis_id); #endif /* ovn/binding.h */ diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 5c74186..5a2daa8 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -314,6 +314,11 @@ static struct hmap patched_datapaths = HMAP_INITIALIZER(&patched_datapaths); static struct lport_index lports; static struct mcgroup_index mcgroups; +/* Contains the names of all logical ports currently bound to the chassis + * managed by this instance of ovn-controller. The contents are managed + * in binding.c, but consumed elsewhere. */ +static struct sset all_lports = SSET_INITIALIZER(&all_lports); + int main(int argc, char *argv[]) { @@ -425,8 +430,6 @@ main(int argc, char *argv[]) update_probe_interval(&ctx); - struct sset all_lports = SSET_INITIALIZER(&all_lports); - const struct ovsrec_bridge *br_int = get_br_int(&ctx); const char *chassis_id = get_chassis_id(ctx.ovs_idl); @@ -434,7 +437,9 @@ main(int argc, char *argv[]) if (chassis_id) { chassis = chassis_run(&ctx, chassis_id); encaps_run(&ctx, br_int, chassis_id); - binding_run(&ctx, br_int, chassis_id, &local_datapaths); + binding_run(&ctx, br_int, chassis_id, &local_datapaths, + &all_lports); + VLOG_INFO("all_lports size: %lu", sset_count(&all_lports)); } if (br_int && chassis_id) { @@ -466,8 +471,6 @@ main(int argc, char *argv[]) } } - sset_destroy(&all_lports); - unixctl_server_run(unixctl); unixctl_server_wait(unixctl);