[dpdk-dev] [PATCH] doc: announce renaming of rte_ether_hdr fields

[dpdk-dev] [PATCH] doc: announce renaming of rte_ether_hdr fields

[Dmitry Kozlyuk]

It is proposed to rename fields of `struct rte_ether_hdr`,
`s_addr` tp `src_addr` and `d_addr` to `dst_addr`,
due to the clash with system macro on Windows.
Until remaining is done in 21.11, a workaround can be used.

Windows Sockets headers contain `#define s_addr S_un.S_addr`, which
conflicts with `s_addr` field of `struct rte_ether_hdr`. Undefining
this macro in <rte_ether.h> is breaking some usages of DPDK
and Windows headers in one file.

Example 1:

    #include <winsock2.h>
    #include <rte_ether.h>
    struct in_addr addr;
    /* addr.s_addr = 0;     ERROR: s_addr undefined by DPDK */

Example 2:

    #include <rte_ether.h>
    #include <winsock2.h>
    struct rte_ether_hdr eh;
    /* eh.s_addr.addr_bytes[0] = 0;     ERROR: `addr_s` is a macro */

It is not mandatory to rename `d_addr`, this is for consistency only.
Naming in `rte_ether_hdr` will also resemble `rte_ipv4/6_hdr`.

Workaround is to define `struct rte_ether_hdr` in such a away that
it can be used with or without `s_addr` macro (as defined on Windows)
This can be done for Windows only or for all platforms to save space.

    #pragma push_macro("s_addr")
    #ifdef s_addr
    #undef s_addr
    #endif

    struct rte_ether_hdr {
        struct rte_ether_addr d_addr; /**< Destination address. */
        RTE_STD_C11
        union {
            struct rte_ether_addr s_addr; /**< Source address. */
            struct {
                struct rte_ether_addr S_un;
                /**< MUST NOT be used directly, only via s_addr */
            } S_addr;
            /*< MUST NOT be used directly, only via s_addr */
        };
        uint16_t ether_type; /**< Frame type. */
    } __rte_aligned(2);

    #pragma pop_macro("s_addr")

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 doc/guides/rel_notes/deprecation.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 82c1a90a3..f7be10543 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -125,3 +125,7 @@ Deprecation Notices
 * cmdline: ``cmdline`` structure will be made opaque to hide platform-specific
   content. On Linux and FreeBSD, supported prior to DPDK 20.11,
   original structure will be kept until DPDK 21.11.
+
+* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
+  will be renamed to ``src_addr`` and ``dst_addr`` respectively in DPDK 20.11
+  in order to avoid conflict with Windows Sockets headers.
-- 
2.29.2

Re: [dpdk-dev] [PATCH] doc: announce renaming of rte_ether_hdr fields

[Stephen Hemminger]

> +
> +* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
> +  will be renamed to ``src_addr`` and ``dst_addr`` respectively in DPDK 20.11
> +  in order to avoid conflict with Windows Sockets headers.

If those fields were a problem now, there might be others in future.
Don't use src_addr and dst_addr because those are already used in rte_ipv4_hdr.

Linux and FreeBSD use:

struct ether_header
{
  uint8_t  ether_dhost[ETH_ALEN];	/* destination eth addr	*/
  uint8_t  ether_shost[ETH_ALEN];	/* source ether addr	*/
  uint16_t ether_type;		        /* packet type ID field	*/
} __attribute__ ((__packed__));

So why not ether_dhost/ether_shost?

Re: [dpdk-dev] [PATCH] doc: announce renaming of rte_ether_hdr fields

[Dmitry Kozlyuk]

2021-03-03 15:54, Stephen Hemminger:
> > +
> > +* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
> > +  will be renamed to ``src_addr`` and ``dst_addr`` respectively in DPDK 20.11
> > +  in order to avoid conflict with Windows Sockets headers.  
> 
> If those fields were a problem now, there might be others in future.

One I can think of is `min` and `max` macros from `windows.h`: they are used
as field names in `rte_compressdev.h` and `rte_cryptodev.h` (and more than
once have they been worked around in PMD code, see i40e and ice patches).
Do you prefer a single notice for all such conflicts we can identify now?

> Don't use src_addr and dst_addr because those are already used in rte_ipv4_hdr.

Not sure what DPDK policy is: `rte_ipv4/6_hdr` use completely custom names,
while `rte_arp_hdr` uses traditional names with `arp_` prefix.
Coming from C++, I chose the former approach, but it's not a strong opinion.

> Linux and FreeBSD use:
> 
> struct ether_header
> {
>   uint8_t  ether_dhost[ETH_ALEN];	/* destination eth addr	*/
>   uint8_t  ether_shost[ETH_ALEN];	/* source ether addr	*/
>   uint16_t ether_type;		        /* packet type ID field	*/
> } __attribute__ ((__packed__));
> 
> So why not ether_dhost/ether_shost?

Works for me, let's see what others think.

[dpdk-dev] [PATCH v2] doc: announce API changes for Windows compatibility

[Dmitry Kozlyuk]

Windows socket headers define `s_addr`, `min`, and `max` macros which
break structure definitions containing fields with one of these names.
Undefining those macros would break some consumers as well.

Example 1:

    #include <winsock2.h>
    #include <rte_ether.h>
    struct in_addr addr;
    /* addr.s_addr = 0;     ERROR: s_addr undefined by DPDK */

Example 2:

    #include <rte_ether.h>
    #include <winsock2.h>
    struct rte_ether_hdr eh;
    /* eh.s_addr.addr_bytes[0] = 0;     ERROR: `addr_s` is a macro */

It is proposed to rename the conflicting fields on DPDK side,
because Win32 API has wider use and is slower and harder to change.

New names are left unspecified, open suggestions:

* `struct rte_ether_hdr` (by Stephen Hemminger):

    * `s_addr` -> `ether_shost`
    * `d_addr` -> `ether_dhost` (for consistency)

* `struct rte_param_log2_range`, `struct rte_crypto_param_range`:

    * `min` -> `minimum`
    * `max` -> `maximum`

