From patchwork Thu Dec 15 14:34:14 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mickey Spiegel X-Patchwork-Id: 706134 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3tfbd25XYRz9t0q for ; Fri, 16 Dec 2016 01:37:26 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="l7G+1rdR"; dkim-atps=neutral Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 9AA8BC05; Thu, 15 Dec 2016 14:34:38 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 17197BE0 for ; Thu, 15 Dec 2016 14:34:35 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pf0-f196.google.com (mail-pf0-f196.google.com [209.85.192.196]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 5D9F320A for ; Thu, 15 Dec 2016 14:34:34 +0000 (UTC) Received: by mail-pf0-f196.google.com with SMTP id i88so2985842pfk.2 for ; Thu, 15 Dec 2016 06:34:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=UtZJ+RXicNFkwWhTMowNS8oyg65+jFL21HXhZ+c0L10=; b=l7G+1rdRGbWKz6Vp6peJBlPcbE99b1Ufpd02TvnGmIMOdd7Bm4BnQlOBoGzd8RsHJ7 qhyqMUW/hSpnJC8sGaSPgWW0YxmfMURlHD0g5/wW5LtL0SeCIFn7OeR69QLAdgR2IkZB TqCXRuW4PcG50yqaMXeicDHJDBw0GQ80Ve/7UP1wyVAGqN3WDvgvPRSSlhJZ1/SFgwwk zs/nzIjVjV/H9y0N0Det3HgdRNy2uzHIPx+9LAy5xqojUCfHbpG5H1ITxxfnEt0hE1MU E8jM4XTsjbAljP+vrAPVNcdQNKBSwJRPsaUbm6u5jCzYTlv54eHR9OATL2sjbHwV4X6g hebA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=UtZJ+RXicNFkwWhTMowNS8oyg65+jFL21HXhZ+c0L10=; b=FQrgcZAJTr/MkvXs0VHG7zaua/Np0S9auBWUkTGPYz3E+eRCEqSwHGNPwPGZkAQ/zl vX/DUJOTTT0e5OnaSHkdAznUHX4ffYyEw3c7MfaE3CwYdOngoBcmqp3UziYatrChqwLw L7EeOWntKJuua7lZP6ZV+AovVG1mf1dSqVj7tBc4azzwVkYAzE848ooFrHk5gcft+FCz XjxF9NAMOkN1tkkQZNJOvYXzDszBETLekR09H7zWUDJkxDp84CzwM6vd+iXuuWz0inYG 8gma/GrAC6u88RrN7j3BbAPOtlht7TPzzf/i++4ZNkIHwsxSgRQ4LmNG3KHie4UouP3R WEWA== X-Gm-Message-State: AKaTC03kRcofzONEsUqFJcoVO0olXqnF8ohegdv36S7q/QeVyIGM4rljV447ZqAVXgIAfg== X-Received: by 10.84.212.144 with SMTP id e16mr3204559pli.140.1481812473724; Thu, 15 Dec 2016 06:34:33 -0800 (PST) Received: from localhost.localdomain (c-73-202-53-195.hsd1.ca.comcast.net. [73.202.53.195]) by smtp.gmail.com with ESMTPSA id t21sm5191050pfa.1.2016.12.15.06.34.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 15 Dec 2016 06:34:33 -0800 (PST) From: Mickey Spiegel To: dev@openvswitch.org Date: Thu, 15 Dec 2016 06:34:14 -0800 Message-Id: <1481812455-13149-5-git-send-email-mickeys.dev@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1481812455-13149-1-git-send-email-mickeys.dev@gmail.com> References: <1481812455-13149-1-git-send-email-mickeys.dev@gmail.com> X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [RFC v3 4/5] ovn: avoid snat recirc only on gateway routers X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Currently, for performance reasons on gateway routers, ct_snat that does not specify an IP address does not immediately trigger recirculation. On gateway routers, ct_snat that does not specify an IP address happens in the UNSNAT pipeline stage, which is followed by the DNAT pipeline stage that triggers recirculation for all packets. This DNAT pipeline stage recirculation takes care of the recirculation needs of UNSNAT as well as other cases such as UNDNAT. On distributed routers, UNDNAT is handled in the egress pipeline stage, separately from DNAT in the ingress pipeline stages. The DNAT pipeline stage only triggers recirculation for some packets. Due to this difference in design, UNSNAT needs to trigger its own recirculation. This patch restricts the logic that avoids recirculation for ct_snat, so that it only applies to datapaths representing gateway routers. Signed-off-by: Mickey Spiegel --- include/ovn/actions.h | 3 +++ ovn/controller/lflow.c | 10 ++++++++++ ovn/lib/actions.c | 15 +++++++++------ tests/ovn.at | 2 +- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/include/ovn/actions.h b/include/ovn/actions.h index 0bf6145..0451c08 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -417,6 +417,9 @@ struct ovnact_encode_params { /* 'true' if the flow is for a switch. */ bool is_switch; + /* 'true' if the flow is for a gateway router. */ + bool is_gateway_router; + /* A map from a port name to its connection tracking zone. */ const struct simap *ct_zones; diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c index a6aea48..2c879a0 100644 --- a/ovn/controller/lflow.c +++ b/ovn/controller/lflow.c @@ -126,6 +126,15 @@ is_switch(const struct sbrec_datapath_binding *ldp) } +static bool +is_gateway_router(const struct sbrec_datapath_binding *ldp, + const struct hmap *patched_datapaths) +{ + struct patched_datapath *pd = + get_patched_datapath(patched_datapaths, ldp->tunnel_key); + return pd ? pd->local : false; +} + /* Adds the logical flows from the Logical_Flow table to flow tables. */ static void add_logical_flows(struct controller_ctx *ctx, const struct lport_index *lports, @@ -275,6 +284,7 @@ consider_logical_flow(const struct lport_index *lports, .lookup_port = lookup_port_cb, .aux = &aux, .is_switch = is_switch(ldp), + .is_gateway_router = is_gateway_router(ldp, patched_datapaths), .ct_zones = ct_zones, .group_table = group_table, diff --git a/ovn/lib/actions.c b/ovn/lib/actions.c index df526c0..95725ca 100644 --- a/ovn/lib/actions.c +++ b/ovn/lib/actions.c @@ -787,12 +787,15 @@ encode_ct_nat(const struct ovnact_ct_nat *cn, ct = ofpacts->header; if (cn->ip) { ct->flags |= NX_CT_F_COMMIT; - } else if (snat) { - /* XXX: For performance reasons, we try to prevent additional - * recirculations. So far, ct_snat which is used in a gateway router - * does not need a recirculation. ct_snat(IP) does need a - * recirculation. Should we consider a method to let the actions - * specify whether an action needs recirculation if there more use + } else if (snat && ep->is_gateway_router) { + /* For performance reasons, we try to prevent additional + * recirculations. ct_snat which is used in a gateway router + * does not need a recirculation. ct_snat(IP) does need a + * recirculation. ct_snat in a distributed router needs + * recirculation regardless of whether an IP address is + * specified. + * XXX Should we consider a method to let the actions specify + * whether an action needs recirculation if there are more use * cases?. */ ct->recirc_table = NX_CT_RECIRC_NONE; } diff --git a/tests/ovn.at b/tests/ovn.at index d38d2c7..1691b77 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -852,7 +852,7 @@ ct_dnat(); # ct_snat ct_snat; - encodes as ct(zone=NXM_NX_REG12[0..15],nat) + encodes as ct(table=27,zone=NXM_NX_REG12[0..15],nat) has prereqs ip ct_snat(192.168.1.2); encodes as ct(commit,table=27,zone=NXM_NX_REG12[0..15],nat(src=192.168.1.2))