Hi Jie,
General comment: try to avoid #ifdef RTE_EXEC_ENV_WINDOWS except for
inherently non-portable Unix code. It clutters the source and complicates
testing: unless you build on Linux you won't know something broke in code
under #ifdef.
Take device hot-plug for example. Since you disable parameter parsing with
#ifdef, hot_plug variable is always 0. You don't need to disable code guarded
by this variable until it only uses DPDK API. You may need to add stubs to
EAL in a separate commit.
2021-03-19 09:51 (UTC-0700), Jie Zhou:
[...]
> diff --git a/app/meson.build b/app/meson.build
> index 87fc195db..00622933e 100644
> --- a/app/meson.build
> +++ b/app/meson.build
> @@ -1,10 +1,6 @@
> # SPDX-License-Identifier: BSD-3-Clause
> # Copyright(c) 2017-2019 Intel Corporation
>
> -if is_windows
> - subdir_done()
> -endif
> -
> apps = [
> 'pdump',
> 'proc-info',
> @@ -21,6 +17,11 @@ apps = [
> 'test-regex',
> 'test-sad']
>
> +if is_windows
> + apps = [
> + 'test-pmd']
> +endif
> +
Can we disable individual apps instead of having two lists,
like Pallavi did in drivers/net directory?
[...]
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 14110eb2e..35a6dd0d3 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -8,12 +8,14 @@
> #include <stdio.h>
> #include <stdint.h>
> #include <string.h>
> +
> +#ifndef RTE_EXEC_ENV_WINDOWS
> #include <termios.h>
> +#endif
This include is not needed at all: cmdline abstracts termios.
> +#ifdef RTE_EXEC_ENV_WINDOWS
> +#ifndef IPDEFTTL
> +#define IPDEFTTL 64
> +#endif
> +#endif
> +
[...]
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 576d5acab..af7570429 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -38,7 +38,6 @@
> #include <rte_string_fns.h>
> #include <rte_cycles.h>
> #include <rte_flow.h>
> -#include <rte_errno.h>
> #ifdef RTE_NET_IXGBE
> #include <rte_pmd_ixgbe.h>
> #endif
> @@ -170,6 +169,27 @@ print_ethaddr(const char *name, struct rte_ether_addr
> *eth_addr)
> printf("%s%s", name, buf);
> }
>
> +#ifdef RTE_EXEC_ENV_WINDOWS
> +static int
> +clock_gettime_monotonic(struct timespec *tp)
> +{
> + LARGE_INTEGER pf, pc;
> + LONGLONG nsec;
> +
> + if (QueryPerformanceFrequency(&pf) == 0)
> + return -1;
> +
> + if (QueryPerformanceCounter(&pc) == 0)
> + return -1;
> +
> + nsec = pc.QuadPart * NS_PER_SEC / pf.QuadPart;
> + tp->tv_sec = nsec / NS_PER_SEC;
> + tp->tv_nsec = nsec - tp->tv_sec * NS_PER_SEC;
> +
> + return 0;
> +}
> +#endif
> +
> void
> nic_stats_display(portid_t port_id)
> {
> @@ -182,6 +202,7 @@ nic_stats_display(portid_t port_id)
> uint64_t diff_pkts_rx, diff_pkts_tx, diff_bytes_rx, diff_bytes_tx,
> diff_ns;
> uint64_t mpps_rx, mpps_tx, mbps_rx, mbps_tx;
> + int ret;
> struct rte_eth_stats stats;
>
> static const char *nic_stats_border = "########################";
> @@ -202,7 +223,13 @@ nic_stats_display(portid_t port_id)
> "%-"PRIu64"\n", stats.opackets, stats.oerrors, stats.obytes);
>
> diff_ns = 0;
> - if (clock_gettime(CLOCK_TYPE_ID, &cur_time) == 0) {
> +
> +#ifdef RTE_EXEC_ENV_WINDOWS
> + ret = clock_gettime_monotonic(&cur_time);
> +#else
> + ret = clock_gettime(CLOCK_TYPE_ID, &cur_time);
> +#endif
> + if (ret == 0) {
I suggest to use a single wrapped defined using #ifdef to avoid an #ifdef
each time it is called (although it's the only occurrence for now).
[...]
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index 6b4df335f..0fb03b9f9 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -696,7 +696,11 @@ pkt_copy_split(const struct rte_mbuf *pkt)
> mp = current_fwd_lcore()->mbp;
>
> if (tx_pkt_split == TX_PKT_SPLIT_RND)
> +#ifndef RTE_EXEC_ENV_WINDOWS
> nb_seg = random() % tx_pkt_nb_segs + 1;
> +#else
> + nb_seg = rand() % tx_pkt_nb_segs + 1;
> +#endif
Can rte_rand() be exported from EAL and used here?
[...]
>
> +#ifndef RTE_EXEC_ENV_WINDOWS
> ret = rte_eal_cleanup();
> if (ret != 0)
> rte_exit(EXIT_FAILURE,
> "EAL cleanup failed: %s\n", strerror(-ret));
> -
> +#endif
> return EXIT_SUCCESS;
> }
rte_eal_cleanup() is fixed, this can be removed.
