From patchwork Fri Oct 29 17:26:47 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1548135 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=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (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 4Hgq9r0b5Qz9sRK for ; Sat, 30 Oct 2021 04:27:10 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id EA8FE4028E; Fri, 29 Oct 2021 17:27:06 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id HiNzohpg_9SL; Fri, 29 Oct 2021 17:27:06 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 29D1440246; Fri, 29 Oct 2021 17:27:05 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id DA583C001E; Fri, 29 Oct 2021 17:27:04 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id E4C6EC0012 for ; Fri, 29 Oct 2021 17:27:03 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id C102880F0F for ; Fri, 29 Oct 2021 17:27:03 +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 HK9KXUxjURWq for ; Fri, 29 Oct 2021 17:27:02 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay12.mail.gandi.net (relay12.mail.gandi.net [217.70.178.232]) by smtp1.osuosl.org (Postfix) with ESMTPS id 4893780EB6 for ; Fri, 29 Oct 2021 17:27:01 +0000 (UTC) Received: (Authenticated sender: numans@ovn.org) by relay12.mail.gandi.net (Postfix) with ESMTPSA id 1C3FF200008; Fri, 29 Oct 2021 17:26:57 +0000 (UTC) From: numans@ovn.org To: dev@openvswitch.org Date: Fri, 29 Oct 2021 13:26:47 -0400 Message-Id: <20211029172648.3859172-1-numans@ovn.org> X-Mailer: git-send-email 2.31.1 MIME-Version: 1.0 Subject: [ovs-dev] [PATCH] ofproto-dpif-xlate: Fix check_pkt_larger incomplete translation. 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" From: Numan Siddique xlate_check_pkt_larger() sets ctx->exit to 'true' at the end causing the translation to stop. This results in incomplete datapath rules. For example, for the below OF rules configured on a bridge, table=0,in_port=1 actions=load:0x1->NXM_NX_REG1[[]],resubmit(,1),load:0x2->NXM_NX_REG1[[]],resubmit(,1),load:0x3->NXM_NX_REG1[[]],resubmit(,1) table=1,in_port=1,reg1=0x1 actions=check_pkt_larger(200)->NXM_NX_REG0[[0]],resubmit(,4) table=1,in_port=1,reg1=0x2 actions=output:2 table=1,in_port=1,reg1=0x3 actions=output:4 table=4,in_port=1 actions=output:3 the datapath flow should be check_pkt_len(size=200,gt(3),le(3)),2,4 But right now it is: check_pkt_len(size=200,gt(3),le(3)) Actions after the first resubmit(,1) in the first flow in table 0 are never applied. This patch fixes this issue. Fixes: 5b34f8fc3b38 ("Add a new OVS action check_pkt_larger") Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2018365 Reported-by: Ihar Hrachyshka Signed-off-by: Numan Siddique --- ofproto/ofproto-dpif-xlate.c | 9 ++++++++- tests/ofproto-dpif.at | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 9d336bc6a6..e22a39a9f4 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -6371,6 +6371,7 @@ xlate_check_pkt_larger(struct xlate_ctx *ctx, * then ctx->exit would be true. Reset to false so that we can * do flow translation for 'IF_LESS_EQUAL' case. finish_freezing() * would have taken care of Undoing the changes done for freeze. */ + bool old_exit = ctx->exit; ctx->exit = false; offset_attr = nl_msg_start_nested( @@ -6395,7 +6396,7 @@ xlate_check_pkt_larger(struct xlate_ctx *ctx, ctx->was_mpls = old_was_mpls; ctx->conntracked = old_conntracked; ctx->xin->flow = old_flow; - ctx->exit = true; + ctx->exit = old_exit; } static void @@ -7192,6 +7193,12 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len, ofpacts_len); xlate_check_pkt_larger(ctx, ofpact_get_CHECK_PKT_LARGER(a), remaining_acts, remaining_acts_len); + if (ctx->xbridge->support.check_pkt_len) { + /* If datapath supports check_pkt_len, then + * xlate_check_pkt_larger() does the translation for the + * ofpacts following 'a'. */ + a = ofpact_end(ofpacts, ofpacts_len); + } break; } } diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 31fb163a91..f7c8f98ce5 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -11452,6 +11452,23 @@ Megaflow: recirc_id=0x3,eth,ip,in_port=1,nw_frag=no Datapath actions: 4 ]) +ovs-ofctl del-flows br0 + +AT_DATA([flows.txt], [dnl +table=0,in_port=1 actions=load:0x1->NXM_NX_REG1[[]],resubmit(,1),load:0x2->NXM_NX_REG1[[]],resubmit(,1),load:0x3->NXM_NX_REG1[[]],resubmit(,1) +table=1,in_port=1,reg1=0x1 actions=check_pkt_larger(200)->NXM_NX_REG0[[0]],resubmit(,4) +table=1,in_port=1,reg1=0x2 actions=output:2 +table=1,in_port=1,reg1=0x3 actions=output:4 +table=4,in_port=1 actions=output:3 +]) + +AT_CHECK([ovs-ofctl --protocols=OpenFlow10 add-flows br0 flows.txt]) +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) +AT_CHECK([cat stdout | grep Datapath -B1], [0], [dnl +Megaflow: recirc_id=0,eth,ip,in_port=1,nw_frag=no +Datapath actions: check_pkt_len(size=200,gt(3),le(3)),2,4 +]) + OVS_VSWITCHD_STOP AT_CLEANUP