[PATCH] acl: fix the value of the trans table

[PATCH] acl: fix the value of the trans table

[Huichao Cai]

The node_array[RTE_ACL_DFA_SIZE] is assigned to
RTE_ACL_IDLE_NODE and is used as a node of
RTE_ACL_NODE_SINGLE type, but it is currently based
on the implementation of idle arrays with a value of all
0 to point to itself, which is unsafe, if the value of the
idle array is not 0, it will produce undefined behavior,
so it needs to be given node_array[RTE_ACL_DFA_ SIZE]
plus RTE_ACL_QUAD_SINGLE.

Signed-off-by: Huichao Cai <chcchc88@163.com>
---
 lib/acl/acl_gen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/acl/acl_gen.c b/lib/acl/acl_gen.c
index 3c53d24056..95c7888162 100644
--- a/lib/acl/acl_gen.c
+++ b/lib/acl/acl_gen.c
@@ -497,7 +497,7 @@ rte_acl_gen(struct rte_acl_ctx *ctx, struct rte_acl_trie *trie,
 	 * highest index, that points to itself)
 	 */
 
-	node_array[RTE_ACL_DFA_SIZE] = RTE_ACL_IDLE_NODE;
+	node_array[RTE_ACL_DFA_SIZE] = RTE_ACL_QUAD_SINGLE | RTE_ACL_IDLE_NODE;
 
 	for (n = 0; n < RTE_ACL_DFA_SIZE; n++)
 		node_array[n] = no_match;
-- 
2.27.0

RE: [PATCH] acl: fix the value of the trans table

[Konstantin Ananyev]

Hi

> The node_array[RTE_ACL_DFA_SIZE] is assigned to
> RTE_ACL_IDLE_NODE and is used as a node of
> RTE_ACL_NODE_SINGLE type, but it is currently based
> on the implementation of idle arrays with a value of all
> 0 to point to itself, which is unsafe, if the value of the
> idle array is not 0, it will produce undefined behavior,
> so it needs to be given node_array[RTE_ACL_DFA_ SIZE]
> plus RTE_ACL_QUAD_SINGLE.

 Not sure I understand what is the problem you are trying to fix?
Any test-case which can demonstrate the failure?

> Signed-off-by: Huichao Cai <chcchc88@163.com>
> ---
>  lib/acl/acl_gen.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/acl/acl_gen.c b/lib/acl/acl_gen.c
> index 3c53d24056..95c7888162 100644
> --- a/lib/acl/acl_gen.c
> +++ b/lib/acl/acl_gen.c
> @@ -497,7 +497,7 @@ rte_acl_gen(struct rte_acl_ctx *ctx, struct rte_acl_trie *trie,
>  	 * highest index, that points to itself)
>  	 */
> 
> -	node_array[RTE_ACL_DFA_SIZE] = RTE_ACL_IDLE_NODE;
> +	node_array[RTE_ACL_DFA_SIZE] = RTE_ACL_QUAD_SINGLE | RTE_ACL_IDLE_NODE;
> 
>  	for (n = 0; n < RTE_ACL_DFA_SIZE; n++)
>  		node_array[n] = no_match;
> --
> 2.27.0

[PATCH] acl: fix the value of the trans table

[Huichao Cai]

Hi, Konstantin

To illustrate this issue, I made the following modifications to the test cases:
Modify the “test_data” content of the function "test_acl.c: test_build_ports_range":
delete the second test data, and leave only one test message data. This way, when
the function "acl_run_scalar.c: rte_acl_classify_scalar" processes the second message
data, it will use the idle data. Then modify the idle data in the function "acl_run.h:
acl_start_next_trie" to (to modify the idle, the const attribute was removed):
parms[n].data = (uint8_t *)idle;
parms[n].data[0] = 0x04;
parms[n].data[4] = 0x01;
When the function "acl_run_scalar.c: scalar_transition" processing this idle message,
because the ranges are 0 and the message data is not 0(0x01), the message data will
exceed all ranges. The calculated result x is 4, while the correct result should be 0. The
following is a comparison of the settlement results of x before and after modification
(I have printed each line of the calculation results of the function "acl_run_stcalar. c: scalar_translation"):

