From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konstantin Ananyev Subject: [PATCHv2 1/3] ACL: fix a problem in acl_merge_trie Date: Wed, 3 Jun 2015 18:45:17 +0100 Message-ID: <1433353519-20589-2-git-send-email-konstantin.ananyev@intel.com> References: <1433353519-20589-1-git-send-email-konstantin.ananyev@intel.com> To: dev@dpdk.org Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 2CAB25693 for ; Wed, 3 Jun 2015 19:45:25 +0200 (CEST) In-Reply-To: <1433353519-20589-1-git-send-email-konstantin.ananyev@intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Reported by Zi Hu : "... 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 --- 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; } -- 2.4.2