[dpdk-users] A confusion caused by the inconsistency in the source code of acl_gen.c. A possible bug?

[James Cape]

Please note there's still a bug in this function if dfa[128] is not some kind of magic value, because that position will never be checked against the "index" variable.


James Cape  Platform Team
IEX Group, Inc.  |  4 World Trade Center, 150 Greenwich St, 44th Floor, New York, NY 10007  |  w. 646.343.2242  |  www.iextrading.com<http://www.iextrading.com/>

________________________________
From: Wu, Xiaoban <Xiaoban_Wu at student.uml.edu>
Sent: Wednesday, February 8, 2017 1:41:03 PM
To: James Cape; users at dpdk.org
Subject: Re: [dpdk-users] A confusion caused by the inconsistency in the source code of acl_gen.c. A possible bug?


Dear James,


Thank you very much for your reply. You are right, I am so sorry that I did not pay attention to the difference between "x < UINT8_MAX + 1" and "x < INT8_MAX + 1". The two for-loops will never overlap. The confusion now is cleared off.


Best wishes,

Xiaoban




________________________________
From: James Cape <james.cape at iextrading.com>
Sent: Wednesday, February 8, 2017 9:18:43 AM
To: Wu, Xiaoban; users at dpdk.org
Subject: Re: [dpdk-users] A confusion caused by the inconsistency in the source code of acl_gen.c. A possible bug?


[Disclaimer: I'm not at all familiar with this code, or what it's doing, and I'm also probably an idiot [?] ]



I can't answer your specific question, but the loops aren't overlapping because QRANGE_MIN is defined as 0x80. However, it also looks like it's never going to check the (dfa[0x80] != index) case, because the +1's make the loops [0x081 - 0x100), and [0x00 - 0x80).



James Cape  Platform Team
IEX Group, Inc.  |  4 World Trade Center, 150 Greenwich St, 44th Floor, New York, NY 10007  |  w. 646.343.2242  |  www.iextrading.com<http://www.iextrading.com/>

________________________________
From: users <users-bounces at dpdk.org> on behalf of Wu, Xiaoban <Xiaoban_Wu at student.uml.edu>
Sent: Wednesday, February 8, 2017 3:26:14 AM
To: users at dpdk.org
Subject: [dpdk-users] A confusion caused by the inconsistency in the source code of acl_gen.c. A possible bug?

Dear DPDK users,


I have been reading and studying the source codes of the librte_acl, since I am very curious to know how to algorithmically implement a fast lookup process.


When I read the function acl_add_ptrs() in the acl_gen.c, there is one comment saying that

/*
* Rather than going from 0 to 256, the range count and
* the layout are from 80-ff then 0-7f due to signed compare
* for SSE (cmpgt).
*/


However, in the following codes, it is


              for (x = QRANGE_MIN + 1; x < UINT8_MAX + 1; x++) {
if (dfa[x] != index) {
index = dfa[x];
*node_a++ = index;
node->transitions[m++] = (uint8_t)(x - 1);
}
}

for (x = 0; x < INT8_MAX + 1; x++) {
if (dfa[x] != index) {
index = dfa[x];
*node_a++ = index;
node->transitions[m++] = (uint8_t)(x - 1);
}
}


As you can see that there are two for-loops, and the second for-loop include the first for-loop. And, this is not consistent with the comment. If the comment is followed, in the second for-loop, it should be "for (x = 0; x < QRANGE_MIN + 1; x++)", if we assume QRANGE_MIN is ff?


Moreover, due to that the first for-loop is included in the second for-loop and the "m" variable is increased by 1 whenever the if-statement is true, I am thinking about a case in which during the first for-loop, the "m" is increased by 3, hence after the second for-loop, the "m" is at least 6, this will finally break the check RTE_ACL_VERIFY(m <= RTE_ACL_QUAD_SIZE) in the following code, since RTE_ACL_QUAD_SIZE is 4. I am wondering if this case will ever happen?


Am I missing something here? Thanks very much for your help.


Best wishes,

Xiaoban

This email is subject to important notices and disclaimers regarding legal privilege, confidentiality, viruses and monitoring, available at www.iextrading.com/electroniccommunications<http://www.iextrading.com/electroniccommunications>

This email is subject to important notices and disclaimers regarding legal privilege, confidentiality, viruses and monitoring, available at www.iextrading.com/electroniccommunications<http://www.iextrading.com/electroniccommunications>