* [dpdk-dev] [PATCH v1 1/1] app/test: fix --socket-mem option in eal flag autotest
@ 2019-04-10 7:25 Vamsi Attunuru
2019-04-10 7:25 ` Vamsi Attunuru
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Vamsi Attunuru @ 2019-04-10 7:25 UTC (permalink / raw)
To: dev; +Cc: Vamsi Attunuru
"argv2[]" positive test case fails with RTE_MAX_NUMA_NODES=1 config
because of "--socket-mem=0,0,0,0" option, which passes memory sizes
for multiple sockets. This patch fixes the issue by passing memory
size for node 0 alone.
Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
---
app/test/test_eal_flags.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c
index 9112c96..90df7b3 100644
--- a/app/test/test_eal_flags.c
+++ b/app/test/test_eal_flags.c
@@ -1161,7 +1161,7 @@ test_memory_flags(void)
/* valid (zero) --socket-mem flag */
const char *argv2[] = {prgname, "-c", "10", "-n", "2",
- "--file-prefix=" memtest, "--socket-mem=0,0,0,0"};
+ "--file-prefix=" memtest, "--socket-mem=0"};
/* invalid (incomplete) --socket-mem flag */
const char *argv3[] = {prgname, "-c", "10", "-n", "2",
--
2.8.4
^ permalink raw reply [flat|nested] 14+ messages in thread* [dpdk-dev] [PATCH v1 1/1] app/test: fix --socket-mem option in eal flag autotest 2019-04-10 7:25 [dpdk-dev] [PATCH v1 1/1] app/test: fix --socket-mem option in eal flag autotest Vamsi Attunuru @ 2019-04-10 7:25 ` Vamsi Attunuru 2019-04-10 8:26 ` David Marchand ` (2 subsequent siblings) 3 siblings, 0 replies; 14+ messages in thread From: Vamsi Attunuru @ 2019-04-10 7:25 UTC (permalink / raw) To: dev; +Cc: Vamsi Attunuru "argv2[]" positive test case fails with RTE_MAX_NUMA_NODES=1 config because of "--socket-mem=0,0,0,0" option, which passes memory sizes for multiple sockets. This patch fixes the issue by passing memory size for node 0 alone. Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com> --- app/test/test_eal_flags.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c index 9112c96..90df7b3 100644 --- a/app/test/test_eal_flags.c +++ b/app/test/test_eal_flags.c @@ -1161,7 +1161,7 @@ test_memory_flags(void) /* valid (zero) --socket-mem flag */ const char *argv2[] = {prgname, "-c", "10", "-n", "2", - "--file-prefix=" memtest, "--socket-mem=0,0,0,0"}; + "--file-prefix=" memtest, "--socket-mem=0"}; /* invalid (incomplete) --socket-mem flag */ const char *argv3[] = {prgname, "-c", "10", "-n", "2", -- 2.8.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v1 1/1] app/test: fix --socket-mem option in eal flag autotest 2019-04-10 7:25 [dpdk-dev] [PATCH v1 1/1] app/test: fix --socket-mem option in eal flag autotest Vamsi Attunuru 2019-04-10 7:25 ` Vamsi Attunuru @ 2019-04-10 8:26 ` David Marchand 2019-04-10 8:26 ` David Marchand 2019-04-10 11:43 ` [dpdk-dev] [EXT] " Vamsi Krishna Attunuru 2019-07-11 5:15 ` [dpdk-dev] [PATCH v2 " vattunuru 2019-07-29 8:08 ` [dpdk-dev] [PATCH v4] test: " David Marchand 3 siblings, 2 replies; 14+ messages in thread From: David Marchand @ 2019-04-10 8:26 UTC (permalink / raw) To: Vamsi Attunuru; +Cc: dev, Michael Santana, Aaron Conole On Wed, Apr 10, 2019 at 9:26 AM Vamsi Attunuru <vattunuru@marvell.com> wrote: > "argv2[]" positive test case fails with RTE_MAX_NUMA_NODES=1 config > because of "--socket-mem=0,0,0,0" option, which passes memory sizes > for multiple sockets. This patch fixes the issue by passing memory > size for node 0 alone. > How about modifying the test so that it also validates the format is consistent with the RTE_MAX_NUMA_NODES value ? -- David Marchand ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v1 1/1] app/test: fix --socket-mem option in eal flag autotest 2019-04-10 8:26 ` David Marchand @ 2019-04-10 8:26 ` David Marchand 2019-04-10 11:43 ` [dpdk-dev] [EXT] " Vamsi Krishna Attunuru 1 sibling, 0 replies; 14+ messages in thread From: David Marchand @ 2019-04-10 8:26 UTC (permalink / raw) To: Vamsi Attunuru; +Cc: dev, Michael Santana, Aaron Conole On Wed, Apr 10, 2019 at 9:26 AM Vamsi Attunuru <vattunuru@marvell.com> wrote: > "argv2[]" positive test case fails with RTE_MAX_NUMA_NODES=1 config > because of "--socket-mem=0,0,0,0" option, which passes memory sizes > for multiple sockets. This patch fixes the issue by passing memory > size for node 0 alone. > How about modifying the test so that it also validates the format is consistent with the RTE_MAX_NUMA_NODES value ? -- David Marchand ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [EXT] Re: [PATCH v1 1/1] app/test: fix --socket-mem option in eal flag autotest 2019-04-10 8:26 ` David Marchand 2019-04-10 8:26 ` David Marchand @ 2019-04-10 11:43 ` Vamsi Krishna Attunuru 2019-04-10 11:43 ` Vamsi Krishna Attunuru 1 sibling, 1 reply; 14+ messages in thread From: Vamsi Krishna Attunuru @ 2019-04-10 11:43 UTC (permalink / raw) To: David Marchand; +Cc: dev, Michael Santana, Aaron Conole ________________________________ From: David Marchand <david.marchand@redhat.com> Sent: Wednesday, April 10, 2019 1:56 PM To: Vamsi Krishna Attunuru Cc: dev; Michael Santana; Aaron Conole Subject: [EXT] Re: [dpdk-dev] [PATCH v1 1/1] app/test: fix --socket-mem option in eal flag autotest External Email ________________________________ On Wed, Apr 10, 2019 at 9:26 AM Vamsi Attunuru <vattunuru@marvell.com<mailto:vattunuru@marvell.com>> wrote: "argv2[]" positive test case fails with RTE_MAX_NUMA_NODES=1 config because of "--socket-mem=0,0,0,0" option, which passes memory sizes for multiple sockets. This patch fixes the issue by passing memory size for node 0 alone. How about modifying the test so that it also validates the format is consistent with the RTE_MAX_NUMA_NODES value ? Vamsi > Yes, started working on it, it should also fix other negative tests(false negative) beneath argv2[] test. -- David Marchand ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [EXT] Re: [PATCH v1 1/1] app/test: fix --socket-mem option in eal flag autotest 2019-04-10 11:43 ` [dpdk-dev] [EXT] " Vamsi Krishna Attunuru @ 2019-04-10 11:43 ` Vamsi Krishna Attunuru 0 siblings, 0 replies; 14+ messages in thread From: Vamsi Krishna Attunuru @ 2019-04-10 11:43 UTC (permalink / raw) To: David Marchand; +Cc: dev, Michael Santana, Aaron Conole ________________________________ From: David Marchand <david.marchand@redhat.com> Sent: Wednesday, April 10, 2019 1:56 PM To: Vamsi Krishna Attunuru Cc: dev; Michael Santana; Aaron Conole Subject: [EXT] Re: [dpdk-dev] [PATCH v1 1/1] app/test: fix --socket-mem option in eal flag autotest External Email ________________________________ On Wed, Apr 10, 2019 at 9:26 AM Vamsi Attunuru <vattunuru@marvell.com<mailto:vattunuru@marvell.com>> wrote: "argv2[]" positive test case fails with RTE_MAX_NUMA_NODES=1 config because of "--socket-mem=0,0,0,0" option, which passes memory sizes for multiple sockets. This patch fixes the issue by passing memory size for node 0 alone. How about modifying the test so that it also validates the format is consistent with the RTE_MAX_NUMA_NODES value ? Vamsi > Yes, started working on it, it should also fix other negative tests(false negative) beneath argv2[] test. -- David Marchand ^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v2 1/1] app/test: fix --socket-mem option in eal flag autotest 2019-04-10 7:25 [dpdk-dev] [PATCH v1 1/1] app/test: fix --socket-mem option in eal flag autotest Vamsi Attunuru 2019-04-10 7:25 ` Vamsi Attunuru 2019-04-10 8:26 ` David Marchand @ 2019-07-11 5:15 ` vattunuru 2019-07-11 13:56 ` Aaron Conole 2019-07-12 6:43 ` [dpdk-dev] [PATCH v3 " vattunuru 2019-07-29 8:08 ` [dpdk-dev] [PATCH v4] test: " David Marchand 3 siblings, 2 replies; 14+ messages in thread From: vattunuru @ 2019-07-11 5:15 UTC (permalink / raw) To: dev; +Cc: thomas, jerinj, david.marchand, Vamsi Attunuru From: Vamsi Attunuru <vattunuru@marvell.com> eal flag autotest fails when multiple mem size flags are passed to --socket-mem option irrespective of RTE_MAX_NUMA_NODES. Patch fixes --socket-mem option by setting enough numbers of numa node mem flags based on RTE_MAX_NUMA_NODES config. Fixes: 45f1b6e8680a ("app: add new tests on eal flags") Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com> --- V2 changes: * Add routine to populate --socket-mem option and use it to form valid and invalid test commands. app/test/test_eal_flags.c | 118 ++++++++++++++++++++++++++-------------------- 1 file changed, 68 insertions(+), 50 deletions(-) diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c index 672ca0a..c8e5c46 100644 --- a/app/test/test_eal_flags.c +++ b/app/test/test_eal_flags.c @@ -1250,12 +1250,45 @@ test_file_prefix(void) return 0; } +static void +populate_socket_mem_param(int num_sockets, const char *mem_size, char *buf) +{ + char tmp[SOCKET_MEM_STRLEN]; + int i; + + snprintf(buf, SOCKET_MEM_STRLEN, "--socket-mem="); + + for (i = 0; i < num_sockets ; i++) { + snprintf(tmp, sizeof(tmp), "%s%s", buf, mem_size); + strlcpy(buf, tmp, sizeof(tmp)); + + if (num_sockets + 1 - i > 1) { + snprintf(tmp, sizeof(tmp), "%s,", buf); + strlcpy(buf, tmp, sizeof(tmp)); + } + } +} + /* * Tests for correct handling of -m and --socket-mem flags */ static int test_memory_flags(void) { + char buf[SOCKET_MEM_STRLEN]; /* to avoid copying string onto itself */ + +#ifdef RTE_EXEC_ENV_FREEBSD + int num_sockets = 1; +#else + int num_sockets = RTE_MIN(get_number_of_sockets(), + RTE_MAX_NUMA_NODES); +#endif + + if (num_sockets <= 0) { + printf("Error - cannot get number of sockets!\n"); + return -1; + } + #ifdef RTE_EXEC_ENV_FREEBSD /* BSD target doesn't support prefixes at this point */ const char * prefix = ""; @@ -1277,85 +1310,70 @@ test_memory_flags(void) "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE}; /* valid (zero) --socket-mem flag */ + char arg2_socket_mem[SOCKET_MEM_STRLEN]; + populate_socket_mem_param(num_sockets - 1, "0", buf); + snprintf(arg2_socket_mem, sizeof(arg2_socket_mem), "%s%s", buf, "0"); const char *argv2[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=0,0,0,0"}; + "--file-prefix=" memtest, arg2_socket_mem}; /* invalid (incomplete) --socket-mem flag */ + char arg3_socket_mem[SOCKET_MEM_STRLEN]; + populate_socket_mem_param(num_sockets - 1, "2", buf); + snprintf(arg3_socket_mem, sizeof(arg3_socket_mem), "%s%s", buf, "2,"); const char *argv3[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=2,2,"}; + "--file-prefix=" memtest, arg3_socket_mem}; /* invalid (mixed with invalid data) --socket-mem flag */ + char arg4_socket_mem[SOCKET_MEM_STRLEN]; + populate_socket_mem_param(num_sockets - 1, "2", buf); + snprintf(arg4_socket_mem, sizeof(arg4_socket_mem), "%s%s", buf, "Fred"); const char *argv4[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=2,2,Fred"}; + "--file-prefix=" memtest, arg4_socket_mem}; /* invalid (with numeric value as last character) --socket-mem flag */ + char arg5_socket_mem[SOCKET_MEM_STRLEN]; + populate_socket_mem_param(num_sockets - 1, "2", buf); + snprintf(arg5_socket_mem, sizeof(arg5_socket_mem), + "%s%s", buf, "Fred0"); const char *argv5[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=2,2,Fred0"}; + "--file-prefix=" memtest, arg5_socket_mem}; /* invalid (with empty socket) --socket-mem flag */ + char arg6_socket_mem[SOCKET_MEM_STRLEN]; + populate_socket_mem_param(num_sockets - 1, "2", buf); + snprintf(arg6_socket_mem, sizeof(arg6_socket_mem), "%s%s", buf, ",,"); const char *argv6[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=2,,2"}; + "--file-prefix=" memtest, arg6_socket_mem}; /* invalid (null) --socket-mem flag */ const char *argv7[] = {prgname, "--file-prefix=" memtest, "--socket-mem="}; /* valid --socket-mem specified together with -m flag */ + char arg8_socket_mem[SOCKET_MEM_STRLEN]; + populate_socket_mem_param(num_sockets - 1, "2", buf); + snprintf(arg8_socket_mem, sizeof(arg8_socket_mem), "%s%s", buf, "2"); const char *argv8[] = {prgname, - "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE, "--socket-mem=2,2"}; + "--file-prefix=" memtest, "-m", + DEFAULT_MEM_SIZE, arg8_socket_mem}; /* construct an invalid socket mask with 2 megs on each socket plus * extra 2 megs on socket that doesn't exist on current system */ char invalid_socket_mem[SOCKET_MEM_STRLEN]; - char buf[SOCKET_MEM_STRLEN]; /* to avoid copying string onto itself */ - -#ifdef RTE_EXEC_ENV_FREEBSD - int i, num_sockets = 1; -#else - int i, num_sockets = RTE_MIN(get_number_of_sockets(), - RTE_MAX_NUMA_NODES); -#endif - - if (num_sockets <= 0) { - printf("Error - cannot get number of sockets!\n"); - return -1; - } - - snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), "--socket-mem="); - - /* add one extra socket */ - for (i = 0; i < num_sockets + 1; i++) { - snprintf(buf, sizeof(buf), "%s%s", invalid_socket_mem, DEFAULT_MEM_SIZE); - strlcpy(invalid_socket_mem, buf, sizeof(invalid_socket_mem)); - - if (num_sockets + 1 - i > 1) { - snprintf(buf, sizeof(buf), "%s,", invalid_socket_mem); - strlcpy(invalid_socket_mem, buf, - sizeof(invalid_socket_mem)); - } - } - - /* construct a valid socket mask with 2 megs on each existing socket */ - char valid_socket_mem[SOCKET_MEM_STRLEN]; - - snprintf(valid_socket_mem, sizeof(valid_socket_mem), "--socket-mem="); - - /* add one extra socket */ - for (i = 0; i < num_sockets; i++) { - snprintf(buf, sizeof(buf), "%s%s", valid_socket_mem, DEFAULT_MEM_SIZE); - strlcpy(valid_socket_mem, buf, sizeof(valid_socket_mem)); - - if (num_sockets - i > 1) { - snprintf(buf, sizeof(buf), "%s,", valid_socket_mem); - strlcpy(valid_socket_mem, buf, - sizeof(valid_socket_mem)); - } - } + populate_socket_mem_param(num_sockets, DEFAULT_MEM_SIZE, buf); + snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), + "%s%s", buf, DEFAULT_MEM_SIZE); /* invalid --socket-mem flag (with extra socket) */ const char *argv9[] = {prgname, "--file-prefix=" memtest, invalid_socket_mem}; + /* construct a valid socket mask with 2 megs on each existing socket */ + char valid_socket_mem[SOCKET_MEM_STRLEN]; + populate_socket_mem_param(num_sockets - 1, DEFAULT_MEM_SIZE, buf); + snprintf(valid_socket_mem, sizeof(valid_socket_mem), + "%s%s", buf, DEFAULT_MEM_SIZE); + /* valid --socket-mem flag */ const char *argv10[] = {prgname, "--file-prefix=" memtest, valid_socket_mem}; -- 2.8.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v2 1/1] app/test: fix --socket-mem option in eal flag autotest 2019-07-11 5:15 ` [dpdk-dev] [PATCH v2 " vattunuru @ 2019-07-11 13:56 ` Aaron Conole 2019-07-12 6:43 ` [dpdk-dev] [PATCH v3 " vattunuru 1 sibling, 0 replies; 14+ messages in thread From: Aaron Conole @ 2019-07-11 13:56 UTC (permalink / raw) To: vattunuru; +Cc: dev, thomas, jerinj, david.marchand <vattunuru@marvell.com> writes: > From: Vamsi Attunuru <vattunuru@marvell.com> > > eal flag autotest fails when multiple mem size flags are passed > to --socket-mem option irrespective of RTE_MAX_NUMA_NODES. > > Patch fixes --socket-mem option by setting enough numbers > of numa node mem flags based on RTE_MAX_NUMA_NODES config. > > Fixes: 45f1b6e8680a ("app: add new tests on eal flags") > > Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com> > --- > V2 changes: > * Add routine to populate --socket-mem option and use it to form > valid and invalid test commands. > > app/test/test_eal_flags.c | 118 ++++++++++++++++++++++++++-------------------- > 1 file changed, 68 insertions(+), 50 deletions(-) > > diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c > index 672ca0a..c8e5c46 100644 > --- a/app/test/test_eal_flags.c > +++ b/app/test/test_eal_flags.c > @@ -1250,12 +1250,45 @@ test_file_prefix(void) > return 0; > } > > +static void > +populate_socket_mem_param(int num_sockets, const char *mem_size, char *buf) > +{ > + char tmp[SOCKET_MEM_STRLEN]; > + int i; > + > + snprintf(buf, SOCKET_MEM_STRLEN, "--socket-mem="); > + > + for (i = 0; i < num_sockets ; i++) { > + snprintf(tmp, sizeof(tmp), "%s%s", buf, mem_size); > + strlcpy(buf, tmp, sizeof(tmp)); strlcpy requires the 3rd parameter be the size of the destination not the size of the source. You'll need to pass sizeof(buf) into this function. Same goes for everywhere strlcpy is used. > + if (num_sockets + 1 - i > 1) { > + snprintf(tmp, sizeof(tmp), "%s,", buf); > + strlcpy(buf, tmp, sizeof(tmp)); > + } > + } > +} > + > /* > * Tests for correct handling of -m and --socket-mem flags > */ > static int > test_memory_flags(void) > { > + char buf[SOCKET_MEM_STRLEN]; /* to avoid copying string onto itself */ > + > +#ifdef RTE_EXEC_ENV_FREEBSD > + int num_sockets = 1; > +#else > + int num_sockets = RTE_MIN(get_number_of_sockets(), > + RTE_MAX_NUMA_NODES); > +#endif > + > + if (num_sockets <= 0) { > + printf("Error - cannot get number of sockets!\n"); > + return -1; > + } > + > #ifdef RTE_EXEC_ENV_FREEBSD > /* BSD target doesn't support prefixes at this point */ > const char * prefix = ""; > @@ -1277,85 +1310,70 @@ test_memory_flags(void) > "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE}; > > /* valid (zero) --socket-mem flag */ > + char arg2_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "0", buf); > + snprintf(arg2_socket_mem, sizeof(arg2_socket_mem), "%s%s", buf, "0"); > const char *argv2[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=0,0,0,0"}; > + "--file-prefix=" memtest, arg2_socket_mem}; > > /* invalid (incomplete) --socket-mem flag */ > + char arg3_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf); > + snprintf(arg3_socket_mem, sizeof(arg3_socket_mem), "%s%s", buf, "2,"); > const char *argv3[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=2,2,"}; > + "--file-prefix=" memtest, arg3_socket_mem}; > > /* invalid (mixed with invalid data) --socket-mem flag */ > + char arg4_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf); > + snprintf(arg4_socket_mem, sizeof(arg4_socket_mem), "%s%s", buf, "Fred"); > const char *argv4[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=2,2,Fred"}; > + "--file-prefix=" memtest, arg4_socket_mem}; > > /* invalid (with numeric value as last character) --socket-mem flag */ > + char arg5_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf); > + snprintf(arg5_socket_mem, sizeof(arg5_socket_mem), > + "%s%s", buf, "Fred0"); > const char *argv5[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=2,2,Fred0"}; > + "--file-prefix=" memtest, arg5_socket_mem}; > > /* invalid (with empty socket) --socket-mem flag */ > + char arg6_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf); > + snprintf(arg6_socket_mem, sizeof(arg6_socket_mem), "%s%s", buf, ",,"); > const char *argv6[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=2,,2"}; > + "--file-prefix=" memtest, arg6_socket_mem}; > > /* invalid (null) --socket-mem flag */ > const char *argv7[] = {prgname, > "--file-prefix=" memtest, "--socket-mem="}; > > /* valid --socket-mem specified together with -m flag */ > + char arg8_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf); > + snprintf(arg8_socket_mem, sizeof(arg8_socket_mem), "%s%s", buf, "2"); > const char *argv8[] = {prgname, > - "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE, "--socket-mem=2,2"}; > + "--file-prefix=" memtest, "-m", > + DEFAULT_MEM_SIZE, arg8_socket_mem}; > > /* construct an invalid socket mask with 2 megs on each socket plus > * extra 2 megs on socket that doesn't exist on current system */ > char invalid_socket_mem[SOCKET_MEM_STRLEN]; > - char buf[SOCKET_MEM_STRLEN]; /* to avoid copying string onto itself */ > - > -#ifdef RTE_EXEC_ENV_FREEBSD > - int i, num_sockets = 1; > -#else > - int i, num_sockets = RTE_MIN(get_number_of_sockets(), > - RTE_MAX_NUMA_NODES); > -#endif > - > - if (num_sockets <= 0) { > - printf("Error - cannot get number of sockets!\n"); > - return -1; > - } > - > - snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), "--socket-mem="); > - > - /* add one extra socket */ > - for (i = 0; i < num_sockets + 1; i++) { > - snprintf(buf, sizeof(buf), "%s%s", invalid_socket_mem, DEFAULT_MEM_SIZE); > - strlcpy(invalid_socket_mem, buf, sizeof(invalid_socket_mem)); > - > - if (num_sockets + 1 - i > 1) { > - snprintf(buf, sizeof(buf), "%s,", invalid_socket_mem); > - strlcpy(invalid_socket_mem, buf, > - sizeof(invalid_socket_mem)); > - } > - } > - > - /* construct a valid socket mask with 2 megs on each existing socket */ > - char valid_socket_mem[SOCKET_MEM_STRLEN]; > - > - snprintf(valid_socket_mem, sizeof(valid_socket_mem), "--socket-mem="); > - > - /* add one extra socket */ > - for (i = 0; i < num_sockets; i++) { > - snprintf(buf, sizeof(buf), "%s%s", valid_socket_mem, DEFAULT_MEM_SIZE); > - strlcpy(valid_socket_mem, buf, sizeof(valid_socket_mem)); > - > - if (num_sockets - i > 1) { > - snprintf(buf, sizeof(buf), "%s,", valid_socket_mem); > - strlcpy(valid_socket_mem, buf, > - sizeof(valid_socket_mem)); > - } > - } > + populate_socket_mem_param(num_sockets, DEFAULT_MEM_SIZE, buf); > + snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), > + "%s%s", buf, DEFAULT_MEM_SIZE); > > /* invalid --socket-mem flag (with extra socket) */ > const char *argv9[] = {prgname, > "--file-prefix=" memtest, invalid_socket_mem}; > > + /* construct a valid socket mask with 2 megs on each existing socket */ > + char valid_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, DEFAULT_MEM_SIZE, buf); > + snprintf(valid_socket_mem, sizeof(valid_socket_mem), > + "%s%s", buf, DEFAULT_MEM_SIZE); > + > /* valid --socket-mem flag */ > const char *argv10[] = {prgname, > "--file-prefix=" memtest, valid_socket_mem}; ^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v3 1/1] app/test: fix --socket-mem option in eal flag autotest 2019-07-11 5:15 ` [dpdk-dev] [PATCH v2 " vattunuru 2019-07-11 13:56 ` Aaron Conole @ 2019-07-12 6:43 ` vattunuru 2019-07-23 6:13 ` Vamsi Krishna Attunuru 2019-07-26 13:39 ` David Marchand 1 sibling, 2 replies; 14+ messages in thread From: vattunuru @ 2019-07-12 6:43 UTC (permalink / raw) To: dev; +Cc: thomas, jerinj, david.marchand, aconole, Vamsi Attunuru From: Vamsi Attunuru <vattunuru@marvell.com> eal flag autotest fails when multiple mem size flags are passed to --socket-mem option irrespective of RTE_MAX_NUMA_NODES. Patch fixes --socket-mem option by setting enough numbers of numa node mem flags based on RTE_MAX_NUMA_NODES config. Fixes: 45f1b6e8680a ("app: add new tests on eal flags") Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com> --- V3 changes: * Pass buffer size info to populate socket mem routine which strlcpy requires. V2 changes: * Add routine to populate --socket-mem option and use it to form valid and invalid test commands. app/test/test_eal_flags.c | 121 +++++++++++++++++++++++++++------------------- 1 file changed, 71 insertions(+), 50 deletions(-) diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c index 672ca0a..053d694 100644 --- a/app/test/test_eal_flags.c +++ b/app/test/test_eal_flags.c @@ -1250,12 +1250,46 @@ test_file_prefix(void) return 0; } +static void +populate_socket_mem_param(int num_sockets, const char *mem, + char *buf, size_t buf_size) +{ + char tmp[SOCKET_MEM_STRLEN]; + int i; + + snprintf(buf, SOCKET_MEM_STRLEN, "--socket-mem="); + + for (i = 0; i < num_sockets ; i++) { + snprintf(tmp, sizeof(tmp), "%s%s", buf, mem); + strlcpy(buf, tmp, buf_size); + + if (num_sockets + 1 - i > 1) { + snprintf(tmp, sizeof(tmp), "%s,", buf); + strlcpy(buf, tmp, buf_size); + } + } +} + /* * Tests for correct handling of -m and --socket-mem flags */ static int test_memory_flags(void) { + char buf[SOCKET_MEM_STRLEN]; /* to avoid copying string onto itself */ + +#ifdef RTE_EXEC_ENV_FREEBSD + int num_sockets = 1; +#else + int num_sockets = RTE_MIN(get_number_of_sockets(), + RTE_MAX_NUMA_NODES); +#endif + + if (num_sockets <= 0) { + printf("Error - cannot get number of sockets!\n"); + return -1; + } + #ifdef RTE_EXEC_ENV_FREEBSD /* BSD target doesn't support prefixes at this point */ const char * prefix = ""; @@ -1277,85 +1311,72 @@ test_memory_flags(void) "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE}; /* valid (zero) --socket-mem flag */ + char arg2_socket_mem[SOCKET_MEM_STRLEN]; + populate_socket_mem_param(num_sockets - 1, "0", buf, sizeof(buf)); + snprintf(arg2_socket_mem, sizeof(arg2_socket_mem), "%s%s", buf, "0"); const char *argv2[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=0,0,0,0"}; + "--file-prefix=" memtest, arg2_socket_mem}; /* invalid (incomplete) --socket-mem flag */ + char arg3_socket_mem[SOCKET_MEM_STRLEN]; + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); + snprintf(arg3_socket_mem, sizeof(arg3_socket_mem), "%s%s", buf, "2,"); const char *argv3[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=2,2,"}; + "--file-prefix=" memtest, arg3_socket_mem}; /* invalid (mixed with invalid data) --socket-mem flag */ + char arg4_socket_mem[SOCKET_MEM_STRLEN]; + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); + snprintf(arg4_socket_mem, sizeof(arg4_socket_mem), "%s%s", buf, "Fred"); const char *argv4[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=2,2,Fred"}; + "--file-prefix=" memtest, arg4_socket_mem}; /* invalid (with numeric value as last character) --socket-mem flag */ + char arg5_socket_mem[SOCKET_MEM_STRLEN]; + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); + snprintf(arg5_socket_mem, sizeof(arg5_socket_mem), + "%s%s", buf, "Fred0"); const char *argv5[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=2,2,Fred0"}; + "--file-prefix=" memtest, arg5_socket_mem}; /* invalid (with empty socket) --socket-mem flag */ + char arg6_socket_mem[SOCKET_MEM_STRLEN]; + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); + snprintf(arg6_socket_mem, sizeof(arg6_socket_mem), "%s%s", buf, ",,"); const char *argv6[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=2,,2"}; + "--file-prefix=" memtest, arg6_socket_mem}; /* invalid (null) --socket-mem flag */ const char *argv7[] = {prgname, "--file-prefix=" memtest, "--socket-mem="}; /* valid --socket-mem specified together with -m flag */ + char arg8_socket_mem[SOCKET_MEM_STRLEN]; + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); + snprintf(arg8_socket_mem, sizeof(arg8_socket_mem), "%s%s", buf, "2"); const char *argv8[] = {prgname, - "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE, "--socket-mem=2,2"}; + "--file-prefix=" memtest, "-m", + DEFAULT_MEM_SIZE, arg8_socket_mem}; /* construct an invalid socket mask with 2 megs on each socket plus * extra 2 megs on socket that doesn't exist on current system */ char invalid_socket_mem[SOCKET_MEM_STRLEN]; - char buf[SOCKET_MEM_STRLEN]; /* to avoid copying string onto itself */ - -#ifdef RTE_EXEC_ENV_FREEBSD - int i, num_sockets = 1; -#else - int i, num_sockets = RTE_MIN(get_number_of_sockets(), - RTE_MAX_NUMA_NODES); -#endif - - if (num_sockets <= 0) { - printf("Error - cannot get number of sockets!\n"); - return -1; - } - - snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), "--socket-mem="); - - /* add one extra socket */ - for (i = 0; i < num_sockets + 1; i++) { - snprintf(buf, sizeof(buf), "%s%s", invalid_socket_mem, DEFAULT_MEM_SIZE); - strlcpy(invalid_socket_mem, buf, sizeof(invalid_socket_mem)); - - if (num_sockets + 1 - i > 1) { - snprintf(buf, sizeof(buf), "%s,", invalid_socket_mem); - strlcpy(invalid_socket_mem, buf, - sizeof(invalid_socket_mem)); - } - } - - /* construct a valid socket mask with 2 megs on each existing socket */ - char valid_socket_mem[SOCKET_MEM_STRLEN]; - - snprintf(valid_socket_mem, sizeof(valid_socket_mem), "--socket-mem="); - - /* add one extra socket */ - for (i = 0; i < num_sockets; i++) { - snprintf(buf, sizeof(buf), "%s%s", valid_socket_mem, DEFAULT_MEM_SIZE); - strlcpy(valid_socket_mem, buf, sizeof(valid_socket_mem)); - - if (num_sockets - i > 1) { - snprintf(buf, sizeof(buf), "%s,", valid_socket_mem); - strlcpy(valid_socket_mem, buf, - sizeof(valid_socket_mem)); - } - } + populate_socket_mem_param(num_sockets, DEFAULT_MEM_SIZE, + buf, sizeof(buf)); + snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), + "%s%s", buf, DEFAULT_MEM_SIZE); /* invalid --socket-mem flag (with extra socket) */ const char *argv9[] = {prgname, "--file-prefix=" memtest, invalid_socket_mem}; + /* construct a valid socket mask with 2 megs on each existing socket */ + char valid_socket_mem[SOCKET_MEM_STRLEN]; + populate_socket_mem_param(num_sockets - 1, DEFAULT_MEM_SIZE, + buf, sizeof(buf)); + snprintf(valid_socket_mem, sizeof(valid_socket_mem), + "%s%s", buf, DEFAULT_MEM_SIZE); + /* valid --socket-mem flag */ const char *argv10[] = {prgname, "--file-prefix=" memtest, valid_socket_mem}; -- 2.8.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v3 1/1] app/test: fix --socket-mem option in eal flag autotest 2019-07-12 6:43 ` [dpdk-dev] [PATCH v3 " vattunuru @ 2019-07-23 6:13 ` Vamsi Krishna Attunuru 2019-07-26 13:39 ` David Marchand 1 sibling, 0 replies; 14+ messages in thread From: Vamsi Krishna Attunuru @ 2019-07-23 6:13 UTC (permalink / raw) To: dev Cc: thomas, Jerin Jacob Kollanukkaran, david.marchand, aconole, Anatoly Burakov Ping V3 review request.. > -----Original Message----- > From: vattunuru@marvell.com <vattunuru@marvell.com> > Sent: Friday, July 12, 2019 12:13 PM > To: dev@dpdk.org > Cc: thomas@monjalon.net; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; > david.marchand@redhat.com; aconole@redhat.com; Vamsi Krishna Attunuru > <vattunuru@marvell.com> > Subject: [PATCH v3 1/1] app/test: fix --socket-mem option in eal flag autotest > > From: Vamsi Attunuru <vattunuru@marvell.com> > > eal flag autotest fails when multiple mem size flags are passed to --socket-mem > option irrespective of RTE_MAX_NUMA_NODES. > > Patch fixes --socket-mem option by setting enough numbers of numa node mem > flags based on RTE_MAX_NUMA_NODES config. > > Fixes: 45f1b6e8680a ("app: add new tests on eal flags") > > Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com> > --- > V3 changes: > * Pass buffer size info to populate socket mem routine which strlcpy requires. > > V2 changes: > * Add routine to populate --socket-mem option and use it to form valid and > invalid test commands. > > app/test/test_eal_flags.c | 121 +++++++++++++++++++++++++++------------------ > - > 1 file changed, 71 insertions(+), 50 deletions(-) > > diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c index > 672ca0a..053d694 100644 > --- a/app/test/test_eal_flags.c > +++ b/app/test/test_eal_flags.c > @@ -1250,12 +1250,46 @@ test_file_prefix(void) > return 0; > } > > +static void > +populate_socket_mem_param(int num_sockets, const char *mem, > + char *buf, size_t buf_size) > +{ > + char tmp[SOCKET_MEM_STRLEN]; > + int i; > + > + snprintf(buf, SOCKET_MEM_STRLEN, "--socket-mem="); > + > + for (i = 0; i < num_sockets ; i++) { > + snprintf(tmp, sizeof(tmp), "%s%s", buf, mem); > + strlcpy(buf, tmp, buf_size); > + > + if (num_sockets + 1 - i > 1) { > + snprintf(tmp, sizeof(tmp), "%s,", buf); > + strlcpy(buf, tmp, buf_size); > + } > + } > +} > + > /* > * Tests for correct handling of -m and --socket-mem flags > */ > static int > test_memory_flags(void) > { > + char buf[SOCKET_MEM_STRLEN]; /* to avoid copying string onto itself > */ > + > +#ifdef RTE_EXEC_ENV_FREEBSD > + int num_sockets = 1; > +#else > + int num_sockets = RTE_MIN(get_number_of_sockets(), > + RTE_MAX_NUMA_NODES); > +#endif > + > + if (num_sockets <= 0) { > + printf("Error - cannot get number of sockets!\n"); > + return -1; > + } > + > #ifdef RTE_EXEC_ENV_FREEBSD > /* BSD target doesn't support prefixes at this point */ > const char * prefix = ""; > @@ -1277,85 +1311,72 @@ test_memory_flags(void) > "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE}; > > /* valid (zero) --socket-mem flag */ > + char arg2_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "0", buf, sizeof(buf)); > + snprintf(arg2_socket_mem, sizeof(arg2_socket_mem), "%s%s", buf, > "0"); > const char *argv2[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=0,0,0,0"}; > + "--file-prefix=" memtest, arg2_socket_mem}; > > /* invalid (incomplete) --socket-mem flag */ > + char arg3_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); > + snprintf(arg3_socket_mem, sizeof(arg3_socket_mem), "%s%s", buf, > "2,"); > const char *argv3[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=2,2,"}; > + "--file-prefix=" memtest, arg3_socket_mem}; > > /* invalid (mixed with invalid data) --socket-mem flag */ > + char arg4_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); > + snprintf(arg4_socket_mem, sizeof(arg4_socket_mem), "%s%s", buf, > +"Fred"); > const char *argv4[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=2,2,Fred"}; > + "--file-prefix=" memtest, arg4_socket_mem}; > > /* invalid (with numeric value as last character) --socket-mem flag */ > + char arg5_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); > + snprintf(arg5_socket_mem, sizeof(arg5_socket_mem), > + "%s%s", buf, "Fred0"); > const char *argv5[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=2,2,Fred0"}; > + "--file-prefix=" memtest, arg5_socket_mem}; > > /* invalid (with empty socket) --socket-mem flag */ > + char arg6_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); > + snprintf(arg6_socket_mem, sizeof(arg6_socket_mem), "%s%s", buf, > ",,"); > const char *argv6[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=2,,2"}; > + "--file-prefix=" memtest, arg6_socket_mem}; > > /* invalid (null) --socket-mem flag */ > const char *argv7[] = {prgname, > "--file-prefix=" memtest, "--socket-mem="}; > > /* valid --socket-mem specified together with -m flag */ > + char arg8_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); > + snprintf(arg8_socket_mem, sizeof(arg8_socket_mem), "%s%s", buf, > "2"); > const char *argv8[] = {prgname, > - "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE, "- > -socket-mem=2,2"}; > + "--file-prefix=" memtest, "-m", > + DEFAULT_MEM_SIZE, arg8_socket_mem}; > > /* construct an invalid socket mask with 2 megs on each socket plus > * extra 2 megs on socket that doesn't exist on current system */ > char invalid_socket_mem[SOCKET_MEM_STRLEN]; > - char buf[SOCKET_MEM_STRLEN]; /* to avoid copying string onto > itself */ > - > -#ifdef RTE_EXEC_ENV_FREEBSD > - int i, num_sockets = 1; > -#else > - int i, num_sockets = RTE_MIN(get_number_of_sockets(), > - RTE_MAX_NUMA_NODES); > -#endif > - > - if (num_sockets <= 0) { > - printf("Error - cannot get number of sockets!\n"); > - return -1; > - } > - > - snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), "--socket- > mem="); > - > - /* add one extra socket */ > - for (i = 0; i < num_sockets + 1; i++) { > - snprintf(buf, sizeof(buf), "%s%s", invalid_socket_mem, > DEFAULT_MEM_SIZE); > - strlcpy(invalid_socket_mem, buf, sizeof(invalid_socket_mem)); > - > - if (num_sockets + 1 - i > 1) { > - snprintf(buf, sizeof(buf), "%s,", invalid_socket_mem); > - strlcpy(invalid_socket_mem, buf, > - sizeof(invalid_socket_mem)); > - } > - } > - > - /* construct a valid socket mask with 2 megs on each existing socket */ > - char valid_socket_mem[SOCKET_MEM_STRLEN]; > - > - snprintf(valid_socket_mem, sizeof(valid_socket_mem), "--socket- > mem="); > - > - /* add one extra socket */ > - for (i = 0; i < num_sockets; i++) { > - snprintf(buf, sizeof(buf), "%s%s", valid_socket_mem, > DEFAULT_MEM_SIZE); > - strlcpy(valid_socket_mem, buf, sizeof(valid_socket_mem)); > - > - if (num_sockets - i > 1) { > - snprintf(buf, sizeof(buf), "%s,", valid_socket_mem); > - strlcpy(valid_socket_mem, buf, > - sizeof(valid_socket_mem)); > - } > - } > + populate_socket_mem_param(num_sockets, DEFAULT_MEM_SIZE, > + buf, sizeof(buf)); > + snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), > + "%s%s", buf, DEFAULT_MEM_SIZE); > > /* invalid --socket-mem flag (with extra socket) */ > const char *argv9[] = {prgname, > "--file-prefix=" memtest, invalid_socket_mem}; > > + /* construct a valid socket mask with 2 megs on each existing socket */ > + char valid_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, DEFAULT_MEM_SIZE, > + buf, sizeof(buf)); > + snprintf(valid_socket_mem, sizeof(valid_socket_mem), > + "%s%s", buf, DEFAULT_MEM_SIZE); > + > /* valid --socket-mem flag */ > const char *argv10[] = {prgname, > "--file-prefix=" memtest, valid_socket_mem}; > -- > 2.8.4 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v3 1/1] app/test: fix --socket-mem option in eal flag autotest 2019-07-12 6:43 ` [dpdk-dev] [PATCH v3 " vattunuru 2019-07-23 6:13 ` Vamsi Krishna Attunuru @ 2019-07-26 13:39 ` David Marchand 2019-07-28 4:19 ` [dpdk-dev] [EXT] " Vamsi Krishna Attunuru 1 sibling, 1 reply; 14+ messages in thread From: David Marchand @ 2019-07-26 13:39 UTC (permalink / raw) To: Vamsi Attunuru Cc: dev, Thomas Monjalon, Jerin Jacob Kollanukkaran, Aaron Conole Hello, On Fri, Jul 12, 2019 at 8:43 AM <vattunuru@marvell.com> wrote: > > From: Vamsi Attunuru <vattunuru@marvell.com> > > eal flag autotest fails when multiple mem size flags are passed > to --socket-mem option irrespective of RTE_MAX_NUMA_NODES. > > Patch fixes --socket-mem option by setting enough numbers > of numa node mem flags based on RTE_MAX_NUMA_NODES config. > > Fixes: 45f1b6e8680a ("app: add new tests on eal flags") > > Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com> > --- > V3 changes: > * Pass buffer size info to populate socket mem routine which > strlcpy requires. > > V2 changes: > * Add routine to populate --socket-mem option and use it to form > valid and invalid test commands. > > app/test/test_eal_flags.c | 121 +++++++++++++++++++++++++++------------------- > 1 file changed, 71 insertions(+), 50 deletions(-) > > diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c > index 672ca0a..053d694 100644 > --- a/app/test/test_eal_flags.c > +++ b/app/test/test_eal_flags.c > @@ -1250,12 +1250,46 @@ test_file_prefix(void) > return 0; > } > > +static void > +populate_socket_mem_param(int num_sockets, const char *mem, > + char *buf, size_t buf_size) > +{ > + char tmp[SOCKET_MEM_STRLEN]; You don't need this temp buffer. > + int i; > + > + snprintf(buf, SOCKET_MEM_STRLEN, "--socket-mem="); buf is buf_size large, not SOCKET_MEM_STRLEN. > + > + for (i = 0; i < num_sockets ; i++) { > + snprintf(tmp, sizeof(tmp), "%s%s", buf, mem); > + strlcpy(buf, tmp, buf_size); > + > + if (num_sockets + 1 - i > 1) { > + snprintf(tmp, sizeof(tmp), "%s,", buf); > + strlcpy(buf, tmp, buf_size); > + } > + } > +} > + How about the following: /* This function writes in passed buf pointer a valid --socket-mem= option * for num_sockets then concatenates the provided suffix string. * * Example for num_sockets 4, mem "2", suffix "plop" * --socket-mem=2,2,2,2plop */ static void populate_socket_mem_param(int num_sockets, const char *mem, const char *suffix, char *buf, size_t buf_size) { unsigned int offset = 0; int written; int i; written = snprintf(&buf[offset], buf_size - offset, "--socket-mem="); if (written < 0 || written + offset >= buf_size) return; offset += written; for (i = 0; i < num_sockets - 1; i++) { written = snprintf(&buf[offset], buf_size - offset, "%s,", mem); if (written < 0 || written + offset >= buf_size) return; offset += written; } written = snprintf(&buf[offset], buf_size - offset, "%s%s", mem, suffix); if (written < 0 || written + offset >= buf_size) return; offset += written; } See below for usage. > /* > * Tests for correct handling of -m and --socket-mem flags > */ > static int > test_memory_flags(void) > { > + char buf[SOCKET_MEM_STRLEN]; /* to avoid copying string onto itself */ You don't need this temp buffer. > + > +#ifdef RTE_EXEC_ENV_FREEBSD > + int num_sockets = 1; > +#else > + int num_sockets = RTE_MIN(get_number_of_sockets(), > + RTE_MAX_NUMA_NODES); > +#endif > + > + if (num_sockets <= 0) { > + printf("Error - cannot get number of sockets!\n"); > + return -1; > + } > + > #ifdef RTE_EXEC_ENV_FREEBSD > /* BSD target doesn't support prefixes at this point */ > const char * prefix = ""; > @@ -1277,85 +1311,72 @@ test_memory_flags(void) > "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE}; > > /* valid (zero) --socket-mem flag */ > + char arg2_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "0", buf, sizeof(buf)); > + snprintf(arg2_socket_mem, sizeof(arg2_socket_mem), "%s%s", buf, "0"); With the proposed populate_socket_mem_param, it becomes: populate_socket_mem_param(num_sockets, "0", "", arg2_socket_mem, sizeof(arg2_socket_mem)); > const char *argv2[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=0,0,0,0"}; > + "--file-prefix=" memtest, arg2_socket_mem}; > > /* invalid (incomplete) --socket-mem flag */ > + char arg3_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); What happens when num_socket == 1? With the proposed populate_socket_mem_param, it would become: populate_socket_mem_param(num_sockets - 1, "2", ",", arg3_socket_mem, sizeof(arg3_socket_mem)); But we need to ensure that we have at least two sockets before starting the test on argv3. > + snprintf(arg3_socket_mem, sizeof(arg3_socket_mem), "%s%s", buf, "2,"); > const char *argv3[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=2,2,"}; > + "--file-prefix=" memtest, arg3_socket_mem}; > > /* invalid (mixed with invalid data) --socket-mem flag */ > + char arg4_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); > + snprintf(arg4_socket_mem, sizeof(arg4_socket_mem), "%s%s", buf, "Fred"); > const char *argv4[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=2,2,Fred"}; > + "--file-prefix=" memtest, arg4_socket_mem}; Idem argv3. > > /* invalid (with numeric value as last character) --socket-mem flag */ > + char arg5_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); > + snprintf(arg5_socket_mem, sizeof(arg5_socket_mem), > + "%s%s", buf, "Fred0"); > const char *argv5[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=2,2,Fred0"}; > + "--file-prefix=" memtest, arg5_socket_mem}; Idem argv3. > > /* invalid (with empty socket) --socket-mem flag */ > + char arg6_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); > + snprintf(arg6_socket_mem, sizeof(arg6_socket_mem), "%s%s", buf, ",,"); > const char *argv6[] = {prgname, > - "--file-prefix=" memtest, "--socket-mem=2,,2"}; > + "--file-prefix=" memtest, arg6_socket_mem}; Here, this test requires at least 3 sockets so that we can test at minimum 2,,2. > > /* invalid (null) --socket-mem flag */ > const char *argv7[] = {prgname, > "--file-prefix=" memtest, "--socket-mem="}; > > /* valid --socket-mem specified together with -m flag */ > + char arg8_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); > + snprintf(arg8_socket_mem, sizeof(arg8_socket_mem), "%s%s", buf, "2"); > const char *argv8[] = {prgname, > - "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE, "--socket-mem=2,2"}; > + "--file-prefix=" memtest, "-m", > + DEFAULT_MEM_SIZE, arg8_socket_mem}; > > /* construct an invalid socket mask with 2 megs on each socket plus > * extra 2 megs on socket that doesn't exist on current system */ > char invalid_socket_mem[SOCKET_MEM_STRLEN]; > - char buf[SOCKET_MEM_STRLEN]; /* to avoid copying string onto itself */ > - > -#ifdef RTE_EXEC_ENV_FREEBSD > - int i, num_sockets = 1; > -#else > - int i, num_sockets = RTE_MIN(get_number_of_sockets(), > - RTE_MAX_NUMA_NODES); > -#endif > - > - if (num_sockets <= 0) { > - printf("Error - cannot get number of sockets!\n"); > - return -1; > - } > - > - snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), "--socket-mem="); > - > - /* add one extra socket */ > - for (i = 0; i < num_sockets + 1; i++) { > - snprintf(buf, sizeof(buf), "%s%s", invalid_socket_mem, DEFAULT_MEM_SIZE); > - strlcpy(invalid_socket_mem, buf, sizeof(invalid_socket_mem)); > - > - if (num_sockets + 1 - i > 1) { > - snprintf(buf, sizeof(buf), "%s,", invalid_socket_mem); > - strlcpy(invalid_socket_mem, buf, > - sizeof(invalid_socket_mem)); > - } > - } > - > - /* construct a valid socket mask with 2 megs on each existing socket */ > - char valid_socket_mem[SOCKET_MEM_STRLEN]; > - > - snprintf(valid_socket_mem, sizeof(valid_socket_mem), "--socket-mem="); > - > - /* add one extra socket */ > - for (i = 0; i < num_sockets; i++) { > - snprintf(buf, sizeof(buf), "%s%s", valid_socket_mem, DEFAULT_MEM_SIZE); > - strlcpy(valid_socket_mem, buf, sizeof(valid_socket_mem)); > - > - if (num_sockets - i > 1) { > - snprintf(buf, sizeof(buf), "%s,", valid_socket_mem); > - strlcpy(valid_socket_mem, buf, > - sizeof(valid_socket_mem)); > - } > - } > + populate_socket_mem_param(num_sockets, DEFAULT_MEM_SIZE, > + buf, sizeof(buf)); > + snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), > + "%s%s", buf, DEFAULT_MEM_SIZE); The comment says you want to generate a string with 2M per socket, not DEFAULT_MEM_SIZE. > > /* invalid --socket-mem flag (with extra socket) */ > const char *argv9[] = {prgname, > "--file-prefix=" memtest, invalid_socket_mem}; > > + /* construct a valid socket mask with 2 megs on each existing socket */ > + char valid_socket_mem[SOCKET_MEM_STRLEN]; > + populate_socket_mem_param(num_sockets - 1, DEFAULT_MEM_SIZE, > + buf, sizeof(buf)); > + snprintf(valid_socket_mem, sizeof(valid_socket_mem), > + "%s%s", buf, DEFAULT_MEM_SIZE); > + Idem 2M, not DEFAULT_MEM_SIZE. > /* valid --socket-mem flag */ > const char *argv10[] = {prgname, > "--file-prefix=" memtest, valid_socket_mem}; > -- > 2.8.4 > I prepared a patch with my comments addressed, can you have a look ? https://github.com/david-marchand/dpdk/commit/a78b7572a402579d775f1d0622d1454eb36e0ad4 Thanks. -- David Marchand ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [EXT] Re: [PATCH v3 1/1] app/test: fix --socket-mem option in eal flag autotest 2019-07-26 13:39 ` David Marchand @ 2019-07-28 4:19 ` Vamsi Krishna Attunuru 0 siblings, 0 replies; 14+ messages in thread From: Vamsi Krishna Attunuru @ 2019-07-28 4:19 UTC (permalink / raw) To: David Marchand Cc: dev, Thomas Monjalon, Jerin Jacob Kollanukkaran, Aaron Conole Hi, > -----Original Message----- > From: David Marchand <david.marchand@redhat.com> > Sent: Friday, July 26, 2019 7:10 PM > To: Vamsi Krishna Attunuru <vattunuru@marvell.com> > Cc: dev <dev@dpdk.org>; Thomas Monjalon <thomas@monjalon.net>; Jerin > Jacob Kollanukkaran <jerinj@marvell.com>; Aaron Conole > <aconole@redhat.com> > Subject: [EXT] Re: [PATCH v3 1/1] app/test: fix --socket-mem option in eal flag > autotest > > External Email > > ---------------------------------------------------------------------- > Hello, > > On Fri, Jul 12, 2019 at 8:43 AM <vattunuru@marvell.com> wrote: > > > > From: Vamsi Attunuru <vattunuru@marvell.com> > > > > eal flag autotest fails when multiple mem size flags are passed to > > --socket-mem option irrespective of RTE_MAX_NUMA_NODES. > > > > Patch fixes --socket-mem option by setting enough numbers of numa node > > mem flags based on RTE_MAX_NUMA_NODES config. > > > > Fixes: 45f1b6e8680a ("app: add new tests on eal flags") > > > > Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com> > > --- > > V3 changes: > > * Pass buffer size info to populate socket mem routine which strlcpy > > requires. > > > > V2 changes: > > * Add routine to populate --socket-mem option and use it to form valid > > and invalid test commands. > > > > app/test/test_eal_flags.c | 121 > > +++++++++++++++++++++++++++------------------- > > 1 file changed, 71 insertions(+), 50 deletions(-) > > > > diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c > > index 672ca0a..053d694 100644 > > --- a/app/test/test_eal_flags.c > > +++ b/app/test/test_eal_flags.c > > @@ -1250,12 +1250,46 @@ test_file_prefix(void) > > return 0; > > } > > > > +static void > > +populate_socket_mem_param(int num_sockets, const char *mem, > > + char *buf, size_t buf_size) { > > + char tmp[SOCKET_MEM_STRLEN]; > > You don't need this temp buffer. > > > + int i; > > + > > + snprintf(buf, SOCKET_MEM_STRLEN, "--socket-mem="); > > buf is buf_size large, not SOCKET_MEM_STRLEN. > > > > + > > + for (i = 0; i < num_sockets ; i++) { > > + snprintf(tmp, sizeof(tmp), "%s%s", buf, mem); > > + strlcpy(buf, tmp, buf_size); > > + > > + if (num_sockets + 1 - i > 1) { > > + snprintf(tmp, sizeof(tmp), "%s,", buf); > > + strlcpy(buf, tmp, buf_size); > > + } > > + } > > +} > > + > > > How about the following: > > /* This function writes in passed buf pointer a valid --socket-mem= option > * for num_sockets then concatenates the provided suffix string. > * > * Example for num_sockets 4, mem "2", suffix "plop" > * --socket-mem=2,2,2,2plop > */ > static void > populate_socket_mem_param(int num_sockets, const char *mem, > const char *suffix, char *buf, size_t buf_size) { > unsigned int offset = 0; > int written; > int i; > > written = snprintf(&buf[offset], buf_size - offset, "--socket-mem="); > if (written < 0 || written + offset >= buf_size) > return; > offset += written; > > for (i = 0; i < num_sockets - 1; i++) { > written = snprintf(&buf[offset], buf_size - offset, > "%s,", mem); > if (written < 0 || written + offset >= buf_size) > return; > offset += written; > } > > written = snprintf(&buf[offset], buf_size - offset, "%s%s", mem, > suffix); > if (written < 0 || written + offset >= buf_size) > return; > offset += written; > } > > See below for usage. > > > > > /* > > * Tests for correct handling of -m and --socket-mem flags > > */ > > static int > > test_memory_flags(void) > > { > > + char buf[SOCKET_MEM_STRLEN]; /* to avoid copying string onto > > + itself */ > > You don't need this temp buffer. > > > > + > > +#ifdef RTE_EXEC_ENV_FREEBSD > > + int num_sockets = 1; > > +#else > > + int num_sockets = RTE_MIN(get_number_of_sockets(), > > + RTE_MAX_NUMA_NODES); #endif > > + > > + if (num_sockets <= 0) { > > + printf("Error - cannot get number of sockets!\n"); > > + return -1; > > + } > > + > > #ifdef RTE_EXEC_ENV_FREEBSD > > /* BSD target doesn't support prefixes at this point */ > > const char * prefix = ""; > > @@ -1277,85 +1311,72 @@ test_memory_flags(void) > > "--file-prefix=" memtest, "-m", > > DEFAULT_MEM_SIZE}; > > > > /* valid (zero) --socket-mem flag */ > > + char arg2_socket_mem[SOCKET_MEM_STRLEN]; > > + populate_socket_mem_param(num_sockets - 1, "0", buf, sizeof(buf)); > > + snprintf(arg2_socket_mem, sizeof(arg2_socket_mem), "%s%s", > > + buf, "0"); > > With the proposed populate_socket_mem_param, it becomes: > > populate_socket_mem_param(num_sockets, "0", "", > arg2_socket_mem, sizeof(arg2_socket_mem)); > > > const char *argv2[] = {prgname, > > - "--file-prefix=" memtest, "--socket-mem=0,0,0,0"}; > > + "--file-prefix=" memtest, arg2_socket_mem}; > > > > /* invalid (incomplete) --socket-mem flag */ > > + char arg3_socket_mem[SOCKET_MEM_STRLEN]; > > + populate_socket_mem_param(num_sockets - 1, "2", buf, > > + sizeof(buf)); > > What happens when num_socket == 1? > > With the proposed populate_socket_mem_param, it would become: > > populate_socket_mem_param(num_sockets - 1, "2", ",", > arg3_socket_mem, sizeof(arg3_socket_mem)); > > But we need to ensure that we have at least two sockets before starting the > test on argv3. > > > > + snprintf(arg3_socket_mem, sizeof(arg3_socket_mem), "%s%s", > > + buf, "2,"); > > const char *argv3[] = {prgname, > > - "--file-prefix=" memtest, "--socket-mem=2,2,"}; > > + "--file-prefix=" memtest, arg3_socket_mem}; > > > > /* invalid (mixed with invalid data) --socket-mem flag */ > > + char arg4_socket_mem[SOCKET_MEM_STRLEN]; > > + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); > > + snprintf(arg4_socket_mem, sizeof(arg4_socket_mem), "%s%s", > > + buf, "Fred"); > > const char *argv4[] = {prgname, > > - "--file-prefix=" memtest, "--socket-mem=2,2,Fred"}; > > + "--file-prefix=" memtest, arg4_socket_mem}; > > Idem argv3. > > > > > /* invalid (with numeric value as last character) --socket-mem > > flag */ > > + char arg5_socket_mem[SOCKET_MEM_STRLEN]; > > + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); > > + snprintf(arg5_socket_mem, sizeof(arg5_socket_mem), > > + "%s%s", buf, "Fred0"); > > const char *argv5[] = {prgname, > > - "--file-prefix=" memtest, "--socket-mem=2,2,Fred0"}; > > + "--file-prefix=" memtest, arg5_socket_mem}; > > Idem argv3. > > > > > /* invalid (with empty socket) --socket-mem flag */ > > + char arg6_socket_mem[SOCKET_MEM_STRLEN]; > > + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); > > + snprintf(arg6_socket_mem, sizeof(arg6_socket_mem), "%s%s", > > + buf, ",,"); > > const char *argv6[] = {prgname, > > - "--file-prefix=" memtest, "--socket-mem=2,,2"}; > > + "--file-prefix=" memtest, arg6_socket_mem}; > > Here, this test requires at least 3 sockets so that we can test at minimum 2,,2. > > > > > > /* invalid (null) --socket-mem flag */ > > const char *argv7[] = {prgname, > > "--file-prefix=" memtest, "--socket-mem="}; > > > > /* valid --socket-mem specified together with -m flag */ > > + char arg8_socket_mem[SOCKET_MEM_STRLEN]; > > + populate_socket_mem_param(num_sockets - 1, "2", buf, sizeof(buf)); > > + snprintf(arg8_socket_mem, sizeof(arg8_socket_mem), "%s%s", > > + buf, "2"); > > const char *argv8[] = {prgname, > > - "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE, "-- > socket-mem=2,2"}; > > + "--file-prefix=" memtest, "-m", > > + DEFAULT_MEM_SIZE, arg8_socket_mem}; > > > > /* construct an invalid socket mask with 2 megs on each socket plus > > * extra 2 megs on socket that doesn't exist on current system */ > > char invalid_socket_mem[SOCKET_MEM_STRLEN]; > > - char buf[SOCKET_MEM_STRLEN]; /* to avoid copying string onto > itself */ > > - > > -#ifdef RTE_EXEC_ENV_FREEBSD > > - int i, num_sockets = 1; > > -#else > > - int i, num_sockets = RTE_MIN(get_number_of_sockets(), > > - RTE_MAX_NUMA_NODES); > > -#endif > > - > > - if (num_sockets <= 0) { > > - printf("Error - cannot get number of sockets!\n"); > > - return -1; > > - } > > - > > - snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), "--socket- > mem="); > > - > > - /* add one extra socket */ > > - for (i = 0; i < num_sockets + 1; i++) { > > - snprintf(buf, sizeof(buf), "%s%s", invalid_socket_mem, > DEFAULT_MEM_SIZE); > > - strlcpy(invalid_socket_mem, buf, sizeof(invalid_socket_mem)); > > - > > - if (num_sockets + 1 - i > 1) { > > - snprintf(buf, sizeof(buf), "%s,", invalid_socket_mem); > > - strlcpy(invalid_socket_mem, buf, > > - sizeof(invalid_socket_mem)); > > - } > > - } > > - > > - /* construct a valid socket mask with 2 megs on each existing socket */ > > - char valid_socket_mem[SOCKET_MEM_STRLEN]; > > - > > - snprintf(valid_socket_mem, sizeof(valid_socket_mem), "--socket- > mem="); > > - > > - /* add one extra socket */ > > - for (i = 0; i < num_sockets; i++) { > > - snprintf(buf, sizeof(buf), "%s%s", valid_socket_mem, > DEFAULT_MEM_SIZE); > > - strlcpy(valid_socket_mem, buf, sizeof(valid_socket_mem)); > > - > > - if (num_sockets - i > 1) { > > - snprintf(buf, sizeof(buf), "%s,", valid_socket_mem); > > - strlcpy(valid_socket_mem, buf, > > - sizeof(valid_socket_mem)); > > - } > > - } > > + populate_socket_mem_param(num_sockets, DEFAULT_MEM_SIZE, > > + buf, sizeof(buf)); > > + snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), > > + "%s%s", buf, DEFAULT_MEM_SIZE); > > The comment says you want to generate a string with 2M per socket, not > DEFAULT_MEM_SIZE. > > > > > /* invalid --socket-mem flag (with extra socket) */ > > const char *argv9[] = {prgname, > > "--file-prefix=" memtest, invalid_socket_mem}; > > > > + /* construct a valid socket mask with 2 megs on each existing socket */ > > + char valid_socket_mem[SOCKET_MEM_STRLEN]; > > + populate_socket_mem_param(num_sockets - 1, DEFAULT_MEM_SIZE, > > + buf, sizeof(buf)); > > + snprintf(valid_socket_mem, sizeof(valid_socket_mem), > > + "%s%s", buf, DEFAULT_MEM_SIZE); > > + > > Idem 2M, not DEFAULT_MEM_SIZE. > > > > /* valid --socket-mem flag */ > > const char *argv10[] = {prgname, > > "--file-prefix=" memtest, valid_socket_mem}; > > -- > > 2.8.4 > > > > I prepared a patch with my comments addressed, can you have a look ? > https://github.com/david- > marchand/dpdk/commit/a78b7572a402579d775f1d0622d1454eb36e0ad4 Thanks for the comments and new patch, reviewed and verified it. Patch looks good to me. Reviewed-by: Vamsi Attunuru <vattunuru@marvell.com> Tested-by: Vamsi Attunuru <vattunuru@marvell.com> > > Thanks. > > > -- > David Marchand ^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v4] test: fix --socket-mem option in eal flag autotest 2019-04-10 7:25 [dpdk-dev] [PATCH v1 1/1] app/test: fix --socket-mem option in eal flag autotest Vamsi Attunuru ` (2 preceding siblings ...) 2019-07-11 5:15 ` [dpdk-dev] [PATCH v2 " vattunuru @ 2019-07-29 8:08 ` David Marchand 2019-07-30 9:24 ` Thomas Monjalon 3 siblings, 1 reply; 14+ messages in thread From: David Marchand @ 2019-07-29 8:08 UTC (permalink / raw) To: dev; +Cc: Vamsi Attunuru, stable From: Vamsi Attunuru <vattunuru@marvell.com> eal flag autotest fails when multiple mem size flags are passed to --socket-mem option irrespective of RTE_MAX_NUMA_NODES and the number of available sockets on the test system. Fixes: 45f1b6e8680a ("app: add new tests on eal flags") Cc: stable@dpdk.org Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com> Signed-off-by: David Marchand <david.marchand@redhat.com> Reviewed-by: Vamsi Attunuru <vattunuru@marvell.com> Tested-by: Vamsi Attunuru <vattunuru@marvell.com> --- app/test/test_eal_flags.c | 144 +++++++++++++++++++++++++++------------------- 1 file changed, 86 insertions(+), 58 deletions(-) diff --git a/app/test/test_eal_flags.c b/app/test/test_eal_flags.c index 672ca0a..827ea88 100644 --- a/app/test/test_eal_flags.c +++ b/app/test/test_eal_flags.c @@ -1250,6 +1250,40 @@ test_file_prefix(void) return 0; } +/* This function writes in passed buf pointer a valid --socket-mem= option + * for num_sockets then concatenates the provided suffix string. + * + * Example for num_sockets 4, mem "2", suffix "plop" + * --socket-mem=2,2,2,2plop + */ +static void +populate_socket_mem_param(int num_sockets, const char *mem, + const char *suffix, char *buf, size_t buf_size) +{ + unsigned int offset = 0; + int written; + int i; + + written = snprintf(&buf[offset], buf_size - offset, "--socket-mem="); + if (written < 0 || written + offset >= buf_size) + return; + offset += written; + + for (i = 0; i < num_sockets - 1; i++) { + written = snprintf(&buf[offset], buf_size - offset, + "%s,", mem); + if (written < 0 || written + offset >= buf_size) + return; + offset += written; + } + + written = snprintf(&buf[offset], buf_size - offset, "%s%s", mem, + suffix); + if (written < 0 || written + offset >= buf_size) + return; + offset += written; +} + /* * Tests for correct handling of -m and --socket-mem flags */ @@ -1277,42 +1311,44 @@ test_memory_flags(void) "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE}; /* valid (zero) --socket-mem flag */ + char arg2_socket_mem[SOCKET_MEM_STRLEN]; const char *argv2[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=0,0,0,0"}; + "--file-prefix=" memtest, arg2_socket_mem}; /* invalid (incomplete) --socket-mem flag */ + char arg3_socket_mem[SOCKET_MEM_STRLEN]; const char *argv3[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=2,2,"}; + "--file-prefix=" memtest, arg3_socket_mem}; /* invalid (mixed with invalid data) --socket-mem flag */ + char arg4_socket_mem[SOCKET_MEM_STRLEN]; const char *argv4[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=2,2,Fred"}; + "--file-prefix=" memtest, arg4_socket_mem}; /* invalid (with numeric value as last character) --socket-mem flag */ + char arg5_socket_mem[SOCKET_MEM_STRLEN]; const char *argv5[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=2,2,Fred0"}; + "--file-prefix=" memtest, arg5_socket_mem}; /* invalid (with empty socket) --socket-mem flag */ + char arg6_socket_mem[SOCKET_MEM_STRLEN]; const char *argv6[] = {prgname, - "--file-prefix=" memtest, "--socket-mem=2,,2"}; + "--file-prefix=" memtest, arg6_socket_mem}; /* invalid (null) --socket-mem flag */ const char *argv7[] = {prgname, "--file-prefix=" memtest, "--socket-mem="}; /* valid --socket-mem specified together with -m flag */ + char arg8_socket_mem[SOCKET_MEM_STRLEN]; const char *argv8[] = {prgname, - "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE, "--socket-mem=2,2"}; - - /* construct an invalid socket mask with 2 megs on each socket plus - * extra 2 megs on socket that doesn't exist on current system */ - char invalid_socket_mem[SOCKET_MEM_STRLEN]; - char buf[SOCKET_MEM_STRLEN]; /* to avoid copying string onto itself */ + "--file-prefix=" memtest, "-m", DEFAULT_MEM_SIZE, + arg8_socket_mem}; #ifdef RTE_EXEC_ENV_FREEBSD - int i, num_sockets = 1; + int num_sockets = 1; #else - int i, num_sockets = RTE_MIN(get_number_of_sockets(), + int num_sockets = RTE_MIN(get_number_of_sockets(), RTE_MAX_NUMA_NODES); #endif @@ -1321,42 +1357,13 @@ test_memory_flags(void) return -1; } - snprintf(invalid_socket_mem, sizeof(invalid_socket_mem), "--socket-mem="); - - /* add one extra socket */ - for (i = 0; i < num_sockets + 1; i++) { - snprintf(buf, sizeof(buf), "%s%s", invalid_socket_mem, DEFAULT_MEM_SIZE); - strlcpy(invalid_socket_mem, buf, sizeof(invalid_socket_mem)); - - if (num_sockets + 1 - i > 1) { - snprintf(buf, sizeof(buf), "%s,", invalid_socket_mem); - strlcpy(invalid_socket_mem, buf, - sizeof(invalid_socket_mem)); - } - } - - /* construct a valid socket mask with 2 megs on each existing socket */ - char valid_socket_mem[SOCKET_MEM_STRLEN]; - - snprintf(valid_socket_mem, sizeof(valid_socket_mem), "--socket-mem="); - - /* add one extra socket */ - for (i = 0; i < num_sockets; i++) { - snprintf(buf, sizeof(buf), "%s%s", valid_socket_mem, DEFAULT_MEM_SIZE); - strlcpy(valid_socket_mem, buf, sizeof(valid_socket_mem)); - - if (num_sockets - i > 1) { - snprintf(buf, sizeof(buf), "%s,", valid_socket_mem); - strlcpy(valid_socket_mem, buf, - sizeof(valid_socket_mem)); - } - } - /* invalid --socket-mem flag (with extra socket) */ + char invalid_socket_mem[SOCKET_MEM_STRLEN]; const char *argv9[] = {prgname, "--file-prefix=" memtest, invalid_socket_mem}; /* valid --socket-mem flag */ + char valid_socket_mem[SOCKET_MEM_STRLEN]; const char *argv10[] = {prgname, "--file-prefix=" memtest, valid_socket_mem}; @@ -1374,34 +1381,49 @@ test_memory_flags(void) printf("Error - process failed with valid -m flag!\n"); return -1; } + + populate_socket_mem_param(num_sockets, "0", "", + arg2_socket_mem, sizeof(arg2_socket_mem)); if (launch_proc(argv2) != 0) { printf("Error - process failed with valid (zero) --socket-mem!\n"); return -1; } - if (launch_proc(argv3) == 0) { - printf("Error - process run ok with invalid " + if (num_sockets > 1) { + populate_socket_mem_param(num_sockets - 1, "2", ",", + arg3_socket_mem, sizeof(arg3_socket_mem)); + if (launch_proc(argv3) == 0) { + printf("Error - process run ok with invalid " "(incomplete) --socket-mem!\n"); - return -1; - } + return -1; + } - if (launch_proc(argv4) == 0) { - printf("Error - process run ok with invalid " + populate_socket_mem_param(num_sockets - 1, "2", ",Fred", + arg4_socket_mem, sizeof(arg4_socket_mem)); + if (launch_proc(argv4) == 0) { + printf("Error - process run ok with invalid " "(mixed with invalid input) --socket-mem!\n"); - return -1; - } + return -1; + } - if (launch_proc(argv5) == 0) { - printf("Error - process run ok with invalid " + populate_socket_mem_param(num_sockets - 1, "2", ",Fred0", + arg5_socket_mem, sizeof(arg5_socket_mem)); + if (launch_proc(argv5) == 0) { + printf("Error - process run ok with invalid " "(mixed with invalid input with a numeric value as " "last character) --socket-mem!\n"); - return -1; + return -1; + } } - if (launch_proc(argv6) == 0) { - printf("Error - process run ok with invalid " + if (num_sockets > 2) { + populate_socket_mem_param(num_sockets - 2, "2", ",,2", + arg6_socket_mem, sizeof(arg6_socket_mem)); + if (launch_proc(argv6) == 0) { + printf("Error - process run ok with invalid " "(with empty socket) --socket-mem!\n"); - return -1; + return -1; + } } if (launch_proc(argv7) == 0) { @@ -1409,16 +1431,22 @@ test_memory_flags(void) return -1; } + populate_socket_mem_param(num_sockets, "2", "", + arg8_socket_mem, sizeof(arg8_socket_mem)); if (launch_proc(argv8) == 0) { printf("Error - process run ok with --socket-mem and -m specified!\n"); return -1; } + populate_socket_mem_param(num_sockets + 1, "2", "", + invalid_socket_mem, sizeof(invalid_socket_mem)); if (launch_proc(argv9) == 0) { printf("Error - process run ok with extra socket in --socket-mem!\n"); return -1; } + populate_socket_mem_param(num_sockets, "2", "", + valid_socket_mem, sizeof(valid_socket_mem)); if (launch_proc(argv10) != 0) { printf("Error - process failed with valid --socket-mem!\n"); return -1; -- 1.8.3.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v4] test: fix --socket-mem option in eal flag autotest 2019-07-29 8:08 ` [dpdk-dev] [PATCH v4] test: " David Marchand @ 2019-07-30 9:24 ` Thomas Monjalon 0 siblings, 0 replies; 14+ messages in thread From: Thomas Monjalon @ 2019-07-30 9:24 UTC (permalink / raw) To: David Marchand, Vamsi Attunuru; +Cc: dev, stable 29/07/2019 10:08, David Marchand: > From: Vamsi Attunuru <vattunuru@marvell.com> > > eal flag autotest fails when multiple mem size flags are passed to > --socket-mem option irrespective of RTE_MAX_NUMA_NODES and the number of > available sockets on the test system. > > Fixes: 45f1b6e8680a ("app: add new tests on eal flags") > Cc: stable@dpdk.org > > Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com> > Signed-off-by: David Marchand <david.marchand@redhat.com> > Reviewed-by: Vamsi Attunuru <vattunuru@marvell.com> > Tested-by: Vamsi Attunuru <vattunuru@marvell.com> Applied, thanks ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-07-30 9:24 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-10 7:25 [dpdk-dev] [PATCH v1 1/1] app/test: fix --socket-mem option in eal flag autotest Vamsi Attunuru 2019-04-10 7:25 ` Vamsi Attunuru 2019-04-10 8:26 ` David Marchand 2019-04-10 8:26 ` David Marchand 2019-04-10 11:43 ` [dpdk-dev] [EXT] " Vamsi Krishna Attunuru 2019-04-10 11:43 ` Vamsi Krishna Attunuru 2019-07-11 5:15 ` [dpdk-dev] [PATCH v2 " vattunuru 2019-07-11 13:56 ` Aaron Conole 2019-07-12 6:43 ` [dpdk-dev] [PATCH v3 " vattunuru 2019-07-23 6:13 ` Vamsi Krishna Attunuru 2019-07-26 13:39 ` David Marchand 2019-07-28 4:19 ` [dpdk-dev] [EXT] " Vamsi Krishna Attunuru 2019-07-29 8:08 ` [dpdk-dev] [PATCH v4] test: " David Marchand 2019-07-30 9:24 ` Thomas Monjalon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).