DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] eal/windows: definition for ETOOMANYREFS errno
@ 2020-11-14 21:11 Tal Shnaiderman
  2020-11-14 22:01 ` Dmitry Kozlyuk
  2020-11-14 22:21 ` [dpdk-dev] [PATCH v2] " Tal Shnaiderman
  0 siblings, 2 replies; 14+ messages in thread
From: Tal Shnaiderman @ 2020-11-14 21:11 UTC (permalink / raw)
  To: dev; +Cc: thomas, dmitry.kozliuk, navasile, dmitrym, pallavi.kadam

The ETOOMANYREFS errno is missing from the Windows clang build
is it used in initialization of flow error structures.

The commit will define it as it is done in the minGW Windows build.

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
---
 lib/librte_eal/windows/include/rte_os.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_eal/windows/include/rte_os.h b/lib/librte_eal/windows/include/rte_os.h
index 569ed92d51..2a91ebf6a1 100644
--- a/lib/librte_eal/windows/include/rte_os.h
+++ b/lib/librte_eal/windows/include/rte_os.h
@@ -90,6 +90,7 @@ eal_strerror(int code)
 }
 
 #define strerror eal_strerror
+#define ETOOMANYREFS WSAETOOMANYREFS
 
 #endif /* RTE_TOOLCHAIN_GCC */
 
-- 
2.16.1.windows.4


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH] eal/windows: definition for ETOOMANYREFS errno
  2020-11-14 21:11 [dpdk-dev] [PATCH] eal/windows: definition for ETOOMANYREFS errno Tal Shnaiderman
@ 2020-11-14 22:01 ` Dmitry Kozlyuk
  2020-11-14 22:11   ` Tal Shnaiderman
  2020-11-14 22:21 ` [dpdk-dev] [PATCH v2] " Tal Shnaiderman
  1 sibling, 1 reply; 14+ messages in thread
From: Dmitry Kozlyuk @ 2020-11-14 22:01 UTC (permalink / raw)
  To: Tal Shnaiderman; +Cc: dev, thomas, navasile, dmitrym, pallavi.kadam

On Sat, 14 Nov 2020 23:11:56 +0200, Tal Shnaiderman wrote:
> The ETOOMANYREFS errno is missing from the Windows clang build
> is it used in initialization of flow error structures.

"is it" -> "it is"
 
> The commit will define it as it is done in the minGW Windows build.

"The commit will" is unnecessary.

"minGW" -> "MinGW"
 
> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> ---
>  lib/librte_eal/windows/include/rte_os.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_eal/windows/include/rte_os.h b/lib/librte_eal/windows/include/rte_os.h
> index 569ed92d51..2a91ebf6a1 100644
> --- a/lib/librte_eal/windows/include/rte_os.h
> +++ b/lib/librte_eal/windows/include/rte_os.h
> @@ -90,6 +90,7 @@ eal_strerror(int code)
>  }
>  
>  #define strerror eal_strerror
> +#define ETOOMANYREFS WSAETOOMANYREFS
>  
>  #endif /* RTE_TOOLCHAIN_GCC */

Should be #define ETOOMANYREFS 10059 /* WSAETOOMANYREFS */ for all toolchains:

1. Users of librte_ethdev, who check for ETOOMANYREFS, may not wish to include
<winsock2.h> because of its defines that break librte_net headers.

2. Now that I looked closely, MinGW-w64's #define ETOOMANYREFS
WSAETOOMANYREFS is under #if 0 clause (for documentation?). Apologies for
earlier misinformation.


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH] eal/windows: definition for ETOOMANYREFS errno
  2020-11-14 22:01 ` Dmitry Kozlyuk
@ 2020-11-14 22:11   ` Tal Shnaiderman
  2020-11-15 10:51     ` Thomas Monjalon
  0 siblings, 1 reply; 14+ messages in thread
From: Tal Shnaiderman @ 2020-11-14 22:11 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, NBU-Contact-Thomas Monjalon, navasile, dmitrym, pallavi.kadam

