From patchwork Tue Nov 2 19:31:03 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1549934 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=EwXEXgyL; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HkKlT3jpSz9sRK for ; Wed, 3 Nov 2021 06:31:33 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 99E5180E2B; Tue, 2 Nov 2021 19:31:30 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id F9_zfljBOCdc; Tue, 2 Nov 2021 19:31:28 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id C847680E32; Tue, 2 Nov 2021 19:31:26 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id E8D5BC0038; Tue, 2 Nov 2021 19:31:24 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3B53CC000E for ; Tue, 2 Nov 2021 19:31:23 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id B444980E0C for ; Tue, 2 Nov 2021 19:31:18 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id FmODu1SeBIVk for ; Tue, 2 Nov 2021 19:31:17 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by smtp1.osuosl.org (Postfix) with ESMTPS id 882B680E26 for ; Tue, 2 Nov 2021 19:31:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1635881476; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=/TygHexRNBeaa1vmTiKxoboYJYIjp79i17xM/RfHLFg=; b=EwXEXgyLzscifIyRVFJrjYLngXte0U8MCdpdIZOFu99thCnH1wIX6AKqeQ2g/Qj8ztVUdd YKX2c4AblF57gc1PdYDjPFeRgA3fVDY1rhKL66O4We+z2dfRYVUDDHjWN3i955lDpPBjyB zIPCW7un2CtKb2BTE7UOrpF3bCPfklU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-414-NfIJxisqOcWm1QJJ1hAskA-1; Tue, 02 Nov 2021 15:31:13 -0400 X-MC-Unique: NfIJxisqOcWm1QJJ1hAskA-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 362EABAF82; Tue, 2 Nov 2021 19:31:12 +0000 (UTC) Received: from dceara.remote.csb (unknown [10.39.193.68]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5309F6F920; Tue, 2 Nov 2021 19:31:07 +0000 (UTC) From: Dumitru Ceara To: ovs-dev@openvswitch.org Date: Tue, 2 Nov 2021 20:31:03 +0100 Message-Id: <20211102193101.615.23638.stgit@dceara.remote.csb> In-Reply-To: <20211102193004.615.42236.stgit@dceara.remote.csb> References: <20211102193004.615.42236.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: trozet@redhat.com Subject: [ovs-dev] [PATCH ovn branch-21.09 3/3] northd: Always generate valid load balancer address set names. 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" Address set names have a fixed format ([a-zA-Z_.][a-zA-Z_.0-9]*) while logical router names are free form. This means we cannot directly embed the logical router name inside the address set name. To make sure that address set names are unique and valid use instead the Datapath_Binding.tunnel_key value which is guaranteed to be unique across logical routers. Fixes: c1e3896c0a39 ("northd: Use address sets for ARP responder flows for VIPs.") Signed-off-by: Dumitru Ceara (cherry picked from commit beed00c9206d439c89da3bcaf924163abf606215) --- northd/northd.c | 39 +++++++++++++++++++++++++++++++++++---- tests/ovn-northd.at | 27 +++++++++++++++------------ 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/northd/northd.c b/northd/northd.c index c1d83548e..d237f693d 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -878,6 +878,37 @@ lr_has_lb_vip(struct ovn_datapath *od) return false; } +/* Builds a unique address set compatible name ([a-zA-Z_.][a-zA-Z_.0-9]*) + * for the router's load balancer VIP address set, combining the logical + * router's datapath tunnel key and address family. + * + * Also prefixes the name with 'prefix'. + */ +static char * +lr_lb_address_set_name_(const struct ovn_datapath *od, const char *prefix, + int addr_family) +{ + ovs_assert(od->nbr); + return xasprintf("%s_rtr_lb_%"PRIu32"_ip%s", prefix, od->tunnel_key, + addr_family == AF_INET ? "4" : "6"); +} + +/* Builds the router's load balancer VIP address set name. */ +static char * +lr_lb_address_set_name(const struct ovn_datapath *od, int addr_family) +{ + return lr_lb_address_set_name_(od, "", addr_family); +} + +/* Builds a string that refers to the the router's load balancer VIP address + * set name, that is: $. + */ +static char * +lr_lb_address_set_ref(const struct ovn_datapath *od, int addr_family) +{ + return lr_lb_address_set_name_(od, "$", addr_family); +} + static void init_lb_for_datapath(struct ovn_datapath *od) { @@ -12040,7 +12071,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, } /* Create a single ARP rule for all IPs that are used as VIPs. */ - char *lb_ips_v4_as = xasprintf("$%s_lb_ip4", op->od->nbr->name); + char *lb_ips_v4_as = lr_lb_address_set_ref(op->od, AF_INET); build_lrouter_arp_flow(op->od, op, lb_ips_v4_as, REG_INPORT_ETH_ADDR, match, false, 90, NULL, lflows); @@ -12056,7 +12087,7 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op, } /* Create a single ND rule for all IPs that are used as VIPs. */ - char *lb_ips_v6_as = xasprintf("$%s_lb_ip6", op->od->nbr->name); + char *lb_ips_v6_as = lr_lb_address_set_ref(op->od, AF_INET6); build_lrouter_nd_flow(op->od, op, "nd_na", lb_ips_v6_as, NULL, REG_INPORT_ETH_ADDR, match, false, 90, NULL, lflows, meter_groups); @@ -13687,7 +13718,7 @@ sync_address_sets(struct northd_context *ctx, struct hmap *datapaths) } if (sset_count(&od->lb_ips_v4)) { - char *ipv4_addrs_name = xasprintf("%s_lb_ip4", od->nbr->name); + char *ipv4_addrs_name = lr_lb_address_set_name(od, AF_INET); const char **ipv4_addrs = sset_array(&od->lb_ips_v4); sync_address_set(ctx, ipv4_addrs_name, ipv4_addrs, @@ -13697,7 +13728,7 @@ sync_address_sets(struct northd_context *ctx, struct hmap *datapaths) } if (sset_count(&od->lb_ips_v6)) { - char *ipv6_addrs_name = xasprintf("%s_lb_ip6", od->nbr->name); + char *ipv6_addrs_name = lr_lb_address_set_name(od, AF_INET6); const char **ipv6_addrs = sset_array(&od->lb_ips_v6); sync_address_set(ctx, ipv6_addrs_name, ipv6_addrs, diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 166723502..3f21edee9 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -1701,10 +1701,13 @@ ovn-nbctl lr-lb-add lr lb4 ovn-nbctl lr-lb-add lr lb5 ovn-nbctl --wait=sb sync +lr_key=$(fetch_column sb:datapath_binding tunnel_key external_ids:name=lr) +lb_as_v4="_rtr_lb_${lr_key}_ip4" +lb_as_v6="_rtr_lb_${lr_key}_ip6" # Check generated VIP address sets. -check_column '192.168.2.1 192.168.2.4 192.168.2.5 192.168.2.6' Address_Set addresses name=lr_lb_ip4 -check_column 'fe80::200:ff:fe00:101 fe80::200:ff:fe00:102' Address_Set addresses name=lr_lb_ip6 +check_column '192.168.2.1 192.168.2.4 192.168.2.5 192.168.2.6' Address_Set addresses name=${lb_as_v4} +check_column 'fe80::200:ff:fe00:101 fe80::200:ff:fe00:102' Address_Set addresses name=${lb_as_v6} # Ingress router port ETH address is stored in lr_in_admission. AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_admission.*xreg0\[[0..47\]]" | sort], [0], [dnl @@ -1723,7 +1726,7 @@ action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;) ]) # Ingress router port ETH address is used for ARP reply/NA in lr_in_ip_input. -AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl +AT_CHECK_UNQUOTED([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl table=3 (lr_in_ip_input ), priority=90 , dnl match=(arp.op == 1 && arp.tpa == 43.43.43.150), dnl action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;) @@ -1737,7 +1740,7 @@ action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ match=(arp.op == 1 && arp.tpa == 43.43.43.4), dnl action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;) table=3 (lr_in_ip_input ), priority=90 , dnl -match=(inport == "lrp" && arp.op == 1 && arp.tpa == $lr_lb_ip4), dnl +match=(inport == "lrp" && arp.op == 1 && arp.tpa == \$${lb_as_v4}), dnl action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;) table=3 (lr_in_ip_input ), priority=90 , dnl match=(inport == "lrp" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.0/24), dnl @@ -1746,10 +1749,10 @@ action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ match=(inport == "lrp" && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd_ns && nd.target == fe80::200:ff:fe00:1), dnl action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };) table=3 (lr_in_ip_input ), priority=90 , dnl -match=(inport == "lrp" && nd_ns && nd.target == $lr_lb_ip6), dnl +match=(inport == "lrp" && nd_ns && nd.target == \$${lb_as_v6}), dnl action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };) table=3 (lr_in_ip_input ), priority=90 , dnl -match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == $lr_lb_ip4), dnl +match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == \$${lb_as_v4}), dnl action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;) table=3 (lr_in_ip_input ), priority=90 , dnl match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.1 && arp.spa == 43.43.43.0/24), dnl @@ -1758,7 +1761,7 @@ action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ match=(inport == "lrp-public" && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd_ns && nd.target == fe80::200:ff:fe00:100), dnl action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };) table=3 (lr_in_ip_input ), priority=90 , dnl -match=(inport == "lrp-public" && nd_ns && nd.target == $lr_lb_ip6), dnl +match=(inport == "lrp-public" && nd_ns && nd.target == \$${lb_as_v6}), dnl action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };) ]) @@ -1792,7 +1795,7 @@ action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;) # Ingress router port is used for ARP reply/NA in lr_in_ip_input. # xxreg0[0..47] is used unless external_mac is set. # Priority 90 flows (per router). -AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl +AT_CHECK_UNQUOTED([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl table=3 (lr_in_ip_input ), priority=90 , dnl match=(arp.op == 1 && arp.tpa == 43.43.43.150), dnl action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;) @@ -1806,7 +1809,7 @@ action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ match=(arp.op == 1 && arp.tpa == 43.43.43.4), dnl action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;) table=3 (lr_in_ip_input ), priority=90 , dnl -match=(inport == "lrp" && arp.op == 1 && arp.tpa == $lr_lb_ip4), dnl +match=(inport == "lrp" && arp.op == 1 && arp.tpa == \$${lb_as_v4}), dnl action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;) table=3 (lr_in_ip_input ), priority=90 , dnl match=(inport == "lrp" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.0/24), dnl @@ -1815,10 +1818,10 @@ action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ match=(inport == "lrp" && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd_ns && nd.target == fe80::200:ff:fe00:1), dnl action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };) table=3 (lr_in_ip_input ), priority=90 , dnl -match=(inport == "lrp" && nd_ns && nd.target == $lr_lb_ip6), dnl +match=(inport == "lrp" && nd_ns && nd.target == \$${lb_as_v6}), dnl action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };) table=3 (lr_in_ip_input ), priority=90 , dnl -match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == $lr_lb_ip4 && is_chassis_resident("cr-lrp-public")), dnl +match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == \$${lb_as_v4} && is_chassis_resident("cr-lrp-public")), dnl action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;) table=3 (lr_in_ip_input ), priority=90 , dnl match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.1 && arp.spa == 43.43.43.0/24), dnl @@ -1827,7 +1830,7 @@ action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ match=(inport == "lrp-public" && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd_ns && nd.target == fe80::200:ff:fe00:100 && is_chassis_resident("cr-lrp-public")), dnl action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };) table=3 (lr_in_ip_input ), priority=90 , dnl -match=(inport == "lrp-public" && nd_ns && nd.target == $lr_lb_ip6 && is_chassis_resident("cr-lrp-public")), dnl +match=(inport == "lrp-public" && nd_ns && nd.target == \$${lb_as_v6} && is_chassis_resident("cr-lrp-public")), dnl action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };) ])