From patchwork Thu Sep 17 12:51:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1366108 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=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=Sl4dsLbO; dkim-atps=neutral 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 4BscN22y3mz9sSC for ; Thu, 17 Sep 2020 22:53:38 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id DA0092E1A0; Thu, 17 Sep 2020 12:53:36 +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 e+dhDipyqdVe; Thu, 17 Sep 2020 12:53:15 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 763052E1B6; Thu, 17 Sep 2020 12:52:03 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 66AC5C0864; Thu, 17 Sep 2020 12:52:03 +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 ABA44C0051 for ; Thu, 17 Sep 2020 12:52:01 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by whitealder.osuosl.org (Postfix) with ESMTP id A8A0C878AE for ; Thu, 17 Sep 2020 12:52:01 +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 COKaByrVpz19 for ; Thu, 17 Sep 2020 12:52:00 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by whitealder.osuosl.org (Postfix) with ESMTPS id 5E83A878A9 for ; Thu, 17 Sep 2020 12:52:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1600347119; 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=kmIwumqn6rJB1GZtqrlamE2gyGA8YHpviLeeiv/DuZs=; b=Sl4dsLbOlEOGOuH3c5QS1Q7a+Gqt0/Gkn24EC6MZGzkQe9+e42GhLvEPceEz4DBAy27DF5 xuNsPlfnOGYZHKxRn7g1gKSWdsYVz+yBY0amhnYlEaJbMBNJmfGUPX7zBkfJUoocJwgf1z /e5eo7XJHWpmi+9ezA8oVwLcQpadblI= 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-218-7W6esefaOlClfP_sSXj2Ew-1; Thu, 17 Sep 2020 08:51:57 -0400 X-MC-Unique: 7W6esefaOlClfP_sSXj2Ew-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 47C401091068 for ; Thu, 17 Sep 2020 12:51:56 +0000 (UTC) Received: from dceara.remote.csb (ovpn-113-244.ams2.redhat.com [10.36.113.244]) by smtp.corp.redhat.com (Postfix) with ESMTP id A6A455DEBB for ; Thu, 17 Sep 2020 12:51:55 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Thu, 17 Sep 2020 14:51:53 +0200 Message-Id: <20200917125145.19729.68870.stgit@dceara.remote.csb> In-Reply-To: <20200917125025.19729.19409.stgit@dceara.remote.csb> References: <20200917125025.19729.19409.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.14 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 v3 ovn 4/4] ovn-northd: Refactor processing of SNAT IPs. 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" Instead of building string sets every time we need to generate logical flows for unique SNAT IPs we now prebuild the set of unique SNAT IPs and store the list of NAT entries that refer it. Signed-off-by: Dumitru Ceara Acked-by: Han Zhou --- northd/ovn-northd.c | 326 ++++++++++++++++++++++++++++----------------------- 1 file changed, 176 insertions(+), 150 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 43bd7b5..1e88cb9 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -623,8 +623,9 @@ struct ovn_datapath { /* NAT entries configured on the router. */ struct ovn_nat *nat_entries; - /* SNAT IPs used by the router. */ - struct sset snat_ips; + /* SNAT IPs owned by the router (shash of 'struct ovn_snat_ip'). */ + struct shash snat_ips; + struct lport_addresses dnat_force_snat_addrs; struct lport_addresses lb_force_snat_addrs; @@ -644,6 +645,18 @@ struct ovn_datapath { struct ovn_nat { const struct nbrec_nat *nb; struct lport_addresses ext_addrs; + struct ovs_list ext_addr_list_node; /* Linkage in the per-external IP + * list of nat entries. Currently + * only used for SNAT. + */ +}; + +/* Stores the list of SNAT entries referencing a unique SNAT IP address. + * The 'snat_entries' list will be empty if the SNAT IP is used only for + * dnat_force_snat_ip or lb_force_snat_ip. + */ +struct ovn_snat_ip { + struct ovs_list snat_entries; }; static bool @@ -670,32 +683,49 @@ nat_entry_is_v6(const struct ovn_nat *nat_entry) } static void +snat_ip_add(struct ovn_datapath *od, const char *ip, struct ovn_nat *nat_entry) +{ + struct ovn_snat_ip *snat_ip = shash_find_data(&od->snat_ips, ip); + + if (!snat_ip) { + snat_ip = xzalloc(sizeof *snat_ip); + ovs_list_init(&snat_ip->snat_entries); + shash_add(&od->snat_ips, ip, snat_ip); + } + + if (nat_entry) { + ovs_list_push_back(&snat_ip->snat_entries, + &nat_entry->ext_addr_list_node); + } +} + +static void init_nat_entries(struct ovn_datapath *od) { if (!od->nbr) { return; } - sset_init(&od->snat_ips); + shash_init(&od->snat_ips); if (get_force_snat_ip(od, "dnat", &od->dnat_force_snat_addrs)) { - if (&od->dnat_force_snat_addrs.n_ipv4_addrs) { - sset_add(&od->snat_ips, - od->dnat_force_snat_addrs.ipv4_addrs[0].addr_s); + if (od->dnat_force_snat_addrs.n_ipv4_addrs) { + snat_ip_add(od, od->dnat_force_snat_addrs.ipv4_addrs[0].addr_s, + NULL); } - if (&od->dnat_force_snat_addrs.n_ipv6_addrs) { - sset_add(&od->snat_ips, - od->dnat_force_snat_addrs.ipv6_addrs[0].addr_s); + if (od->dnat_force_snat_addrs.n_ipv6_addrs) { + snat_ip_add(od, od->dnat_force_snat_addrs.ipv6_addrs[0].addr_s, + NULL); } } if (get_force_snat_ip(od, "lb", &od->lb_force_snat_addrs)) { if (od->lb_force_snat_addrs.n_ipv4_addrs) { - sset_add(&od->snat_ips, - od->lb_force_snat_addrs.ipv4_addrs[0].addr_s); + snat_ip_add(od, od->lb_force_snat_addrs.ipv4_addrs[0].addr_s, + NULL); } if (od->lb_force_snat_addrs.n_ipv6_addrs) { - sset_add(&od->snat_ips, - od->lb_force_snat_addrs.ipv6_addrs[0].addr_s); + snat_ip_add(od, od->lb_force_snat_addrs.ipv6_addrs[0].addr_s, + NULL); } } @@ -721,10 +751,15 @@ init_nat_entries(struct ovn_datapath *od) continue; } - if (!nat_entry_is_v6(nat_entry)) { - sset_add(&od->snat_ips, nat_entry->ext_addrs.ipv4_addrs[0].addr_s); - } else { - sset_add(&od->snat_ips, nat_entry->ext_addrs.ipv6_addrs[0].addr_s); + /* If this is a SNAT rule add the IP to the set of unique SNAT IPs. */ + if (!strcmp(nat->type, "snat")) { + if (!nat_entry_is_v6(nat_entry)) { + snat_ip_add(od, nat_entry->ext_addrs.ipv4_addrs[0].addr_s, + nat_entry); + } else { + snat_ip_add(od, nat_entry->ext_addrs.ipv6_addrs[0].addr_s, + nat_entry); + } } } } @@ -736,7 +771,7 @@ destroy_nat_entries(struct ovn_datapath *od) return; } - sset_destroy(&od->snat_ips); + shash_destroy_free_data(&od->snat_ips); destroy_lport_addresses(&od->dnat_force_snat_addrs); destroy_lport_addresses(&od->lb_force_snat_addrs); @@ -8728,6 +8763,65 @@ build_lrouter_port_nat_arp_nd_flow(struct ovn_port *op, } static void +build_lrouter_drop_own_dest(struct ovn_port *op, enum ovn_stage stage, + uint16_t priority, bool drop_snat, + struct hmap *lflows) +{ + struct ds match_ips = DS_EMPTY_INITIALIZER; + + if (op->lrp_networks.n_ipv4_addrs) { + for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { + const char *ip = op->lrp_networks.ipv4_addrs[i].addr_s; + + if (drop_snat && !shash_find(&op->od->snat_ips, ip)) { + continue; + } else if (!drop_snat && shash_find(&op->od->snat_ips, ip)) { + continue; + } + ds_put_format(&match_ips, "%s, ", ip); + } + + if (ds_last(&match_ips) != EOF) { + ds_chomp(&match_ips, ' '); + ds_chomp(&match_ips, ','); + + char *match = xasprintf("ip4.dst == {%s}", ds_cstr(&match_ips)); + ovn_lflow_add_with_hint(lflows, op->od, stage, priority, + match, "drop;", + &op->nbrp->header_); + free(match); + } + } + + if (op->lrp_networks.n_ipv6_addrs) { + ds_clear(&match_ips); + + for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { + const char *ip = op->lrp_networks.ipv6_addrs[i].addr_s; + + if (drop_snat && !shash_find(&op->od->snat_ips, ip)) { + continue; + } else if (!drop_snat && shash_find(&op->od->snat_ips, ip)) { + continue; + } + ds_put_format(&match_ips, "%s, ", ip); + } + + if (ds_last(&match_ips) != EOF) { + ds_chomp(&match_ips, ' '); + ds_chomp(&match_ips, ','); + + char *match = xasprintf("ip6.dst == {%s}", ds_cstr(&match_ips)); + ovn_lflow_add_with_hint(lflows, op->od, stage, priority, + match, "drop;", + &op->nbrp->header_); + free(match); + } + } + ds_destroy(&match_ips); +} + +static void build_lrouter_force_snat_flows(struct hmap *lflows, struct ovn_datapath *od, const char *ip_version, const char *ip_addr, const char *context) @@ -8879,68 +8973,6 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, op, lflows, &match, &actions); } - /* Drop IP traffic destined to router owned IPs. Part of it is dropped - * in stage "lr_in_ip_input" but traffic that could have been unSNATed - * but didn't match any existing session might still end up here. - */ - HMAP_FOR_EACH (op, key_node, ports) { - if (!op->nbrp) { - continue; - } - - if (op->lrp_networks.n_ipv4_addrs) { - ds_clear(&match); - for (size_t i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { - if (!sset_find(&op->od->snat_ips, - op->lrp_networks.ipv4_addrs[i].addr_s)) { - continue; - } - ds_put_format(&match, "%s, ", - op->lrp_networks.ipv4_addrs[i].addr_s); - } - - if (ds_last(&match) != EOF) { - ds_chomp(&match, ' '); - ds_chomp(&match, ','); - - char *drop_match = xasprintf("ip4.dst == {%s}", - ds_cstr(&match)); - /* Drop traffic with IP.dest == router-ip. */ - ovn_lflow_add_with_hint(lflows, op->od, - S_ROUTER_IN_ARP_RESOLVE, 1, - drop_match, "drop;", - &op->nbrp->header_); - free(drop_match); - } - } - - if (op->lrp_networks.n_ipv6_addrs) { - ds_clear(&match); - for (size_t i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { - if (!sset_find(&op->od->snat_ips, - op->lrp_networks.ipv6_addrs[i].addr_s)) { - continue; - } - ds_put_format(&match, "%s, ", - op->lrp_networks.ipv6_addrs[i].addr_s); - } - - if (ds_last(&match) != EOF) { - ds_chomp(&match, ' '); - ds_chomp(&match, ','); - - char *drop_match = xasprintf("ip6.dst == {%s}", - ds_cstr(&match)); - /* Drop traffic with IP.dest == router-ip. */ - ovn_lflow_add_with_hint(lflows, op->od, - S_ROUTER_IN_ARP_RESOLVE, 1, - drop_match, "drop;", - &op->nbrp->header_); - free(drop_match); - } - } - } - HMAP_FOR_EACH (od, key_node, datapaths) { if (!od->nbr) { continue; @@ -8966,29 +8998,37 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, * port to handle the special cases. In case we get the packet * on a regular port, just reply with the port's ETH address. */ - struct sset snat_ips = SSET_INITIALIZER(&snat_ips); for (int i = 0; i < od->nbr->n_nat; i++) { struct ovn_nat *nat_entry = &od->nat_entries[i]; - const struct nbrec_nat *nat = nat_entry->nb; /* Skip entries we failed to parse. */ if (!nat_entry_is_valid(nat_entry)) { continue; } - struct lport_addresses *ext_addrs = &nat_entry->ext_addrs; - char *ext_addr = nat_entry_is_v6(nat_entry) ? - ext_addrs->ipv6_addrs[0].addr_s : - ext_addrs->ipv4_addrs[0].addr_s; + /* Skip SNAT entries for now, we handle unique SNAT IPs separately + * below. + */ + if (!strcmp(nat_entry->nb->type, "snat")) { + continue; + } + build_lrouter_nat_arp_nd_flow(od, nat_entry, lflows); + } - if (!strcmp(nat->type, "snat")) { - if (!sset_add(&snat_ips, ext_addr)) { - continue; - } + /* Now handle SNAT entries too, one per unique SNAT IP. */ + struct shash_node *snat_snode; + SHASH_FOR_EACH (snat_snode, &od->snat_ips) { + struct ovn_snat_ip *snat_ip = snat_snode->data; + + if (ovs_list_is_empty(&snat_ip->snat_entries)) { + continue; } + + struct ovn_nat *nat_entry = + CONTAINER_OF(ovs_list_front(&snat_ip->snat_entries), + struct ovn_nat, ext_addr_list_node); build_lrouter_nat_arp_nd_flow(od, nat_entry, lflows); } - sset_destroy(&snat_ips); /* Drop ARP packets (priority 85). ARP request packets for router's own * IPs are handled with priority-90 flows. @@ -9219,82 +9259,59 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports, } } - /* A gateway router can have 4 SNAT IP addresses to force DNATed and - * LBed traffic respectively to be SNATed. In addition, there can be - * a number of SNAT rules in the NAT table. - * Skip all of them for drop flows. */ - ds_clear(&match); - ds_put_cstr(&match, "ip4.dst == {"); - bool has_drop_ips = false; - for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { - if (sset_find(&op->od->snat_ips, - op->lrp_networks.ipv4_addrs[i].addr_s)) { - continue; - } - ds_put_format(&match, "%s, ", - op->lrp_networks.ipv4_addrs[i].addr_s); - has_drop_ips = true; - } - if (has_drop_ips) { - ds_chomp(&match, ' '); - ds_chomp(&match, ','); - ds_put_cstr(&match, "} || ip6.dst == {"); - } else { - ds_clear(&match); - ds_put_cstr(&match, "ip6.dst == {"); - } - - for (int i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { - if (sset_find(&op->od->snat_ips, - op->lrp_networks.ipv6_addrs[i].addr_s)) { - continue; - } - ds_put_format(&match, "%s, ", - op->lrp_networks.ipv6_addrs[i].addr_s); - has_drop_ips = true; - } - - ds_chomp(&match, ' '); - ds_chomp(&match, ','); - ds_put_cstr(&match, "}"); - - if (has_drop_ips) { - /* Drop IP traffic to this router. */ - ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_INPUT, 60, - ds_cstr(&match), "drop;", - &op->nbrp->header_); - } + /* Drop IP traffic destined to router owned IPs except if the IP is + * also a SNAT IP. Those are dropped later, in stage + * "lr_in_arp_resolve", if unSNAT was unsuccessful. + * + * Priority 60. + */ + build_lrouter_drop_own_dest(op, S_ROUTER_IN_IP_INPUT, 60, false, + lflows); - /* ARP/NS packets are taken care of per router. The only exception - * is on the l3dgw_port where we might need to use a different - * ETH address. + /* ARP / ND handling for external IP addresses. + * + * DNAT and SNAT IP addresses are external IP addresses that need ARP + * handling. + * + * These are already taken care globally, per router. The only + * exception is on the l3dgw_port where we might need to use a + * different ETH address. */ if (op != op->od->l3dgw_port) { continue; } - struct sset sset_snat_ips = SSET_INITIALIZER(&sset_snat_ips); 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; /* Skip entries we failed to parse. */ if (!nat_entry_is_valid(nat_entry)) { continue; } - struct lport_addresses *ext_addrs = &nat_entry->ext_addrs; - char *ext_addr = (nat_entry_is_v6(nat_entry) - ? ext_addrs->ipv6_addrs[0].addr_s - : ext_addrs->ipv4_addrs[0].addr_s); - if (!strcmp(nat->type, "snat")) { - if (!sset_add(&sset_snat_ips, ext_addr)) { - continue; - } + /* Skip SNAT entries for now, we handle unique SNAT IPs separately + * below. + */ + if (!strcmp(nat_entry->nb->type, "snat")) { + continue; + } + build_lrouter_port_nat_arp_nd_flow(op, nat_entry, lflows); + } + + /* Now handle SNAT entries too, one per unique SNAT IP. */ + struct shash_node *snat_snode; + SHASH_FOR_EACH (snat_snode, &op->od->snat_ips) { + struct ovn_snat_ip *snat_ip = snat_snode->data; + + if (ovs_list_is_empty(&snat_ip->snat_entries)) { + continue; } + + struct ovn_nat *nat_entry = + CONTAINER_OF(ovs_list_front(&snat_ip->snat_entries), + struct ovn_nat, ext_addr_list_node); build_lrouter_port_nat_arp_nd_flow(op, nat_entry, lflows); } - sset_destroy(&sset_snat_ips); } /* DHCPv6 reply handling */ @@ -10813,6 +10830,15 @@ build_arp_resolve_flows_for_lrouter_port( &op->nbrp->header_); } } + + /* Drop IP traffic destined to router owned IPs. Part of it is dropped + * in stage "lr_in_ip_input" but traffic that could have been unSNATed + * but didn't match any existing session might still end up here. + * + * Priority 1. + */ + build_lrouter_drop_own_dest(op, S_ROUTER_IN_ARP_RESOLVE, 1, true, + lflows); } else if (op->od->n_router_ports && strcmp(op->nbsp->type, "router") && strcmp(op->nbsp->type, "virtual")) { /* This is a logical switch port that backs a VM or a container.