From patchwork Mon Sep 28 10:08:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1372498 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=iE3uyirP; 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 4C0JBp0x6bz9ryj for ; Mon, 28 Sep 2020 20:08:50 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 7F0578494E; Mon, 28 Sep 2020 10:08: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 CSazz9C_dn5x; Mon, 28 Sep 2020 10:08:47 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id 274C084545; Mon, 28 Sep 2020 10:08:47 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1A7C0C016F; Mon, 28 Sep 2020 10:08:47 +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 A9BEBC0051 for ; Mon, 28 Sep 2020 10:08:46 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 988E586F89 for ; Mon, 28 Sep 2020 10:08:46 +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 jzROuD+x53VZ for ; Mon, 28 Sep 2020 10:08:45 +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 [216.205.24.124]) by hemlock.osuosl.org (Postfix) with ESMTPS id 0E83786F88 for ; Mon, 28 Sep 2020 10:08:44 +0000 (UTC) Dkim-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1601287724; 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=W45vILrPQi98Uoba6o4MTPtvr1MQR60KZqJIlCtZ5ic=; b=iE3uyirPFXvIO1HUAQvefvoMkiO5qzmGY/ZlKWxTo3r4F3fwH4PM8yumHfonFwTEu965vU duULB44ptftK56bLZwj+VUIC6hTia5kFcjeSxwES6bUsDswpPodCxpXxducX3RPHgKTiL7 qjQeoVV+5XadV5MIKV9MT975kvx+Z+8= 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-209-KqAZ4fyAODGci4Ug6IIK1Q-1; Mon, 28 Sep 2020 06:08:42 -0400 X-MC-Unique: KqAZ4fyAODGci4Ug6IIK1Q-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 02DE0802B61 for ; Mon, 28 Sep 2020 10:08:41 +0000 (UTC) Received: from dceara.remote.csb (ovpn-113-160.ams2.redhat.com [10.36.113.160]) by smtp.corp.redhat.com (Postfix) with ESMTP id 56C0D81C56 for ; Mon, 28 Sep 2020 10:08:40 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Mon, 28 Sep 2020 12:08:37 +0200 Message-Id: <20200928100835.25333.47638.stgit@dceara.remote.csb> In-Reply-To: <20200928100802.25333.36491.stgit@dceara.remote.csb> References: <20200928100802.25333.36491.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.13 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 v4 ovn 2/2] 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. Acked-by: Han Zhou Signed-off-by: Dumitru Ceara --- northd/ovn-northd.c | 322 ++++++++++++++++++++++++++++----------------------- 1 file changed, 174 insertions(+), 148 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 5fddc5e..a39cb0e 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); + 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); + 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) @@ -8897,68 +8991,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; @@ -8972,29 +9004,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); } HMAP_FOR_EACH (od, key_node, datapaths) { @@ -9193,82 +9233,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); } HMAP_FOR_EACH (op, key_node, ports) { @@ -10642,6 +10659,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.