From patchwork Fri Jul 8 07:38:06 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gurucharan Shetty X-Patchwork-Id: 646524 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 3rmM9k5sN4z9t0P for ; Sat, 9 Jul 2016 03:36:42 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id B8E2C10580; Fri, 8 Jul 2016 10:36:40 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx1e4.cudamail.com (mx1.cudamail.com [69.90.118.67]) by archives.nicira.com (Postfix) with ESMTPS id B207F10283 for ; Fri, 8 Jul 2016 10:36:39 -0700 (PDT) Received: from bar5.cudamail.com (unknown [192.168.21.12]) by mx1e4.cudamail.com (Postfix) with ESMTPS id 0719B1E004B for ; Fri, 8 Jul 2016 11:36:39 -0600 (MDT) X-ASG-Debug-ID: 1467999398-09eadd1b184449a0001-byXFYA Received: from mx1-pf1.cudamail.com ([192.168.24.1]) by bar5.cudamail.com with ESMTP id 7GwKLna4jjx4oX7Z (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO) for ; Fri, 08 Jul 2016 11:36:38 -0600 (MDT) X-Barracuda-Envelope-From: guru.ovn@gmail.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.24.1 Received: from unknown (HELO mail-pa0-f68.google.com) (209.85.220.68) by mx1-pf1.cudamail.com with ESMTPS (AES128-SHA encrypted); 8 Jul 2016 17:36:38 -0000 Received-SPF: pass (mx1-pf1.cudamail.com: SPF record at _netblocks.google.com designates 209.85.220.68 as permitted sender) X-Barracuda-Apparent-Source-IP: 209.85.220.68 X-Barracuda-RBL-IP: 209.85.220.68 Received: by mail-pa0-f68.google.com with SMTP id ib6so8491864pad.3 for ; Fri, 08 Jul 2016 10:36:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:subject:date:message-id; bh=4g++3QI7PRG1R7X0TsIUSItEgkVEiJJoBbrrOaVhJpk=; b=Cq10T2AH5bmPSLfKnbsU36RNhIlNL1bux4PdKiMhiW5+odTqG4UIOtebbZxhzTvp23 CmifjRItr4m5fLgBSj70TrpyTIlUo9bfPTXG0oWqaLjWSxCYi30GQkrwMrvnfaWCjka5 3SvP3L+vNmdZ/ri6fsUXGd9YXd+1yu08vKEXDbenzenkX4UG4gduIcJ0FL7zYGszolOs jzfGuQWBLBYMzFA0KLrAGxyUg/tWdCmbRkHHUzy2e7d9igm1/qQlmZuiaxWfPp064GV0 0toP/xZ+AWHTRQkRd2hfVBVOhrA+RHm16iwgG76XzguZ8+5PflRA5lZGRcZWN3RBt3O8 n7wA== X-Gm-Message-State: ALyK8tJHY96qISslG/09Vmj5qbYS5HZbkm86CAqz2ChU7d2pClM2re2wPI7Qigs5vQJh4g== X-Received: by 10.66.54.35 with SMTP id g3mr12129620pap.30.1467999397181; Fri, 08 Jul 2016 10:36:37 -0700 (PDT) Received: from ubuntu.eng.vmware.com ([208.91.1.34]) by smtp.gmail.com with ESMTPSA id b7sm1919453pat.27.2016.07.08.10.36.35 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 08 Jul 2016 10:36:35 -0700 (PDT) X-CudaMail-Envelope-Sender: guru.ovn@gmail.com From: Gurucharan Shetty To: dev@openvswitch.org X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-E1-707040800 X-CudaMail-DTE: 070816 X-CudaMail-Originating-IP: 209.85.220.68 Date: Fri, 8 Jul 2016 00:38:06 -0700 X-ASG-Orig-Subj: [##CM-E1-707040800##][PATCH 2/2] ovn-controller: Change strategy for gateway conntrack zone allocation. Message-Id: <1467963486-32094-1-git-send-email-guru@ovn.org> X-Mailer: git-send-email 1.9.1 X-Barracuda-Connect: UNKNOWN[192.168.24.1] X-Barracuda-Start-Time: 1467999398 X-Barracuda-Encrypted: ECDHE-RSA-AES256-GCM-SHA384 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 2/2] ovn-controller: Change strategy for gateway conntrack zone allocation. 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" Commit 263064aeaa31e7 (Convert binding_run to incremental processing.) changed the way patched_datapaths were handled. Previously we would destroy the datastructure in every run and re-create it fresh. The new way causes problems with the way conntrack zones are allocated as now we can have stale port_binding entries causing segmentation faults. With this commit, we simply don't depend on port_binding records in conntrack zone allocation and instead store the UUID as a string in the patch_datapath datastructure. Fixes: 263064aeaa31e7 ("Convert binding_run to incremental processing.") Signed-off-by: Gurucharan Shetty --- This applies on top of https://patchwork.ozlabs.org/patch/646268/. I will rebase this if the dependent patch changes. --- ovn/controller/ovn-controller.c | 4 ++-- ovn/controller/ovn-controller.h | 2 +- ovn/controller/patch.c | 4 +++- ovn/controller/physical.c | 8 ++++++-- ovn/lib/ovn-util.c | 8 +++----- ovn/lib/ovn-util.h | 3 +-- 6 files changed, 16 insertions(+), 13 deletions(-) diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 8471f64..28ee13e 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -251,8 +251,8 @@ update_ct_zones(struct sset *lports, struct hmap *patched_datapaths, continue; } - char *dnat = alloc_nat_zone_key(pd->port_binding, "dnat"); - char *snat = alloc_nat_zone_key(pd->port_binding, "snat"); + char *dnat = alloc_nat_zone_key(pd->key, "dnat"); + char *snat = alloc_nat_zone_key(pd->key, "snat"); sset_add(&all_users, dnat); sset_add(&all_users, snat); free(dnat); diff --git a/ovn/controller/ovn-controller.h b/ovn/controller/ovn-controller.h index f3edc43..470b1f5 100644 --- a/ovn/controller/ovn-controller.h +++ b/ovn/controller/ovn-controller.h @@ -51,7 +51,7 @@ struct local_datapath *get_local_datapath(const struct hmap *, * with at least one logical patch port binding. */ struct patched_datapath { struct hmap_node hmap_node; - const struct sbrec_port_binding *port_binding; + char *key; /* Holds the uuid of the corresponding datapath. */ bool local; /* 'True' if the datapath is for gateway router. */ bool stale; /* 'True' if the datapath is not referenced by any patch * port. */ diff --git a/ovn/controller/patch.c b/ovn/controller/patch.c index 3825f31..9a5c96f 100644 --- a/ovn/controller/patch.c +++ b/ovn/controller/patch.c @@ -263,7 +263,8 @@ add_patched_datapath(struct hmap *patched_datapaths, pd = xzalloc(sizeof *pd); pd->local = local; - pd->port_binding = binding_rec; + pd->key = xasprintf(UUID_FMT, + UUID_ARGS(&binding_rec->datapath->header_.uuid)); /* stale is set to false. */ hmap_insert(patched_datapaths, &pd->hmap_node, binding_rec->datapath->tunnel_key); @@ -291,6 +292,7 @@ add_logical_patch_ports_postprocess(struct hmap *patched_datapaths) patched_datapaths) { if (pd_cur_node->stale == true) { hmap_remove(patched_datapaths, &pd_cur_node->hmap_node); + free(pd_cur_node->key); free(pd_cur_node); } } diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c index f7389ea..d1b40c2 100644 --- a/ovn/controller/physical.c +++ b/ovn/controller/physical.c @@ -297,8 +297,12 @@ consider_port_binding(struct hmap *flow_table, } int zone_id_dnat, zone_id_snat; - char *dnat = alloc_nat_zone_key(binding, "dnat"); - char *snat = alloc_nat_zone_key(binding, "snat"); + char *key = xasprintf(UUID_FMT, + UUID_ARGS(&binding->datapath->header_.uuid)); + char *dnat = alloc_nat_zone_key(key, "dnat"); + char *snat = alloc_nat_zone_key(key, "snat"); + free(key); + zone_id_dnat = simap_get(ct_zones, dnat); if (zone_id_dnat) { put_load(zone_id_dnat, MFF_LOG_DNAT_ZONE, 0, 32, ofpacts_p); diff --git a/ovn/lib/ovn-util.c b/ovn/lib/ovn-util.c index 6113087..6e94083 100644 --- a/ovn/lib/ovn-util.c +++ b/ovn/lib/ovn-util.c @@ -105,13 +105,11 @@ extract_lsp_addresses(char *address, struct lport_addresses *laddrs, } /* Allocates a key for NAT conntrack zone allocation for a provided - * 'port_binding' record and a 'type'. + * 'key' record and a 'type'. * * It is the caller's responsibility to free the allocated memory. */ char * -alloc_nat_zone_key(const struct sbrec_port_binding *port_binding, - const char *type) +alloc_nat_zone_key(const char *key, const char *type) { - return xasprintf(UUID_FMT"_%s", - UUID_ARGS(&port_binding->datapath->header_.uuid), type); + return xasprintf("%s_%s", key, type); } diff --git a/ovn/lib/ovn-util.h b/ovn/lib/ovn-util.h index d23f4af..98b1426 100644 --- a/ovn/lib/ovn-util.h +++ b/ovn/lib/ovn-util.h @@ -43,6 +43,5 @@ extract_lsp_addresses(char *address, struct lport_addresses *laddrs, bool store_ipv6); char * -alloc_nat_zone_key(const struct sbrec_port_binding *port_binding, - const char *type); +alloc_nat_zone_key(const char *key, const char *type); #endif