From patchwork Tue Sep 8 18:51:52 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 515520 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from archives.nicira.com (unknown [IPv6:2600:3c00::f03c:91ff:fe6e:bdf7]) by ozlabs.org (Postfix) with ESMTP id 9897D1400CB for ; Wed, 9 Sep 2015 04:51:51 +1000 (AEST) Received: from archives.nicira.com (localhost [127.0.0.1]) by archives.nicira.com (Postfix) with ESMTP id 03508105C9; Tue, 8 Sep 2015 11:51:49 -0700 (PDT) X-Original-To: dev@openvswitch.org Delivered-To: dev@openvswitch.org Received: from mx3v1.cudamail.com (mx3.cudamail.com [64.34.241.5]) by archives.nicira.com (Postfix) with ESMTPS id B0537105C0 for ; Tue, 8 Sep 2015 11:51:47 -0700 (PDT) Received: from bar4.cudamail.com (bar2 [192.168.15.2]) by mx3v1.cudamail.com (Postfix) with ESMTP id 1F9526190F6 for ; Tue, 8 Sep 2015 12:51:47 -0600 (MDT) X-ASG-Debug-ID: 1441738306-03dc21257705a90001-byXFYA Received: from mx3-pf3.cudamail.com ([192.168.14.3]) by bar4.cudamail.com with ESMTP id H6kv3Z0ykAmToMWK (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Tue, 08 Sep 2015 12:51:46 -0600 (MDT) X-Barracuda-Envelope-From: blp@nicira.com X-Barracuda-RBL-Trusted-Forwarder: 192.168.14.3 Received: from unknown (HELO mail-pa0-f50.google.com) (209.85.220.50) by mx3-pf3.cudamail.com with ESMTPS (RC4-SHA encrypted); 8 Sep 2015 18:51:44 -0000 Received-SPF: unknown (mx3-pf3.cudamail.com: Multiple SPF records returned) X-Barracuda-Apparent-Source-IP: 209.85.220.50 X-Barracuda-RBL-IP: 209.85.220.50 Received: by pacfv12 with SMTP id fv12so134385644pac.2 for ; Tue, 08 Sep 2015 11:51:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=MHD94Ivze4SQUpEMAnwq3BPWMIG2LPJTTXbcl8Nd2C0=; b=ewzMS6xG9uBKMge+r7ltO6ymFu17W56LC0InCTS87glkIjGV2qtTwSonvVBJtnKee1 aOTRdz3bjY0ahRDRvQ2GPhGIc+P0T4lp4wWdMSjIsvTurE60fOuvYrwMLbEt66b9KjDc 2NNByIocBwl2USxfFm4+DnnfS+5LeuT5Penke8cuhOOnlsDsf3LH9MHZ9EOV45m5bBTX g2p4JgwNc5vJCijjaYCi59V5DtZxZol0rTOYMQyKVo3+84WCMGDH3jiXYUoqn4zLZhLk 0VcB+sw46NK1S7PMot0KRoZ9rzfaZPDpXB49JwVocEBHm9MoBV5wAzXMF6aYTfWexh+M 5k4A== X-Gm-Message-State: ALoCoQnaV6C+1De+Vf0Nr+2fTFcCIxtYKFwjfNkIPEfd5iMrVWHz89bJVVrbRfxeUrML5mUg9aPa X-Received: by 10.66.219.5 with SMTP id pk5mr51118334pac.111.1441738303967; Tue, 08 Sep 2015 11:51:43 -0700 (PDT) Received: from nicira.com (173-228-112-165.dsl.dynamic.fusionbroadband.com. [173.228.112.165]) by smtp.gmail.com with ESMTPSA id kw3sm4266391pbc.30.2015.09.08.11.51.42 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 08 Sep 2015 11:51:42 -0700 (PDT) Date: Tue, 8 Sep 2015 11:51:52 -0700 X-CudaMail-Envelope-Sender: blp@nicira.com From: Ben Pfaff To: Russell Bryant X-CudaMail-Whitelist-To: dev@openvswitch.org X-CudaMail-MID: CM-V3-907044835 X-CudaMail-DTE: 090815 X-CudaMail-Originating-IP: 209.85.220.50 Message-ID: <20150908185152.GD3766@nicira.com> X-ASG-Orig-Subj: [##CM-V3-907044835##]Re: [ovs-dev] [PATCH v8 2/2] ovn: Add "localnet" logical port type. References: <1440601674-27185-1-git-send-email-rbryant@redhat.com> <1441298701-32163-1-git-send-email-rbryant@redhat.com> <1441298701-32163-3-git-send-email-rbryant@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1441298701-32163-3-git-send-email-rbryant@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Barracuda-Connect: UNKNOWN[192.168.14.3] X-Barracuda-Start-Time: 1441738306 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 Cc: dev@openvswitch.org Subject: Re: [ovs-dev] [PATCH v8 2/2] ovn: Add "localnet" logical port type. 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: , Errors-To: dev-bounces@openvswitch.org Sender: "dev" On Thu, Sep 03, 2015 at 12:45:01PM -0400, Russell Bryant wrote: > Introduce a new logical port type called "localnet". A logical port > with this type also has an option called "network_name". A "localnet" > logical port represents a connection to a network that is locally > accessible from each chassis running ovn-controller. ovn-controller > will use the ovn-bridge-mappings configuration to figure out which > patch port on br-int should be used for this port. Thanks! I folded in the following incrementals, which are mostly for style (I updated one comment that had rotted, adjusted line lengths, etc.). Please do read them to make sure that you agree. Then I applied this to master. diff --git a/ovn/controller/physical.c b/ovn/controller/physical.c index ba2cddf..bdb02da 100644 --- a/ovn/controller/physical.c +++ b/ovn/controller/physical.c @@ -214,22 +214,34 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, ofp_port_t ofport; struct ovs_list bindings; }; - /* A list of localnet port bindings hashed by network name */ + /* Maps from network name to "struct localnet_bindings". */ struct shash localnet_inputs = SHASH_INITIALIZER(&localnet_inputs); - /* Datapaths with at least one local port binding */ + /* Contains bare "struct hmap_node"s whose hash values are the tunnel_key + * of datapaths with at least one local port binding. */ struct hmap local_datapaths = HMAP_INITIALIZER(&local_datapaths); /* Set up flows in table 0 for physical-to-logical translation and in table * 64 for logical-to-physical translation. */ const struct sbrec_port_binding *binding; SBREC_PORT_BINDING_FOR_EACH (binding, ctx->ovnsb_idl) { - /* Find the OpenFlow port for the logical port, as 'ofport'. If it's - * on a remote chassis, this is the OpenFlow port for the tunnel to - * that chassis (and set 'local' to false). Otherwise, if it's on the - * chassis we're managing, this is the OpenFlow port for the vif itself - * (and set 'local' to true). When 'parent_port' is set for a binding, - * it implies a container sitting inside a VM reachable via a 'tag'. + /* Find the OpenFlow port for the logical port, as 'ofport'. This is + * one of: + * + * - If the port is a VIF on the chassis we're managing, the + * OpenFlow port for the VIF. 'tun' will be NULL. + * + * In this or the next case, for a container nested inside a VM + * and accessible via a VLAN, 'tag' is the VLAN ID; otherwise + * 'tag' is 0. + * + * - If the port is on a remote chassis, the OpenFlow port for a + * tunnel to the VIF's remote chassis. 'tun' identifies that + * tunnel. + * + * - If the port is a "localnet" port for a network that is + * attached to the chassis we're managing, the OpenFlow port for + * the localnet port (a patch port). */ int tag = 0; @@ -287,18 +299,17 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, * attached to more than one logical datapath, so keep track of * all associated bindings and add a flow at the end. */ - const char *network = smap_get(&binding->options, "network_name"); - struct shash_node *node; + const char *network + = smap_get(&binding->options, "network_name"); struct localnet_bindings *ln_bindings; - node = shash_find(&localnet_inputs, network); - if (!node) { + ln_bindings = shash_find_data(&localnet_inputs, network); + if (!ln_bindings) { ln_bindings = xmalloc(sizeof *ln_bindings); ln_bindings->ofport = ofport; list_init(&ln_bindings->bindings); - node = shash_add(&localnet_inputs, network, ln_bindings); + shash_add(&localnet_inputs, network, ln_bindings); } - ln_bindings = node->data; struct binding_elem *b = xmalloc(sizeof *b); b->binding = binding; @@ -594,14 +605,14 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, /* Table 0, Priority 100 * ===================== * - * We have now determined the full set of port bindings associated with each - * "localnet" network. Only create flows for datapaths that have another - * local binding. Otherwise, we know it would just be dropped. + * We have now determined the full set of port bindings associated with + * each "localnet" network. Only create flows for datapaths that have + * another local binding. Otherwise, we know it would just be dropped. */ struct shash_node *ln_bindings_node, *ln_bindings_node_next; - struct localnet_bindings *ln_bindings; - SHASH_FOR_EACH_SAFE (ln_bindings_node, ln_bindings_node_next, &localnet_inputs) { - ln_bindings = ln_bindings_node->data; + SHASH_FOR_EACH_SAFE (ln_bindings_node, ln_bindings_node_next, + &localnet_inputs) { + struct localnet_bindings *ln_bindings = ln_bindings_node->data; struct match match; match_init_catchall(&match); @@ -610,16 +621,12 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, struct ofpbuf ofpacts; ofpbuf_init(&ofpacts, 0); - bool found_local = false; struct binding_elem *b; LIST_FOR_EACH_POP (b, list_elem, &ln_bindings->bindings) { struct hmap_node *ld; ld = hmap_first_with_hash(&local_datapaths, b->binding->datapath->tunnel_key); if (ld) { - /* This datapath has local port bindings. */ - found_local = true; - /* Set MFF_LOG_DATAPATH and MFF_LOG_INPORT. */ put_load(b->binding->datapath->tunnel_key, MFF_LOG_DATAPATH, 0, 64, &ofpacts); @@ -631,7 +638,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve, free(b); } - if (found_local) { + if (ofpacts.size) { ofctrl_add_flow(flow_table, 0, 100, &match, &ofpacts); } diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml index 0dfdab5..42a94b9 100644 --- a/ovn/ovn-nb.xml +++ b/ovn/ovn-nb.xml @@ -116,24 +116,26 @@

- When this column is set to localnet, this logical port represents a - connection to a locally accessible network from each ovn-controller instance. - A logical switch can only have a single localnet port attached - and at most one regular logical port. This is used to model direct - connectivity to an existing network. + When this column is set to localnet, this logical port + represents a connection to a locally accessible network from each + ovn-controller instance. A logical switch can only have a + single localnet port attached and at most one regular + logical port. This is used to model direct connectivity to an existing + network.

- This column provides key/value settings specific to the logical port - . + This column provides key/value settings specific to the logical port + .

- When is set to localnet, you must set the option - network_name. ovn-controller uses local configuration to determine - exactly how to connect to this locally accessible network. + When is set to localnet, you must set + the option network_name. ovn-controller uses + local configuration to determine exactly how to connect to this locally + accessible network.

diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index d696745..74631f9 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -964,39 +964,42 @@

- When this column is set to localnet, this logical port represents a - connection to a locally accessible network from each ovn-controller instance. - A logical switch can only have a single localnet port attached - and at most one regular logical port. This is used to model direct - connectivity to an existing network. + When this column is set to localnet, this logical port + represents a connection to a locally accessible network from each + ovn-controller instance. A logical switch can only have a single + localnet port attached and at most one regular logical + port. This is used to model direct connectivity to an existing + network.

- This column provides key/value settings specific to the logical port - . + This column provides key/value settings specific to the logical port + .

- When is set to localnet, you must set the option - network_name. ovn-controller uses the configuration entry - ovn-bridge-mappings to determine how to connect to this network. - ovn-bridge-mappings is a list of network names mapped to a local - OVS bridge that provides access to that network. An example of configuring - ovn-bridge-mappings would be: + When is set to localnet, you must set + the option network_name. ovn-controller uses + the configuration entry ovn-bridge-mappings to determine + how to connect to this network. ovn-bridge-mappings is a + list of network names mapped to a local OVS bridge that provides access + to that network. An example of configuring + ovn-bridge-mappings would be:

- $ ovs-vsctl set open . external-ids:ovn-bridge-mappings=physnet1:br-eth0,physnet2:br-eth1 + $ ovs-vsctl set open + . external-ids:ovn-bridge-mappings=physnet1:br-eth0,physnet2:br-eth1

- Also note that when a logical switch has a localnet port attached, - every chassis that may have a local vif attached to that logical switch - must have a bridge mapping configured to reach that localnet. - Traffic that arrives on a localnet port is never forwarded over a tunnel - to another chassis. + Also note that when a logical switch has a localnet port + attached, every chassis that may have a local vif attached to that + logical switch must have a bridge mapping configured to reach that + localnet. Traffic that arrives on a localnet + port is never forwarded over a tunnel to another chassis.