> Subject: Re: [PATCH] eal/windows: definition for ETOOMANYREFS errno
> 
> External email: Use caution opening links or attachments
> 
> 
> On Sat, 14 Nov 2020 23:11:56 +0200, Tal Shnaiderman wrote:
> > The ETOOMANYREFS errno is missing from the Windows clang build is it
> > used in initialization of flow error structures.
> 
> "is it" -> "it is"
> 
> > The commit will define it as it is done in the minGW Windows build.
> 
> "The commit will" is unnecessary.
> 
> "minGW" -> "MinGW"
> 
> > Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> > ---
> >  lib/librte_eal/windows/include/rte_os.h | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/librte_eal/windows/include/rte_os.h
> > b/lib/librte_eal/windows/include/rte_os.h
> > index 569ed92d51..2a91ebf6a1 100644
> > --- a/lib/librte_eal/windows/include/rte_os.h
> > +++ b/lib/librte_eal/windows/include/rte_os.h
> > @@ -90,6 +90,7 @@ eal_strerror(int code)  }
> >
> >  #define strerror eal_strerror
> > +#define ETOOMANYREFS WSAETOOMANYREFS
> >
> >  #endif /* RTE_TOOLCHAIN_GCC */
> 
> Should be #define ETOOMANYREFS 10059 /* WSAETOOMANYREFS */ for all
> toolchains:
> 
> 1. Users of librte_ethdev, who check for ETOOMANYREFS, may not wish to
> include <winsock2.h> because of its defines that break librte_net headers.
> 
> 2. Now that I looked closely, MinGW-w64's #define ETOOMANYREFS
> WSAETOOMANYREFS is under #if 0 clause (for documentation?). Apologies
> for earlier misinformation.

Thank you for the comments Dmitry, will send a v2 promptly.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] [PATCH v2] eal/windows: definition for ETOOMANYREFS errno
  2020-11-14 21:11 [dpdk-dev] [PATCH] eal/windows: definition for ETOOMANYREFS errno Tal Shnaiderman
  2020-11-14 22:01 ` Dmitry Kozlyuk
@ 2020-11-14 22:21 ` Tal Shnaiderman
  2020-11-14 23:13   ` Dmitry Kozlyuk
  1 sibling, 1 reply; 14+ messages in thread
From: Tal Shnaiderman @ 2020-11-14 22:21 UTC (permalink / raw)
  To: dev; +Cc: thomas, dmitry.kozliuk, navasile, dmitrym, pallavi.kadam

The ETOOMANYREFS errno is missing from the Windows build.
it is used in initialization of flow error structures.

The commit will define it with the same error code used by
WSAETOOMANYREFS.

Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>

---
v2: log message fix, apply errno to both Windows builds
and remove dependency on winsock2.h [DmitryK]
---
---
 lib/librte_eal/windows/include/rte_os.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_eal/windows/include/rte_os.h b/lib/librte_eal/windows/include/rte_os.h
index 569ed92d51..8300ea483a 100644
--- a/lib/librte_eal/windows/include/rte_os.h
+++ b/lib/librte_eal/windows/include/rte_os.h
@@ -51,6 +51,8 @@ extern "C" {
 /* as in <windows.h> */
 typedef long long ssize_t;
 
+#define ETOOMANYREFS 10059 /* WSAETOOMANYREFS */
+
 #ifndef RTE_TOOLCHAIN_GCC
 
 static inline int
-- 
2.16.1.windows.4


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal/windows: definition for ETOOMANYREFS errno
  2020-11-14 22:21 ` [dpdk-dev] [PATCH v2] " Tal Shnaiderman
@ 2020-11-14 23:13   ` Dmitry Kozlyuk
  2020-11-15 23:10     ` Thomas Monjalon
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Kozlyuk @ 2020-11-14 23:13 UTC (permalink / raw)
  To: Tal Shnaiderman; +Cc: dev, thomas, navasile, dmitrym, pallavi.kadam

