acl: fix matching rule
authorKonstantin Ananyev <konstantin.ananyev@intel.com>
Wed, 3 Jun 2015 17:45:17 +0000 (18:45 +0100)
committerThomas Monjalon <thomas.monjalon@6wind.com>
Thu, 4 Jun 2015 09:14:45 +0000 (11:14 +0200)
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 <huzilucky@gmail.com>
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
lib/librte_acl/acl_bld.c

index a406737..92a85df 100644 (file)
@@ -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;
        }