From patchwork Mon Oct 26 18:16:20 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 1387965 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=fraxinus.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CKjhl41Z1z9sSW for ; Tue, 27 Oct 2020 05:16:39 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 25F10855DF; Mon, 26 Oct 2020 18:16:38 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id aeXWcFjmyQmj; Mon, 26 Oct 2020 18:16:37 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 8E8E1852E9; Mon, 26 Oct 2020 18:16:37 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 70B5AC0859; Mon, 26 Oct 2020 18:16:37 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 14FFCC0051 for ; Mon, 26 Oct 2020 18:16:37 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id F17D78700C for ; Mon, 26 Oct 2020 18:16:36 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id WJWmgWnbICuO for ; Mon, 26 Oct 2020 18:16:36 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by hemlock.osuosl.org (Postfix) with ESMTPS id 046A58700B for ; Mon, 26 Oct 2020 18:16:35 +0000 (UTC) Received: from sigfpe.attlocal.net (75-54-222-30.lightspeed.rdcyca.sbcglobal.net [75.54.222.30]) (Authenticated sender: blp@ovn.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id AE15D100008; Mon, 26 Oct 2020 18:16:31 +0000 (UTC) From: Ben Pfaff To: dev@openvswitch.org Date: Mon, 26 Oct 2020 11:16:20 -0700 Message-Id: <20201026181626.1827014-1-blp@ovn.org> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 Cc: Ben Pfaff Subject: [ovs-dev] [PATCH ovn 1/7] northd: Count mask length and priority correctly for IPv6 addresses. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Before this commit, the IPv4 calculation was used even for IPv6 addresses, which was wrong. Signed-off-by: Ben Pfaff Acked-by: Numan Siddique --- northd/ovn-northd.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 59b7b3d2ee3a..d677f357f5d0 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -9303,10 +9303,13 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, /* Check the validity of nat->logical_ip. 'logical_ip' can * be a subnet when the type is "snat". */ + int cidr_bits; if (is_v6) { error = ipv6_parse_masked(nat->logical_ip, &ipv6, &mask_v6); + cidr_bits = ipv6_count_cidr_bits(&mask_v6); } else { error = ip_parse_masked(nat->logical_ip, &ip, &mask); + cidr_bits = ip_count_cidr_bits(mask); } if (!strcmp(nat->type, "snat")) { if (error) { @@ -9612,11 +9615,11 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, * nat->logical_ip with the longest mask gets a higher * priority. */ ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_SNAT, - count_1bits(ntohl(mask)) + 1, + cidr_bits + 1, ds_cstr(&match), ds_cstr(&actions), &nat->header_); } else { - uint16_t priority = count_1bits(ntohl(mask)) + 1; + uint16_t priority = cidr_bits + 1; /* Distributed router. */ ds_clear(&match); From patchwork Mon Oct 26 18:16:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 1387968 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.136; helo=silver.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CKjhv0Ykzz9sSW for ; Tue, 27 Oct 2020 05:16:47 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 527E32E14A; Mon, 26 Oct 2020 18:16:45 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id xUnUJ4XI-iyC; Mon, 26 Oct 2020 18:16:40 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 4C3042E13D; Mon, 26 Oct 2020 18:16:40 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 2F1A5C0859; Mon, 26 Oct 2020 18:16:40 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id A098DC0051 for ; Mon, 26 Oct 2020 18:16:38 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id 9A7DA866D4 for ; Mon, 26 Oct 2020 18:16:38 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hUhkDBv2Zqqe for ; Mon, 26 Oct 2020 18:16:37 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by whitealder.osuosl.org (Postfix) with ESMTPS id 4D6AD86499 for ; Mon, 26 Oct 2020 18:16:37 +0000 (UTC) Received: from sigfpe.attlocal.net (75-54-222-30.lightspeed.rdcyca.sbcglobal.net [75.54.222.30]) (Authenticated sender: blp@ovn.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id 16701100009; Mon, 26 Oct 2020 18:16:32 +0000 (UTC) From: Ben Pfaff To: dev@openvswitch.org Date: Mon, 26 Oct 2020 11:16:21 -0700 Message-Id: <20201026181626.1827014-2-blp@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20201026181626.1827014-1-blp@ovn.org> References: <20201026181626.1827014-1-blp@ovn.org> MIME-Version: 1.0 Cc: Ben Pfaff Subject: [ovs-dev] [PATCH ovn 2/7] northd: Add missing space in log message. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Signed-off-by: Ben Pfaff Fixes: fc79d690b9e5 ("External IP based NAT: NORTHD changes to use allowed/exempted external ip") Acked-by: Numan Siddique --- northd/ovn-northd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index d677f357f5d0..4457cdc2d392 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -9276,7 +9276,7 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, if (allowed_ext_ips && exempted_ext_ips) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, "NAT rule: "UUID_FMT" not applied, since" + VLOG_WARN_RL(&rl, "NAT rule: "UUID_FMT" not applied, since " "both allowed and exempt external ips set", UUID_ARGS(&(nat->header_.uuid))); continue; From patchwork Mon Oct 26 18:16:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 1387966 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=fraxinus.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CKjhs12gDz9sSW for ; Tue, 27 Oct 2020 05:16:45 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 6A7C485A74; Mon, 26 Oct 2020 18:16:43 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id M53Fv7Llrh4D; Mon, 26 Oct 2020 18:16:41 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 6939B85EBE; Mon, 26 Oct 2020 18:16:41 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 453D1C08A1; Mon, 26 Oct 2020 18:16:41 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from whitealder.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id DD8A5C0051 for ; Mon, 26 Oct 2020 18:16:38 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id CC16F866C7 for ; Mon, 26 Oct 2020 18:16:38 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from whitealder.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id pqLfabfDNKVU for ; Mon, 26 Oct 2020 18:16:37 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by whitealder.osuosl.org (Postfix) with ESMTPS id 05BAE85FAC for ; Mon, 26 Oct 2020 18:16:36 +0000 (UTC) Received: from sigfpe.attlocal.net (75-54-222-30.lightspeed.rdcyca.sbcglobal.net [75.54.222.30]) (Authenticated sender: blp@ovn.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id 6678D10000B; Mon, 26 Oct 2020 18:16:34 +0000 (UTC) From: Ben Pfaff To: dev@openvswitch.org Date: Mon, 26 Oct 2020 11:16:22 -0700 Message-Id: <20201026181626.1827014-3-blp@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20201026181626.1827014-1-blp@ovn.org> References: <20201026181626.1827014-1-blp@ovn.org> MIME-Version: 1.0 Cc: Ben Pfaff Subject: [ovs-dev] [PATCH ovn 3/7] northd: Improve comments. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Signed-off-by: Ben Pfaff Acked-by: Numan Siddique --- northd/ovn-northd.c | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 4457cdc2d392..e69845fbb219 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -1386,9 +1386,29 @@ build_datapaths(struct northd_context *ctx, struct hmap *datapaths, } } +/* A logical switch port or logical router port. + * + * In steady state, an ovn_port points to a northbound Logical_Switch_Port + * record (via 'nbsp') *or* a Logical_Router_Port record (via 'nbrp'), and to a + * southbound Port_Binding record (via 'sb'). As the state of the system + * changes, join_logical_ports() may determine that there is a new LSP or LRP + * that has no corresponding Port_Binding record (in which case build_ports()) + * will create the missing Port_Binding) or that a Port_Binding record exists + * that has no coresponding LSP (in which case build_ports() will delete the + * spurious Port_Binding). Thus, after build_ports() runs, any given ovn_port + * will have 'sb' nonnull, and 'nbsp' xor 'nbrp' nonnull. + * + * Ordinarily there is only one ovn_port that points to a given LSP or LRP (but + * distributed gateway ports point a "derived" ovn_port to a duplicate LRP). + */ struct ovn_port { + /* Port name aka key. + * + * This is ordinarily the same as nbsp->name or nbrp->name and + * sb->logical_port. (A distributed gateway port creates a "derived" + * ovn_port with key "cr-%s" % nbrp->name.) */ struct hmap_node key_node; /* Index on 'key'. */ - char *key; /* nbs->name, nbr->name, sb->logical_port. */ + char *key; /* nbsp->name, nbrp->name, sb->logical_port. */ char *json_key; /* 'key', quoted for use in JSON. */ const struct sbrec_port_binding *sb; /* May be NULL. */ @@ -1410,15 +1430,20 @@ struct ovn_port { /* Logical port multicast data. */ struct mcast_port_info mcast_info; - bool derived; /* Indicates whether this is an additional port - * derived from nbsp or nbrp. */ + /* This is ordinarily false. It is true iff this ovn_port is derived from + * a chassis-redirect port. */ + bool derived; + bool has_unknown; /* If the addresses have 'unknown' defined. */ + /* The port's peer: * * - A switch port S of type "router" has a router port R as a peer, * and R in turn has S has its peer. * - * - Two connected logical router ports have each other as peer. */ + * - Two connected logical router ports have each other as peer. + * + * - Other kinds of ports have no peer. */ struct ovn_port *peer; struct ovn_datapath *od; From patchwork Mon Oct 26 18:16:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 1387969 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=fraxinus.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CKjhx2ZDBz9sT6 for ; Tue, 27 Oct 2020 05:16:49 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id EDDA885C95; Mon, 26 Oct 2020 18:16:47 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id F2uVxYGJTNg4; Mon, 26 Oct 2020 18:16:45 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 23EDF85FB4; Mon, 26 Oct 2020 18:16:45 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id EE9A8C1AD8; Mon, 26 Oct 2020 18:16:44 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 10822C0051 for ; Mon, 26 Oct 2020 18:16:42 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 002652E141 for ; Mon, 26 Oct 2020 18:16:41 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id MnyByBGlRHii for ; Mon, 26 Oct 2020 18:16:39 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by silver.osuosl.org (Postfix) with ESMTPS id 6058C2E134 for ; Mon, 26 Oct 2020 18:16:38 +0000 (UTC) Received: from sigfpe.attlocal.net (75-54-222-30.lightspeed.rdcyca.sbcglobal.net [75.54.222.30]) (Authenticated sender: blp@ovn.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id C481C10000D; Mon, 26 Oct 2020 18:16:35 +0000 (UTC) From: Ben Pfaff To: dev@openvswitch.org Date: Mon, 26 Oct 2020 11:16:23 -0700 Message-Id: <20201026181626.1827014-4-blp@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20201026181626.1827014-1-blp@ovn.org> References: <20201026181626.1827014-1-blp@ovn.org> MIME-Version: 1.0 Cc: Ben Pfaff Subject: [ovs-dev] [PATCH ovn 4/7] northd: Don't redundantly set Port_Binding options. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" This is called twice within a few lines of code. Signed-off-by: Ben Pfaff Acked-by: Numan Siddique --- northd/ovn-northd.c | 1 - 1 file changed, 1 deletion(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index e69845fbb219..f25f5cd82f39 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -10223,7 +10223,6 @@ build_ND_RA_flows_for_lrouter_port( } smap_add(&options, "ipv6_prefix_delegation", prefix_delegation ? "true" : "false"); - sbrec_port_binding_set_options(op->sb, &options); bool ipv6_prefix = smap_get_bool(&op->nbrp->options, "prefix", false); From patchwork Mon Oct 26 18:16:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 1387967 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.137; helo=fraxinus.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CKjhv0s09z9sT6 for ; Tue, 27 Oct 2020 05:16:47 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 94BC685B3D; Mon, 26 Oct 2020 18:16:45 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ZPaSGpNvGyee; Mon, 26 Oct 2020 18:16:43 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id BED1685F66; Mon, 26 Oct 2020 18:16:43 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id A3791C1ADC; Mon, 26 Oct 2020 18:16:43 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 99749C1AD8 for ; Mon, 26 Oct 2020 18:16:40 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 8B7F187093 for ; Mon, 26 Oct 2020 18:16:40 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 56K2T5dsREQR for ; Mon, 26 Oct 2020 18:16:39 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by hemlock.osuosl.org (Postfix) with ESMTPS id 6431687053 for ; Mon, 26 Oct 2020 18:16:39 +0000 (UTC) Received: from sigfpe.attlocal.net (75-54-222-30.lightspeed.rdcyca.sbcglobal.net [75.54.222.30]) (Authenticated sender: blp@ovn.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id 24CD6100008; Mon, 26 Oct 2020 18:16:36 +0000 (UTC) From: Ben Pfaff To: dev@openvswitch.org Date: Mon, 26 Oct 2020 11:16:24 -0700 Message-Id: <20201026181626.1827014-5-blp@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20201026181626.1827014-1-blp@ovn.org> References: <20201026181626.1827014-1-blp@ovn.org> MIME-Version: 1.0 Cc: Ben Pfaff Subject: [ovs-dev] [PATCH ovn 5/7] northd: Use address set for service monitor MAC. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" Until now, the service monitor MAC has been inlined into logical flow matches. This makes it a little hard to compare flow tables from one run or deployment to another, since the service monitor MAC is random and will always differ. This commit changes flow matches to use an address set $svc_monitor_mac everywhere that it can be. This makes the flow matches the same in every deployment. The service monitor MAC is also used in actions to set Ethernet addresses. This can't be replaced by an address set, so these flows will still have some differences. Signed-off-by: Ben Pfaff Acked-by: Numan Siddique --- northd/ovn-northd.c | 43 +++++++++++++++++-------------------------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index f25f5cd82f39..b96e0db516be 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -5004,15 +5004,11 @@ build_pre_acls(struct ovn_datapath *od, struct hmap *lflows) ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 0, "1", "next;"); ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 0, "1", "next;"); - char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); - ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, svc_check_match, - "next;"); - free(svc_check_match); + ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110, + "eth.dst == $svc_monitor_mac", "next;"); - svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); - ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, svc_check_match, - "next;"); - free(svc_check_match); + ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_ACL, 110, + "eth.src == $svc_monitor_mac", "next;"); /* If there are any stateful ACL rules in this datapath, we must * send all IP packets through the conntrack action, which handles @@ -5170,15 +5166,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap *lflows, "next;"); /* Do not send service monitor packets to conntrack. */ - char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110, - svc_check_match, "next;"); - free(svc_check_match); - - svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); + "eth.dst == $svc_monitor_mac", "next;"); ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110, - svc_check_match, "next;"); - free(svc_check_match); + "eth.src == $svc_monitor_mac", "next;"); /* Allow all packets to go to next tables by default. */ ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 0, "1", "next;"); @@ -5831,17 +5822,13 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows, /* Add a 34000 priority flow to advance the service monitor reply * packets to skip applying ingress ACLs. */ - char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); - ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, 34000, svc_check_match, - "next;"); - free(svc_check_match); + ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, 34000, + "eth.dst == $svc_monitor_mac", "next;"); /* Add a 34000 priority flow to advance the service monitor packets * generated by ovn-controller to skip applying egress ACLs. */ - svc_check_match = xasprintf("eth.src == %s", svc_monitor_mac); - ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, 34000, svc_check_match, - "next;"); - free(svc_check_match); + ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, 34000, + "eth.src == $svc_monitor_mac", "next;"); } static void @@ -7172,7 +7159,6 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports, } } - char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac); /* Ingress table 19: Destination lookup, broadcast and multicast handling * (priority 70 - 100). */ HMAP_FOR_EACH (od, key_node, datapaths) { @@ -7180,7 +7166,8 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports, continue; } - ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 110, svc_check_match, + ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 110, + "eth.dst == $svc_monitor_mac", "handle_svc_check(inport);"); struct mcast_switch_info *mcast_sw_info = &od->mcast_info.sw; @@ -7253,7 +7240,6 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports, ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 70, "eth.mcast", "outport = \""MC_FLOOD"\"; output;"); } - free(svc_check_match); /* Ingress table 19: Add IP multicast flows learnt from IGMP/MLD * (priority 90). */ @@ -11537,6 +11523,11 @@ sync_address_sets(struct northd_context *ctx) shash_add(&sb_address_sets, sb_address_set->name, sb_address_set); } + /* Service monitor MAC. */ + const char *svc_monitor_macp = svc_monitor_mac; + sync_address_set(ctx, "svc_monitor_mac", &svc_monitor_macp, 1, + &sb_address_sets); + /* sync port group generated address sets first */ const struct nbrec_port_group *nb_port_group; NBREC_PORT_GROUP_FOR_EACH (nb_port_group, ctx->ovnnb_idl) { From patchwork Mon Oct 26 18:16:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 1387971 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.133; helo=hemlock.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CKjjB32stz9sSW for ; Tue, 27 Oct 2020 05:17:02 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id C92078729A; Mon, 26 Oct 2020 18:17:00 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id pt8SCsr5c6u2; Mon, 26 Oct 2020 18:16:57 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by hemlock.osuosl.org (Postfix) with ESMTP id 07C2987275; Mon, 26 Oct 2020 18:16:57 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C41FCC1DA0; Mon, 26 Oct 2020 18:16:56 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 80D22C1AE0 for ; Mon, 26 Oct 2020 18:16:54 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 610542E14B for ; Mon, 26 Oct 2020 18:16:54 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tl1N0TIIxqrX for ; Mon, 26 Oct 2020 18:16:41 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by silver.osuosl.org (Postfix) with ESMTPS id EF6552E140 for ; Mon, 26 Oct 2020 18:16:40 +0000 (UTC) Received: from sigfpe.attlocal.net (75-54-222-30.lightspeed.rdcyca.sbcglobal.net [75.54.222.30]) (Authenticated sender: blp@ovn.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id 7C5AE10000B; Mon, 26 Oct 2020 18:16:38 +0000 (UTC) From: Ben Pfaff To: dev@openvswitch.org Date: Mon, 26 Oct 2020 11:16:25 -0700 Message-Id: <20201026181626.1827014-6-blp@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20201026181626.1827014-1-blp@ovn.org> References: <20201026181626.1827014-1-blp@ovn.org> MIME-Version: 1.0 Cc: Ben Pfaff Subject: [ovs-dev] [PATCH ovn 6/7] northd: Enhance implementation of datapath tunnel key requests. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" When a tunnel key was requested, the implementation was not smart enough to reassign a datapath that had been automatically assigned the same key. This fixes the problem and adds a test. I didn't see a reason not to make this feature available for logical routers so I added that feature too. Signed-off-by: Ben Pfaff Acked-by: Numan Siddique --- lib/ovn-util.c | 26 ++++------ lib/ovn-util.h | 3 +- northd/ovn-northd.c | 124 ++++++++++++++++++++++++-------------------- ovn-nb.xml | 8 +++ tests/ovn-northd.at | 46 ++++++++++++++++ 5 files changed, 135 insertions(+), 72 deletions(-) diff --git a/lib/ovn-util.c b/lib/ovn-util.c index 1daf6650379f..321fc89e275a 100644 --- a/lib/ovn-util.c +++ b/lib/ovn-util.c @@ -517,24 +517,21 @@ ovn_destroy_tnlids(struct hmap *tnlids) hmap_destroy(tnlids); } -void -ovn_add_tnlid(struct hmap *set, uint32_t tnlid) -{ - struct tnlid_node *node = xmalloc(sizeof *node); - hmap_insert(set, &node->hmap_node, hash_int(tnlid, 0)); - node->tnlid = tnlid; -} - bool -ovn_tnlid_in_use(const struct hmap *set, uint32_t tnlid) +ovn_add_tnlid(struct hmap *set, uint32_t tnlid) { - const struct tnlid_node *node; - HMAP_FOR_EACH_IN_BUCKET (node, hmap_node, hash_int(tnlid, 0), set) { + uint32_t hash = hash_int(tnlid, 0); + struct tnlid_node *node; + HMAP_FOR_EACH_IN_BUCKET (node, hmap_node, hash, set) { if (node->tnlid == tnlid) { - return true; + return false; } } - return false; + + node = xmalloc(sizeof *node); + hmap_insert(set, &node->hmap_node, hash); + node->tnlid = tnlid; + return true; } static uint32_t @@ -549,8 +546,7 @@ ovn_allocate_tnlid(struct hmap *set, const char *name, uint32_t min, { for (uint32_t tnlid = next_tnlid(*hint, min, max); tnlid != *hint; tnlid = next_tnlid(tnlid, min, max)) { - if (!ovn_tnlid_in_use(set, tnlid)) { - ovn_add_tnlid(set, tnlid); + if (ovn_add_tnlid(set, tnlid)) { *hint = tnlid; return tnlid; } diff --git a/lib/ovn-util.h b/lib/ovn-util.h index 1d22a691f56f..6162f30225b3 100644 --- a/lib/ovn-util.h +++ b/lib/ovn-util.h @@ -114,8 +114,7 @@ void ovn_conn_show(struct unixctl_conn *conn, int argc OVS_UNUSED, struct hmap; void ovn_destroy_tnlids(struct hmap *tnlids); -void ovn_add_tnlid(struct hmap *set, uint32_t tnlid); -bool ovn_tnlid_in_use(const struct hmap *set, uint32_t tnlid); +bool ovn_add_tnlid(struct hmap *set, uint32_t tnlid); uint32_t ovn_allocate_tnlid(struct hmap *set, const char *name, uint32_t min, uint32_t max, uint32_t *hint); diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index b96e0db516be..92d578c405a2 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -596,6 +596,8 @@ struct ovn_datapath { struct ovs_list list; /* In list of similar records. */ + uint32_t tunnel_key; + /* Logical switch data. */ struct ovn_port **router_ports; size_t n_router_ports; @@ -1296,12 +1298,45 @@ get_ovn_max_dp_key_local(struct northd_context *ctx) return OVN_MAX_DP_KEY - OVN_MAX_DP_GLOBAL_NUM; } -static uint32_t -ovn_datapath_allocate_key(struct northd_context *ctx, struct hmap *dp_tnlids) +static void +ovn_datapath_allocate_key(struct northd_context *ctx, + struct hmap *datapaths, struct hmap *dp_tnlids, + struct ovn_datapath *od, uint32_t *hint) +{ + if (!od->tunnel_key) { + od->tunnel_key = ovn_allocate_tnlid(dp_tnlids, "datapath", + OVN_MIN_DP_KEY_LOCAL, + get_ovn_max_dp_key_local(ctx), + hint); + if (!od->tunnel_key) { + if (od->sb) { + sbrec_datapath_binding_delete(od->sb); + } + ovs_list_remove(&od->list); + ovn_datapath_destroy(datapaths, od); + } + } +} + +static void +ovn_datapath_assign_requested_tnl_id(struct hmap *dp_tnlids, + struct ovn_datapath *od) { - static uint32_t hint; - return ovn_allocate_tnlid(dp_tnlids, "datapath", OVN_MIN_DP_KEY_LOCAL, - get_ovn_max_dp_key_local(ctx), &hint); + const struct smap *other_config = (od->nbs + ? &od->nbs->other_config + : &od->nbr->options); + uint32_t tunnel_key = smap_get_int(other_config, "requested-tnl-key", 0); + if (tunnel_key) { + if (ovn_add_tnlid(dp_tnlids, tunnel_key)) { + od->tunnel_key = tunnel_key; + } else { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "Logical %s %s requests same tunnel key " + "%"PRIu32" as another logical switch or router", + od->nbs ? "switch" : "router", od->nbs->name, + tunnel_key); + } + } } /* Updates the southbound Datapath_Binding table so that it contains the @@ -1317,65 +1352,44 @@ build_datapaths(struct northd_context *ctx, struct hmap *datapaths, join_datapaths(ctx, datapaths, &sb_only, &nb_only, &both, lr_list); - /* First index the in-use datapath tunnel IDs. */ + /* Assign explicitly requested tunnel ids first. */ struct hmap dp_tnlids = HMAP_INITIALIZER(&dp_tnlids); struct ovn_datapath *od, *next; - if (!ovs_list_is_empty(&nb_only) || !ovs_list_is_empty(&both)) { - LIST_FOR_EACH (od, list, &both) { - ovn_add_tnlid(&dp_tnlids, od->sb->tunnel_key); - } + LIST_FOR_EACH (od, list, &both) { + ovn_datapath_assign_requested_tnl_id(&dp_tnlids, od); } - - /* Add southbound record for each unmatched northbound record. */ LIST_FOR_EACH (od, list, &nb_only) { - int64_t tunnel_key = 0; - if (od->nbs) { - tunnel_key = smap_get_int(&od->nbs->other_config, - "requested-tnl-key", - 0); - if (tunnel_key && ovn_tnlid_in_use(&dp_tnlids, tunnel_key)) { - static struct vlog_rate_limit rl = - VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, "Cannot create datapath binding for " - "logical switch %s due to duplicate key set " - "in other_config:requested-tnl-key: %"PRId64, - od->nbs->name, tunnel_key); - continue; - } - } - if (!tunnel_key) { - tunnel_key = ovn_datapath_allocate_key(ctx, &dp_tnlids); - if (!tunnel_key) { - break; - } + ovn_datapath_assign_requested_tnl_id(&dp_tnlids, od); + } + + /* Keep nonconflicting tunnel IDs that are already assigned. */ + LIST_FOR_EACH (od, list, &both) { + if (!od->tunnel_key && ovn_add_tnlid(&dp_tnlids, od->sb->tunnel_key)) { + od->tunnel_key = od->sb->tunnel_key; } + } - od->sb = sbrec_datapath_binding_insert(ctx->ovnsb_txn); - ovn_datapath_update_external_ids(od); - sbrec_datapath_binding_set_tunnel_key(od->sb, tunnel_key); + /* Assign new tunnel ids where needed. */ + uint32_t hint = 0; + LIST_FOR_EACH_SAFE (od, next, list, &both) { + ovn_datapath_allocate_key(ctx, datapaths, &dp_tnlids, od, &hint); + } + LIST_FOR_EACH_SAFE (od, next, list, &nb_only) { + ovn_datapath_allocate_key(ctx, datapaths, &dp_tnlids, od, &hint); } - /* Sync from northbound to southbound record for od existed in both. */ + /* Sync tunnel ids from nb to sb. */ LIST_FOR_EACH (od, list, &both) { - if (od->nbs) { - int64_t tunnel_key = smap_get_int(&od->nbs->other_config, - "requested-tnl-key", - 0); - if (tunnel_key && tunnel_key != od->sb->tunnel_key) { - if (ovn_tnlid_in_use(&dp_tnlids, tunnel_key)) { - static struct vlog_rate_limit rl = - VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, "Cannot update datapath binding key for " - "logical switch %s due to duplicate key set " - "in other_config:requested-tnl-key: %"PRId64, - od->nbs->name, tunnel_key); - continue; - } - sbrec_datapath_binding_set_tunnel_key(od->sb, tunnel_key); - } + if (od->sb->tunnel_key != od->tunnel_key) { + sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key); } + ovn_datapath_update_external_ids(od); + } + LIST_FOR_EACH (od, list, &nb_only) { + od->sb = sbrec_datapath_binding_insert(ctx->ovnsb_txn); + ovn_datapath_update_external_ids(od); + sbrec_datapath_binding_set_tunnel_key(od->sb, od->tunnel_key); } - ovn_destroy_tnlids(&dp_tnlids); /* Delete southbound records without northbound matches. */ @@ -3292,7 +3306,7 @@ ovn_port_update_sbrec(struct northd_context *ctx, } int64_t tnl_key = op_get_requested_tnl_key(op); if (tnl_key && tnl_key != op->sb->tunnel_key) { - if (ovn_tnlid_in_use(&op->od->port_tnlids, tnl_key)) { + if (!ovn_add_tnlid(&op->od->port_tnlids, tnl_key)) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); VLOG_WARN_RL(&rl, "Cannot update port binding for " "%s due to duplicate key set " @@ -3749,7 +3763,7 @@ build_ports(struct northd_context *ctx, /* Add southbound record for each unmatched northbound record. */ LIST_FOR_EACH_SAFE (op, next, list, &nb_only) { int64_t tunnel_key = op_get_requested_tnl_key(op); - if (tunnel_key && ovn_tnlid_in_use(&op->od->port_tnlids, tunnel_key)) { + if (tunnel_key && !ovn_add_tnlid(&op->od->port_tnlids, tunnel_key)) { static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); VLOG_WARN_RL(&rl, "Cannot create port binding for " "%s due to duplicate key set " diff --git a/ovn-nb.xml b/ovn-nb.xml index b0ceb5051966..a3979caf13ce 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -1933,6 +1933,14 @@ binding table.

+ + + Configures the datapath tunnel key for the logical router. + This is not needed because ovn-northd will assign an + unique key for each datapath by itself. However, if it is configured, + ovn-northd honors the configured value. + diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index e155e26f897c..9ad562ee15f7 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -2067,3 +2067,49 @@ action=(reg0 = 0; reject { /* eth.dst <-> eth.src; ip.dst <-> ip.src; is implici ]) AT_CLEANUP + +AT_SETUP([requested-tnl-key]) +AT_KEYWORDS([requested tnl tunnel key keys]) +ovn_start + +get_tunnel_keys() { + set $(ovn-sbctl get datapath_binding ls0 tunnel_key \ + -- get datapath_binding ls1 tunnel_key \ + -- get datapath_binding ls2 tunnel_key) + echo "ls0=$ls0 ls1=$ls1 ls2=$ls2" + ls0=$1 ls1=$2 ls2=$3 + AT_CHECK([test "$ls0" != "$ls1" && \ + test "$ls1" != "$ls2" && \ + test "$ls0" != "$ls2"]) +} + +echo +echo "__file__:__line__: Add three logical switches, check tunnel ids" +AT_CHECK( + [ovn-nbctl --wait=sb ls-add ls0 + ovn-nbctl --wait=sb ls-add ls1 + ovn-nbctl --wait=sb ls-add ls2]) +get_tunnel_keys +AT_CHECK([test $ls0 = 1 && test $ls1 = 2 && test $ls2 = 3]) + +echo +echo "__file__:__line__: Assign ls0 new tunnel key, others don't change." +AT_CHECK( + [ovn-nbctl --wait=sb set logical-switch ls0 other-config:requested-tnl-key=4]) +get_tunnel_keys +AT_CHECK([test $ls0 = 4 && test $ls1 = 2 && test $ls2 = 3]) + +echo +echo "__file__:__line__: Assign ls0 a conflict with ls1, which moves aside." +AT_CHECK( + [ovn-nbctl --wait=sb set logical-switch ls0 other-config:requested-tnl-key=2]) +get_tunnel_keys +AT_CHECK([test $ls0 = 2 && test $ls2 = 3]) + +echo +echo "__file__:__line__: Assign ls0 and ls1 conflicts and verify that they end up different and ls2 doesn't change." +AT_CHECK( + [ovn-nbctl --wait=sb set logical-switch ls1 other-config:requested-tnl-key=2]) +get_tunnel_keys +AT_CHECK([test $ls2 = 3]) +AT_CLEANUP From patchwork Mon Oct 26 18:16:26 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 1387972 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.136; helo=silver.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4CKjjb0v5pz9sSW for ; Tue, 27 Oct 2020 05:17:22 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 5AC002E15E; Mon, 26 Oct 2020 18:17:21 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id U6HosyGXIXxw; Mon, 26 Oct 2020 18:17:13 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id F0E882E15D; Mon, 26 Oct 2020 18:16:59 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id C9B29C1D7C; Mon, 26 Oct 2020 18:16:59 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 88D80C1DA4 for ; Mon, 26 Oct 2020 18:16:57 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id E45A12E150 for ; Mon, 26 Oct 2020 18:16:56 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 3YiDapJ+k-wZ for ; Mon, 26 Oct 2020 18:16:43 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by silver.osuosl.org (Postfix) with ESMTPS id 5484C2E141 for ; Mon, 26 Oct 2020 18:16:42 +0000 (UTC) Received: from sigfpe.attlocal.net (75-54-222-30.lightspeed.rdcyca.sbcglobal.net [75.54.222.30]) (Authenticated sender: blp@ovn.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id 073A1100006; Mon, 26 Oct 2020 18:16:39 +0000 (UTC) From: Ben Pfaff To: dev@openvswitch.org Date: Mon, 26 Oct 2020 11:16:26 -0700 Message-Id: <20201026181626.1827014-7-blp@ovn.org> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20201026181626.1827014-1-blp@ovn.org> References: <20201026181626.1827014-1-blp@ovn.org> MIME-Version: 1.0 Cc: Ben Pfaff Subject: [ovs-dev] [PATCH ovn 7/7] northd: Enhance implementation of port tunnel key requests. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" When a tunnel key was requested, the implementation was not smart enough to reassign a port that had been automatically assigned the same key. This fixes the problem and adds a test. Signed-off-by: Ben Pfaff Acked-by: Numan Siddique --- northd/ovn-northd.c | 142 ++++++++++++++++++++++++-------------------- tests/ovn-northd.at | 59 +++++++++++++++++- 2 files changed, 136 insertions(+), 65 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 92d578c405a2..66b0a81267cc 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -1427,6 +1427,8 @@ struct ovn_port { const struct sbrec_port_binding *sb; /* May be NULL. */ + uint32_t tunnel_key; + /* Logical switch port data. */ const struct nbrec_logical_switch_port *nbsp; /* May be NULL. */ @@ -1465,13 +1467,6 @@ struct ovn_port { struct ovs_list list; /* In list of similar records. */ }; -static void -ovn_port_set_sb(struct ovn_port *op, - const struct sbrec_port_binding *sb) -{ - op->sb = sb; -} - static void ovn_port_set_nb(struct ovn_port *op, const struct nbrec_logical_switch_port *nbsp, @@ -1495,7 +1490,7 @@ ovn_port_create(struct hmap *ports, const char *key, op->json_key = ds_steal_cstr(&json_key); op->key = xstrdup(key); - ovn_port_set_sb(op, sb); + op->sb = sb; ovn_port_set_nb(op, nbsp, nbrp); op->derived = false; hmap_insert(ports, &op->key_node, hash_string(op->key, 0)); @@ -1541,13 +1536,6 @@ ovn_port_find(const struct hmap *ports, const char *name) return NULL; } -static uint32_t -ovn_port_allocate_key(struct ovn_datapath *od) -{ - return ovn_allocate_tnlid(&od->port_tnlids, "port", - 1, (1u << 15) - 1, &od->port_key_hint); -} - /* Returns true if the logical switch port 'enabled' column is empty or * set to true. Otherwise, returns false. */ static bool @@ -2979,15 +2967,6 @@ copy_gw_chassis_from_nbrp_to_sbpb( free(sb_ha_chassis); } -static int64_t -op_get_requested_tnl_key(const struct ovn_port *op) -{ - ovs_assert(op->nbsp || op->nbrp); - const struct smap *op_options = op->nbsp ? &op->nbsp->options - : &op->nbrp->options; - return smap_get_int(op_options, "requested-tnl-key", 0); -} - static const char* op_get_name(const struct ovn_port *op) { @@ -3304,17 +3283,8 @@ ovn_port_update_sbrec(struct northd_context *ctx, sbrec_port_binding_set_external_ids(op->sb, &ids); smap_destroy(&ids); } - int64_t tnl_key = op_get_requested_tnl_key(op); - if (tnl_key && tnl_key != op->sb->tunnel_key) { - if (!ovn_add_tnlid(&op->od->port_tnlids, tnl_key)) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, "Cannot update port binding for " - "%s due to duplicate key set " - "in options:requested-tnl-key: %"PRId64, - op_get_name(op), tnl_key); - } else { - sbrec_port_binding_set_tunnel_key(op->sb, tnl_key); - } + if (op->tunnel_key != op->sb->tunnel_key) { + sbrec_port_binding_set_tunnel_key(op->sb, op->tunnel_key); } } @@ -3707,6 +3677,52 @@ destroy_ovn_lbs(struct hmap *lbs) } } +static bool +ovn_port_add_tnlid(struct ovn_port *op, uint32_t tunnel_key) +{ + bool added = ovn_add_tnlid(&op->od->port_tnlids, tunnel_key); + if (added) { + op->tunnel_key = tunnel_key; + if (tunnel_key > op->od->port_key_hint) { + op->od->port_key_hint = tunnel_key; + } + } + return added; +} + +static void +ovn_port_assign_requested_tnl_id(struct ovn_port *op) +{ + const struct smap *options = (op->nbsp + ? &op->nbsp->options + : &op->nbrp->options); + uint32_t tunnel_key = smap_get_int(options, "requested-tnl-key", 0); + if (tunnel_key && !ovn_port_add_tnlid(op, tunnel_key)) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&rl, "Logical %s port %s requests same tunnel key " + "%"PRIu32" as another LSP or LRP", + op->nbsp ? "switch" : "router", + op_get_name(op), tunnel_key); + } +} + +static void +ovn_port_allocate_key(struct hmap *ports, struct ovn_port *op) +{ + if (!op->tunnel_key) { + op->tunnel_key = ovn_allocate_tnlid(&op->od->port_tnlids, "port", + 1, (1u << 15) - 1, + &op->od->port_key_hint); + if (!op->tunnel_key) { + if (op->sb) { + sbrec_port_binding_delete(op->sb); + } + ovs_list_remove(&op->list); + ovn_port_destroy(ports, op); + } + } +} + /* Updates the southbound Port_Binding table so that it contains the logical * switch ports specified by the northbound database. * @@ -3732,15 +3748,30 @@ build_ports(struct northd_context *ctx, /* Purge stale Mac_Bindings if ports are deleted. */ bool remove_mac_bindings = !ovs_list_is_empty(&sb_only); + /* Assign explicitly requested tunnel ids first. */ struct ovn_port *op, *next; - /* For logical ports that are in both databases, index the in-use - * tunnel_keys. */ LIST_FOR_EACH (op, list, &both) { - ovn_add_tnlid(&op->od->port_tnlids, op->sb->tunnel_key); - if (op->sb->tunnel_key > op->od->port_key_hint) { - op->od->port_key_hint = op->sb->tunnel_key; + ovn_port_assign_requested_tnl_id(op); + } + LIST_FOR_EACH (op, list, &nb_only) { + ovn_port_assign_requested_tnl_id(op); + } + + /* Keep nonconflicting tunnel IDs that are already assigned. */ + LIST_FOR_EACH (op, list, &both) { + if (!op->tunnel_key) { + ovn_port_add_tnlid(op, op->sb->tunnel_key); } } + + /* Assign new tunnel ids where needed. */ + LIST_FOR_EACH_SAFE (op, next, list, &both) { + ovn_port_allocate_key(ports, op); + } + LIST_FOR_EACH_SAFE (op, next, list, &nb_only) { + ovn_port_allocate_key(ports, op); + } + /* For logical ports that are in both databases, update the southbound * record based on northbound data. * For logical ports that are in NB database, do any tag allocation @@ -3762,38 +3793,21 @@ build_ports(struct northd_context *ctx, /* Add southbound record for each unmatched northbound record. */ LIST_FOR_EACH_SAFE (op, next, list, &nb_only) { - int64_t tunnel_key = op_get_requested_tnl_key(op); - if (tunnel_key && !ovn_add_tnlid(&op->od->port_tnlids, tunnel_key)) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1); - VLOG_WARN_RL(&rl, "Cannot create port binding for " - "%s due to duplicate key set " - "in options:requested-tnl-key: %"PRId64, - op_get_name(op), tunnel_key); - continue; - } - - if (!tunnel_key) { - tunnel_key = ovn_port_allocate_key(op->od); - if (!tunnel_key) { - continue; - } - } - - ovn_port_set_sb(op, sbrec_port_binding_insert(ctx->ovnsb_txn)); + op->sb = sbrec_port_binding_insert(ctx->ovnsb_txn); ovn_port_update_sbrec(ctx, sbrec_chassis_by_name, op, &chassis_qdisc_queues, &active_ha_chassis_grps); sbrec_port_binding_set_logical_port(op->sb, op->key); - sbrec_port_binding_set_tunnel_key(op->sb, tunnel_key); } /* Delete southbound records without northbound matches. */ - LIST_FOR_EACH_SAFE(op, next, list, &sb_only) { - ovs_list_remove(&op->list); - sbrec_port_binding_delete(op->sb); - ovn_port_destroy(ports, op); + if (!ovs_list_is_empty(&sb_only)) { + LIST_FOR_EACH_SAFE(op, next, list, &sb_only) { + ovs_list_remove(&op->list); + sbrec_port_binding_delete(op->sb); + ovn_port_destroy(ports, op); + } } - if (remove_mac_bindings) { cleanup_mac_bindings(ctx, datapaths, ports); } diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 9ad562ee15f7..946f20b6a176 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -2068,7 +2068,7 @@ action=(reg0 = 0; reject { /* eth.dst <-> eth.src; ip.dst <-> ip.src; is implici AT_CLEANUP -AT_SETUP([requested-tnl-key]) +AT_SETUP([datapath requested-tnl-key]) AT_KEYWORDS([requested tnl tunnel key keys]) ovn_start @@ -2113,3 +2113,60 @@ AT_CHECK( get_tunnel_keys AT_CHECK([test $ls2 = 3]) AT_CLEANUP +]) + +AT_SETUP([port requested-tnl-key]) +AT_KEYWORDS([requested tnl tunnel key keys]) +ovn_start + +get_tunnel_keys() { + set $(ovn-sbctl get port_binding lsp00 tunnel_key \ + -- get port_binding lsp01 tunnel_key \ + -- get port_binding lsp02 tunnel_key \ + -- get port_binding lsp10 tunnel_key \ + -- get port_binding lsp11 tunnel_key \ + -- get port_binding lsp12 tunnel_key) + lsp00=$1 lsp01=$2 lsp02=$3 lsp10=$4 lsp11=$5 lsp12=$6 + ls0=$1$2$3 ls1=$4$5$6 + echo "ls0=$1$2$3 ls1=$4$5$6" + AT_CHECK([test "$lsp00" != "$lsp01" && \ + test "$lsp01" != "$lsp02" && \ + test "$lsp00" != "$lsp02"]) + AT_CHECK([test "$lsp10" != "$lsp11" && \ + test "$lsp11" != "$lsp12" && \ + test "$lsp10" != "$lsp12"]) +} + +echo +echo "__file__:__line__: Add two logical switches with three ports each, check tunnel ids" +AT_CHECK( + [for i in 0 1; do + ovn-nbctl --wait=sb ls-add ls$i || exit $? + for j in 0 1 2; do + ovn-nbctl --wait=sb lsp-add ls$i lsp$i$j || exit $? + done + done]) +get_tunnel_keys +AT_CHECK([test $ls0 = 123 && test $ls1 = 123]) + +echo +echo "__file__:__line__: Assign lsp00 new tunnel key, others don't change." +AT_CHECK( + [ovn-nbctl --wait=sb set logical-switch-port lsp00 options:requested-tnl-key=4]) +get_tunnel_keys +AT_CHECK([test $ls0 = 423 && test $ls1 = 123]) + +echo +echo "__file__:__line__: Assign lsp00 a conflict with lsp01, which moves aside." +AT_CHECK( + [ovn-nbctl --wait=sb set logical-switch-port lsp00 options:requested-tnl-key=2]) +get_tunnel_keys +AT_CHECK([test $lsp00 = 2 && test $lsp02 = 3 && test $ls1 = 123]) + +echo +echo "__file__:__line__: Assign lsp00 and lsp01 conflicts and verify that they end up different and lsp02 doesn't change." +AT_CHECK( + [ovn-nbctl --wait=sb set logical-switch-port lsp01 options:requested-tnl-key=2]) +get_tunnel_keys +AT_CHECK([test $lsp02 = 3 && test $ls1 = 123]) +AT_CLEANUP