On Sun, 15 Nov 2020 00:21:29 +0200, Tal Shnaiderman wrote:
> The ETOOMANYREFS errno is missing from the Windows build.
> it is used in initialization of flow error structures.
> 
> The commit will define it with the same error code used by
> WSAETOOMANYREFS.
> 
> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> 
> ---
> v2: log message fix, apply errno to both Windows builds
> and remove dependency on winsock2.h [DmitryK]

Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH] eal/windows: definition for ETOOMANYREFS errno
  2020-11-14 22:11   ` Tal Shnaiderman
@ 2020-11-15 10:51     ` Thomas Monjalon
  2020-11-15 14:23       ` Tal Shnaiderman
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2020-11-15 10:51 UTC (permalink / raw)
  To: Tal Shnaiderman; +Cc: Dmitry Kozlyuk, dev, navasile, dmitrym, pallavi.kadam

14/11/2020 23:11, Tal Shnaiderman:
> > On Sat, 14 Nov 2020 23:11:56 +0200, Tal Shnaiderman wrote:
> > > The ETOOMANYREFS errno is missing from the Windows clang build is it
> > > used in initialization of flow error structures.
> > 
> > "is it" -> "it is"
> > 
> > > The commit will define it as it is done in the minGW Windows build.
> > 
> > "The commit will" is unnecessary.
> > 
> > "minGW" -> "MinGW"

[...]
> Thank you for the comments Dmitry, will send a v2 promptly.

Just a detail, I see you missed this comment in v2:
	"The commit will" is unnecessary.




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH] eal/windows: definition for ETOOMANYREFS errno
  2020-11-15 10:51     ` Thomas Monjalon
@ 2020-11-15 14:23       ` Tal Shnaiderman
  0 siblings, 0 replies; 14+ messages in thread
From: Tal Shnaiderman @ 2020-11-15 14:23 UTC (permalink / raw)
  To: NBU-Contact-Thomas Monjalon
  Cc: Dmitry Kozlyuk, dev, navasile, dmitrym, pallavi.kadam

> Subject: Re: [PATCH] eal/windows: definition for ETOOMANYREFS errno
> 
> External email: Use caution opening links or attachments
> 
> 
> 14/11/2020 23:11, Tal Shnaiderman:
> > > On Sat, 14 Nov 2020 23:11:56 +0200, Tal Shnaiderman wrote:
> > > > The ETOOMANYREFS errno is missing from the Windows clang build is
> > > > it used in initialization of flow error structures.
> > >
> > > "is it" -> "it is"
> > >
> > > > The commit will define it as it is done in the minGW Windows build.
> > >
> > > "The commit will" is unnecessary.
> > >
> > > "minGW" -> "MinGW"
> 
> [...]
> > Thank you for the comments Dmitry, will send a v2 promptly.
> 
> Just a detail, I see you missed this comment in v2:
>         "The commit will" is unnecessary.
> 

Yes I missed it, Thanks for noticing Thomas.

> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] [PATCH v2] eal/windows: definition for ETOOMANYREFS errno
  2020-11-14 23:13   ` Dmitry Kozlyuk
@ 2020-11-15 23:10     ` Thomas Monjalon
  2020-11-17 10:51       ` [dpdk-dev] Windows: A fundamental issue (was eal/windows: definition for ETOOMANYREFS errno) Nick Connolly
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2020-11-15 23:10 UTC (permalink / raw)
  To: Tal Shnaiderman; +Cc: dev, navasile, dmitrym, pallavi.kadam, Dmitry Kozlyuk

15/11/2020 00:13, Dmitry Kozlyuk:
> On Sun, 15 Nov 2020 00:21:29 +0200, Tal Shnaiderman wrote:
> > The ETOOMANYREFS errno is missing from the Windows build.
> > it is used in initialization of flow error structures.
> > 
> > The commit will define it with the same error code used by
> > WSAETOOMANYREFS.
> > 
> > Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
> > 
> > ---
> > v2: log message fix, apply errno to both Windows builds
> > and remove dependency on winsock2.h [DmitryK]
> 
> Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

