[v4] efd: change data type of parameter
Checks
Commit Message
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 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>
---
v4: Set socket id in EFD tests
v3: Fixed commit message
v2: Fixed EFD tests
---
app/test/test_efd.c | 5 +++--
app/test/test_efd_perf.c | 4 ++--
lib/efd/rte_efd.c | 2 +-
lib/efd/rte_efd.h | 2 +-
4 files changed, 7 insertions(+), 6 deletions(-)
Comments
On 9/28/21 6:58 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 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>
> ---
>
> v4: Set socket id in EFD tests
>
> v3: Fixed commit message
>
> v2: Fixed EFD tests
Results with v4 on a non-consecutive NUMA system:
$ lscpu
Architecture: ppc64le
Byte Order: Little Endian
CPU(s): 128
On-line CPU(s) list: 0-127
Thread(s) per core: 4
Core(s) per socket: 16
Socket(s): 2
NUMA node(s): 6
Model: 2.3 (pvr 004e 1203)
Model name: POWER9, altivec supported
CPU max MHz: 3800.0000
CPU min MHz: 2300.0000
L1d cache: 32K
L1i cache: 32K
L2 cache: 512K
L3 cache: 10240K
NUMA node0 CPU(s): 0-63
NUMA node8 CPU(s): 64-127
NUMA node252 CPU(s):
NUMA node253 CPU(s):
NUMA node254 CPU(s):
NUMA node255 CPU(s):
$ sudo /home/drc/src/dpdk/build/app/test/dpdk-test -l 64-127 -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 8 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>>efd_perf_autotest
Measuring performance, please wait
..........
Results (in CPU cycles/operation)
-----------------------------------
Keysize Add Lookup Lookup_bulk
Delete
4 471 17 11 59
8 502 18 14 73
16 542 23 21 81
32 662 37 36 97
48 782 54 53 110
64 881 75 74 119
9 512 18 16 75
13 539 23 22 84
37 726 48 47 115
40 726 47 46 112
Test OK
Hi
> -----Original Message-----
> From: David Christensen <drc@linux.vnet.ibm.com>
> Sent: Tuesday, September 28, 2021 4:53 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Wang, Yipeng1
> <yipeng1.wang@intel.com>; Marohn, Byron <byron.marohn@intel.com>
> Cc: dev@dpdk.org; Mcnamara, John <john.mcnamara@intel.com>
> Subject: Re: [PATCH v4] efd: change data type of parameter
>
>
>
> On 9/28/21 6:58 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 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>
> > ---
> >
> > v4: Set socket id in EFD tests
> >
> > v3: Fixed commit message
> >
> > v2: Fixed EFD tests
>
> Results with v4 on a non-consecutive NUMA system:
...
> Test OK
Great! Thanks a lot for checking. Would you mind adding tested-by to the patch?
Pablo
On 9/29/21 12:51 AM, De Lara Guarch, Pablo wrote:
> Hi
>
>> -----Original Message-----
>> From: David Christensen <drc@linux.vnet.ibm.com>
>> Sent: Tuesday, September 28, 2021 4:53 PM
>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Wang, Yipeng1
>> <yipeng1.wang@intel.com>; Marohn, Byron <byron.marohn@intel.com>
>> Cc: dev@dpdk.org; Mcnamara, John <john.mcnamara@intel.com>
>> Subject: Re: [PATCH v4] efd: change data type of parameter
>>
>>
>>
>> On 9/28/21 6:58 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 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>
>>> ---
>>>
>>> v4: Set socket id in EFD tests
>>>
>>> v3: Fixed commit message
>>>
>>> v2: Fixed EFD tests
>>
>> Results with v4 on a non-consecutive NUMA system:
>
> ...
>
>> Test OK
>
> Great! Thanks a lot for checking. Would you mind adding tested-by to the patch?
>
> Pablo
>
Tested-by: David Christensen <drc@linux.vnet.ibm.com>
> -----Original Message-----
> From: David Christensen <drc@linux.vnet.ibm.com>
> Sent: Tuesday, September 28, 2021 8:53 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Wang, Yipeng1
> <yipeng1.wang@intel.com>; Marohn, Byron <byron.marohn@intel.com>
> Cc: dev@dpdk.org; Mcnamara, John <john.mcnamara@intel.com>
> Subject: Re: [PATCH v4] efd: change data type of parameter
>
>
>
> On 9/28/21 6:58 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 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>
> > ---
> >
> > v4: Set socket id in EFD tests
> >
> > v3: Fixed commit message
> >
> > v2: Fixed EFD tests
>
[Wang, Yipeng]
Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
Thanks Pablo!
> > > 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 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>
> Acked-by: Yipeng Wang <yipeng1.wang@intel.com>
Applied, thanks.
@@ -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;
@@ -443,6 +443,7 @@ static int test_efd_creation_with_bad_parameters(void)
static int
test_efd(void)
{
+ test_socket_id = rte_socket_id();
/* Unit tests */
if (test_add_delete() < 0)
@@ -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;
@@ -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;
@@ -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