For `s_addr`, a workaround is possible to use it until 21.11.
For `min` and `max`, no workaround seems available. If cryptodev or
compressdev is going to be enabled on Windows before 21.11, the only
option seems to use a new name on Windows (using #ifdef).

Workaround for `s_addr` is to define `struct rte_ether_hdr`
in such a away that it can be used with or without `s_addr` macro
(as defined on Windows):

    #pragma push_macro("s_addr")
    #ifdef s_addr
    #undef s_addr
    #endif

    struct rte_ether_hdr {
        struct rte_ether_addr d_addr; /**< Destination address. */
        RTE_STD_C11
        union {
            struct rte_ether_addr s_addr; /**< Source address. */
            struct {
                struct rte_ether_addr S_un;
                /**< MUST NOT be used directly, only via s_addr */
            } S_addr;
            /**< MUST NOT be used directly, only via s_addr */
        };
        uint16_t ether_type; /**< Frame type. */
    } __rte_aligned(2);

    #pragma pop_macro("s_addr")

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
v2: * Propose to rename all problematic fields identified so far.
    * Leave future names unspecified, no need to promise now.
    * Propose better names for MAC addresses (Stephen Hemminger).

Thread about `s_addr` workaround:
https://mails.dpdk.org/archives/dev/2021-March/200700.html
Tyler Retzlaff confirmed offline that Microsoft took similar approach.

 doc/guides/rel_notes/deprecation.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 64629e0641..854618f091 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -130,3 +130,12 @@ Deprecation Notices
 * cmdline: ``cmdline`` structure will be made opaque to hide platform-specific
   content. On Linux and FreeBSD, supported prior to DPDK 20.11,
   original structure will be kept until DPDK 21.11.
+
+* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
+  will be renamed in DPDK 20.11 to avoid conflict with Windows Sockets headers.
+
+* compressdev: ``min`` and ``max`` fields of ``rte_param_log2_range`` structure
+  will be renamed in DPDK 20.11 to avoid conflict with Windows Sockets headers.
+
+* cryptodev: ``min`` and ``max`` fields of ``rte_crypto_param_range`` structure
+  will be renamed in DPDK 20.11 to avoid conflict with Windows Sockets headers.
-- 
2.29.2

Re: [dpdk-dev] [PATCH v2] doc: announce API changes for Windows compatibility

[John Alexander]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dmitry Kozlyuk
> Sent: 10 March 2021 23:54
> To: dev@dpdk.org
> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; Pallavi Kadam
> <pallavi.kadam@intel.com>; Dmitry Malloy <dmitrym@microsoft.com>;
> Narcisa Ana Maria Vasile <navasile@linux.microsoft.com>; Stephen
> Hemminger <stephen@networkplumber.org>; Tyler Retzlaff
> <roretzla@linux.microsoft.com>; Declan Doherty
> <declan.doherty@intel.com>; Fiona Trahe <fiona.trahe@intel.com>; Ashish
> Gupta <ashish.gupta@marvell.com>; Dmitry Kozlyuk
> <dmitry.kozliuk@gmail.com>; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
> <nhorman@tuxdriver.com>
> Subject: [dpdk-dev] [PATCH v2] doc: announce API changes for Windows
> compatibility
> 
> CAUTION: This email originated from outside of the organization. Do not click
> links or open attachments unless you recognize the sender and know the
> content is safe.
> 
> Windows socket headers define `s_addr`, `min`, and `max` macros which
> break structure definitions containing fields with one of these names.
> Undefining those macros would break some consumers as well.
> 
> Example 1:
> 
>     #include <winsock2.h>
>     #include <rte_ether.h>
>     struct in_addr addr;
>     /* addr.s_addr = 0;     ERROR: s_addr undefined by DPDK */
> 
> Example 2:
> 
>     #include <rte_ether.h>
>     #include <winsock2.h>
>     struct rte_ether_hdr eh;
>     /* eh.s_addr.addr_bytes[0] = 0;     ERROR: `addr_s` is a macro */
> 
> It is proposed to rename the conflicting fields on DPDK side, because Win32
> API has wider use and is slower and harder to change.
> 
> New names are left unspecified, open suggestions:
> 
> * `struct rte_ether_hdr` (by Stephen Hemminger):
> 
>     * `s_addr` -> `ether_shost`
>     * `d_addr` -> `ether_dhost` (for consistency)
> 
> * `struct rte_param_log2_range`, `struct rte_crypto_param_range`:
> 
>     * `min` -> `minimum`
>     * `max` -> `maximum`


The min/max macros in the Windows headers cause issues with C++ projects also (breaks std::min/std::max).  The fix there is to "#define NOMINMAX" prior to including windows.h, maybe that's appropriate here too?

> 
> For `s_addr`, a workaround is possible to use it until 21.11.
> For `min` and `max`, no workaround seems available. If cryptodev or
> compressdev is going to be enabled on Windows before 21.11, the only
> option seems to use a new name on Windows (using #ifdef).
> 
> Workaround for `s_addr` is to define `struct rte_ether_hdr` in such a away
> that it can be used with or without `s_addr` macro (as defined on Windows):
> 
>     #pragma push_macro("s_addr")
>     #ifdef s_addr
>     #undef s_addr
>     #endif
> 
>     struct rte_ether_hdr {
>         struct rte_ether_addr d_addr; /**< Destination address. */
>         RTE_STD_C11
>         union {
>             struct rte_ether_addr s_addr; /**< Source address. */
>             struct {
>                 struct rte_ether_addr S_un;
>                 /**< MUST NOT be used directly, only via s_addr */
>             } S_addr;
>             /**< MUST NOT be used directly, only via s_addr */
>         };
>         uint16_t ether_type; /**< Frame type. */
>     } __rte_aligned(2);
> 
>     #pragma pop_macro("s_addr")
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
> v2: * Propose to rename all problematic fields identified so far.
>     * Leave future names unspecified, no need to promise now.
>     * Propose better names for MAC addresses (Stephen Hemminger).
> 
> Thread about `s_addr` workaround:
> https://mails.dpdk.org/archives/dev/2021-March/200700.html
> Tyler Retzlaff confirmed offline that Microsoft took similar approach.
> 
>  doc/guides/rel_notes/deprecation.rst | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 64629e0641..854618f091 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -130,3 +130,12 @@ Deprecation Notices
>  * cmdline: ``cmdline`` structure will be made opaque to hide platform-
> specific
>    content. On Linux and FreeBSD, supported prior to DPDK 20.11,
>    original structure will be kept until DPDK 21.11.
> +
> +* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
> +  will be renamed in DPDK 20.11 to avoid conflict with Windows Sockets
> headers.
> +
> +* compressdev: ``min`` and ``max`` fields of ``rte_param_log2_range``
> +structure
> +  will be renamed in DPDK 20.11 to avoid conflict with Windows Sockets
> headers.
> +
> +* cryptodev: ``min`` and ``max`` fields of ``rte_crypto_param_range``
> +structure
> +  will be renamed in DPDK 20.11 to avoid conflict with Windows Sockets
> headers.
> --
> 2.29.2

Re: [dpdk-dev] [PATCH v2] doc: announce API changes for Windows compatibility

[Dmitry Kozlyuk]

2021-03-11 16:19 (UTC+0000), John Alexander:
[...]
> > * `struct rte_param_log2_range`, `struct rte_crypto_param_range`:
> > 
> >     * `min` -> `minimum`
> >     * `max` -> `maximum`  
> 
> 
> The min/max macros in the Windows headers cause issues with C++ projects also (breaks std::min/std::max).  The fix there is to "#define NOMINMAX" prior to including windows.h, maybe that's appropriate here too?

We don't control include order in user code and we shouldn't #undef system
macros in public headers. We could push_macro/pop_macro around structure
definition and have min/max undefined in DPDK internal code, so that including
this header always works. Then, if user wants to access the fields, they
should take care of macros themselves.

Re: [dpdk-dev] [PATCH v2] doc: announce API changes for Windows compatibility

[Tyler Retzlaff]

On Thu, Mar 11, 2021 at 08:01:01PM +0300, Dmitry Kozlyuk wrote:
> 2021-03-11 16:19 (UTC+0000), John Alexander:
> [...]
> > > * `struct rte_param_log2_range`, `struct rte_crypto_param_range`:
> > > 
> > >     * `min` -> `minimum`
> > >     * `max` -> `maximum`  
> > 
> > 
> > The min/max macros in the Windows headers cause issues with C++ projects also (breaks std::min/std::max).  The fix there is to "#define NOMINMAX" prior to including windows.h, maybe that's appropriate here too?
> 
> We don't control include order in user code and we shouldn't #undef system
> macros in public headers. We could push_macro/pop_macro around structure
> definition and have min/max undefined in DPDK internal code, so that including
> this header always works. Then, if user wants to access the fields, they
> should take care of macros themselves.

agreed, nothing stops an application from including windows.h before
dpdk includes windows.h and similarly the application shouldn't have
visibility of min/max (broken as they are) disappear from the
application namespace after including dpdk headers.

the approach of altering what appears in the namespace for the scope of
the dpdk header preprocessing is about the best that can be achieved.

Re: [dpdk-dev] [PATCH v2] doc: announce API changes for Windows compatibility

[Thomas Monjalon]

11/03/2021 00:54, Dmitry Kozlyuk:
> Windows socket headers define `s_addr`, `min`, and `max` macros which
> break structure definitions containing fields with one of these names.
> Undefining those macros would break some consumers as well.
> 
> Example 1:
> 
>     #include <winsock2.h>
>     #include <rte_ether.h>
>     struct in_addr addr;
>     /* addr.s_addr = 0;     ERROR: s_addr undefined by DPDK */
> 
> Example 2:
> 
>     #include <rte_ether.h>
>     #include <winsock2.h>
>     struct rte_ether_hdr eh;
>     /* eh.s_addr.addr_bytes[0] = 0;     ERROR: `addr_s` is a macro */
> 
> It is proposed to rename the conflicting fields on DPDK side,
> because Win32 API has wider use and is slower and harder to change.
> 
> New names are left unspecified, open suggestions:
> 
> * `struct rte_ether_hdr` (by Stephen Hemminger):
> 
>     * `s_addr` -> `ether_shost`
>     * `d_addr` -> `ether_dhost` (for consistency)

I prefer "addr" over "host".


> * `struct rte_param_log2_range`, `struct rte_crypto_param_range`:
> 
>     * `min` -> `minimum`
>     * `max` -> `maximum`
> 
> For `s_addr`, a workaround is possible to use it until 21.11.
> For `min` and `max`, no workaround seems available. If cryptodev or
> compressdev is going to be enabled on Windows before 21.11, the only
> option seems to use a new name on Windows (using #ifdef).

I think we have enough work on ethdev until 21.11.

Re: [dpdk-dev] [PATCH] doc: announce renaming of rte_ether_hdr fields

[Ferruh Yigit]

On 3/3/2021 10:51 PM, Dmitry Kozlyuk wrote:
> It is proposed to rename fields of `struct rte_ether_hdr`,
> `s_addr` tp `src_addr` and `d_addr` to `dst_addr`,

s/tp/to/

> due to the clash with system macro on Windows.
> Until remaining is done in 21.11, a workaround can be used.

s/remaining/renaming/  ??

> 
> Windows Sockets headers contain `#define s_addr S_un.S_addr`, which
> conflicts with `s_addr` field of `struct rte_ether_hdr`. Undefining
> this macro in <rte_ether.h> is breaking some usages of DPDK
> and Windows headers in one file.
> 
> Example 1:
> 
>     #include <winsock2.h>
>     #include <rte_ether.h>
>     struct in_addr addr;
>     /* addr.s_addr = 0;     ERROR: s_addr undefined by DPDK */
> 
> Example 2:
> 
>     #include <rte_ether.h>
>     #include <winsock2.h>
>     struct rte_ether_hdr eh;
>     /* eh.s_addr.addr_bytes[0] = 0;     ERROR: `addr_s` is a macro */

s/addr_s/s_addr/  ??

> 
> It is not mandatory to rename `d_addr`, this is for consistency only.
> Naming in `rte_ether_hdr` will also resemble `rte_ipv4/6_hdr`.
> 
> Workaround is to define `struct rte_ether_hdr` in such a away that
> it can be used with or without `s_addr` macro (as defined on Windows)
> This can be done for Windows only or for all platforms to save space.
> 
>     #pragma push_macro("s_addr")
>     #ifdef s_addr
>     #undef s_addr
>     #endif
> 
>     struct rte_ether_hdr {
>         struct rte_ether_addr d_addr; /**< Destination address. */
>         RTE_STD_C11
>         union {
>             struct rte_ether_addr s_addr; /**< Source address. */
>             struct {
>                 struct rte_ether_addr S_un;
>                 /**< MUST NOT be used directly, only via s_addr */
>             } S_addr;
>             /*< MUST NOT be used directly, only via s_addr */
>         };
>         uint16_t ether_type; /**< Frame type. */
>     } __rte_aligned(2);
> 
>     #pragma pop_macro("s_addr")
> 

What is the problem with the workaround, why we can't live with it?

It requires an order in include files, right?


> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>  doc/guides/rel_notes/deprecation.rst | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index 82c1a90a3..f7be10543 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -125,3 +125,7 @@ Deprecation Notices
>  * cmdline: ``cmdline`` structure will be made opaque to hide platform-specific
>    content. On Linux and FreeBSD, supported prior to DPDK 20.11,
>    original structure will be kept until DPDK 21.11.
> +
> +* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
> +  will be renamed to ``src_addr`` and ``dst_addr`` respectively in DPDK 20.11

v21.11

> +  in order to avoid conflict with Windows Sockets headers.
> 

Re: [dpdk-dev] [PATCH] doc: announce renaming of rte_ether_hdr fields

[Ferruh Yigit]

On 3/4/2021 7:09 AM, Dmitry Kozlyuk wrote:
> 2021-03-03 15:54, Stephen Hemminger:
>>> +
>>> +* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
>>> +  will be renamed to ``src_addr`` and ``dst_addr`` respectively in DPDK 20.11
>>> +  in order to avoid conflict with Windows Sockets headers.  
>>
>> If those fields were a problem now, there might be others in future.
> 
> One I can think of is `min` and `max` macros from `windows.h`: they are used
> as field names in `rte_compressdev.h` and `rte_cryptodev.h` (and more than
> once have they been worked around in PMD code, see i40e and ice patches).
> Do you prefer a single notice for all such conflicts we can identify now?
> 
>> Don't use src_addr and dst_addr because those are already used in rte_ipv4_hdr.
> 

Why src_addr being used in rte_ipv4_hdr is a problem?

> Not sure what DPDK policy is: `rte_ipv4/6_hdr` use completely custom names,
> while `rte_arp_hdr` uses traditional names with `arp_` prefix.
> Coming from C++, I chose the former approach, but it's not a strong opinion.
> 

I am now aware of a DPDK policy for it, but +1 to former approach.

>> Linux and FreeBSD use:
>>
>> struct ether_header
>> {
>>   uint8_t  ether_dhost[ETH_ALEN];	/* destination eth addr	*/
>>   uint8_t  ether_shost[ETH_ALEN];	/* source ether addr	*/
>>   uint16_t ether_type;		        /* packet type ID field	*/
>> } __attribute__ ((__packed__));
>>
>> So why not ether_dhost/ether_shost?
> 
> Works for me, let's see what others think.
> 

Re: [dpdk-dev] [PATCH] doc: announce renaming of rte_ether_hdr fields

[Dmitry Kozlyuk]

2021-05-20 15:24 (UTC+0100), Ferruh Yigit:
> On 3/3/2021 10:51 PM, Dmitry Kozlyuk wrote:
[...]
> > 
> > It is not mandatory to rename `d_addr`, this is for consistency only.
> > Naming in `rte_ether_hdr` will also resemble `rte_ipv4/6_hdr`.
> > 
> > Workaround is to define `struct rte_ether_hdr` in such a away that
> > it can be used with or without `s_addr` macro (as defined on Windows)
> > This can be done for Windows only or for all platforms to save space.
> > 
> >     #pragma push_macro("s_addr")
> >     #ifdef s_addr
> >     #undef s_addr
> >     #endif
> > 
> >     struct rte_ether_hdr {
> >         struct rte_ether_addr d_addr; /**< Destination address. */
> >         RTE_STD_C11
> >         union {
> >             struct rte_ether_addr s_addr; /**< Source address. */
> >             struct {
> >                 struct rte_ether_addr S_un;
> >                 /**< MUST NOT be used directly, only via s_addr */
> >             } S_addr;
> >             /*< MUST NOT be used directly, only via s_addr */
> >         };
> >         uint16_t ether_type; /**< Frame type. */
> >     } __rte_aligned(2);
> > 
> >     #pragma pop_macro("s_addr")
> >   
> 
> What is the problem with the workaround, why we can't live with it?
> 
> It requires an order in include files, right?

There's no problem except a tricky structure definition with fields that
violate DPDK coding rules. It works with any include order.

Will fix typos in v3, thanks.

Re: [dpdk-dev] [PATCH] doc: announce renaming of rte_ether_hdr fields

[Ferruh Yigit]

On 5/20/2021 4:06 PM, Dmitry Kozlyuk wrote:
> 2021-05-20 15:24 (UTC+0100), Ferruh Yigit:
>> On 3/3/2021 10:51 PM, Dmitry Kozlyuk wrote:
> [...]
>>>
>>> It is not mandatory to rename `d_addr`, this is for consistency only.
>>> Naming in `rte_ether_hdr` will also resemble `rte_ipv4/6_hdr`.
>>>
>>> Workaround is to define `struct rte_ether_hdr` in such a away that
>>> it can be used with or without `s_addr` macro (as defined on Windows)
>>> This can be done for Windows only or for all platforms to save space.
>>>
>>>     #pragma push_macro("s_addr")
>>>     #ifdef s_addr
>>>     #undef s_addr
>>>     #endif
>>>
>>>     struct rte_ether_hdr {
>>>         struct rte_ether_addr d_addr; /**< Destination address. */
>>>         RTE_STD_C11
>>>         union {
>>>             struct rte_ether_addr s_addr; /**< Source address. */
>>>             struct {
>>>                 struct rte_ether_addr S_un;
>>>                 /**< MUST NOT be used directly, only via s_addr */
>>>             } S_addr;
>>>             /*< MUST NOT be used directly, only via s_addr */
>>>         };
>>>         uint16_t ether_type; /**< Frame type. */
>>>     } __rte_aligned(2);
>>>
>>>     #pragma pop_macro("s_addr")
>>>   
>>
>> What is the problem with the workaround, why we can't live with it?
>>
>> It requires an order in include files, right?
> 
> There's no problem except a tricky structure definition with fields that
> violate DPDK coding rules. It works with any include order.
> 
> Will fix typos in v3, thanks.
> 

For following case, won't compiler take 's_addr' as macro?

    #include <rte_ether.h>
    #include <winsock2.h>
    struct rte_ether_hdr eh;
    /* eh.s_addr.addr_bytes[0] = 0;

Re: [dpdk-dev] [PATCH] doc: announce renaming of rte_ether_hdr fields

[Dmitry Kozlyuk]

2021-05-20 16:27 (UTC+0100), Ferruh Yigit:
> On 5/20/2021 4:06 PM, Dmitry Kozlyuk wrote:
> > 2021-05-20 15:24 (UTC+0100), Ferruh Yigit:  
> >> On 3/3/2021 10:51 PM, Dmitry Kozlyuk wrote:  
> > [...]  
> >>>
> >>> It is not mandatory to rename `d_addr`, this is for consistency only.
> >>> Naming in `rte_ether_hdr` will also resemble `rte_ipv4/6_hdr`.
> >>>
> >>> Workaround is to define `struct rte_ether_hdr` in such a away that
> >>> it can be used with or without `s_addr` macro (as defined on Windows)
> >>> This can be done for Windows only or for all platforms to save space.
> >>>
> >>>     #pragma push_macro("s_addr")
> >>>     #ifdef s_addr
> >>>     #undef s_addr
> >>>     #endif
> >>>
> >>>     struct rte_ether_hdr {
> >>>         struct rte_ether_addr d_addr; /**< Destination address. */
> >>>         RTE_STD_C11
> >>>         union {
> >>>             struct rte_ether_addr s_addr; /**< Source address. */
> >>>             struct {
> >>>                 struct rte_ether_addr S_un;
> >>>                 /**< MUST NOT be used directly, only via s_addr */
> >>>             } S_addr;
> >>>             /*< MUST NOT be used directly, only via s_addr */
> >>>         };
> >>>         uint16_t ether_type; /**< Frame type. */
> >>>     } __rte_aligned(2);
> >>>
> >>>     #pragma pop_macro("s_addr")
> >>>     
> >>
> >> What is the problem with the workaround, why we can't live with it?
> >>
> >> It requires an order in include files, right?  
> > 
> > There's no problem except a tricky structure definition with fields that
> > violate DPDK coding rules. It works with any include order.
> > 
> > Will fix typos in v3, thanks.
> >   
> 
> For following case, won't compiler take 's_addr' as macro?
> 
>     #include <rte_ether.h>
>     #include <winsock2.h>
>     struct rte_ether_hdr eh;
>     /* eh.s_addr.addr_bytes[0] = 0;
> 

Yes, it will. The macro will expand to `S_addr.S_un` and compile successfully.
In theory, Microsoft can change the definition of `s_addr`, and while I doubt
they will, it's a valid concern and a reason to remove workaround in 21.11.

Re: [dpdk-dev] [PATCH] doc: announce renaming of rte_ether_hdr fields

[Ferruh Yigit]

On 5/20/2021 4:50 PM, Dmitry Kozlyuk wrote:
> 2021-05-20 16:27 (UTC+0100), Ferruh Yigit:
>> On 5/20/2021 4:06 PM, Dmitry Kozlyuk wrote:
>>> 2021-05-20 15:24 (UTC+0100), Ferruh Yigit:  
>>>> On 3/3/2021 10:51 PM, Dmitry Kozlyuk wrote:  
>>> [...]  
>>>>>
>>>>> It is not mandatory to rename `d_addr`, this is for consistency only.
>>>>> Naming in `rte_ether_hdr` will also resemble `rte_ipv4/6_hdr`.
>>>>>
>>>>> Workaround is to define `struct rte_ether_hdr` in such a away that
>>>>> it can be used with or without `s_addr` macro (as defined on Windows)
>>>>> This can be done for Windows only or for all platforms to save space.
>>>>>
>>>>>     #pragma push_macro("s_addr")
>>>>>     #ifdef s_addr
>>>>>     #undef s_addr
>>>>>     #endif
>>>>>
>>>>>     struct rte_ether_hdr {
>>>>>         struct rte_ether_addr d_addr; /**< Destination address. */
>>>>>         RTE_STD_C11
>>>>>         union {
>>>>>             struct rte_ether_addr s_addr; /**< Source address. */
>>>>>             struct {
>>>>>                 struct rte_ether_addr S_un;
>>>>>                 /**< MUST NOT be used directly, only via s_addr */
>>>>>             } S_addr;
>>>>>             /*< MUST NOT be used directly, only via s_addr */
>>>>>         };
>>>>>         uint16_t ether_type; /**< Frame type. */
>>>>>     } __rte_aligned(2);
>>>>>
>>>>>     #pragma pop_macro("s_addr")
>>>>>     
>>>>
>>>> What is the problem with the workaround, why we can't live with it?
>>>>
>>>> It requires an order in include files, right?  
>>>
>>> There's no problem except a tricky structure definition with fields that
>>> violate DPDK coding rules. It works with any include order.
>>>
>>> Will fix typos in v3, thanks.
>>>   
>>
>> For following case, won't compiler take 's_addr' as macro?
>>
>>     #include <rte_ether.h>
>>     #include <winsock2.h>
>>     struct rte_ether_hdr eh;
>>     /* eh.s_addr.addr_bytes[0] = 0;
>>
> 
> Yes, it will. The macro will expand to `S_addr.S_un` and compile successfully.

will 'eh.S_addr.S_un.addr_bytes[0] = 0;' compile successfully?

> In theory, Microsoft can change the definition of `s_addr`, and while I doubt
> they will, it's a valid concern and a reason to remove workaround in 21.11.
> 

Re: [dpdk-dev] [PATCH] doc: announce renaming of rte_ether_hdr fields

[Dmitry Kozlyuk]

2021-05-20 17:04 (UTC+0100), Ferruh Yigit:
> On 5/20/2021 4:50 PM, Dmitry Kozlyuk wrote:
> > 2021-05-20 16:27 (UTC+0100), Ferruh Yigit:  
> >> On 5/20/2021 4:06 PM, Dmitry Kozlyuk wrote:  
> >>> 2021-05-20 15:24 (UTC+0100), Ferruh Yigit:    
> >>>> On 3/3/2021 10:51 PM, Dmitry Kozlyuk wrote:    
> >>> [...]    
> >>>>>
> >>>>> It is not mandatory to rename `d_addr`, this is for consistency only.
> >>>>> Naming in `rte_ether_hdr` will also resemble `rte_ipv4/6_hdr`.
> >>>>>
> >>>>> Workaround is to define `struct rte_ether_hdr` in such a away that
> >>>>> it can be used with or without `s_addr` macro (as defined on Windows)
> >>>>> This can be done for Windows only or for all platforms to save space.
> >>>>>
> >>>>>     #pragma push_macro("s_addr")
> >>>>>     #ifdef s_addr
> >>>>>     #undef s_addr
> >>>>>     #endif
> >>>>>
> >>>>>     struct rte_ether_hdr {
> >>>>>         struct rte_ether_addr d_addr; /**< Destination address. */
> >>>>>         RTE_STD_C11
> >>>>>         union {
> >>>>>             struct rte_ether_addr s_addr; /**< Source address. */
> >>>>>             struct {
> >>>>>                 struct rte_ether_addr S_un;
> >>>>>                 /**< MUST NOT be used directly, only via s_addr */
> >>>>>             } S_addr;
> >>>>>             /*< MUST NOT be used directly, only via s_addr */
> >>>>>         };
> >>>>>         uint16_t ether_type; /**< Frame type. */
> >>>>>     } __rte_aligned(2);
> >>>>>
> >>>>>     #pragma pop_macro("s_addr")
> >>>>>       
> >>>>
> >>>> What is the problem with the workaround, why we can't live with it?
> >>>>
> >>>> It requires an order in include files, right?    
> >>>
> >>> There's no problem except a tricky structure definition with fields that
> >>> violate DPDK coding rules. It works with any include order.
> >>>
> >>> Will fix typos in v3, thanks.
> >>>     
> >>
> >> For following case, won't compiler take 's_addr' as macro?
> >>
> >>     #include <rte_ether.h>
> >>     #include <winsock2.h>
> >>     struct rte_ether_hdr eh;
> >>     /* eh.s_addr.addr_bytes[0] = 0;
> >>  
> > 
> > Yes, it will. The macro will expand to `S_addr.S_un` and compile successfully.  
> 
> will 'eh.S_addr.S_un.addr_bytes[0] = 0;' compile successfully?

Yes, only it's `S_un.S_addr`, sorry for the typo in my explanation.
Both code snippets from commit message compile successfully.

> 
> > In theory, Microsoft can change the definition of `s_addr`, and while I doubt
> > they will, it's a valid concern and a reason to remove workaround in 21.11.
> >   
> 

Re: [dpdk-dev] [PATCH] doc: announce renaming of rte_ether_hdr fields

[Ferruh Yigit]

On 5/20/2021 5:16 PM, Dmitry Kozlyuk wrote:
> 2021-05-20 17:04 (UTC+0100), Ferruh Yigit:
>> On 5/20/2021 4:50 PM, Dmitry Kozlyuk wrote:
>>> 2021-05-20 16:27 (UTC+0100), Ferruh Yigit:  
>>>> On 5/20/2021 4:06 PM, Dmitry Kozlyuk wrote:  
>>>>> 2021-05-20 15:24 (UTC+0100), Ferruh Yigit:    
>>>>>> On 3/3/2021 10:51 PM, Dmitry Kozlyuk wrote:    
>>>>> [...]    
>>>>>>>
>>>>>>> It is not mandatory to rename `d_addr`, this is for consistency only.
>>>>>>> Naming in `rte_ether_hdr` will also resemble `rte_ipv4/6_hdr`.
>>>>>>>
>>>>>>> Workaround is to define `struct rte_ether_hdr` in such a away that
>>>>>>> it can be used with or without `s_addr` macro (as defined on Windows)
>>>>>>> This can be done for Windows only or for all platforms to save space.
>>>>>>>
>>>>>>>     #pragma push_macro("s_addr")
>>>>>>>     #ifdef s_addr
>>>>>>>     #undef s_addr
>>>>>>>     #endif
>>>>>>>
>>>>>>>     struct rte_ether_hdr {
>>>>>>>         struct rte_ether_addr d_addr; /**< Destination address. */
>>>>>>>         RTE_STD_C11
>>>>>>>         union {
>>>>>>>             struct rte_ether_addr s_addr; /**< Source address. */
>>>>>>>             struct {
>>>>>>>                 struct rte_ether_addr S_un;
>>>>>>>                 /**< MUST NOT be used directly, only via s_addr */
>>>>>>>             } S_addr;
>>>>>>>             /*< MUST NOT be used directly, only via s_addr */
>>>>>>>         };
>>>>>>>         uint16_t ether_type; /**< Frame type. */
>>>>>>>     } __rte_aligned(2);
>>>>>>>
>>>>>>>     #pragma pop_macro("s_addr")
>>>>>>>       
>>>>>>
>>>>>> What is the problem with the workaround, why we can't live with it?
>>>>>>
>>>>>> It requires an order in include files, right?    
>>>>>
>>>>> There's no problem except a tricky structure definition with fields that
>>>>> violate DPDK coding rules. It works with any include order.
>>>>>
>>>>> Will fix typos in v3, thanks.
>>>>>     
>>>>
>>>> For following case, won't compiler take 's_addr' as macro?
>>>>
>>>>     #include <rte_ether.h>
>>>>     #include <winsock2.h>
>>>>     struct rte_ether_hdr eh;
>>>>     /* eh.s_addr.addr_bytes[0] = 0;
>>>>  
>>>
>>> Yes, it will. The macro will expand to `S_addr.S_un` and compile successfully.  
>>
>> will 'eh.S_addr.S_un.addr_bytes[0] = 0;' compile successfully?
> 
> Yes, only it's `S_un.S_addr`, sorry for the typo in my explanation.
> Both code snippets from commit message compile successfully.
> 

Ah, I was missing the union on the struct, yes it will build,

And +1 to deprecation notice and clean the "struct rte_ether_hdr" whenever possible.

>>
>>> In theory, Microsoft can change the definition of `s_addr`, and while I doubt
>>> they will, it's a valid concern and a reason to remove workaround in 21.11.
>>>   
>>
> 

[dpdk-dev] [PATCH v3] doc: announce API changes for Windows compatibility

[Dmitry Kozlyuk]

Windows system headers define `s_addr`, `min`, and `max` macros which
break structure definitions containing fields with one of these names.
Undefining those macros would break consumer code that relies on them.

Example 1:

    #include <winsock2.h>
    #include <rte_ether.h>
    struct in_addr addr;
    /* addr.s_addr = 0;     ERROR: s_addr undefined by DPDK */