Applied, thanks



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [dpdk-dev] Windows: A fundamental issue (was eal/windows: definition for ETOOMANYREFS errno)
  2020-11-15 23:10     ` Thomas Monjalon
@ 2020-11-17 10:51       ` Nick Connolly
  2020-11-17 12:53         ` Dmitry Kozlyuk
  0 siblings, 1 reply; 14+ messages in thread
From: Nick Connolly @ 2020-11-17 10:51 UTC (permalink / raw)
  To: Thomas Monjalon, Tal Shnaiderman
  Cc: dev, navasile, dmitrym, pallavi.kadam, Dmitry Kozlyuk

Unfortunately, this change has broken the build for SPDK on Windows.

To use the DPDK libraries, an application needs to include the rte_* 
header, which includes rte_os.h via rte_common.h.  The definition of 
ETOOMANYREFS clashes with the value used when building the SPDK on Windows.

The fundamental issue here is how to support missing POSIX/Linux 
functionality.  If I understand correctly, the EAL should be responsible 
for providing all such functionality.  The support should be private to 
the EAL and only exported through rte_* definitions.

This means that rte_os.h should not include POSIX/Linux definitions to 
avoid clashes such as the one seen with this change.  It's clearly not 
sustainable if applications have to be modified every time we add more 
Windows support to the DPDK.

Note that this is not an isolated issue - most of the definitions in 
rte_os.h (redefining close, unlink, strdup etc) should not be present if 
other layers (application, other libraries, etc) are to be able to 
implement their own POSIX/Linux support.

Please can we back this change out until we have a strategy that allows 
us to make these definitions available for 'internal' use, but prevent 
them being visible outside of the DPDK tree.  If we can't wrap them with 
rte_* yet, perhaps the short term solution could be as simple as setting 
RTE_DEFINE_POSIX when building DPDK code and hiding them if it is not set?

Apologies that I didn't spot the issue earlier.

Thanks,
Nick


On 15/11/2020 23:10, Thomas Monjalon wrote:
> 15/11/2020 00:13, Dmitry Kozlyuk:
>> On Sun, 15 Nov 2020 00:21:29 +0200, Tal Shnaiderman wrote:
>>> The ETOOMANYREFS errno is missing from the Windows build.
>>> it is used in initialization of flow error structures.
>>>
>>> The commit will define it with the same error code used by
>>> WSAETOOMANYREFS.
>>>
>>> Signed-off-by: Tal Shnaiderman <talshn@nvidia.com>
>>>
>>> ---
>>> v2: log message fix, apply errno to both Windows builds
>>> and remove dependency on winsock2.h [DmitryK]
>> Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Applied, thanks
>
>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] Windows: A fundamental issue (was eal/windows: definition for ETOOMANYREFS errno)
  2020-11-17 10:51       ` [dpdk-dev] Windows: A fundamental issue (was eal/windows: definition for ETOOMANYREFS errno) Nick Connolly
@ 2020-11-17 12:53         ` Dmitry Kozlyuk
  2020-11-19 13:21           ` Tal Shnaiderman
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Kozlyuk @ 2020-11-17 12:53 UTC (permalink / raw)
  To: Nick Connolly
  Cc: Thomas Monjalon, Tal Shnaiderman, dev, navasile, dmitrym, pallavi.kadam

Hi Nick,

> This means that rte_os.h should not include POSIX/Linux definitions to 
> avoid clashes such as the one seen with this change.  It's clearly not 
> sustainable if applications have to be modified every time we add more 
> Windows support to the DPDK.
> 
> Note that this is not an isolated issue - most of the definitions in 
> rte_os.h (redefining close, unlink, strdup etc) should not be present if 
> other layers (application, other libraries, etc) are to be able to 
> implement their own POSIX/Linux support.

The purpose of rte_os.h must be clarified. It now says:

/**
 * This is header should contain any function/macro definition
 * which are not supported natively or named differently in the
 * ... OS. Functions will be added in future releases.
 */

This doesn't specify if the file should expose wrappers or POSIX-named
bits. Linux and FreeBSD, however, only use it for RTE_CPU_xxx() macros for
CPU_xxx() and don't define anything with POSIX names. So should Windows.

> Please can we back this change out until we have a strategy that allows 
> us to make these definitions available for 'internal' use, but prevent 
> them being visible outside of the DPDK tree.  If we can't wrap them with 
> rte_* yet, perhaps the short term solution could be as simple as setting 
> RTE_DEFINE_POSIX when building DPDK code and hiding them if it is not set?

You need the same value both inside DPDK to return it and outside of DPDK to
match on it. Returning an unnamed, unspecified code is not an option. RTE_
prefix is a way to go. We can just rename ETOOMANYREFS.

