From patchwork Fri Sep 13 20:49:19 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 1162239 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com 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 46VSS24Bgcz9s7T for ; Sat, 14 Sep 2019 06:49:38 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 4CA3CDC1; Fri, 13 Sep 2019 20:49:34 +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 94ED7D9E for ; Fri, 13 Sep 2019 20:49:33 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 1A80C7D2 for ; Fri, 13 Sep 2019 20:49:33 +0000 (UTC) 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 mx1.redhat.com (Postfix) with ESMTPS id 86657300DA2E for ; Fri, 13 Sep 2019 20:49:32 +0000 (UTC) Received: from nummac.local (unknown [10.67.116.11]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9B09660A9F; Fri, 13 Sep 2019 20:49:30 +0000 (UTC) From: nusiddiq@redhat.com To: dev@openvswitch.org Date: Sat, 14 Sep 2019 02:19:19 +0530 Message-Id: <20190913204919.3596-1-nusiddiq@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.47]); Fri, 13 Sep 2019 20:49:32 +0000 (UTC) X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI 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] [PATCH ovn] Exclude inport and outport symbol tables from conjunction 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: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org From: Numan Siddique If there are multiple ACLs associated with a port group and they match on a range of some field, then ovn-controller doesn't install the flows properly and this results in broken ACL functionality. For example, if there is a port group - pg1 with logical ports - [p1, p2] and if there are below ACLs (only match condition is shown) 1 - outport == @pg1 && ip4 && tcp.dst >= 500 && tcp.dst <= 501 2 - outport == @pg1 && ip4 && tcp.dst >= 600 && tcp.dst <= 601 The first ACL will result in the below OF flows 1. conj_id=1,tcp 2. tcp,reg15=0x11: conjunction(1, 1/2) 3. tcp,reg15=0x12: conjunction(1, 1/2) 5. tcp,tp_dst=500: conjunction(1, 2/2) 6. tcp,tp_dst=501: conjunction(1, 2/2) The second ACL will result in the below OF flows 7. conj_id=2,tcp 8. tcp,reg15=0x11: conjunction(2, 1/2) 9. tcp,reg15=0x12: conjunction(2, 1/2) 11. tcp,tp_dst=600: conjunction(2, 2/2) 12. tcp,tp_dst=601: conjunction(2, 3/2) The OF flows (2) and (8) have the exact match but with different action. This results in only one of the flows getting installed. The same goes for the flows (3) and (9). And this completely breaks the ACL functionality for such scenarios. In order to fix this issue, this patch excludes the 'inport' and 'outport' symbols from conjunction. With this patch we will have the below flows. tcp,reg15=0x11,tp_dst=500 tcp,reg15=0x11,tp_dst=501 tcp,reg15=0x12,tp_dst=500 tcp,reg15=0x12,tp_dst=501 tcp,reg15=0x13,tp_dst=500 tcp,reg15=0x13,tp_dst=501 tcp,reg15=0x11,tp_dst=600 tcp,reg15=0x11,tp_dst=601 tcp,reg15=0x12,tp_dst=600 tcp,reg15=0x12,tp_dst=601 tcp,reg15=0x13,tp_dst=600 tcp,reg15=0x13,tp_dst=601 Signed-off-by: Numan Siddique Acked-by: Mark Michelson Acked-by: Daniel Alvarez --- lib/expr.c | 2 +- tests/ovn.at | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/lib/expr.c b/lib/expr.c index e4c650f7c..c0871e1e8 100644 --- a/lib/expr.c +++ b/lib/expr.c @@ -1499,7 +1499,7 @@ expr_symtab_add_string(struct shash *symtab, const char *name, const struct mf_field *field = mf_from_id(id); struct expr_symbol *symbol; - symbol = add_symbol(symtab, name, 0, prereqs, EXPR_L_NOMINAL, false, + symbol = add_symbol(symtab, name, 0, prereqs, EXPR_L_NOMINAL, true, field->writable); symbol->field = field; return symbol; diff --git a/tests/ovn.at b/tests/ovn.at index 2a35b4e15..14d9f59b0 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -589,6 +589,24 @@ ip,reg14=0x6 ipv6,reg14=0x5 ipv6,reg14=0x6 ]) +AT_CHECK([expr_to_flow 'inport == {"eth0", "eth1", "eth2"} && ip4 && tcp && tcp.dst == {500, 501}'], [0], [dnl +tcp,reg14=0x5,tp_dst=500 +tcp,reg14=0x5,tp_dst=501 +tcp,reg14=0x6,tp_dst=500 +tcp,reg14=0x6,tp_dst=501 +]) +AT_CHECK([expr_to_flow 'outport == {"eth0", "eth1", "eth2"} && ip4 && tcp && tcp.src == {400, 401} && tcp.dst == {500, 501}'], [0], [dnl +conj_id=1,tcp,reg15=0x5 +conj_id=2,tcp,reg15=0x6 +tcp,reg15=0x5,tp_dst=500: conjunction(1, 0/2) +tcp,reg15=0x5,tp_dst=501: conjunction(1, 0/2) +tcp,reg15=0x5,tp_src=400: conjunction(1, 1/2) +tcp,reg15=0x5,tp_src=401: conjunction(1, 1/2) +tcp,reg15=0x6,tp_dst=500: conjunction(2, 0/2) +tcp,reg15=0x6,tp_dst=501: conjunction(2, 0/2) +tcp,reg15=0x6,tp_src=400: conjunction(2, 1/2) +tcp,reg15=0x6,tp_src=401: conjunction(2, 1/2) +]) AT_CHECK([expr_to_flow 'inport == "eth0" && inport == "eth1"'], [0], [dnl (no flows) ]) @@ -693,6 +711,14 @@ reg15=0x11 reg15=0x12 reg15=0x13 ]) +AT_CHECK([expr_to_flow 'outport == @pg1 && ip4.src == {10.0.0.4, 10.0.0.5}'], [0], [dnl +ip,reg15=0x11,nw_src=10.0.0.4 +ip,reg15=0x11,nw_src=10.0.0.5 +ip,reg15=0x12,nw_src=10.0.0.4 +ip,reg15=0x12,nw_src=10.0.0.5 +ip,reg15=0x13,nw_src=10.0.0.4 +ip,reg15=0x13,nw_src=10.0.0.5 +]) AT_CHECK([expr_to_flow 'outport == {@pg_empty}'], [0], [dnl (no flows) ])