From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <kananye1@ecsmtp.ir.intel.com>
Received: from mga11.intel.com (mga11.intel.com [192.55.52.93])
 by dpdk.org (Postfix) with ESMTP id 4F1B05A30
 for <dev@dpdk.org>; Fri, 22 May 2015 19:49:05 +0200 (CEST)
Received: from fmsmga001.fm.intel.com ([10.253.24.23])
 by fmsmga102.fm.intel.com with ESMTP; 22 May 2015 10:49:04 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.13,477,1427785200"; d="scan'208";a="714364858"
Received: from irvmail001.ir.intel.com ([163.33.26.43])
 by fmsmga001.fm.intel.com with ESMTP; 22 May 2015 10:49:03 -0700
Received: from sivswdev02.ir.intel.com (sivswdev02.ir.intel.com
 [10.237.217.46])
 by irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id
 t4MHn2rA025961; Fri, 22 May 2015 18:49:02 +0100
Received: from sivswdev02.ir.intel.com (localhost [127.0.0.1])
 by sivswdev02.ir.intel.com with ESMTP id t4MHn2LP018457;
 Fri, 22 May 2015 18:49:02 +0100
Received: (from kananye1@localhost)
 by sivswdev02.ir.intel.com with  id t4MHn2cG018453;
 Fri, 22 May 2015 18:49:02 +0100
From: Konstantin Ananyev <konstantin.ananyev@intel.com>
To: dev@dpdk.org
Date: Fri, 22 May 2015 18:48:49 +0100
Message-Id: <1432316931-18406-2-git-send-email-konstantin.ananyev@intel.com>
X-Mailer: git-send-email 1.7.4.1
In-Reply-To: <1432316931-18406-1-git-send-email-konstantin.ananyev@intel.com>
References: <1432316931-18406-1-git-send-email-konstantin.ananyev@intel.com>
Subject: [dpdk-dev] [PATCH 1/3] ACL: fix a problem in acl_merge_trie
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Fri, 22 May 2015 17:49:05 -0000

Reported by Zi Hu <huzilucky@gmail.com>:

"...
cat test_data/rule1
@192.168.0.0/24 192.168.0.0/24 400 : 500 0 : 52 6/0xff
@192.168.0.0/24 192.168.0.0/24 400 : 500 54 : 65280 6/0xff
@192.168.0.0/24 192.168.0.0/24 400 : 500 0 : 65535 6/0xff

cat test_data/trace1
0xc0a80005 0xc0a80009 450 53 0x06

I run the test by:
sudo ./testacl -n 2 -c 4 -- --rulesf=./test_data/rule1
 --tracef=./test_data/trace1

The result shows that the packet matches the second rule,  which is wrong.
The dest port of the pkt is 53, so it should match the third rule."

Indeed there is problem at ACL build stage.
Sometimes acl_merge_trie() is too aggressive in trying to conserve
space at build time.
So it takes a wrong assumptions and didn't duplicate a node,
even when it should.
The easiest and safest fix seems to always duplicate a left non-root/non-leaf
node first, and let the further code to destroy the node, if it is not needed.

Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 lib/librte_acl/acl_bld.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lib/librte_acl/acl_bld.c b/lib/librte_acl/acl_bld.c
index a406737..92a85df 100644
--- a/lib/librte_acl/acl_bld.c
+++ b/lib/librte_acl/acl_bld.c
@@ -1044,9 +1044,7 @@ acl_merge_trie(struct acl_build_context *context,
 	 * a subtree of the merging tree (node B side). Otherwise,
 	 * just use node A.
 	 */
-	if (level > 0 &&
-			node_a->subtree_id !=
-			(subtree_id | RTE_ACL_SUBTREE_NODE)) {
+	if (level > 0) {
 		node_c = acl_dup_node(context, node_a);
 		node_c->subtree_id = subtree_id | RTE_ACL_SUBTREE_NODE;
 	}
-- 
1.8.5.3