Strictly speaking, C standard defines very few errno, so using POSIX values
in API is incorrect anyway. It has to be deprecated and removed eventually,
we already had issues with MMAP_FAILED.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] Windows: A fundamental issue (was eal/windows: definition for ETOOMANYREFS errno)
  2020-11-17 12:53         ` Dmitry Kozlyuk
@ 2020-11-19 13:21           ` Tal Shnaiderman
  2020-11-19 14:46             ` Thomas Monjalon
  0 siblings, 1 reply; 14+ messages in thread
From: Tal Shnaiderman @ 2020-11-19 13:21 UTC (permalink / raw)
  To: Dmitry Kozlyuk, Nick Connolly
  Cc: NBU-Contact-Thomas Monjalon, dev, navasile, dmitrym, pallavi.kadam

> Subject: Re: Windows: A fundamental issue (was eal/windows: definition for
> ETOOMANYREFS errno)
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Nick,
> 
> > This means that rte_os.h should not include POSIX/Linux definitions to
> > avoid clashes such as the one seen with this change.  It's clearly not
> > sustainable if applications have to be modified every time we add more
> > Windows support to the DPDK.
> >
> > Note that this is not an isolated issue - most of the definitions in
> > rte_os.h (redefining close, unlink, strdup etc) should not be present
> > if other layers (application, other libraries, etc) are to be able to
> > implement their own POSIX/Linux support.
> 
> The purpose of rte_os.h must be clarified. It now says:
> 
> /**
>  * This is header should contain any function/macro definition
>  * which are not supported natively or named differently in the
>  * ... OS. Functions will be added in future releases.
>  */
> 
> This doesn't specify if the file should expose wrappers or POSIX-named bits.
> Linux and FreeBSD, however, only use it for RTE_CPU_xxx() macros for
> CPU_xxx() and don't define anything with POSIX names. So should Windows.
> 
> > Please can we back this change out until we have a strategy that
> > allows us to make these definitions available for 'internal' use, but
> > prevent them being visible outside of the DPDK tree.  If we can't wrap
> > them with
> > rte_* yet, perhaps the short term solution could be as simple as
> > setting RTE_DEFINE_POSIX when building DPDK code and hiding them if it is
> not set?
> 
> You need the same value both inside DPDK to return it and outside of DPDK
> to match on it. Returning an unnamed, unspecified code is not an option.
> RTE_ prefix is a way to go. We can just rename ETOOMANYREFS.

Thanks for the info Nick.
Dmitry, If we go with RTE_ETOOMANYREFS, I assume we need to define it for Linux and FreeBSD as well?

> 
> Strictly speaking, C standard defines very few errno, so using POSIX values in
> API is incorrect anyway. It has to be deprecated and removed eventually, we
> already had issues with MMAP_FAILED.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] Windows: A fundamental issue (was eal/windows: definition for ETOOMANYREFS errno)
  2020-11-19 13:21           ` Tal Shnaiderman
@ 2020-11-19 14:46             ` Thomas Monjalon
  2020-11-19 15:27               ` Tal Shnaiderman
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Monjalon @ 2020-11-19 14:46 UTC (permalink / raw)
  To: Dmitry Kozlyuk, Nick Connolly, Tal Shnaiderman
  Cc: dev, navasile, dmitrym, pallavi.kadam, Andrey Vesnovaty, asafp

19/11/2020 14:21, Tal Shnaiderman:
> > Subject: Re: Windows: A fundamental issue (was eal/windows: definition for
> > ETOOMANYREFS errno)
> > 
> > External email: Use caution opening links or attachments
> > 
> > 
> > Hi Nick,
> > 
> > > This means that rte_os.h should not include POSIX/Linux definitions to
> > > avoid clashes such as the one seen with this change.  It's clearly not
> > > sustainable if applications have to be modified every time we add more
> > > Windows support to the DPDK.
> > >
> > > Note that this is not an isolated issue - most of the definitions in
> > > rte_os.h (redefining close, unlink, strdup etc) should not be present
> > > if other layers (application, other libraries, etc) are to be able to
> > > implement their own POSIX/Linux support.
> > 
> > The purpose of rte_os.h must be clarified. It now says:
> > 
> > /**
> >  * This is header should contain any function/macro definition
> >  * which are not supported natively or named differently in the
> >  * ... OS. Functions will be added in future releases.
> >  */
> > 
> > This doesn't specify if the file should expose wrappers or POSIX-named bits.
> > Linux and FreeBSD, however, only use it for RTE_CPU_xxx() macros for
> > CPU_xxx() and don't define anything with POSIX names. So should Windows.
> > 
> > > Please can we back this change out until we have a strategy that
> > > allows us to make these definitions available for 'internal' use, but
> > > prevent them being visible outside of the DPDK tree.  If we can't wrap
> > > them with
> > > rte_* yet, perhaps the short term solution could be as simple as
> > > setting RTE_DEFINE_POSIX when building DPDK code and hiding them if it is
> > not set?
> > 
> > You need the same value both inside DPDK to return it and outside of DPDK
> > to match on it. Returning an unnamed, unspecified code is not an option.
> > RTE_ prefix is a way to go. We can just rename ETOOMANYREFS.
> 
> Thanks for the info Nick.
> Dmitry, If we go with RTE_ETOOMANYREFS, I assume we need to define it for Linux and FreeBSD as well?

