Re: [dpdk-dev] DPDK ACL bug? pkt matches the wrong ACL rule.

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

Hi Zi,

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zi Hu
> Sent: Friday, May 15, 2015 1:27 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] DPDK ACL bug? pkt matches the wrong ACL rule.
> 
> Hi, there,
> 
> I recently noticed that sometimes packets are matched with the wrong ACL
> rules when using the DPDK ACL library.
> 
> I tested it with the "testacl" under dpdk/build/app:
> Here are my rule file and trace file:
> 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
> 
> Result:
> .....
> acl context <TESTACL>@0x7f5b43effac0
>   socket_id=-1
>   alg=2
>   max_rules=65536
>   rule_size=96
>   num_rules=3
>   num_categories=3
>   num_tries=1
> ipv4_5tuple: 1, category: 0, result: 1
> search_ip5tuples_once(1, 256, sse) returns 1
> search_ip5tuples  @lcore 2: 1 iterations, 1 pkts, 1 categories, 21812
> cycles, 21812.000000 cycles/pkt
> 
> 
> 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.
> How possible could it match the second rule?  Anyone see similar situation
> before?
> 
> Another interesting I found  is that if we make the dest port range to be
> 54 : 65279 in the second rule (only change 65280 to 65279, all other stuff
> remains the same):
> 
> 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 : 65279 6/0xff
> @192.168.0.0/24 192.168.0.0/24 400 : 500 0 : 65535 6/0xff
> 
> Then run the test again, the packet matches the third rule as expected.
> 
> 
> This seems really weird to me. Anyone has an explanation for that?
> 
> thanks
> -Zi

I think I found the culprit.
Problem is that acl_merge_trie()  sometimes is too aggressive in trying to conserve some space at build time.
So didn't duplicate a node, even when it should.
Could you try a patch below?
Thanks
Konstantin

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