From patchwork Wed Jul 8 15:03:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1325266 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=fail (p=none dis=none) header.from=redhat.com Authentication-Results: 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=asiRJxjE; dkim-atps=neutral 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 4B22d22CGYz9sRN for ; Thu, 9 Jul 2020 01:03:49 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 531A687C5B; Wed, 8 Jul 2020 15:03:48 +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 YBumj1fN8grX; Wed, 8 Jul 2020 15:03:47 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 2133B86A47; Wed, 8 Jul 2020 15:03:47 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id F3AC9C077B; Wed, 8 Jul 2020 15:03:46 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 77720C016F for ; Wed, 8 Jul 2020 15:03:45 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 6717D86A47 for ; Wed, 8 Jul 2020 15:03: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 DEZnEn1dRvo1 for ; Wed, 8 Jul 2020 15:03:44 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) by fraxinus.osuosl.org (Postfix) with ESMTPS id E064286A1A for ; Wed, 8 Jul 2020 15:03:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1594220622; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=bAuSG9uI/eIe6hh5Tmv5xMGiDUUOBro2G3WRui3aR/w=; b=asiRJxjEqKh+2RXpxilpfk51hglEDkUO55dfSep5RxzpogLGjJL1GieEU9Ag/3dH/u8kz2 h637N+xSIZUvf0znwAO9mOPwiHszrTml7TBvaon0GK63DotQ8cx0soDSgKsjq4SXp2cMFM qzuw/E63FNshXBsNUcH2kpZOOCNoxKs= 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-307-EhZJqGWLNo2yr168boN3Sg-1; Wed, 08 Jul 2020 11:03:25 -0400 X-MC-Unique: EhZJqGWLNo2yr168boN3Sg-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 6BEBD8B00B7 for ; Wed, 8 Jul 2020 15:03:07 +0000 (UTC) Received: from dceara.remote.csb (ovpn-114-175.ams2.redhat.com [10.36.114.175]) by smtp.corp.redhat.com (Postfix) with ESMTP id AB37C10013D7 for ; Wed, 8 Jul 2020 15:03:06 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Wed, 8 Jul 2020 17:03:04 +0200 Message-Id: <20200708150301.10666.86582.stgit@dceara.remote.csb> In-Reply-To: <20200708150241.10666.61782.stgit@dceara.remote.csb> References: <20200708150241.10666.61782.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 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 Subject: [ovs-dev] [PATCH v2 ovn 2/2] ovn-northd: Fix logical flows to limit ARP/NS broadcast domain. 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" Logical flows that limit the ARP/NS broadcast domain on a logical switch should only match on ARP requests/NS for IPs that can actually be replied to on the connected router port (i.e., an IP on the same network is configured on the router port). Reported-by: Girish Moodalbail Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2020-June/050287.html Fixes: 32f5ebb06226 ("ovn-northd: Limit ARP/ND broadcast domain whenever possible.") Signed-off-by: Dumitru Ceara --- northd/ovn-northd.c | 164 ++++++++++++++++++++++++++++++++++++++++++--------- tests/ovn.at | 74 +++++++++++++++++++++++ 2 files changed, 210 insertions(+), 28 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 4c23158..e2e875a 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -6091,6 +6091,44 @@ build_lrouter_groups(struct hmap *ports, struct ovs_list *lr_list) } } +/* Returns 'true' if the IPv4 'addr' is on the same subnet with one of the + * IPs configured on the router port. + */ +static bool +lrouter_port_ipv4_reachable(const struct ovn_port *op, ovs_be32 addr) +{ + for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { + struct ipv4_netaddr *op_addr = &op->lrp_networks.ipv4_addrs[i]; + + if ((op_addr->addr & op_addr->mask) == (addr & op_addr->mask)) { + return true; + } + } + return false; +} + +/* Returns 'true' if the IPv6 'addr' is on the same subnet with one of the + * IPs configured on the router port. + */ +static bool +lrouter_port_ipv6_reachable(const struct ovn_port *op, + const struct in6_addr *addr) +{ + for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { + struct ipv6_netaddr *op_addr = &op->lrp_networks.ipv6_addrs[i]; + + struct in6_addr op_addr6_masked = + ipv6_addr_bitand(&op_addr->addr, &op_addr->mask); + struct in6_addr nat_addr6_masked = + ipv6_addr_bitand(addr, &op_addr->mask); + + if (ipv6_addr_equals(&op_addr6_masked, &nat_addr6_masked)) { + return true; + } + } + return false; +} + /* * Ingress table 19: Flows that flood self originated ARP/ND packets in the * switching domain. @@ -6101,8 +6139,47 @@ build_lswitch_rport_arp_req_self_orig_flow(struct ovn_port *op, struct ovn_datapath *od, struct hmap *lflows) { - struct ds match = DS_EMPTY_INITIALIZER; + struct sset all_eth_addrs = SSET_INITIALIZER(&all_eth_addrs); struct ds eth_src = DS_EMPTY_INITIALIZER; + struct ds match = DS_EMPTY_INITIALIZER; + + sset_add(&all_eth_addrs, op->lrp_networks.ea_s); + + for (size_t i = 0; i < op->od->nbr->n_nat; i++) { + struct ovn_nat *nat_entry = &op->od->nat_entries[i]; + const struct nbrec_nat *nat = nat_entry->nb; + + if (!nat_entry_is_valid(nat_entry)) { + continue; + } + + if (!strcmp(nat->type, "snat")) { + continue; + } + + if (!nat->external_mac) { + continue; + } + + /* Check if there's a network configured on op on which we could + * expect ARP requests/NS for the DNAT external_ip. + */ + if (nat_entry_is_v6(nat_entry)) { + struct in6_addr *addr = &nat_entry->ext_addrs.ipv6_addrs[0].addr; + + if (!lrouter_port_ipv6_reachable(op, addr)) { + continue; + } + } else { + ovs_be32 addr = nat_entry->ext_addrs.ipv4_addrs[0].addr; + + if (!lrouter_port_ipv4_reachable(op, addr)) { + continue; + } + } + sset_add(&all_eth_addrs, nat->external_mac); + } + /* Self originated (G)ARP requests/ND need to be flooded as usual. * Determine that packets are self originated by also matching on @@ -6110,15 +6187,11 @@ build_lswitch_rport_arp_req_self_orig_flow(struct ovn_port *op, * is a VLAN-backed network. * Priority: 80. */ - ds_put_format(ð_src, "{ %s, ", op->lrp_networks.ea_s); - for (size_t i = 0; i < op->od->nbr->n_nat; i++) { - const struct nbrec_nat *nat = op->od->nbr->nat[i]; - - if (!nat->external_mac) { - continue; - } + const char *eth_addr; - ds_put_format(ð_src, "%s, ", nat->external_mac); + ds_put_cstr(ð_src, "{"); + SSET_FOR_EACH (eth_addr, &all_eth_addrs) { + ds_put_format(ð_src, "%s, ", eth_addr); } ds_chomp(ð_src, ' '); ds_chomp(ð_src, ','); @@ -6130,8 +6203,9 @@ build_lswitch_rport_arp_req_self_orig_flow(struct ovn_port *op, ds_cstr(&match), "outport = \""MC_FLOOD"\"; output;"); - ds_destroy(&match); + sset_destroy(&all_eth_addrs); ds_destroy(ð_src); + ds_destroy(&match); } /* @@ -6222,38 +6296,72 @@ build_lswitch_rport_arp_req_flows(struct ovn_port *op, struct sset all_ips_v4 = SSET_INITIALIZER(&all_ips_v4); struct sset all_ips_v6 = SSET_INITIALIZER(&all_ips_v6); - for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { - sset_add(&all_ips_v4, op->lrp_networks.ipv4_addrs[i].addr_s); - } - for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { - sset_add(&all_ips_v6, op->lrp_networks.ipv6_addrs[i].addr_s); + get_router_load_balancer_ips(op->od, &all_ips_v4, &all_ips_v6); + + const char *ip_addr; + const char *ip_addr_next; + SSET_FOR_EACH_SAFE (ip_addr, ip_addr_next, &all_ips_v4) { + ovs_be32 ipv4_addr; + + /* Check if there's a network configured on op on which we could + * expect ARP requests for the LB VIP. + */ + if (ip_parse(ip_addr, &ipv4_addr) && + lrouter_port_ipv4_reachable(op, ipv4_addr)) { + continue; + } + + sset_delete(&all_ips_v4, SSET_NODE_FROM_NAME(ip_addr)); } + SSET_FOR_EACH_SAFE (ip_addr, ip_addr_next, &all_ips_v6) { + struct in6_addr ipv6_addr; - get_router_load_balancer_ips(op->od, &all_ips_v4, &all_ips_v6); + /* Check if there's a network configured on op on which we could + * expect NS requests for the LB VIP. + */ + if (ipv6_parse(ip_addr, &ipv6_addr) && + lrouter_port_ipv6_reachable(op, &ipv6_addr)) { + continue; + } + + sset_delete(&all_ips_v6, SSET_NODE_FROM_NAME(ip_addr)); + } for (size_t i = 0; i < op->od->nbr->n_nat; i++) { - const struct nbrec_nat *nat = op->od->nbr->nat[i]; + struct ovn_nat *nat_entry = &op->od->nat_entries[i]; + const struct nbrec_nat *nat = nat_entry->nb; + + if (!nat_entry_is_valid(nat_entry)) { + continue; + } if (!strcmp(nat->type, "snat")) { continue; } - ovs_be32 ip; - ovs_be32 mask; - struct in6_addr ipv6; - struct in6_addr mask_v6; + /* Check if there's a network configured on op on which we could + * expect ARP requests/NS for the DNAT external_ip. + */ + if (nat_entry_is_v6(nat_entry)) { + struct in6_addr *addr = &nat_entry->ext_addrs.ipv6_addrs[0].addr; - char *error = ip_parse_masked(nat->external_ip, &ip, &mask); - if (error) { - free(error); - error = ipv6_parse_masked(nat->external_ip, &ipv6, &mask_v6); - if (!error) { + if (lrouter_port_ipv6_reachable(op, addr)) { sset_add(&all_ips_v6, nat->external_ip); } } else { - sset_add(&all_ips_v4, nat->external_ip); + ovs_be32 addr = nat_entry->ext_addrs.ipv4_addrs[0].addr; + + if (lrouter_port_ipv4_reachable(op, addr)) { + sset_add(&all_ips_v4, nat->external_ip); + } } - free(error); + } + + for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { + sset_add(&all_ips_v4, op->lrp_networks.ipv4_addrs[i].addr_s); + } + for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { + sset_add(&all_ips_v6, op->lrp_networks.ipv6_addrs[i].addr_s); } if (!sset_is_empty(&all_ips_v4)) { diff --git a/tests/ovn.at b/tests/ovn.at index 24d93bc..88700e8 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -19249,12 +19249,14 @@ ovn-nbctl --wait=hv sync sw_dp_uuid=$(ovn-sbctl --bare --columns _uuid list datapath_binding sw-agg) sw_dp_key=$(ovn-sbctl --bare --columns tunnel_key list datapath_binding sw-agg) +sw1_dp_key=$(ovn-sbctl --bare --columns tunnel_key list datapath_binding sw1) r1_dp_key=$(ovn-sbctl --bare --columns tunnel_key list datapath_binding rtr1) r1_tnl_key=$(ovn-sbctl --bare --columns tunnel_key list port_binding sw-rtr1) r2_tnl_key=$(ovn-sbctl --bare --columns tunnel_key list port_binding sw-rtr2) match_sw_metadata="metadata=0x${sw_dp_key}" +match_sw1_metadata="metadata=0x${sw1_dp_key}" match_r1_metadata="metadata=0x${r1_dp_key}" # Inject ARP request for first router owned IP address. @@ -19370,6 +19372,78 @@ ovn-nbctl lr-nat-add rtr1 dnat_and_snat 10.0.0.122 20.0.0.12 sw1-p2 00:00:00:02: ovn-nbctl lr-nat-add rtr1 dnat_and_snat 10::122 20::12 sw1-p2 00:00:00:02:00:00 ovn-nbctl --wait=hv sync +# Check that broadcast domain limiting flows match only on IPs that are +# directly reachable via the router port. +# For sw-rtr1: +# - 10.0.0.1, 10::1, fe80::200:ff:fe00:100 - interface IPs. +# - 10.0.0.11, 10::11 - LB VIPs. +# - 10.0.0.111, 10.0.0.121, 10.0.0.122, 10::111, 10::121, 10::122 - DNAT IPs. +# For sw-rtr2: +# - 10.0.0.2, 10::2, fe80::200:ff:fe00:200 - interface IPs. +# - 10.0.0.22, 10::22 - LB VIPs. +# - 10.0.0.222, 10::222 - DNAT IPs. +as hv1 +AT_CHECK([ovs-ofctl dump-flows br-int | grep -E "priority=75,.*${match_sw_metadata}" | grep -oE "arp_tpa=[[0-9.]]+" | sort], [0], [dnl +arp_tpa=10.0.0.1 +arp_tpa=10.0.0.11 +arp_tpa=10.0.0.111 +arp_tpa=10.0.0.121 +arp_tpa=10.0.0.122 +arp_tpa=10.0.0.2 +arp_tpa=10.0.0.22 +arp_tpa=10.0.0.222 +]) +AT_CHECK([ovs-ofctl dump-flows br-int | grep -E "priority=75,.*${match_sw_metadata}" | grep -oE "nd_target=[[0-9a-f:]]+" | sort], [0], [dnl +nd_target=10::1 +nd_target=10::11 +nd_target=10::111 +nd_target=10::121 +nd_target=10::122 +nd_target=10::2 +nd_target=10::22 +nd_target=10::222 +nd_target=fe80::200:ff:fe00:100 +nd_target=fe80::200:ff:fe00:200 +]) + +# For sw1-rtr1: +# - 20.0.0.1, 20::1, fe80::200:1ff:fe00:0 - interface IPs. +as hv1 +AT_CHECK([ovs-ofctl dump-flows br-int | grep -E "priority=75,.*${match_sw1_metadata}" | grep -oE "arp_tpa=[[0-9.]]+" | sort], [0], [dnl +arp_tpa=20.0.0.1 +]) +AT_CHECK([ovs-ofctl dump-flows br-int | grep -E "priority=75,.*${match_sw1_metadata}" | grep -oE "nd_target=[[0-9a-f:]]+" | sort], [0], [dnl +nd_target=20::1 +nd_target=fe80::200:1ff:fe00:0 +]) + +# Self originated ARP/NS with SMACs owned by rtr1-sw and rtr2-sw should be +# flooded: +# - 00:00:00:00:01:00 - interface MAC (rtr1-sw). +# - 00:00:00:00:02:00 - interface MAC (rtr2-sw). +# - 00:00:00:01:00:00 - dnat_and_snat external MAC. +# - 00:00:00:02:00:00 - dnat_and_snat external MAC. +as hv1 +AT_CHECK([ovs-ofctl dump-flows br-int | grep -E "priority=80,.*${match_sw_metadata}.*arp_op=1" | grep -oE "dl_src=[[0-9a-f:]]+" | sort], [0], [dnl +dl_src=00:00:00:00:01:00 +dl_src=00:00:00:00:02:00 +dl_src=00:00:00:01:00:00 +dl_src=00:00:00:02:00:00 +]) +AT_CHECK([ovs-ofctl dump-flows br-int | grep -E "priority=80,.*${match_sw_metadata}.*icmp_type=135" | grep -oE "dl_src=[[0-9a-f:]]+" | sort], [0], [dnl +dl_src=00:00:00:00:01:00 +dl_src=00:00:00:00:02:00 +dl_src=00:00:00:01:00:00 +dl_src=00:00:00:02:00:00 +]) + +# Self originated ARP/NS with SMACs owned by rtr1-sw1 should be flooded: +# - 00:00:01:00:00:00 - interface MAC. +as hv1 +AT_CHECK([ovs-ofctl dump-flows br-int | grep -E "priority=80,.*${match_sw1_metadata}.*arp_op=1" | grep -oE "dl_src=[[0-9a-f:]]+" | sort], [0], [dnl +dl_src=00:00:01:00:00:00 +]) + # Inject ARP request for first router owned NAT address. send_arp_request 1 0 ${src_mac} $(ip_to_hex 10 0 0 254) $(ip_to_hex 10 0 0 111)