[v3] doc: announce API changes for Windows compatibility
Checks
Commit Message
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(+)
Comments
> 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
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.
>
> 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.
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.
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.
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.
@@ -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.