DPDK patches and discussions
 help / color / mirror / Atom feed
From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
To: Pallavi Kadam <pallavi.kadam@intel.com>
Cc: dev@dpdk.org, thomas@monjalon.net, ranjit.menon@intel.com,
	Narcisa.Vasile@microsoft.com, talshn@nvidia.com,
	ferruh.yigit@intel.com, beilei.xing@intel.com, jia.guo@intel.com
Subject: Re: [dpdk-dev] [PATCH 2/3] net/i40e: add changes to support i40e PMD on windows
Date: Sat, 5 Dec 2020 16:52:04 +0300
Message-ID: <20201205165204.10e30d17@sovereign> (raw)
In-Reply-To: <20201205011020.6276-3-pallavi.kadam@intel.com>

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@intel.com>
> Reviewed-by: Ranjit Menon <ranjit.menon@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.

  reply	other threads:[~2020-12-05 13:52 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-05  1:10 [dpdk-dev] [PATCH 0/3] Support i40e PMD on Windows Pallavi Kadam
2020-12-05  1:10 ` [dpdk-dev] [PATCH 1/3] eal/windows: add some interrupt functions stub Pallavi Kadam
2020-12-05  1:10 ` [dpdk-dev] [PATCH 2/3] net/i40e: add changes to support i40e PMD on windows Pallavi Kadam
2020-12-05 13:52   ` Dmitry Kozlyuk [this message]
2020-12-09  0:09     ` Kadam, Pallavi
2020-12-06 15:49   ` Thomas Monjalon
2020-12-07  9:14     ` Bruce Richardson
2020-12-09  0:14       ` Kadam, Pallavi
2020-12-09  0:21     ` Kadam, Pallavi
2020-12-09  8:59       ` Thomas Monjalon
2020-12-17 23:17         ` Kadam, Pallavi
2020-12-05  1:10 ` [dpdk-dev] [PATCH 3/3] config/build: ignore enum forward reference warning Pallavi Kadam
2020-12-05 13:51   ` Dmitry Kozlyuk
2020-12-10  0:53   ` Narcisa Ana Maria Vasile
2020-12-17 23:15     ` Kadam, Pallavi
2020-12-17 22:59 ` [dpdk-dev] [PATCH v2 0/2] Support i40e PMD on Windows Pallavi Kadam
2020-12-17 22:59   ` [dpdk-dev] [PATCH v2 1/2] eal: add rte_random.c file on windows Pallavi Kadam
2020-12-18 11:29     ` Thomas Monjalon
2020-12-18 18:49       ` Kadam, Pallavi
2020-12-17 22:59   ` [dpdk-dev] [PATCH v2 2/2] build: i40e PMD on Windows Pallavi Kadam
2020-12-18 11:27     ` Thomas Monjalon
2020-12-18 18:42       ` Kadam, Pallavi
2020-12-22  0:45   ` [dpdk-dev] [PATCH v3 0/2] Support " Pallavi Kadam
2020-12-22  0:45     ` [dpdk-dev] [PATCH v3 1/2] eal: add rte_random.c file on windows Pallavi Kadam
2020-12-22  0:45     ` [dpdk-dev] [PATCH v3 2/2] build: i40e PMD on Windows Pallavi Kadam
2021-01-08 22:58       ` Jie Zhou
2021-01-13 22:52       ` Thomas Monjalon
2021-01-13 22:55         ` Ranjit Menon
2021-01-13 23:10           ` Thomas Monjalon
2021-01-14  1:51             ` Tal Shnaiderman
2021-01-14 22:55     ` [dpdk-dev] [PATCH v3 0/2] Support " Thomas Monjalon
2021-01-14 23:06       ` Ranjit Menon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201205165204.10e30d17@sovereign \
    --to=dmitry.kozliuk@gmail.com \
    --cc=Narcisa.Vasile@microsoft.com \
    --cc=beilei.xing@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=jia.guo@intel.com \
    --cc=pallavi.kadam@intel.com \
    --cc=ranjit.menon@intel.com \
    --cc=talshn@nvidia.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git