From patchwork Fri Jun 24 19:52:09 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell Bryant X-Patchwork-Id: 640403 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 3rbps04KfRz9t0P for ; Sat, 25 Jun 2016 05:52:36 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id EE52310CCF; Fri, 24 Jun 2016 12:52:23 -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 034B410CCA for ; Fri, 24 Jun 2016 12:52:23 -0700 (PDT) Received: from bar5.cudamail.com (unknown [192.168.21.12]) by mx1e4.cudamail.com (Postfix) with ESMTPS id 860531E0630 for ; Fri, 24 Jun 2016 13:52:22 -0600 (MDT) X-ASG-Debug-ID: 1466797941-09eadd24ba22eab0001-byXFYA Received: from mx3-pf2.cudamail.com ([192.168.14.1]) by bar5.cudamail.com with ESMTP id CY6c6456rMOmYTlq (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Fri, 24 Jun 2016 13:52:21 -0600 (MDT) X-Barracuda-Envelope-From: russell@ovn.org X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.1 Received: from unknown (HELO mx1.redhat.com) (209.132.183.28) by mx3-pf2.cudamail.com with ESMTPS (DHE-RSA-AES256-SHA encrypted); 24 Jun 2016 19:52:21 -0000 Received-SPF: neutral (mx3-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-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (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 9662B80088; Fri, 24 Jun 2016 19:52:19 +0000 (UTC) Received: from x1c.redhat.com (ovpn-112-42.phx2.redhat.com [10.3.112.42]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u5OJqEru009539; Fri, 24 Jun 2016 15:52:17 -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-V2-623040050 X-CudaMail-DTE: 062416 X-CudaMail-Originating-IP: 209.132.183.28 Date: Fri, 24 Jun 2016 15:52:09 -0400 X-ASG-Orig-Subj: [##CM-V2-623040050##][PATCH 2/4] Revert "lport: Persist lport_index and mcgroup_index structures." Message-Id: <1466797931-23871-3-git-send-email-russell@ovn.org> In-Reply-To: <1466797931-23871-1-git-send-email-russell@ovn.org> References: <1466797931-23871-1-git-send-email-russell@ovn.org> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 24 Jun 2016 19:52:19 +0000 (UTC) X-Barracuda-Connect: UNKNOWN[192.168.14.1] X-Barracuda-Start-Time: 1466797941 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 2/4] Revert "lport: Persist lport_index and mcgroup_index structures." 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" This reverts commit 4470328bc4e0081479351e39a1964a78fe19be1b. Signed-off-by: Russell Bryant --- ovn/controller/lport.c | 221 +++++++--------------------------------- ovn/controller/lport.h | 22 +--- ovn/controller/ovn-controller.c | 16 ++- 3 files changed, 46 insertions(+), 213 deletions(-) diff --git a/ovn/controller/lport.c b/ovn/controller/lport.c index 2ce6387..a7ae320 100644 --- a/ovn/controller/lport.c +++ b/ovn/controller/lport.c @@ -17,7 +17,6 @@ #include "lport.h" #include "hash.h" -#include "lflow.h" #include "openvswitch/vlog.h" #include "ovn/lib/ovn-sb-idl.h" @@ -25,108 +24,48 @@ VLOG_DEFINE_THIS_MODULE(lport); /* A logical port. */ struct lport { - struct hmap_node name_node; /* Index by name. */ - struct hmap_node key_node; /* Index by (dp_key, port_key). */ - struct hmap_node uuid_node; /* Index by row uuid. */ - const struct uuid *uuid; + struct hmap_node name_node; /* Index by name. */ + struct hmap_node key_node; /* Index by (dp_key, port_key). */ const struct sbrec_port_binding *pb; }; -static bool full_lport_rebuild = false; - -void -lport_index_reset(void) -{ - full_lport_rebuild = true; -} - void -lport_index_init(struct lport_index *lports) +lport_index_init(struct lport_index *lports, struct ovsdb_idl *ovnsb_idl) { hmap_init(&lports->by_name); hmap_init(&lports->by_key); - hmap_init(&lports->by_uuid); -} - -void -lport_index_remove(struct lport_index *lports, const struct uuid *uuid) -{ - const struct lport *port_ = lport_lookup_by_uuid(lports, uuid); - struct lport *port = CONST_CAST(struct lport *, port_); - if (port) { - hmap_remove(&lports->by_name, &port->name_node); - hmap_remove(&lports->by_key, &port->key_node); - hmap_remove(&lports->by_uuid, &port->uuid_node); - free(port); - } -} - -void -lport_index_clear(struct lport_index *lports) -{ - /* Destroy all of the "struct lport"s. - * - * We have to remove the node from all indexes. */ - struct lport *port, *next; - HMAP_FOR_EACH_SAFE (port, next, name_node, &lports->by_name) { - hmap_remove(&lports->by_name, &port->name_node); - hmap_remove(&lports->by_key, &port->key_node); - hmap_remove(&lports->by_uuid, &port->uuid_node); - free(port); - } -} -static void -consider_lport_index(struct lport_index *lports, - const struct sbrec_port_binding *pb) -{ - if (lport_lookup_by_name(lports, pb->logical_port)) { - return; - } - - struct lport *p = xmalloc(sizeof *p); - hmap_insert(&lports->by_name, &p->name_node, - hash_string(pb->logical_port, 0)); - hmap_insert(&lports->by_key, &p->key_node, - hash_int(pb->tunnel_key, pb->datapath->tunnel_key)); - hmap_insert(&lports->by_uuid, &p->uuid_node, - uuid_hash(&pb->header_.uuid)); - p->uuid = &pb->header_.uuid; - p->pb = pb; -} - -void -lport_index_fill(struct lport_index *lports, struct ovsdb_idl *ovnsb_idl) -{ const struct sbrec_port_binding *pb; - if (full_lport_rebuild) { - lport_index_clear(lports); - SBREC_PORT_BINDING_FOR_EACH (pb, ovnsb_idl) { - consider_lport_index(lports, pb); + SBREC_PORT_BINDING_FOR_EACH (pb, ovnsb_idl) { + if (lport_lookup_by_name(lports, pb->logical_port)) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "duplicate logical port name '%s'", + pb->logical_port); + continue; } - full_lport_rebuild = false; - } else { - SBREC_PORT_BINDING_FOR_EACH_TRACKED (pb, ovnsb_idl) { - bool is_delete = sbrec_port_binding_row_get_seqno(pb, - OVSDB_IDL_CHANGE_DELETE) > 0; - if (is_delete) { - lport_index_remove(lports, &pb->header_.uuid); - continue; - } - consider_lport_index(lports, pb); - } + struct lport *p = xmalloc(sizeof *p); + hmap_insert(&lports->by_name, &p->name_node, + hash_string(pb->logical_port, 0)); + hmap_insert(&lports->by_key, &p->key_node, + hash_int(pb->tunnel_key, pb->datapath->tunnel_key)); + p->pb = pb; } } void lport_index_destroy(struct lport_index *lports) { - lport_index_clear(lports); + /* Destroy all of the "struct lport"s. + * + * We don't have to remove the node from both indexes. */ + struct lport *port; + HMAP_FOR_EACH_POP (port, name_node, &lports->by_name) { + free(port); + } hmap_destroy(&lports->by_name); hmap_destroy(&lports->by_key); - hmap_destroy(&lports->by_uuid); } /* Finds and returns the lport with the given 'name', or NULL if no such lport @@ -144,20 +83,6 @@ lport_lookup_by_name(const struct lport_index *lports, const char *name) return NULL; } -const struct lport * -lport_lookup_by_uuid(const struct lport_index *lports, - const struct uuid *uuid) -{ - const struct lport *lport; - HMAP_FOR_EACH_WITH_HASH (lport, uuid_node, uuid_hash(uuid), - &lports->by_uuid) { - if (uuid_equals(uuid, lport->uuid)) { - return lport; - } - } - return NULL; -} - const struct sbrec_port_binding * lport_lookup_by_key(const struct lport_index *lports, uint32_t dp_key, uint16_t port_key) @@ -175,113 +100,43 @@ lport_lookup_by_key(const struct lport_index *lports, struct mcgroup { struct hmap_node dp_name_node; /* Index by (logical datapath, name). */ - struct hmap_node uuid_node; /* Index by insert uuid. */ - const struct uuid *uuid; const struct sbrec_multicast_group *mg; }; -static bool full_mc_rebuild = false; - void -mcgroup_index_reset(void) -{ - full_mc_rebuild = true; -} - -void -mcgroup_index_init(struct mcgroup_index *mcgroups) +mcgroup_index_init(struct mcgroup_index *mcgroups, struct ovsdb_idl *ovnsb_idl) { hmap_init(&mcgroups->by_dp_name); - hmap_init(&mcgroups->by_uuid); -} -void -mcgroup_index_remove(struct mcgroup_index *mcgroups, const struct uuid *uuid) -{ - const struct mcgroup *mcgroup_ = mcgroup_lookup_by_uuid(mcgroups, uuid); - struct mcgroup *mcgroup = CONST_CAST(struct mcgroup *, mcgroup_); - if (mcgroup) { - hmap_remove(&mcgroups->by_dp_name, &mcgroup->dp_name_node); - hmap_remove(&mcgroups->by_uuid, &mcgroup->uuid_node); - free(mcgroup); + const struct sbrec_multicast_group *mg; + SBREC_MULTICAST_GROUP_FOR_EACH (mg, ovnsb_idl) { + const struct uuid *dp_uuid = &mg->datapath->header_.uuid; + if (mcgroup_lookup_by_dp_name(mcgroups, mg->datapath, mg->name)) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "datapath "UUID_FMT" contains duplicate " + "multicast group '%s'", UUID_ARGS(dp_uuid), mg->name); + continue; + } + + struct mcgroup *m = xmalloc(sizeof *m); + hmap_insert(&mcgroups->by_dp_name, &m->dp_name_node, + hash_string(mg->name, uuid_hash(dp_uuid))); + m->mg = mg; } } void -mcgroup_index_clear(struct mcgroup_index *mcgroups) +mcgroup_index_destroy(struct mcgroup_index *mcgroups) { struct mcgroup *mcgroup, *next; HMAP_FOR_EACH_SAFE (mcgroup, next, dp_name_node, &mcgroups->by_dp_name) { hmap_remove(&mcgroups->by_dp_name, &mcgroup->dp_name_node); - hmap_remove(&mcgroups->by_uuid, &mcgroup->uuid_node); free(mcgroup); } -} - -static void -consider_mcgroup_index(struct mcgroup_index *mcgroups, - const struct sbrec_multicast_group *mg) -{ - const struct uuid *dp_uuid = &mg->datapath->header_.uuid; - if (mcgroup_lookup_by_dp_name(mcgroups, mg->datapath, mg->name)) { - return; - } - - struct mcgroup *m = xmalloc(sizeof *m); - hmap_insert(&mcgroups->by_dp_name, &m->dp_name_node, - hash_string(mg->name, uuid_hash(dp_uuid))); - hmap_insert(&mcgroups->by_uuid, &m->uuid_node, - uuid_hash(&mg->header_.uuid)); - m->uuid = &mg->header_.uuid; - m->mg = mg; -} - -void -mcgroup_index_fill(struct mcgroup_index *mcgroups, struct ovsdb_idl *ovnsb_idl) -{ - const struct sbrec_multicast_group *mg; - if (full_mc_rebuild) { - mcgroup_index_clear(mcgroups); - SBREC_MULTICAST_GROUP_FOR_EACH (mg, ovnsb_idl) { - consider_mcgroup_index(mcgroups, mg); - } - full_mc_rebuild = false; - } else { - SBREC_MULTICAST_GROUP_FOR_EACH_TRACKED (mg, ovnsb_idl) { - bool is_delete = sbrec_multicast_group_row_get_seqno(mg, - OVSDB_IDL_CHANGE_DELETE) > 0; - - if (is_delete) { - mcgroup_index_remove(mcgroups, &mg->header_.uuid); - continue; - } - consider_mcgroup_index(mcgroups, mg); - } - } -} - -void -mcgroup_index_destroy(struct mcgroup_index *mcgroups) -{ - mcgroup_index_clear(mcgroups); hmap_destroy(&mcgroups->by_dp_name); } -const struct mcgroup * -mcgroup_lookup_by_uuid(const struct mcgroup_index *mcgroups, - const struct uuid *uuid) -{ - const struct mcgroup *mcgroup; - HMAP_FOR_EACH_WITH_HASH (mcgroup, uuid_node, uuid_hash(uuid), - &mcgroups->by_uuid) { - if (uuid_equals(mcgroup->uuid, uuid)) { - return mcgroup; - } - } - return NULL; -} - const struct sbrec_multicast_group * mcgroup_lookup_by_dp_name(const struct mcgroup_index *mcgroups, const struct sbrec_datapath_binding *dp, diff --git a/ovn/controller/lport.h b/ovn/controller/lport.h index 33f81d5..f09e2eb 100644 --- a/ovn/controller/lport.h +++ b/ovn/controller/lport.h @@ -18,7 +18,6 @@ #include #include "hmap.h" -#include "uuid.h" struct ovsdb_idl; struct sbrec_datapath_binding; @@ -33,24 +32,15 @@ struct sbrec_datapath_binding; struct lport_index { struct hmap by_name; struct hmap by_key; - struct hmap by_uuid; }; -void lport_index_reset(void); -void lport_index_init(struct lport_index *); -void lport_index_fill(struct lport_index *, struct ovsdb_idl *); -void lport_index_remove(struct lport_index *, const struct uuid *); -void lport_index_clear(struct lport_index *); +void lport_index_init(struct lport_index *, struct ovsdb_idl *); void lport_index_destroy(struct lport_index *); const struct sbrec_port_binding *lport_lookup_by_name( const struct lport_index *, const char *name); const struct sbrec_port_binding *lport_lookup_by_key( const struct lport_index *, uint32_t dp_key, uint16_t port_key); - -const struct lport *lport_lookup_by_uuid( - const struct lport_index *, const struct uuid *uuid); - /* Multicast group index * ===================== @@ -64,14 +54,9 @@ const struct lport *lport_lookup_by_uuid( struct mcgroup_index { struct hmap by_dp_name; - struct hmap by_uuid; }; -void mcgroup_index_reset(void); -void mcgroup_index_init(struct mcgroup_index *); -void mcgroup_index_fill(struct mcgroup_index *, struct ovsdb_idl *); -void mcgroup_index_remove(struct mcgroup_index *, const struct uuid *); -void mcgroup_index_clear(struct mcgroup_index *); +void mcgroup_index_init(struct mcgroup_index *, struct ovsdb_idl *); void mcgroup_index_destroy(struct mcgroup_index *); const struct sbrec_multicast_group *mcgroup_lookup_by_dp_name( @@ -79,7 +64,4 @@ const struct sbrec_multicast_group *mcgroup_lookup_by_dp_name( const struct sbrec_datapath_binding *, const char *name); -const struct mcgroup *mcgroup_lookup_by_uuid( - const struct mcgroup_index *, const struct uuid *uuid); - #endif /* ovn/lport.h */ diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 47e6824..8620a71 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -327,9 +327,6 @@ update_ct_zones(struct sset *lports, struct hmap *patched_datapaths, static struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths); static struct hmap patched_datapaths = HMAP_INITIALIZER(&patched_datapaths); -static struct lport_index lports; -static struct mcgroup_index mcgroups; - int main(int argc, char *argv[]) { @@ -360,9 +357,6 @@ main(int argc, char *argv[]) pinctrl_init(); lflow_init(); - lport_index_init(&lports); - mcgroup_index_init(&mcgroups); - /* Connect to OVS OVSDB instance. We do not monitor all tables by * default, so modules must register their interest explicitly. */ struct ovsdb_idl_loop ovs_idl_loop = OVSDB_IDL_LOOP_INITIALIZER( @@ -423,8 +417,6 @@ main(int argc, char *argv[]) ovnsb_remote = new_ovnsb_remote; ovsdb_idl_set_remote(ovnsb_idl_loop.idl, ovnsb_remote, true); binding_reset_processing(); - lport_index_clear(&lports); - mcgroup_index_clear(&mcgroups); } else { free(new_ovnsb_remote); } @@ -451,8 +443,10 @@ main(int argc, char *argv[]) patch_run(&ctx, br_int, chassis_id, &local_datapaths, &patched_datapaths); - lport_index_fill(&lports, ctx.ovnsb_idl); - mcgroup_index_fill(&mcgroups, ctx.ovnsb_idl); + struct lport_index lports; + struct mcgroup_index mcgroups; + lport_index_init(&lports, ctx.ovnsb_idl); + mcgroup_index_init(&mcgroups, ctx.ovnsb_idl); enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int); @@ -470,6 +464,8 @@ main(int argc, char *argv[]) } ofctrl_put(&flow_table); hmap_destroy(&flow_table); + mcgroup_index_destroy(&mcgroups); + lport_index_destroy(&lports); } sset_destroy(&all_lports);