Re: [dpdk-dev] [PATCH v3] efd: change data type of parameter

[David Christensen <drc@linux.vnet.ibm.com>]

On 9/17/21 5:56 AM, Pablo de Lara wrote:
> rte_efd_create() function was using uint8_t for a socket bitmask,
> for one of its parameters.
> This limits the maximum of NUMA sockets to be 8.
> Changing to uint64_t increases it to 64, which should be
> more future-proof.
> 
> Coverity issue: 366390
> Fixes: 56b6ef874f8 ("efd: new Elastic Flow Distributor library")
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> Acked-by: John McNamara <john.mcnamara@intel.com>
> ---
> 
> v3: Fixed commit message
> 
> v2: Fixed EFD tests
> 
> ---
> 
>   app/test/test_efd.c      | 4 ++--
>   app/test/test_efd_perf.c | 4 ++--
>   lib/efd/rte_efd.c        | 2 +-
>   lib/efd/rte_efd.h        | 2 +-
>   4 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/app/test/test_efd.c b/app/test/test_efd.c
> index 180dc4748e..581519c1e0 100644
> --- a/app/test/test_efd.c
> +++ b/app/test/test_efd.c
> @@ -91,9 +91,9 @@ static struct flow_key keys[5] = {
>   /* Array to store the data */
>   static efd_value_t data[5];
> 
> -static inline uint8_t efd_get_all_sockets_bitmask(void)
> +static inline uint64_t efd_get_all_sockets_bitmask(void)
>   {
> -	uint8_t all_cpu_sockets_bitmask = 0;
> +	uint64_t all_cpu_sockets_bitmask = 0;
>   	unsigned int i;
>   	unsigned int next_lcore = rte_get_main_lcore();
>   	const int val_true = 1, val_false = 0;
> diff --git a/app/test/test_efd_perf.c b/app/test/test_efd_perf.c
> index 1c47704475..f3fe3b1736 100644
> --- a/app/test/test_efd_perf.c
> +++ b/app/test/test_efd_perf.c
> @@ -29,9 +29,9 @@
>   #endif
>   static unsigned int test_socket_id;
> 
> -static inline uint8_t efd_get_all_sockets_bitmask(void)
> +static inline uint64_t efd_get_all_sockets_bitmask(void)
>   {
> -	uint8_t all_cpu_sockets_bitmask = 0;
> +	uint64_t all_cpu_sockets_bitmask = 0;
>   	unsigned int i;
>   	unsigned int next_lcore = rte_get_main_lcore();
>   	const int val_true = 1, val_false = 0;
> diff --git a/lib/efd/rte_efd.c b/lib/efd/rte_efd.c
> index 77f46809f8..68a2378e88 100644
> --- a/lib/efd/rte_efd.c
> +++ b/lib/efd/rte_efd.c
> @@ -495,7 +495,7 @@ efd_search_hash(struct rte_efd_table * const table,
> 
>   struct rte_efd_table *
>   rte_efd_create(const char *name, uint32_t max_num_rules, uint32_t key_len,
> -		uint8_t online_cpu_socket_bitmask, uint8_t offline_cpu_socket)
> +		uint64_t online_cpu_socket_bitmask, uint8_t offline_cpu_socket)
>   {
>   	struct rte_efd_table *table = NULL;
>   	uint8_t *key_array = NULL;
> diff --git a/lib/efd/rte_efd.h b/lib/efd/rte_efd.h
> index c2be4c09ae..d3d7befd0c 100644
> --- a/lib/efd/rte_efd.h
> +++ b/lib/efd/rte_efd.h
> @@ -139,7 +139,7 @@ typedef uint16_t efd_hashfunc_t;
>    */
>   struct rte_efd_table *
>   rte_efd_create(const char *name, uint32_t max_num_rules, uint32_t key_len,
> -	uint8_t online_cpu_socket_bitmask, uint8_t offline_cpu_socket);
> +	uint64_t online_cpu_socket_bitmask, uint8_t offline_cpu_socket);
> 
>   /**
>    * Releases the resources from an EFD table
> 

After applying the patch I receive a segmentation fault when I use 
lcores on the second NUMA node (node 8).

$ lscpu
Architecture:        ppc64le
..
NUMA node0 CPU(s):   0-63
NUMA node8 CPU(s):   64-127

Working case:
---------------------------
$ sudo /home/drc/src/dpdk/build/app/test/dpdk-test -l 59-63 -n 4 --no-pci
EAL: Detected 128 lcore(s)
EAL: Detected 2 NUMA nodes
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
EAL: No available 1048576 kB hugepages reported
EAL: VFIO support initialized
APP: HPET is not enabled, using TSC as default timer
RTE>>efd_autotest
Entering test_add_delete
Entering test_efd_find_existing
Entering test_add_update_delete
Entering test_five_keys
Entering test_efd_creation_with_bad_parameters, **Errors are expected **
EFD: Allocating key array on socket 0 failed
EFD: At least one CPU socket must be enabled in the bitmask
EFD: Allocating EFD table management structure on socket 255 failed
# Test successful. No more errors expected
Evaluating table utilization and correctness, please wait
Added    2097152	Succeeded    2097152	Lost          0
Added    2097152	Succeeded    2097152	Lost          0
Added    2097152	Succeeded    2097152	Lost          0

Average table utilization = 100.00% (2097152/2097152)
Test OK
RTE>>quit

Failing case:
---------------------------
sudo /home/drc/src/dpdk/build/app/test/dpdk-test -l 64-69 -n 4 --no-pci
EAL: Detected 128 lcore(s)
EAL: Detected 2 NUMA nodes
EAL: Detected static linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'VA'
EAL: No available 1048576 kB hugepages reported
EAL: VFIO support initialized
APP: HPET is not enabled, using TSC as default timer
RTE>>efd_autotest
Entering test_add_delete
Segmentation fault

What's the purpose of "test_socket_id" in the file app/test/test_efd.c? 
  I don't see it set during the test, default to 0, and it looks like it 
should be 8 in this situation:

sudo gdb --args /home/drc/src/dpdk/build/app/test/dpdk-test -l 64-69 -n 
4 --no-pci
GNU gdb (GDB) Red Hat Enterprise Linux 8.2-12.el8
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 
<http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "ppc64le-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
     <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from /home/drc/src/dpdk/build/app/test/dpdk-test...done.
(gdb) b test_and_delete
Function "test_and_delete" not defined.
Make breakpoint pending on future shared library load? (y or [n]) n
(gdb) b efd_get_choice
Breakpoint 1 at 0x1090dbdc: file ../lib/efd/rte_efd.c, line 319.
(gdb) b test_add_delete
Breakpoint 2 at 0x1009d7a8: test_add_delete. (2 locations)
(gdb) start
Temporary breakpoint 3 at 0x1000f340: file ../app/test/test.c, line 105.
Starting program: /home/drc/src/dpdk/build/app/test/dpdk-test -l 64-69 
-n 4 --no-pci
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/power9/libthread_db.so.1".

Temporary breakpoint 3, main (argc=6, argv=0x7ffffffff138) at 
../app/test/test.c:105
105	{
Missing separate debuginfos, use: yum debuginfo-install 
libibverbs-52mlnx1-1.53100.ppc64le
(gdb) c
Continuing.
EAL: Detected 128 lcore(s)
EAL: Detected 2 NUMA nodes
EAL: Detected static linkage of DPDK
[New Thread 0x7ffff746d090 (LWP 290647)]
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
[New Thread 0x7ffff6c5d090 (LWP 290648)]
EAL: Selected IOVA mode 'VA'
EAL: No available 1048576 kB hugepages reported
EAL: VFIO support initialized
[New Thread 0x7ffff644d090 (LWP 290649)]
[New Thread 0x7ffff5c3d090 (LWP 290650)]
[New Thread 0x7ffff542d090 (LWP 290651)]
[New Thread 0x7ffff4c1d090 (LWP 290652)]
[New Thread 0x7ffff440d090 (LWP 290653)]
[New Thread 0x7ffff3bfd090 (LWP 290654)]
[New Thread 0x7ffff33ed090 (LWP 290655)]
APP: HPET is not enabled, using TSC as default timer
RTE>>efd_autotest

Thread 1 "dpdk-test" hit Breakpoint 2, test_add_delete () at 
../app/test/test_efd.c:120
120		printf("Entering %s\n", __func__);
(gdb) bt
#0  test_add_delete () at ../app/test/test_efd.c:120
#1  0x000000001009ec54 in test_efd () at ../app/test/test_efd.c:448
#2  0x0000000010009b4c in cmd_autotest_parsed 
(parsed_result=0x7fffffff8620, cl=0x200196d0, data=0x0) at 
../app/test/commands.c:71
#3  0x000000001095a808 in cmdline_parse (cl=0x200196d0, buf=0x20019718 
"efd_autotest\n") at ../lib/cmdline/cmdline_parse.c:290
#4  0x0000000010957c90 in cmdline_valid_buffer (rdl=0x200196e0, 
buf=0x20019718 "efd_autotest\n", size=14)
     at ../lib/cmdline/cmdline.c:26
#5  0x000000001095e8a8 in rdline_char_in (rdl=0x200196e0, c=10 '\n') at 
../lib/cmdline/cmdline_rdline.c:421
#6  0x0000000010958258 in cmdline_in (cl=0x200196d0, buf=0x7fffffffe800 
"\n\226\001 ", size=1) at ../lib/cmdline/cmdline.c:149
#7  0x00000000109585f0 in cmdline_interact (cl=0x200196d0) at 
../lib/cmdline/cmdline.c:223
#8  0x000000001000f988 in main (argc=1, argv=0x7ffffffff160) at 
../app/test/test.c:234
(gdb) n
Entering test_add_delete
122		handle = rte_efd_create("test_add_delete",
(gdb) n
125		TEST_ASSERT_NOT_NULL(handle, "Error creating the EFD table\n");
(gdb) n
127		data[0] = mrand48() & VALUE_BITMASK;
(gdb) n
128		TEST_ASSERT_SUCCESS(rte_efd_update(handle, test_socket_id, &keys[0],
(gdb) p test_socket_id
$1 = 0
(gdb) c
Continuing.

Thread 1 "dpdk-test" hit Breakpoint 1, efd_get_choice 
(table=0x1003fff00, socket_id=0, chunk_id=218, bin_id=120)
     at ../lib/efd/rte_efd.c:319
319		struct efd_online_chunk *chunk = &table->chunks[socket_id][chunk_id];
(gdb) p table
$2 = (const struct rte_efd_table * const) 0x1003fff00
(gdb) p *table
$3 = {name = "test_add_delete", '\000' <repeats 16 times>, key_len = 13, 
max_num_rules = 3670016, num_rules = 0,
   num_chunks = 2048, num_chunks_shift = 11, lookup_fn = 
EFD_LOOKUP_SCALAR, chunks = {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
     0x11011dff00, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, offline_chunks = 
0x10327e000, free_slots = 0x1049ffd00,
   keys = 0x100480000 ""}
(gdb) p *table->chunk[0]
There is no member named chunk.
(gdb) p *table->chunks[0]
Cannot access memory at address 0x0
(gdb) p *table->chunks[8]
$4 = {bin_choice_list = '\000' <repeats 63 times>, groups = {{hash_idx = 
{0, 0, 0, 0, 0, 0, 0, 0}, lookup_table = {0, 0, 0, 0, 0,
         0, 0, 0}} <repeats 64 times>}}
(gdb) c
Continuing.

Thread 1 "dpdk-test" received signal SIGSEGV, Segmentation fault.
0x000000001090dc18 in efd_get_choice (table=0x1003fff00, socket_id=0, 
chunk_id=218, bin_id=120) at ../lib/efd/rte_efd.c:325
325		uint8_t choice_chunk =
(gdb) p chunk
$5 = (struct efd_online_chunk *) 0x70680
(gdb) p *chunk
Cannot access memory at address 0x70680
(gdb)

Not clear to me if this is an existing problem with POWER or related to 
your patch.  Can someone more familiar with EFD chime in?

Dave