From patchwork Thu Nov 14 17:55:12 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Venugopal Iyer X-Patchwork-Id: 1195010 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.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=nvidia.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=nvidia.com header.i=@nvidia.com header.b="oIyTFAzg"; dkim-atps=neutral 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 47DTfc12P6z9s7T for ; Fri, 15 Nov 2019 04:55:35 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 697EDCC9; Thu, 14 Nov 2019 17:55:32 +0000 (UTC) X-Original-To: ovs-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 EEB81CBB for ; Thu, 14 Nov 2019 17:55:30 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from hqemgate16.nvidia.com (hqemgate16.nvidia.com [216.228.121.65]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 6EDB18A for ; Thu, 14 Nov 2019 17:55:30 +0000 (UTC) Received: from hqpgpgate101.nvidia.com (Not Verified[216.228.121.13]) by hqemgate16.nvidia.com (using TLS: TLSv1.2, DES-CBC3-SHA) id ; Thu, 14 Nov 2019 09:54:34 -0800 Received: from hqmail.nvidia.com ([172.20.161.6]) by hqpgpgate101.nvidia.com (PGP Universal service); Thu, 14 Nov 2019 09:55:29 -0800 X-PGP-Universal: processed; by hqpgpgate101.nvidia.com on Thu, 14 Nov 2019 09:55:29 -0800 Received: from HQMAIL101.nvidia.com (172.20.187.10) by HQMAIL105.nvidia.com (172.20.187.12) with Microsoft SMTP Server (TLS) id 15.0.1473.3; Thu, 14 Nov 2019 17:55:29 +0000 Received: from hqnvemgw03.nvidia.com (10.124.88.68) by HQMAIL101.nvidia.com (172.20.187.10) with Microsoft SMTP Server (TLS) id 15.0.1473.3 via Frontend Transport; Thu, 14 Nov 2019 17:55:29 +0000 Received: from nviyer-1.nvidia.com (Not Verified[172.16.173.224]) by hqnvemgw03.nvidia.com with Trustwave SEG (v7, 5, 8, 10121) id ; Thu, 14 Nov 2019 09:55:29 -0800 From: To: Date: Thu, 14 Nov 2019 09:55:12 -0800 Message-ID: <20191114175512.10286-1-venugopali@nvidia.com> X-Mailer: git-send-email 2.17.1 X-NVConfidentiality: public MIME-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nvidia.com; s=n1; t=1573754074; bh=vmlREtVHvDSZ87xJUn9kT6nrU9YLxB5/qXG707K1prs=; h=X-PGP-Universal:From:To:CC:Subject:Date:Message-ID:X-Mailer: X-NVConfidentiality:MIME-Version:Content-Type; b=oIyTFAzgLnzLHpHQ/6IvW9G/gLqLD47ZHDOfXu0utxXqGb/CmaUzC3aqd/Hw4utv/ fDel9SVKqzmg51svg2ZW7yYr4r6DmYCn9aiwCnR4jAtm3Uaa6YbJCXv4aqM0U0/W6V DV+enPH+aeCkZ3Yl9n+VYDBTeUBkTSDOXS7unU/2gk0GPrw4EXBaG+oO5tFJq22HB0 9I8XeZKIwnFf9kPsY62Ni05RDl0ix64PCSeFNqxOmZdR6bd+YxB8G0jLrkxcrybJyj AoLJDoNcanGZuhYoTwBv3lR+hJR2dsYYHAAwoMbr1gcFtd8HNitrNFLaBr1uUmrJAx RbKbYVQRl6HEQ== X-Spam-Status: No, score=-7.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, 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 Cc: ovs-dev@openvswitch.org, venu iyer Subject: [ovs-dev] [RFC ovn] ovn-northd: ls_*_acl behavior not consistent for untracked flows 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: venu iyer If one creates a port group and a MAC address set, and create an ACL that prevents packets being output to a port in the Port Group from any MAC address in the address set, the outcome is not consistent. The outcome depends on whether there is a stateful rule on the switch or not. Specifically: Assuming 'l2pg' is a port group with a list of ports and 'macs' is an Address Set with a list of MAC addresses and the intent is to drop all packets with source MAC address in 'macs' to any port in 'l2pg' using: ovn-nbctl acl-add to-lport 5000 \ "outport == @l2pg && eth.src == $macs" drop Without any stateful rule on the logical switch, the corresponding logical flow looks like: table=4 (ls_out_acl ), priority=6000,\ match=(outport == @l2pg && eth.src == $macs), \ action=(/* drop */) Based on this rule, any packet destined to the ports in 'l2pg' with source Address in 'macs' will be dropped - as is expected from the ACL above. While with a Stateful rule on the switch (any stateful rule will do), the same rule looks like: table=4 (ls_out_acl ), priority=6000, \ match=((!ct.est || (ct.est && ct_label.blocked == 1)) && \ (outport == @l2pg && eth.src == $macs)), action=(/* drop */) However, with this, only packets that are tracked will match the rule and be dropped, e.g. IP packets will be dropped, but ARP etc., will go through - this is not expected. Based on whether there are stateful rules or not on the switch, untracked packets will see different behavior. The fix is to complete the rules in the stateful case, i.e. instead of looking for flows that are not established or not new, we should first check if flows are not tracked. The fix was tested in the above scenario. Additionally, the following ACL was added to test the change in the "allow" case (i.e. to drop all the packets based on the above ACL, but have a higher priority rule that selectively allow ARP). ovn-nbctl acl-add ls1 to-lport 6000 \ "outport == @l2pg && eth.type == 0x806" allow with and without the staeful rule to make sure the behavior is the same. OVN test cases were run with this change and no failures were seen. Signed-off-by: venu iyer --- northd/ovn-northd.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 6742bc002..1f5f97796 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -4585,12 +4585,12 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od, * deletion. There is no need to commit here, so we can just * proceed to the next table. We use this to ensure that this * connection is still allowed by the currently defined - * policy. */ + * policy. Match untracked packets too. */ ds_clear(&match); ds_clear(&actions); ds_put_format(&match, - "!ct.new && ct.est && !ct.rpl" - " && ct_label.blocked == 0 && (%s)", + "(!ct.trk || (!ct.new && ct.est && !ct.rpl" + " && ct_label.blocked == 0)) && (%s)", acl->match); build_acl_log(&actions, acl); @@ -4613,10 +4613,10 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od, * depending on whether the connection was previously committed * to the connection tracker with ct_commit. */ if (has_stateful) { - /* If the packet is not part of an established connection, then + /* If the packet is not tracked or part of an established connection, then * we can simply reject/drop it. */ ds_put_cstr(&match, - "(!ct.est || (ct.est && ct_label.blocked == 1))"); + "(!ct.trk || !ct.est || (ct.est && ct_label.blocked == 1))"); if (!strcmp(acl->action, "reject")) { build_reject_acl_rules(od, lflows, stage, acl, &match, &actions);