==before==:
Line 87: transition: 0x20000100
Line 88: ranges: 0x0
Line 89: index: 0x20000000
Line 90: addr: 0x100
Line 91: input: 0x0
Line 96: c: 0x0
Line 99: a: 0x80808080
Line 102: a: 0x80808080
Line 105: b: 0x0
Line 108: a: 0x80808080
Line 111: a: 0x80808080
Line 114: x: 0x0
Line 125: addr: 0x100

==after==:
Line 87: transition: 0x20000100
Line 88: ranges: 0x0
Line 89: index: 0x20000000
Line 90: addr: 0x100
Line 91: input: 0x1
Line 96: c: 0x1010101
Line 99: a: 0x80808080
Line 102: a: 0x7f7f7f7f
Line 105: b: 0x0
Line 108: a: 0x0
Line 111: a: 0x0
Line 114: x: 0x4
Line 125: addr: 0x104

RE: [PATCH] acl: fix the value of the trans table

[Konstantin Ananyev]

Hi,


> Hi, Konstantin
> 
> To illustrate this issue, I made the following modifications to the test cases:
> Modify the “test_data” content of the function "test_acl.c: test_build_ports_range":
> delete the second test data, and leave only one test message data. This way, when
> the function "acl_run_scalar.c: rte_acl_classify_scalar" processes the second message
> data, it will use the idle data. Then modify the idle data in the function "acl_run.h:
> acl_start_next_trie" to (to modify the idle, the const attribute was removed):
> parms[n].data = (uint8_t *)idle;
> parms[n].data[0] = 0x04;
> parms[n].data[4] = 0x01;
> When the function "acl_run_scalar.c: scalar_transition" processing this idle message,
> because the ranges are 0 and the message data is not 0(0x01), the message data will
> exceed all ranges. The calculated result x is 4, while the correct result should be 0. The
> following is a comparison of the settlement results of x before and after modification
> (I have printed each line of the calculation results of the function "acl_run_stcalar. c: scalar_translation"):

Sorry, it is a bit too cryptic for me.
Why do you modify the internal ACL tables contents?
If you can, please create a new test-case in  app/test/test_acl.c to demonstrate the issue.
Thanks
Konstantin 

> ==before==:
> Line 87: transition: 0x20000100
> Line 88: ranges: 0x0
> Line 89: index: 0x20000000
> Line 90: addr: 0x100
> Line 91: input: 0x0
> Line 96: c: 0x0
> Line 99: a: 0x80808080
> Line 102: a: 0x80808080
> Line 105: b: 0x0
> Line 108: a: 0x80808080
> Line 111: a: 0x80808080
> Line 114: x: 0x0
> Line 125: addr: 0x100
> 
> ==after==:
> Line 87: transition: 0x20000100
> Line 88: ranges: 0x0
> Line 89: index: 0x20000000
> Line 90: addr: 0x100
> Line 91: input: 0x1
> Line 96: c: 0x1010101
> Line 99: a: 0x80808080
> Line 102: a: 0x7f7f7f7f
> Line 105: b: 0x0
> Line 108: a: 0x0
> Line 111: a: 0x0
> Line 114: x: 0x4
> Line 125: addr: 0x104

[PATCH] acl: fix the value of the trans table

[Huichao Cai]

Maybe that's not a problem, because ACL can handle idle
packets normally, so I can't reproduce this issue with
external test cases at the moment, but I'm just describing
what idle nomatch node has to do if it wants to implement
an effect pointing to itself:

The reason why RTE_ACL_NODE_SINGLE node can point to itself
(regardless of the value of the packet data) is that it relies
on the node_index high 32-bit assignment to the maximum boundary
value (RTE_ACL_QUAD_SINGLE), and the idle nomatch node has the
same properties, so it should also be like RTE_ACL_NODE_SINGLE
node_ index instead of the current boundary value of 0, the reason
why the current idle nomatch node can point to itself is because
the idle data is 0, which has nothing to do with the node pointing
to its own attributes, it is just a coincidence of the data.