Or we can use a "more standard" error code?


> > Strictly speaking, C standard defines very few errno, so using POSIX values in
> > API is incorrect anyway. It has to be deprecated and removed eventually, we
> > already had issues with MMAP_FAILED.




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] Windows: A fundamental issue (was eal/windows: definition for ETOOMANYREFS errno)
  2020-11-19 14:46             ` Thomas Monjalon
@ 2020-11-19 15:27               ` Tal Shnaiderman
  2020-11-19 15:38                 ` Nick Connolly
  0 siblings, 1 reply; 14+ messages in thread
From: Tal Shnaiderman @ 2020-11-19 15:27 UTC (permalink / raw)
  To: NBU-Contact-Thomas Monjalon, Dmitry Kozlyuk, Nick Connolly
  Cc: dev, navasile, dmitrym, pallavi.kadam, Andrey Vesnovaty, Asaf Penso

> Subject: Re: Windows: A fundamental issue (was eal/windows: definition for
> ETOOMANYREFS errno)
> 
> External email: Use caution opening links or attachments
> 
> 
> 19/11/2020 14:21, Tal Shnaiderman:
> > > Subject: Re: Windows: A fundamental issue (was eal/windows:
> > > definition for ETOOMANYREFS errno)
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > Hi Nick,
> > >
> > > > This means that rte_os.h should not include POSIX/Linux
> > > > definitions to avoid clashes such as the one seen with this
> > > > change.  It's clearly not sustainable if applications have to be
> > > > modified every time we add more Windows support to the DPDK.
> > > >
> > > > Note that this is not an isolated issue - most of the definitions
> > > > in rte_os.h (redefining close, unlink, strdup etc) should not be
> > > > present if other layers (application, other libraries, etc) are to
> > > > be able to implement their own POSIX/Linux support.
> > >
> > > The purpose of rte_os.h must be clarified. It now says:
> > >
> > > /**
> > >  * This is header should contain any function/macro definition
> > >  * which are not supported natively or named differently in the
> > >  * ... OS. Functions will be added in future releases.
> > >  */
> > >
> > > This doesn't specify if the file should expose wrappers or POSIX-named
> bits.
> > > Linux and FreeBSD, however, only use it for RTE_CPU_xxx() macros for
> > > CPU_xxx() and don't define anything with POSIX names. So should
> Windows.
> > >
> > > > Please can we back this change out until we have a strategy that
> > > > allows us to make these definitions available for 'internal' use,
> > > > but prevent them being visible outside of the DPDK tree.  If we
> > > > can't wrap them with
> > > > rte_* yet, perhaps the short term solution could be as simple as
> > > > setting RTE_DEFINE_POSIX when building DPDK code and hiding them
> > > > if it is
> > > not set?
> > >
> > > You need the same value both inside DPDK to return it and outside of
> > > DPDK to match on it. Returning an unnamed, unspecified code is not an
> option.
> > > RTE_ prefix is a way to go. We can just rename ETOOMANYREFS.
> >
> > Thanks for the info Nick.
> > Dmitry, If we go with RTE_ETOOMANYREFS, I assume we need to define it
> for Linux and FreeBSD as well?
> 
> Or we can use a "more standard" error code?
> 

Right, Since it is used rarely and only in our PMD I'll work with the developer on selecting a different errno and will revert this commit, apologies for the inconvenience.

> 
> > > Strictly speaking, C standard defines very few errno, so using POSIX
> > > values in API is incorrect anyway. It has to be deprecated and
> > > removed eventually, we already had issues with MMAP_FAILED.
> 
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [dpdk-dev] Windows: A fundamental issue (was eal/windows: definition for ETOOMANYREFS errno)
  2020-11-19 15:27               ` Tal Shnaiderman
