[dpdk-dev] [PATCH 2/3] net/i40e: add changes to support i40e PMD on windows
Dmitry Kozlyuk
dmitry.kozliuk at gmail.com
Sat Dec 5 14:52:04 CET 2020
On Fri, 4 Dec 2020 17:10:19 -0800, Pallavi Kadam wrote:
You could drop "add changes" and "i40e PMD" from subject line, as any commit
changes something and topic is "net/i40e" already.
> Adding build changes to compile i40e PMD on windows.
This is redundant given the commit subject.
Please use present simple tense for changes description (this applies to
sibling patches).
> Disabling few warnings with Clang such as comparison of integers of
> different signs and macro redefinitions.
> Also, adding linking dependency source file rte_random.c file to
> Windows.
>
> Signed-off-by: Pallavi Kadam <pallavi.kadam at intel.com>
> Reviewed-by: Ranjit Menon <ranjit.menon at intel.com>
> ---
> drivers/net/i40e/base/i40e_osdep.h | 3 +++
> drivers/net/i40e/i40e_ethdev_vf.c | 3 ++-
> drivers/net/i40e/i40e_rxtx_vec_avx2.c | 2 ++
> drivers/net/i40e/i40e_tm.c | 2 +-
> drivers/net/meson.build | 9 ++++++---
> lib/librte_eal/common/meson.build | 1 +
> lib/librte_eal/rte_eal_exports.def | 1 +
> lib/librte_eal/windows/include/rte_windows.h | 5 +++++
> 8 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/i40e/base/i40e_osdep.h b/drivers/net/i40e/base/i40e_osdep.h
> index 9b5033024..fa22df122 100644
> --- a/drivers/net/i40e/base/i40e_osdep.h
> +++ b/drivers/net/i40e/base/i40e_osdep.h
> @@ -67,8 +67,11 @@ typedef enum i40e_status_code i40e_status;
> #define false 0
> #define true 1
>
> +/* Avoid macro redifinition warning on Windows */
> +#ifndef RTE_EXEC_ENV_WINDOWS
> #define min(a,b) RTE_MIN(a,b)
> #define max(a,b) RTE_MAX(a,b)
> +#endif
Windows min() and max() macros evaluate arguments twice [1], which can be
unacceptable in driver code if used with MMIO. Better #undef min and max
first, then let PMD define them.
It seems we'll have to do that for many PMDs, because rte_os.h must not erase
platform-specific macros, and rte_windows.h is not for generic PMDs.
[1]: https://docs.microsoft.com/en-us/windows/win32/multimedia/max
> diff --git a/drivers/net/i40e/i40e_rxtx_vec_avx2.c b/drivers/net/i40e/i40e_rxtx_vec_avx2.c
> index 7a558fc73..cf2dc88c5 100644
> --- a/drivers/net/i40e/i40e_rxtx_vec_avx2.c
> +++ b/drivers/net/i40e/i40e_rxtx_vec_avx2.c
> @@ -12,7 +12,9 @@
> #include "i40e_rxtx.h"
> #include "i40e_rxtx_vec_common.h"
>
> +#ifndef RTE_EXEC_ENV_WINDOWS
> #include <x86intrin.h>
> +#endif
Just #include <rte_vect.h>, it takes care of x86intrin.h for Windows.
> diff --git a/lib/librte_eal/windows/include/rte_windows.h b/lib/librte_eal/windows/include/rte_windows.h
> index b82af34f6..822922c11 100644
> --- a/lib/librte_eal/windows/include/rte_windows.h
> +++ b/lib/librte_eal/windows/include/rte_windows.h
> @@ -18,6 +18,11 @@
> #define WIN32_LEAN_AND_MEAN
> #endif
>
> +#ifdef __clang__
> +#undef _m_prefetchw
> +#define _m_prefetchw __m_prefetchw
> +#endif
> +
Please explain in the commit message which problem this solves.
Can't x86intrin.h be replaced by rte_vect.h in rte_random.c?
If it's still necessary, __clang__ should be RTE_TOOLCHAIN_CLANG.
More information about the dev
mailing list