[v8,03/11] app/test: replace POSIX specific code
Checks
Commit Message
- Include rte_os_shim.h
- Replace sleep and usleep with rte_delay_us_sleep
- #ifndef RTE_EXEC_ENV_WINDOWS for POSIX code only
Signed-off-by: Jie Zhou <jizh@linux.microsoft.com>
---
app/test/packet_burst_generator.c | 1 +
app/test/process.h | 4 +++-
app/test/test.c | 4 ++++
app/test/test_byteorder.c | 2 +-
app/test/test_cmdline.c | 2 ++
app/test/test_crc.c | 1 -
app/test/test_mp_secondary.c | 2 ++
app/test/test_pmd_perf.c | 6 +++++-
app/test/test_ring_stress.c | 3 ++-
app/test/test_ring_stress_impl.h | 2 +-
app/test/test_telemetry_data.c | 2 ++
11 files changed, 23 insertions(+), 6 deletions(-)
Comments
2021-10-25 19:45 (UTC-0700), Jie Zhou:
> - Include rte_os_shim.h
> - Replace sleep and usleep with rte_delay_us_sleep
> - #ifndef RTE_EXEC_ENV_WINDOWS for POSIX code only
>
> Signed-off-by: Jie Zhou <jizh@linux.microsoft.com>
> ---
This patch can be combined with the previous one:
they serve the same purpose---to remove Unix-specific code.
Please try to summarize in the commit message
which parts of the tests suites are excluded, e.g. multi-process.
It is more useful then stating what was changed in the code.
[...]
> diff --git a/app/test/test_cmdline.c b/app/test/test_cmdline.c
> index 115bee966d..9a76bd299f 100644
> --- a/app/test/test_cmdline.c
> +++ b/app/test/test_cmdline.c
> @@ -31,6 +31,7 @@ test_cmdline(void)
> return -1;
> if (test_parse_num_invalid_param() < 0)
> return -1;
> +#ifndef RTE_EXEC_ENV_WINDOWS
> printf("Testing parsing IP addresses...\n");
> if (test_parse_ipaddr_valid() < 0)
> return -1;
> @@ -38,6 +39,7 @@ test_cmdline(void)
> return -1;
> if (test_parse_ipaddr_invalid_param() < 0)
> return -1;
> +#endif
> printf("Testing parsing strings...\n");
> if (test_parse_string_valid() < 0)
> return -1;
What's wrong with parsing IP addresses on Windows?
[...]
> diff --git a/app/test/test_mp_secondary.c b/app/test/test_mp_secondary.c
> index 5b6f05dbb1..da035348bd 100644
> --- a/app/test/test_mp_secondary.c
> +++ b/app/test/test_mp_secondary.c
> @@ -14,7 +14,9 @@
> #include <errno.h>
> #include <string.h>
> #include <unistd.h>
> +#ifndef RTE_EXEC_ENV_WINDOWS
> #include <sys/wait.h>
> +#endif
> #include <libgen.h>
> #include <dirent.h>
> #include <limits.h>
<libgen.h> is absent on Windows for sure, but you don't exclude it.
Does this file even need modification?
It's not going to be compiled for Windows.
[...]
> diff --git a/app/test/test_ring_stress.c b/app/test/test_ring_stress.c
> index 1af45e0fc8..ce3535c6b2 100644
> --- a/app/test/test_ring_stress.c
> +++ b/app/test/test_ring_stress.c
> @@ -43,9 +43,10 @@ test_ring_stress(void)
> n += test_ring_rts_stress.nb_case;
> k += run_test(&test_ring_rts_stress);
>
> +#ifndef RTE_EXEC_ENV_WINDOWS
> n += test_ring_hts_stress.nb_case;
> k += run_test(&test_ring_hts_stress);
> -
> +#endif
Can you please elaborate what is the issue with this case?
It is also one of the details you usually want to put
into the commit message.
On Wed, Nov 24, 2021 at 01:02:06AM +0300, Dmitry Kozlyuk wrote:
> 2021-10-25 19:45 (UTC-0700), Jie Zhou:
> > - Include rte_os_shim.h
> > - Replace sleep and usleep with rte_delay_us_sleep
> > - #ifndef RTE_EXEC_ENV_WINDOWS for POSIX code only
> >
> > Signed-off-by: Jie Zhou <jizh@linux.microsoft.com>
> > ---
>
> This patch can be combined with the previous one:
> they serve the same purpose---to remove Unix-specific code.
>
> Please try to summarize in the commit message
> which parts of the tests suites are excluded, e.g. multi-process.
> It is more useful then stating what was changed in the code.
>
Will combine and revise the message.
> [...]
> > diff --git a/app/test/test_cmdline.c b/app/test/test_cmdline.c
> > index 115bee966d..9a76bd299f 100644
> > --- a/app/test/test_cmdline.c
> > +++ b/app/test/test_cmdline.c
> > @@ -31,6 +31,7 @@ test_cmdline(void)
> > return -1;
> > if (test_parse_num_invalid_param() < 0)
> > return -1;
> > +#ifndef RTE_EXEC_ENV_WINDOWS
> > printf("Testing parsing IP addresses...\n");
> > if (test_parse_ipaddr_valid() < 0)
> > return -1;
> > @@ -38,6 +39,7 @@ test_cmdline(void)
> > return -1;
> > if (test_parse_ipaddr_invalid_param() < 0)
> > return -1;
> > +#endif
> > printf("Testing parsing strings...\n");
> > if (test_parse_string_valid() < 0)
> > return -1;
>
> What's wrong with parsing IP addresses on Windows?
>
test_cmdline_ipaddr.c uses linux netinet/in.h specific u6_addr. Skip these 3 cases for now and prefer a separate patch to make it work on Windows. Or maybe there is already DPDK support on this which I am not aware of? Thanks.
> [...]
> > diff --git a/app/test/test_mp_secondary.c b/app/test/test_mp_secondary.c
> > index 5b6f05dbb1..da035348bd 100644
> > --- a/app/test/test_mp_secondary.c
> > +++ b/app/test/test_mp_secondary.c
> > @@ -14,7 +14,9 @@
> > #include <errno.h>
> > #include <string.h>
> > #include <unistd.h>
> > +#ifndef RTE_EXEC_ENV_WINDOWS
> > #include <sys/wait.h>
> > +#endif
> > #include <libgen.h>
> > #include <dirent.h>
> > #include <limits.h>
>
> <libgen.h> is absent on Windows for sure, but you don't exclude it.
> Does this file even need modification?
> It's not going to be compiled for Windows.
>
This is replaced with the test stub in the patch#11 (of V8) to make it compile on Windows. Sorry for the confusion. Will make sure remove this unnecessary part from V9.
> [...]
> > diff --git a/app/test/test_ring_stress.c b/app/test/test_ring_stress.c
> > index 1af45e0fc8..ce3535c6b2 100644
> > --- a/app/test/test_ring_stress.c
> > +++ b/app/test/test_ring_stress.c
> > @@ -43,9 +43,10 @@ test_ring_stress(void)
> > n += test_ring_rts_stress.nb_case;
> > k += run_test(&test_ring_rts_stress);
> >
> > +#ifndef RTE_EXEC_ENV_WINDOWS
> > n += test_ring_hts_stress.nb_case;
> > k += run_test(&test_ring_hts_stress);
> > -
> > +#endif
>
> Can you please elaborate what is the issue with this case?
> It is also one of the details you usually want to put
> into the commit message.
Cannot remember what caused this case being skipped in the first place, but it can work now. So removed the ifndef. Thanks.
2021-11-30 17:05 (UTC-0800), Jie Zhou:
> On Wed, Nov 24, 2021 at 01:02:06AM +0300, Dmitry Kozlyuk wrote:
[...]
> > [...]
> > > diff --git a/app/test/test_cmdline.c b/app/test/test_cmdline.c
> > > index 115bee966d..9a76bd299f 100644
> > > --- a/app/test/test_cmdline.c
> > > +++ b/app/test/test_cmdline.c
> > > @@ -31,6 +31,7 @@ test_cmdline(void)
> > > return -1;
> > > if (test_parse_num_invalid_param() < 0)
> > > return -1;
> > > +#ifndef RTE_EXEC_ENV_WINDOWS
> > > printf("Testing parsing IP addresses...\n");
> > > if (test_parse_ipaddr_valid() < 0)
> > > return -1;
> > > @@ -38,6 +39,7 @@ test_cmdline(void)
> > > return -1;
> > > if (test_parse_ipaddr_invalid_param() < 0)
> > > return -1;
> > > +#endif
> > > printf("Testing parsing strings...\n");
> > > if (test_parse_string_valid() < 0)
> > > return -1;
> >
> > What's wrong with parsing IP addresses on Windows?
> >
> test_cmdline_ipaddr.c uses linux netinet/in.h specific u6_addr. Skip these
> 3 cases for now and prefer a separate patch to make it work on Windows. Or
> maybe there is already DPDK support on this which I am not aware of? Thanks.
Understood, only please explain this in the commit log.
@@ -5,6 +5,7 @@
#include <rte_byteorder.h>
#include <rte_mbuf.h>
#include <rte_ip.h>
+#include <rte_os_shim.h>
#include "packet_burst_generator.h"
@@ -7,12 +7,14 @@
#include <errno.h> /* errno */
#include <limits.h> /* PATH_MAX */
+#ifndef RTE_EXEC_ENV_WINDOWS
#include <libgen.h> /* basename et al */
+#include <sys/wait.h>
+#endif
#include <stdlib.h> /* NULL */
#include <string.h> /* strerror */
#include <unistd.h> /* readlink */
#include <dirent.h>
-#include <sys/wait.h>
#include <rte_string_fns.h> /* strlcpy */
@@ -62,7 +62,9 @@ do_recursive_call(void)
const char *env_var;
int (*action_fn)(void);
} actions[] = {
+#ifndef RTE_EXEC_ENV_WINDOWS
{ "run_secondary_instances", test_mp_secondary },
+#endif
#ifdef RTE_LIB_PDUMP
#ifdef RTE_NET_RING
{ "run_pdump_server_tests", test_pdump },
@@ -81,7 +83,9 @@ do_recursive_call(void)
{ "test_file_prefix", no_action },
{ "test_no_huge_flag", no_action },
#ifdef RTE_LIB_TIMER
+#ifndef RTE_EXEC_ENV_WINDOWS
{ "timer_secondary_spawn_wait", test_timer_secondary },
+#endif
#endif
};
@@ -46,7 +46,7 @@ test_byteorder(void)
return -1;
res_u16 = rte_bswap16(0x1337);
- printf("const %"PRIx16" -> %"PRIx16"\n", 0x1337, res_u16);
+ printf("const %"PRIx16" -> %"PRIx16"\n", (uint16_t)0x1337, res_u16);
if (res_u16 != 0x3713)
return -1;
@@ -31,6 +31,7 @@ test_cmdline(void)
return -1;
if (test_parse_num_invalid_param() < 0)
return -1;
+#ifndef RTE_EXEC_ENV_WINDOWS
printf("Testing parsing IP addresses...\n");
if (test_parse_ipaddr_valid() < 0)
return -1;
@@ -38,6 +39,7 @@ test_cmdline(void)
return -1;
if (test_parse_ipaddr_invalid_param() < 0)
return -1;
+#endif
printf("Testing parsing strings...\n");
if (test_parse_string_valid() < 0)
return -1;
@@ -14,7 +14,6 @@
#define CRC32_VEC_LEN2 348
#define CRC16_VEC_LEN1 12
#define CRC16_VEC_LEN2 2
-#define LINE_LEN 75
/* CRC test vector */
static const uint8_t crc_vec[CRC_VEC_LEN] = {
@@ -14,7 +14,9 @@
#include <errno.h>
#include <string.h>
#include <unistd.h>
+#ifndef RTE_EXEC_ENV_WINDOWS
#include <sys/wait.h>
+#endif
#include <libgen.h>
#include <dirent.h>
#include <limits.h>
@@ -297,6 +297,7 @@ reset_count(void)
idle = 0;
}
+#ifndef RTE_EXEC_ENV_WINDOWS
static void
stats_display(uint16_t port_id)
{
@@ -326,6 +327,7 @@ signal_handler(int signum)
if (signum == SIGUSR2)
stats_display(0);
}
+#endif
struct rte_mbuf **tx_burst;
@@ -637,7 +639,7 @@ exec_burst(uint32_t flags, int lcore)
i = (i >= conf->nb_ports - 1) ? 0 : (i + 1);
}
- sleep(5);
+ rte_delay_us(5 * US_PER_S);
/* only when polling second */
if (flags == SC_BURST_XMIT_FIRST)
@@ -668,8 +670,10 @@ test_pmd_perf(void)
printf("Start PMD RXTX cycles cost test.\n");
+#ifndef RTE_EXEC_ENV_WINDOWS
signal(SIGUSR1, signal_handler);
signal(SIGUSR2, signal_handler);
+#endif
nb_ports = rte_eth_dev_count_avail();
if (nb_ports < NB_ETHPORTS_USED) {
@@ -43,9 +43,10 @@ test_ring_stress(void)
n += test_ring_rts_stress.nb_case;
k += run_test(&test_ring_rts_stress);
+#ifndef RTE_EXEC_ENV_WINDOWS
n += test_ring_hts_stress.nb_case;
k += run_test(&test_ring_hts_stress);
-
+#endif
n += test_ring_mt_peek_stress.nb_case;
k += run_test(&test_ring_mt_peek_stress);
@@ -360,7 +360,7 @@ test_mt1(int (*test)(void *))
/* signal worker to start test */
__atomic_store_n(&wrk_cmd, WRK_CMD_RUN, __ATOMIC_RELEASE);
- usleep(run_time * US_PER_S);
+ rte_delay_us(run_time * US_PER_S);
/* signal worker to start test */
__atomic_store_n(&wrk_cmd, WRK_CMD_STOP, __ATOMIC_RELEASE);
@@ -4,7 +4,9 @@
#include <string.h>
#include <sys/socket.h>
+#ifndef RTE_EXEC_ENV_WINDOWS
#include <sys/un.h>
+#endif
#include <unistd.h>
#include <limits.h>