@ 2020-11-19 15:38                 ` Nick Connolly
  0 siblings, 0 replies; 14+ messages in thread
From: Nick Connolly @ 2020-11-19 15:38 UTC (permalink / raw)
  To: Tal Shnaiderman, NBU-Contact-Thomas Monjalon, Dmitry Kozlyuk
  Cc: dev, navasile, dmitrym, pallavi.kadam, Andrey Vesnovaty, Asaf Penso

Thanks Tal.


On 19/11/2020 15:27, Tal Shnaiderman wrote:
>> Subject: Re: Windows: A fundamental issue (was eal/windows: definition for
>> ETOOMANYREFS errno)
>>
>> External email: Use caution opening links or attachments
>>
>>
>> 19/11/2020 14:21, Tal Shnaiderman:
>>>> Subject: Re: Windows: A fundamental issue (was eal/windows:
>>>> definition for ETOOMANYREFS errno)
>>>>
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> Hi Nick,
>>>>
>>>>> This means that rte_os.h should not include POSIX/Linux
>>>>> definitions to avoid clashes such as the one seen with this
>>>>> change.  It's clearly not sustainable if applications have to be
>>>>> modified every time we add more Windows support to the DPDK.
>>>>>
>>>>> Note that this is not an isolated issue - most of the definitions
>>>>> in rte_os.h (redefining close, unlink, strdup etc) should not be
>>>>> present if other layers (application, other libraries, etc) are to
>>>>> be able to implement their own POSIX/Linux support.
>>>> The purpose of rte_os.h must be clarified. It now says:
>>>>
>>>> /**
>>>>   * This is header should contain any function/macro definition
>>>>   * which are not supported natively or named differently in the
>>>>   * ... OS. Functions will be added in future releases.
>>>>   */
>>>>
>>>> This doesn't specify if the file should expose wrappers or POSIX-named
>> bits.
>>>> Linux and FreeBSD, however, only use it for RTE_CPU_xxx() macros for
>>>> CPU_xxx() and don't define anything with POSIX names. So should
>> Windows.
>>>>> Please can we back this change out until we have a strategy that
>>>>> allows us to make these definitions available for 'internal' use,
>>>>> but prevent them being visible outside of the DPDK tree.  If we
>>>>> can't wrap them with
>>>>> rte_* yet, perhaps the short term solution could be as simple as
>>>>> setting RTE_DEFINE_POSIX when building DPDK code and hiding them
>>>>> if it is
>>>> not set?
>>>>
>>>> You need the same value both inside DPDK to return it and outside of
>>>> DPDK to match on it. Returning an unnamed, unspecified code is not an
>> option.
>>>> RTE_ prefix is a way to go. We can just rename ETOOMANYREFS.
>>> Thanks for the info Nick.
>>> Dmitry, If we go with RTE_ETOOMANYREFS, I assume we need to define it
>> for Linux and FreeBSD as well?
>>
>> Or we can use a "more standard" error code?
>>
> Right, Since it is used rarely and only in our PMD I'll work with the developer on selecting a different errno and will revert this commit, apologies for the inconvenience.
>
>>>> Strictly speaking, C standard defines very few errno, so using POSIX
>>>> values in API is incorrect anyway. It has to be deprecated and
>>>> removed eventually, we already had issues with MMAP_FAILED.
>>


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-11-19 15:38 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-14 21:11 [dpdk-dev] [PATCH] eal/windows: definition for ETOOMANYREFS errno Tal Shnaiderman
2020-11-14 22:01 ` Dmitry Kozlyuk
2020-11-14 22:11   ` Tal Shnaiderman
2020-11-15 10:51     ` Thomas Monjalon
2020-11-15 14:23       ` Tal Shnaiderman
2020-11-14 22:21 ` [dpdk-dev] [PATCH v2] " Tal Shnaiderman
2020-11-14 23:13   ` Dmitry Kozlyuk
2020-11-15 23:10     ` Thomas Monjalon
2020-11-17 10:51       ` [dpdk-dev] Windows: A fundamental issue (was eal/windows: definition for ETOOMANYREFS errno) Nick Connolly
2020-11-17 12:53         ` Dmitry Kozlyuk
2020-11-19 13:21           ` Tal Shnaiderman
2020-11-19 14:46             ` Thomas Monjalon
2020-11-19 15:27               ` Tal Shnaiderman
2020-11-19 15:38                 ` Nick Connolly

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).