Example 2:

    #include <rte_ether.h>
    #include <winsock2.h>
    struct rte_ether_hdr eh;
    /* eh.s_addr.addr_bytes[0] = 0;     ERROR: `addr_s` is a macro */

Commit 6c068dbd9fea ("net: work around s_addr macro on Windows")
modified definition of `struct rte_ether_hdr` to avoid the issue.
However, the workaround assumes `#define s_addr S_addr.S_un`
in Windows headers, which is not a part of official API.
It also complicates the definition of `struct rte_ether_hdr`.

For `min` and `max`, no workaround seems available. If cryptodev or
compressdev is going to be enabled on Windows before 21.11, the only
option seems to use a new name on Windows (using #ifdef).

It is proposed to rename the conflicting fields on DPDK side,
because Win32 API has wider use and is slower and harder to change.
Exact new names are left for further discussion.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Acked-by: Khoa To <khot@microsoft.com>
---
v3: fix typos (Ferruh), remove naming speculation,
    replace workaround snippet with commit reference.

 doc/guides/rel_notes/deprecation.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 9584d6bfd7..cc6e8db92c 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -147,3 +147,12 @@ Deprecation Notices
 * cmdline: ``cmdline`` structure will be made opaque to hide platform-specific
   content. On Linux and FreeBSD, supported prior to DPDK 20.11,
   original structure will be kept until DPDK 21.11.
+
+* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
+  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets headers.
+
+* compressdev: ``min`` and ``max`` fields of ``rte_param_log2_range`` structure
+  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets headers.
+
+* cryptodev: ``min`` and ``max`` fields of ``rte_crypto_param_range`` structure
+  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets headers.
-- 
2.29.3

Re: [dpdk-dev] [EXT] [PATCH v3] doc: announce API changes for Windows compatibility

[Akhil Goyal]

> Windows system headers define `s_addr`, `min`, and `max` macros which
> break structure definitions containing fields with one of these names.
> Undefining those macros would break consumer code that relies on them.
> 

From the commit message the requirement for changing the structure definitions
Is not clear. Please note that 'min' - 'max' are not macros. These are variables of a
structure which should not break any other structure/Macro in windows.

> Example 1:
> 
>     #include <winsock2.h>
>     #include <rte_ether.h>
>     struct in_addr addr;
>     /* addr.s_addr = 0;     ERROR: s_addr undefined by DPDK */
> 
> Example 2:
> 
>     #include <rte_ether.h>
>     #include <winsock2.h>
>     struct rte_ether_hdr eh;
>     /* eh.s_addr.addr_bytes[0] = 0;     ERROR: `addr_s` is a macro */
> 
> Commit 6c068dbd9fea ("net: work around s_addr macro on Windows")
> modified definition of `struct rte_ether_hdr` to avoid the issue.
> However, the workaround assumes `#define s_addr S_addr.S_un`
> in Windows headers, which is not a part of official API.
> It also complicates the definition of `struct rte_ether_hdr`.
> 
> For `min` and `max`, no workaround seems available. If cryptodev or
> compressdev is going to be enabled on Windows before 21.11, the only
> option seems to use a new name on Windows (using #ifdef).
> 
> It is proposed to rename the conflicting fields on DPDK side,
> because Win32 API has wider use and is slower and harder to change.
> Exact new names are left for further discussion.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Acked-by: Khoa To <khot@microsoft.com>
> ---
> v3: fix typos (Ferruh), remove naming speculation,
>     replace workaround snippet with commit reference.
> 
>  doc/guides/rel_notes/deprecation.rst | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> index 9584d6bfd7..cc6e8db92c 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -147,3 +147,12 @@ Deprecation Notices
>  * cmdline: ``cmdline`` structure will be made opaque to hide platform-
> specific
>    content. On Linux and FreeBSD, supported prior to DPDK 20.11,
>    original structure will be kept until DPDK 21.11.
> +
> +* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
> +  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets
> headers.
> +
> +* compressdev: ``min`` and ``max`` fields of ``rte_param_log2_range``
> structure
> +  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets
> headers.
> +
> +* cryptodev: ``min`` and ``max`` fields of ``rte_crypto_param_range``
> structure
> +  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets
> headers.
> --
> 2.29.3

Re: [dpdk-dev] [EXT] [PATCH v3] doc: announce API changes for Windows compatibility

[Dmitry Kozlyuk]

2021-05-20 18:59 (UTC+0000), Akhil Goyal:
> > Windows system headers define `s_addr`, `min`, and `max` macros which
> > break structure definitions containing fields with one of these names.
> > Undefining those macros would break consumer code that relies on them.
> >   
> 
> From the commit message the requirement for changing the structure definitions
> Is not clear. Please note that 'min' - 'max' are not macros. These are variables of a
> structure which should not break any other structure/Macro in windows.

Err, yes, that's what the commit message says.
Structure fields of course break nothing; they are broken by Windows macros.
Would this make more sense?


	Windows headers define `s_addr`, `min`, and `max` as macros.
	If DPDK headers are included after Windows ones, DPDK structure
	definitions containing fields with these names get broken.
	If DPDK headers undefined these macros, it could break consumer code
	relying on these macros. It is proposed to rename structure fields
	in DPDK, because Win32 headers are more widely used and harder to fix.

Re: [dpdk-dev] [EXT] [PATCH v3] doc: announce API changes for Windows compatibility

[Akhil Goyal]

> 
> 2021-05-20 18:59 (UTC+0000), Akhil Goyal:
> > > Windows system headers define `s_addr`, `min`, and `max` macros which
> > > break structure definitions containing fields with one of these names.
> > > Undefining those macros would break consumer code that relies on
> them.
> > >
> >
> > From the commit message the requirement for changing the structure
> definitions
> > Is not clear. Please note that 'min' - 'max' are not macros. These are
> variables of a
> > structure which should not break any other structure/Macro in windows.
> 
> Err, yes, that's what the commit message says.
> Structure fields of course break nothing; they are broken by Windows
> macros.
> Would this make more sense?
> 
> 
> 	Windows headers define `s_addr`, `min`, and `max` as macros.
> 	If DPDK headers are included after Windows ones, DPDK structure
> 	definitions containing fields with these names get broken.
> 	If DPDK headers undefined these macros, it could break consumer
> code
> 	relying on these macros. It is proposed to rename structure fields
> 	in DPDK, because Win32 headers are more widely used and harder
> to fix.

Yes it makes more sense now. But ideally it should be fixed in windows.
This may be just one such issue, there may be many more.
Will this also mean that nobody can define a local variable 'min'?
Is this acceptable?

Any macro definition in a subsystem should have a prefix to denote that,
Just like in DPDK 'RTE_' is added.
Macros with generic names should be avoided so that we do not get into
these issues.

Adding more people for comments. I don't have a good feeling about
this change.

Re: [dpdk-dev] [EXT] [PATCH v3] doc: announce API changes for Windows compatibility

[Dmitry Kozlyuk]

2021-05-20 20:17 (UTC+0000), Akhil Goyal:
> > 
> > 2021-05-20 18:59 (UTC+0000), Akhil Goyal:  
> > > > Windows system headers define `s_addr`, `min`, and `max` macros which
> > > > break structure definitions containing fields with one of these names.
> > > > Undefining those macros would break consumer code that relies on  
> > them.  
> > > >  
> > >
> > > From the commit message the requirement for changing the structure  
> > definitions  
> > > Is not clear. Please note that 'min' - 'max' are not macros. These are  
> > variables of a  
> > > structure which should not break any other structure/Macro in windows.  
> > 
> > Err, yes, that's what the commit message says.
> > Structure fields of course break nothing; they are broken by Windows
> > macros.
> > Would this make more sense?
> > 
> > 
> > 	Windows headers define `s_addr`, `min`, and `max` as macros.
> > 	If DPDK headers are included after Windows ones, DPDK structure
> > 	definitions containing fields with these names get broken.
> > 	If DPDK headers undefined these macros, it could break consumer
> > code
> > 	relying on these macros. It is proposed to rename structure fields
> > 	in DPDK, because Win32 headers are more widely used and harder
> > to fix.  
> 
> Yes it makes more sense now. But ideally it should be fixed in windows.
> This may be just one such issue, there may be many more.
> Will this also mean that nobody can define a local variable 'min'?
> Is this acceptable?

Only in public headers. There happens to be one such, rte_lru_x86.h.

> Any macro definition in a subsystem should have a prefix to denote that,
> Just like in DPDK 'RTE_' is added.
> Macros with generic names should be avoided so that we do not get into
> these issues.
> 
> Adding more people for comments. I don't have a good feeling about
> this change.

Friendly ping to everyone Akhil cc'ed.
As far as I understand, if we want to fix it in 21.11,
deprecation notice should make it into 21.08.

Re: [dpdk-dev] [EXT] [PATCH v3] doc: announce API changes for Windows compatibility

[Tyler Retzlaff]

On Thu, May 20, 2021 at 08:17:54PM +0000, Akhil Goyal wrote:
> > 
> 
> Yes it makes more sense now. But ideally it should be fixed in windows.

they won't be fixed in windows.  it is extremely rare to withdraw
something from a platform headers namespace and is avoided to maintain
API compatibility no matter how horrible these macros are.

> This may be just one such issue, there may be many more.
> Will this also mean that nobody can define a local variable 'min'?

that is correct. As well as a long list of other commonly used names not
only from windows but other platform headers, standard library headers
and keywords.

> Any macro definition in a subsystem should have a prefix to denote that,
> Just like in DPDK 'RTE_' is added.

completely agree, best practice would be to avoid contaminating the
applications namespace.

> Macros with generic names should be avoided so that we do not get into
> these issues.
> 
> Adding more people for comments. I don't have a good feeling about
> this change.

there is no real choice, the platform header won't be "fixed" so it has
to be dealt with and even thinking you can avoid it by just not
including windows.h here keep in mind the consuming application will
have to anyway so it's difficult to avoid.

Re: [dpdk-dev] [EXT] [PATCH v3] doc: announce API changes for Windows compatibility

[Dmitry Kozlyuk]

2021-06-09 18:52 (UTC+0300), Dmitry Kozlyuk:
> 2021-05-20 20:17 (UTC+0000), Akhil Goyal:
> > > 
> > > 2021-05-20 18:59 (UTC+0000), Akhil Goyal:    
> > > > > Windows system headers define `s_addr`, `min`, and `max` macros which
> > > > > break structure definitions containing fields with one of these names.
> > > > > Undefining those macros would break consumer code that relies on    
> > > them.    
> > > > >    
> > > >
> > > > From the commit message the requirement for changing the structure    
> > > definitions    
> > > > Is not clear. Please note that 'min' - 'max' are not macros. These are    
> > > variables of a    
> > > > structure which should not break any other structure/Macro in windows.    
> > > 
> > > Err, yes, that's what the commit message says.
> > > Structure fields of course break nothing; they are broken by Windows
> > > macros.
> > > Would this make more sense?
> > > 
> > > 
> > > 	Windows headers define `s_addr`, `min`, and `max` as macros.
> > > 	If DPDK headers are included after Windows ones, DPDK structure
> > > 	definitions containing fields with these names get broken.
> > > 	If DPDK headers undefined these macros, it could break consumer
> > > code
> > > 	relying on these macros. It is proposed to rename structure fields
> > > 	in DPDK, because Win32 headers are more widely used and harder
> > > to fix.    
> > 
> > Yes it makes more sense now. But ideally it should be fixed in windows.
> > This may be just one such issue, there may be many more.
> > Will this also mean that nobody can define a local variable 'min'?
> > Is this acceptable?  
> 
> Only in public headers. There happens to be one such, rte_lru_x86.h.
> 
> > Any macro definition in a subsystem should have a prefix to denote that,
> > Just like in DPDK 'RTE_' is added.
> > Macros with generic names should be avoided so that we do not get into
> > these issues.
> > 
> > Adding more people for comments. I don't have a good feeling about
> > this change.  
> 
> Friendly ping to everyone Akhil cc'ed.
> As far as I understand, if we want to fix it in 21.11,
> deprecation notice should make it into 21.08.

Friendly ping v2.
Hopefully Tyler's answer will help with the decision.

[dpdk-dev] [PATCH v3] doc: announce API changes for Windows compatibility

[Dmitry Kozlyuk]

Windows system headers define `s_addr`, `min`, and `max` macros which
break structure definitions containing fields with one of these names.
Undefining those macros would break consumer code that relies on them.

Example 1:

    #include <winsock2.h>
    #include <rte_ether.h>
    struct in_addr addr;
    /* addr.s_addr = 0;     ERROR: s_addr undefined by DPDK */

Example 2:

    #include <rte_ether.h>
    #include <winsock2.h>
    struct rte_ether_hdr eh;
    /* eh.s_addr.addr_bytes[0] = 0;     ERROR: `addr_s` is a macro */

Commit 6c068dbd9fea ("net: work around s_addr macro on Windows")
modified definition of `struct rte_ether_hdr` to avoid the issue.
However, the workaround assumes `#define s_addr S_addr.S_un`
in Windows headers, which is not a part of official API.
It also complicates the definition of `struct rte_ether_hdr`.

For `min` and `max`, no workaround seems available. If cryptodev or
compressdev is going to be enabled on Windows before 21.11, the only
option seems to use a new name on Windows (using #ifdef).

It is proposed to rename the conflicting fields on DPDK side,
because Win32 API has wider use and is slower and harder to change.
Exact new names are left for further discussion.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Acked-by: Khoa To <khot@microsoft.com>
---
v3: fix typos (Ferruh), remove naming speculation,
    replace workaround snippet with commit reference.

 doc/guides/rel_notes/deprecation.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 9584d6bfd7..cc6e8db92c 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -147,3 +147,12 @@ Deprecation Notices
 * cmdline: ``cmdline`` structure will be made opaque to hide platform-specific
   content. On Linux and FreeBSD, supported prior to DPDK 20.11,
   original structure will be kept until DPDK 21.11.
+
+* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
+  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets headers.
+
+* compressdev: ``min`` and ``max`` fields of ``rte_param_log2_range`` structure
+  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets headers.
+
+* cryptodev: ``min`` and ``max`` fields of ``rte_crypto_param_range`` structure
+  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets headers.
-- 
2.29.3

[dpdk-dev] [PATCH v4] doc: announce API changes for Windows compatibility

[Dmitry Kozlyuk]

Windows headers define `s_addr`, `min`, and `max` as macros.
If DPDK headers are included after Windows ones, DPDK structure
definitions containing fields with these names get broken (example 1),
as well as any usage of such fields (example 2). If DPDK headers
undefined these macros, it could break consumer code (example 3).
It is proposed to rename structure fields in DPDK, because Win32 headers
are used more widely than DPDK, as a general-purpose platform compared
to domain-specific kit, and are harder to fix because of that.
Exact new names are left for further discussion.

Example 1:

    /* in DPDK public header included after windows.h */
    struct rte_type {
        int min;    /* ERROR: `min` is a macro */
    };

Example 2:

    #include <rte_ether.h>
    #include <winsock2.h>
    struct rte_ether_hdr eh;
    eh.s_addr.addr_bytes[0] = 0;    /* ERROR: `addr_s` is a macro */

Example 3:

    #include <winsock2.h>
    #include <rte_ether.h>
    struct in_addr addr;
    addr.s_addr = 0;      /* ERROR: there is no `s_addr` field,
                             and `s_addr` macro is undefined by DPDK. */

Commit 6c068dbd9fea ("net: work around s_addr macro on Windows")
modified definition of `struct rte_ether_hdr` to avoid the issue.
However, the workaround assumes `#define s_addr S_addr.S_un`
in Windows headers, which is not a part of official API.
It also complicates the definition of `struct rte_ether_hdr`.

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Acked-by: Khoa To <khot@microsoft.com>
---
v4: improve wording (Akhil).
v3: fix typos (Ferruh), remove naming speculation,
    replace workaround snippet with commit reference.

 doc/guides/rel_notes/deprecation.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 9584d6bfd7..cc6e8db92c 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -147,3 +147,12 @@ Deprecation Notices
 * cmdline: ``cmdline`` structure will be made opaque to hide platform-specific
   content. On Linux and FreeBSD, supported prior to DPDK 20.11,
   original structure will be kept until DPDK 21.11.
+
+* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
+  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets headers.
+
+* compressdev: ``min`` and ``max`` fields of ``rte_param_log2_range`` structure
+  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets headers.
+
+* cryptodev: ``min`` and ``max`` fields of ``rte_crypto_param_range`` structure
+  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets headers.
-- 
2.29.3

Re: [dpdk-dev] [PATCH v4] doc: announce API changes for Windows compatibility

[Thomas Monjalon]

21/07/2021 21:55, Dmitry Kozlyuk:
> Windows headers define `s_addr`, `min`, and `max` as macros.
> If DPDK headers are included after Windows ones, DPDK structure
> definitions containing fields with these names get broken (example 1),
> as well as any usage of such fields (example 2). If DPDK headers
> undefined these macros, it could break consumer code (example 3).
> It is proposed to rename structure fields in DPDK, because Win32 headers
> are used more widely than DPDK, as a general-purpose platform compared
> to domain-specific kit, and are harder to fix because of that.
> Exact new names are left for further discussion.
> 
> Example 1:
> 
>     /* in DPDK public header included after windows.h */
>     struct rte_type {
>         int min;    /* ERROR: `min` is a macro */
>     };
> 
> Example 2:
> 
>     #include <rte_ether.h>
>     #include <winsock2.h>
>     struct rte_ether_hdr eh;
>     eh.s_addr.addr_bytes[0] = 0;    /* ERROR: `addr_s` is a macro */
> 
> Example 3:
> 
>     #include <winsock2.h>
>     #include <rte_ether.h>
>     struct in_addr addr;
>     addr.s_addr = 0;      /* ERROR: there is no `s_addr` field,
>                              and `s_addr` macro is undefined by DPDK. */
> 
> Commit 6c068dbd9fea ("net: work around s_addr macro on Windows")
> modified definition of `struct rte_ether_hdr` to avoid the issue.
> However, the workaround assumes `#define s_addr S_addr.S_un`
> in Windows headers, which is not a part of official API.
> It also complicates the definition of `struct rte_ether_hdr`.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> Acked-by: Khoa To <khot@microsoft.com>
> ---
> +* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
> +  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets headers.
> +
> +* compressdev: ``min`` and ``max`` fields of ``rte_param_log2_range`` structure
> +  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets headers.

The struct rte_param_log2_range should also be renamed to include "compress" prefix.
But as we break the struct API, it is not an issue I guess.

> +* cryptodev: ``min`` and ``max`` fields of ``rte_crypto_param_range`` structure
> +  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets headers.

Acked-by: Thomas Monjalon <thomas@monjalon.net>

Re: [dpdk-dev] [EXT] Re: [PATCH v4] doc: announce API changes for Windows compatibility

[Akhil Goyal]

> 21/07/2021 21:55, Dmitry Kozlyuk:
> > Windows headers define `s_addr`, `min`, and `max` as macros.
> > If DPDK headers are included after Windows ones, DPDK structure
> > definitions containing fields with these names get broken (example 1),
> > as well as any usage of such fields (example 2). If DPDK headers
> > undefined these macros, it could break consumer code (example 3).
> > It is proposed to rename structure fields in DPDK, because Win32 headers
> > are used more widely than DPDK, as a general-purpose platform compared
> > to domain-specific kit, and are harder to fix because of that.
> > Exact new names are left for further discussion.
> >
> > Example 1:
> >
> >     /* in DPDK public header included after windows.h */
> >     struct rte_type {
> >         int min;    /* ERROR: `min` is a macro */
> >     };
> >
> > Example 2:
> >
> >     #include <rte_ether.h>
> >     #include <winsock2.h>
> >     struct rte_ether_hdr eh;
> >     eh.s_addr.addr_bytes[0] = 0;    /* ERROR: `addr_s` is a macro */
> >
> > Example 3:
> >
> >     #include <winsock2.h>
> >     #include <rte_ether.h>
> >     struct in_addr addr;
> >     addr.s_addr = 0;      /* ERROR: there is no `s_addr` field,
> >                              and `s_addr` macro is undefined by DPDK. */
> >
> > Commit 6c068dbd9fea ("net: work around s_addr macro on Windows")
> > modified definition of `struct rte_ether_hdr` to avoid the issue.
> > However, the workaround assumes `#define s_addr S_addr.S_un`
> > in Windows headers, which is not a part of official API.
> > It also complicates the definition of `struct rte_ether_hdr`.
> >
> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > Acked-by: Khoa To <khot@microsoft.com>
> > ---
> > +* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
> > +  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets
> headers.
> > +
> > +* compressdev: ``min`` and ``max`` fields of ``rte_param_log2_range``
> structure
> > +  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets
> headers.
> 
> The struct rte_param_log2_range should also be renamed to include
> "compress" prefix.
> But as we break the struct API, it is not an issue I guess.
> 
> > +* cryptodev: ``min`` and ``max`` fields of ``rte_crypto_param_range``
> structure
> > +  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets
> headers.
> 
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> 
Can we have a local variable named as min/max?
If not, then I believe it is not a good idea.

Re: [dpdk-dev] [EXT] Re: [PATCH v4] doc: announce API changes for Windows compatibility

[Dmitry Kozlyuk]

2021-08-02 12:45 (UTC+0000), Akhil Goyal:
> > 21/07/2021 21:55, Dmitry Kozlyuk:  
> > > Windows headers define `s_addr`, `min`, and `max` as macros.
> > > If DPDK headers are included after Windows ones, DPDK structure
> > > definitions containing fields with these names get broken (example 1),
> > > as well as any usage of such fields (example 2). If DPDK headers
> > > undefined these macros, it could break consumer code (example 3).
> > > It is proposed to rename structure fields in DPDK, because Win32 headers
> > > are used more widely than DPDK, as a general-purpose platform compared
> > > to domain-specific kit, and are harder to fix because of that.
> > > Exact new names are left for further discussion.
> > >
> > > Example 1:
> > >
> > >     /* in DPDK public header included after windows.h */
> > >     struct rte_type {
> > >         int min;    /* ERROR: `min` is a macro */
> > >     };
> > >
> > > Example 2:
> > >
> > >     #include <rte_ether.h>
> > >     #include <winsock2.h>
> > >     struct rte_ether_hdr eh;
> > >     eh.s_addr.addr_bytes[0] = 0;    /* ERROR: `addr_s` is a macro */
> > >
> > > Example 3:
> > >
> > >     #include <winsock2.h>
> > >     #include <rte_ether.h>
> > >     struct in_addr addr;
> > >     addr.s_addr = 0;      /* ERROR: there is no `s_addr` field,
> > >                              and `s_addr` macro is undefined by DPDK. */
> > >
> > > Commit 6c068dbd9fea ("net: work around s_addr macro on Windows")
> > > modified definition of `struct rte_ether_hdr` to avoid the issue.
> > > However, the workaround assumes `#define s_addr S_addr.S_un`
> > > in Windows headers, which is not a part of official API.
> > > It also complicates the definition of `struct rte_ether_hdr`.
> > >
> > > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > > Acked-by: Khoa To <khot@microsoft.com>
> > > ---
> > > +* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
> > > +  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets  
> > headers.  
> > > +
> > > +* compressdev: ``min`` and ``max`` fields of ``rte_param_log2_range``  
> > structure  
> > > +  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets  
> > headers.
> > 
> > The struct rte_param_log2_range should also be renamed to include
> > "compress" prefix.
> > But as we break the struct API, it is not an issue I guess.
> >   
> > > +* cryptodev: ``min`` and ``max`` fields of ``rte_crypto_param_range``  
> > structure  
> > > +  will be renamed in DPDK 21.11 to avoid conflict with Windows Sockets  
> > headers.
> > 
> > Acked-by: Thomas Monjalon <thomas@monjalon.net>
> >   
> Can we have a local variable named as min/max?
> If not, then I believe it is not a good idea.

Yes, except for inline functions in public headers.
The only problematic one I know of is this (rte_lru_x86.h):

static inline int
f_lru_pos(uint64_t lru_list)
{
	__m128i lst = _mm_set_epi64x((uint64_t)-1, lru_list);
	__m128i min = _mm_minpos_epu16(lst); /* <<< */
	return _mm_extract_epi16(min, 1);
}

Fixing it breaks neither API nor ABI, thus no explicit deprecation notice.

Re: [dpdk-dev] [EXT] Re: [PATCH v4] doc: announce API changes for Windows compatibility

[Akhil Goyal]

> 2021-08-02 12:45 (UTC+0000), Akhil Goyal:
> > > 21/07/2021 21:55, Dmitry Kozlyuk:
> > > > Windows headers define `s_addr`, `min`, and `max` as macros.
> > > > If DPDK headers are included after Windows ones, DPDK structure
> > > > definitions containing fields with these names get broken (example 1),
> > > > as well as any usage of such fields (example 2). If DPDK headers
> > > > undefined these macros, it could break consumer code (example 3).
> > > > It is proposed to rename structure fields in DPDK, because Win32
> headers
> > > > are used more widely than DPDK, as a general-purpose platform
> compared
> > > > to domain-specific kit, and are harder to fix because of that.
> > > > Exact new names are left for further discussion.
> > > >
> > > > Example 1:
> > > >
> > > >     /* in DPDK public header included after windows.h */
> > > >     struct rte_type {
> > > >         int min;    /* ERROR: `min` is a macro */
> > > >     };
> > > >
> > > > Example 2:
> > > >
> > > >     #include <rte_ether.h>
> > > >     #include <winsock2.h>
> > > >     struct rte_ether_hdr eh;
> > > >     eh.s_addr.addr_bytes[0] = 0;    /* ERROR: `addr_s` is a macro */
> > > >
> > > > Example 3:
> > > >
> > > >     #include <winsock2.h>
> > > >     #include <rte_ether.h>
> > > >     struct in_addr addr;
> > > >     addr.s_addr = 0;      /* ERROR: there is no `s_addr` field,
> > > >                              and `s_addr` macro is undefined by DPDK. */
> > > >
> > > > Commit 6c068dbd9fea ("net: work around s_addr macro on Windows")
> > > > modified definition of `struct rte_ether_hdr` to avoid the issue.
> > > > However, the workaround assumes `#define s_addr S_addr.S_un`
> > > > in Windows headers, which is not a part of official API.
> > > > It also complicates the definition of `struct rte_ether_hdr`.
> > > >
> > > > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > > > Acked-by: Khoa To <khot@microsoft.com>
> > > > ---
> > > > +* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr`` structure
> > > > +  will be renamed in DPDK 21.11 to avoid conflict with Windows
> Sockets
> > > headers.
> > > > +
> > > > +* compressdev: ``min`` and ``max`` fields of ``rte_param_log2_range``
> > > structure
> > > > +  will be renamed in DPDK 21.11 to avoid conflict with Windows
> Sockets
> > > headers.
> > >
> > > The struct rte_param_log2_range should also be renamed to include
> > > "compress" prefix.
> > > But as we break the struct API, it is not an issue I guess.
> > >
> > > > +* cryptodev: ``min`` and ``max`` fields of ``rte_crypto_param_range``
> > > structure
> > > > +  will be renamed in DPDK 21.11 to avoid conflict with Windows
> Sockets
> > > headers.
> > >
> > > Acked-by: Thomas Monjalon <thomas@monjalon.net>
> > >
> > Can we have a local variable named as min/max?
> > If not, then I believe it is not a good idea.
> 
> Yes, except for inline functions in public headers.
> The only problematic one I know of is this (rte_lru_x86.h):
> 
> static inline int
> f_lru_pos(uint64_t lru_list)
> {
> 	__m128i lst = _mm_set_epi64x((uint64_t)-1, lru_list);
> 	__m128i min = _mm_minpos_epu16(lst); /* <<< */
> 	return _mm_extract_epi16(min, 1);
> }
> 
> Fixing it breaks neither API nor ABI, thus no explicit deprecation notice.
OK,
Acked-by: Akhil Goyal <gakhil@marvell.com>

I hope when windows compilation is enabled, it will be part of CI and it will run
on each patch which goes to patchworks.

Re: [dpdk-dev] [EXT] Re: [PATCH v4] doc: announce API changes for Windows compatibility

[Tal Shnaiderman]

> Subject: Re: [dpdk-dev] [EXT] Re: [PATCH v4] doc: announce API changes for
> Windows compatibility
> 
> External email: Use caution opening links or attachments
> 
> 
> > 2021-08-02 12:45 (UTC+0000), Akhil Goyal:
> > > > 21/07/2021 21:55, Dmitry Kozlyuk:
> > > > > Windows headers define `s_addr`, `min`, and `max` as macros.
> > > > > If DPDK headers are included after Windows ones, DPDK structure
> > > > > definitions containing fields with these names get broken
> > > > > (example 1), as well as any usage of such fields (example 2). If
> > > > > DPDK headers undefined these macros, it could break consumer code
> (example 3).
> > > > > It is proposed to rename structure fields in DPDK, because Win32
> > headers
> > > > > are used more widely than DPDK, as a general-purpose platform
> > compared
> > > > > to domain-specific kit, and are harder to fix because of that.
> > > > > Exact new names are left for further discussion.
> > > > >
> > > > > Example 1:
> > > > >
> > > > >     /* in DPDK public header included after windows.h */
> > > > >     struct rte_type {
> > > > >         int min;    /* ERROR: `min` is a macro */
> > > > >     };
> > > > >
> > > > > Example 2:
> > > > >
> > > > >     #include <rte_ether.h>
> > > > >     #include <winsock2.h>
> > > > >     struct rte_ether_hdr eh;
> > > > >     eh.s_addr.addr_bytes[0] = 0;    /* ERROR: `addr_s` is a macro */
> > > > >
> > > > > Example 3:
> > > > >
> > > > >     #include <winsock2.h>
> > > > >     #include <rte_ether.h>
> > > > >     struct in_addr addr;
> > > > >     addr.s_addr = 0;      /* ERROR: there is no `s_addr` field,
> > > > >                              and `s_addr` macro is undefined by
> > > > > DPDK. */
> > > > >
> > > > > Commit 6c068dbd9fea ("net: work around s_addr macro on
> Windows")
> > > > > modified definition of `struct rte_ether_hdr` to avoid the issue.
> > > > > However, the workaround assumes `#define s_addr S_addr.S_un` in
> > > > > Windows headers, which is not a part of official API.
> > > > > It also complicates the definition of `struct rte_ether_hdr`.
> > > > >
> > > > > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > > > > Acked-by: Khoa To <khot@microsoft.com>
> > > > > ---
> > > > > +* net: ``s_addr`` and ``d_addr`` fields of ``rte_ether_hdr``
> > > > > +structure
> > > > > +  will be renamed in DPDK 21.11 to avoid conflict with Windows
> > Sockets
> > > > headers.
> > > > > +
> > > > > +* compressdev: ``min`` and ``max`` fields of
> > > > > +``rte_param_log2_range``
> > > > structure
> > > > > +  will be renamed in DPDK 21.11 to avoid conflict with Windows
> > Sockets
> > > > headers.
> > > >
> > > > The struct rte_param_log2_range should also be renamed to include
> > > > "compress" prefix.
> > > > But as we break the struct API, it is not an issue I guess.
> > > >
> > > > > +* cryptodev: ``min`` and ``max`` fields of
> > > > > +``rte_crypto_param_range``
> > > > structure
> > > > > +  will be renamed in DPDK 21.11 to avoid conflict with Windows
> > Sockets
> > > > headers.
> > > >
> > > > Acked-by: Thomas Monjalon <thomas@monjalon.net>
> > > >
> > > Can we have a local variable named as min/max?
> > > If not, then I believe it is not a good idea.
> >
> > Yes, except for inline functions in public headers.
> > The only problematic one I know of is this (rte_lru_x86.h):
> >
> > static inline int
> > f_lru_pos(uint64_t lru_list)
> > {
> >       __m128i lst = _mm_set_epi64x((uint64_t)-1, lru_list);
> >       __m128i min = _mm_minpos_epu16(lst); /* <<< */
> >       return _mm_extract_epi16(min, 1); }
> >
> > Fixing it breaks neither API nor ABI, thus no explicit deprecation notice.
> OK,
> Acked-by: Akhil Goyal <gakhil@marvell.com>
> 
> I hope when windows compilation is enabled, it will be part of CI and it will
> run on each patch which goes to patchworks.

Windows compilation is already part of CI in ci/iol-testing and ci/Intel-compilation.

Re: [dpdk-dev] [EXT] Re: [PATCH v4] doc: announce API changes for Windows compatibility

[Thomas Monjalon]

02/08/2021 15:48, Akhil Goyal:
> > 2021-08-02 12:45 (UTC+0000), Akhil Goyal:
> > > > 21/07/2021 21:55, Dmitry Kozlyuk:
> > > > > Windows headers define `s_addr`, `min`, and `max` as macros.
> > > > > If DPDK headers are included after Windows ones, DPDK structure
> > > > > definitions containing fields with these names get broken (example 1),
> > > > > as well as any usage of such fields (example 2). If DPDK headers
> > > > > undefined these macros, it could break consumer code (example 3).
> > > > > It is proposed to rename structure fields in DPDK, because Win32
> > headers
> > > > > are used more widely than DPDK, as a general-purpose platform
> > compared
> > > > > to domain-specific kit, and are harder to fix because of that.
> > > > > Exact new names are left for further discussion.
> > > > >
> > > > > Example 1:
> > > > >
> > > > >     /* in DPDK public header included after windows.h */
> > > > >     struct rte_type {
> > > > >         int min;    /* ERROR: `min` is a macro */
> > > > >     };
> > > > >
> > > > > Example 2:
> > > > >
> > > > >     #include <rte_ether.h>
> > > > >     #include <winsock2.h>
> > > > >     struct rte_ether_hdr eh;
> > > > >     eh.s_addr.addr_bytes[0] = 0;    /* ERROR: `addr_s` is a macro */
> > > > >
> > > > > Example 3:
> > > > >
> > > > >     #include <winsock2.h>
> > > > >     #include <rte_ether.h>
> > > > >     struct in_addr addr;
> > > > >     addr.s_addr = 0;      /* ERROR: there is no `s_addr` field,
> > > > >                              and `s_addr` macro is undefined by DPDK. */
> > > > >
> > > > > Commit 6c068dbd9fea ("net: work around s_addr macro on Windows")
> > > > > modified definition of `struct rte_ether_hdr` to avoid the issue.
> > > > > However, the workaround assumes `#define s_addr S_addr.S_un`
> > > > > in Windows headers, which is not a part of official API.
> > > > > It also complicates the definition of `struct rte_ether_hdr`.
> > > > >
> > > > > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > > > > Acked-by: Khoa To <khot@microsoft.com>
[...]
> > > > Acked-by: Thomas Monjalon <thomas@monjalon.net>
> > > >
> > > Can we have a local variable named as min/max?
> > > If not, then I believe it is not a good idea.
> > 
> > Yes, except for inline functions in public headers.
> > The only problematic one I know of is this (rte_lru_x86.h):
> > 
> > static inline int
> > f_lru_pos(uint64_t lru_list)
> > {
> > 	__m128i lst = _mm_set_epi64x((uint64_t)-1, lru_list);
> > 	__m128i min = _mm_minpos_epu16(lst); /* <<< */
> > 	return _mm_extract_epi16(min, 1);
> > }
> > 
> > Fixing it breaks neither API nor ABI, thus no explicit deprecation notice.
> OK,
> Acked-by: Akhil Goyal <gakhil@marvell.com>

Applied, thanks.