From 2f372ab5c90c8e93e04f65d0fbdd34f6997f4de6 Mon Sep 17 00:00:00 2001 From: Konstantin Ananyev Date: Wed, 3 Jun 2015 18:45:17 +0100 Subject: [PATCH] acl: fix matching rule 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. Reported-by: Zi Hu 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 a4067376c4..92a85dfb91 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.20.1