On 5/16/2023 2:39 PM, Burakov, Anatoly wrote:
Hi Vipin!
Thanks for all of the work on this bug, it is highly appreciated. Below
are suggestions for improvements for this patch.
On 1/13/2023 1:12 PM, Vipin P R wrote:
add test case coverage to cover the ms_idx jump
This message could be expanded to be more informative. Suggested rewording:
test/fbarray: add test case for incorrect lookahead behavior
Cc: sta...@dpdk.org
Signed-off-by: Vipin P R <vip...@vmware.com>
Acked-by: Kumara Parameshwaran <kparamesh...@vmware.com>
---
Depends-on:
0001-Memory-Allocation-Fixes-ms_idx-jump-lookahead-during.patch
Depends-on:
0002-Memory-Allocation-Fixes-ms_idx-jump-lookbehind-durin.patch
This makes no difference for commit, but for future reference:
depends-on should reference link to actual patches, not a patch file name.
---
app/test/test_fbarray.c | 49
+++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/app/test/test_fbarray.c b/app/test/test_fbarray.c
index a691bf4..275449c 100644
--- a/app/test/test_fbarray.c
+++ b/app/test/test_fbarray.c
@@ -11,6 +11,7 @@
#include <rte_debug.h>
#include <rte_errno.h>
#include <rte_fbarray.h>
+#include <rte_memory.h>
This is presumably added to get access to `struct rte_memseg`, but this
is not needed, because the bug is in the mask behavior, which does not
depend on specific data size.
#include "test.h"
@@ -402,6 +403,53 @@ static int check_used_one(void)
return 0;
}
+/* the following test case verifies that the jump in ms_idx for an
fb-array is correct. */
+static int test_jump(void)
I think the test functions would be better named "test_lookahead" and
"test_lookbehind" respectively.
+{
+ struct rte_fbarray test_array;
+ int input[] = {1, 1070, 1, 2, 1, 2, 4, 12, 2, 2, 1, 2, 1};
I've managed to reduce this bug down to a more minimal example:
{ 63, 1, 2 }
I've managed to reduce the test down to an even more minimal example, so
all of the other code, loops etc. is actually not needed:
1. Allocate fbarray with 256 entries
2. Set idx 64 as used
3. Call rte_fbarray_find_next_n_free() starting with index 1 and length
of 64
Returned value should be 65, but without the fix it returns 129.
--
Thanks,
Anatoly
