[v2] doc: announce API changes for Windows compatibility

  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(+)
  

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

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.
  

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.
  

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.