mbox series

[v9,0/4] eal/windows: do not expose POSIX symbols

Message ID 20210410224732.20234-1-dmitry.kozliuk@gmail.com (mailing list archive)
Headers
Series eal/windows: do not expose POSIX symbols |

Message

Dmitry Kozlyuk April 10, 2021, 10:47 p.m. UTC
  On Windows, EAL contains two sets of functions and macros for POSIX
compatibility: <rte_os.h> and a networking shim (socket headers).
The latter conflicts with system headers and should not exist.
Exposing the former from EAL can break consumer own POSIX compatibility
layer and is against standards in general. Hide these symbols from
external consumers, while keeping them available for DPDK code.

v9:
    * Fix missing <rte_os_shim.h> include in rte_common_log.c.
      (This will happen again. Going to add a checkpatch test
       after this series is merged.)
v8:
    * Drop rte_thread_sleep API, use rte_delay_us_sleep (Morten Brørup).

Dmitry Kozlyuk (4):
  eal/windows: hide asprintf() shim
  eal: make OS shims internal
  net: work around s_addr macro on Windows
  net: provide IP-related API on any OS

 drivers/bus/pci/private.h                    |  4 +-
 drivers/bus/vdev/vdev_private.h              |  2 +
 drivers/common/mlx5/mlx5_common.h            |  1 +
 drivers/net/i40e/i40e_ethdev.c               |  1 +
 drivers/net/i40e/i40e_fdir.c                 |  1 +
 drivers/net/mlx5/mlx5.h                      |  1 -
 drivers/net/mlx5/mlx5_flow.c                 |  4 +-
 drivers/net/mlx5/mlx5_flow.h                 |  3 +-
 drivers/net/mlx5/mlx5_mac.c                  |  1 -
 examples/cmdline/commands.c                  |  5 --
 examples/cmdline/parse_obj_list.c            |  2 -
 lib/librte_cmdline/cmdline.c                 |  5 --
 lib/librte_cmdline/cmdline_os_windows.c      |  2 -
 lib/librte_cmdline/cmdline_parse.c           |  2 -
 lib/librte_cmdline/cmdline_parse_etheraddr.c |  6 --
 lib/librte_cmdline/cmdline_parse_ipaddr.c    |  6 --
 lib/librte_cmdline/cmdline_parse_ipaddr.h    |  2 +-
 lib/librte_cmdline/cmdline_private.h         |  1 +
 lib/librte_cmdline/cmdline_socket.c          |  4 -
 lib/librte_eal/common/eal_common_config.c    |  1 -
 lib/librte_eal/common/eal_common_errno.c     |  4 +
 lib/librte_eal/common/eal_common_log.c       |  1 +
 lib/librte_eal/common/eal_common_options.c   |  2 +-
 lib/librte_eal/common/eal_common_timer.c     |  4 +-
 lib/librte_eal/common/eal_internal_cfg.h     |  1 +
 lib/librte_eal/common/eal_private.h          | 11 +++
 lib/librte_eal/freebsd/include/rte_os_shim.h | 14 +++
 lib/librte_eal/linux/include/rte_os_shim.h   | 14 +++
 lib/librte_eal/windows/eal.c                 | 30 +++++++
 lib/librte_eal/windows/eal_hugepages.c       |  1 -
 lib/librte_eal/windows/eal_lcore.c           |  1 -
 lib/librte_eal/windows/eal_memalloc.c        |  1 -
 lib/librte_eal/windows/include/arpa/inet.h   | 30 -------
 lib/librte_eal/windows/include/netinet/in.h  | 38 --------
 lib/librte_eal/windows/include/netinet/ip.h  | 10 ---
 lib/librte_eal/windows/include/rte_os.h      | 92 +-------------------
 lib/librte_eal/windows/include/rte_os_shim.h | 36 ++++++++
 lib/librte_eal/windows/include/sys/socket.h  | 24 -----
 lib/librte_ethdev/ethdev_private.h           |  2 +
 lib/librte_ethdev/rte_ethdev.c               | 12 +--
 lib/librte_ethdev/rte_ethdev_core.h          |  1 -
 lib/librte_kvargs/rte_kvargs.c               |  1 +
 lib/librte_net/rte_ether.h                   | 26 ++++--
 lib/librte_net/rte_ip.h                      |  7 ++
 lib/librte_net/rte_net.c                     |  1 +
 45 files changed, 165 insertions(+), 253 deletions(-)
 create mode 100644 lib/librte_eal/freebsd/include/rte_os_shim.h
 create mode 100644 lib/librte_eal/linux/include/rte_os_shim.h
 delete mode 100644 lib/librte_eal/windows/include/arpa/inet.h
 delete mode 100644 lib/librte_eal/windows/include/netinet/in.h
 delete mode 100644 lib/librte_eal/windows/include/netinet/ip.h
 create mode 100644 lib/librte_eal/windows/include/rte_os_shim.h
 delete mode 100644 lib/librte_eal/windows/include/sys/socket.h
  

Comments

Menon, Ranjit April 13, 2021, 4:46 a.m. UTC | #1
Hi Dmitry,

On 4/10/2021 3:47 PM, Dmitry Kozlyuk wrote:
> On Windows, EAL contains two sets of functions and macros for POSIX
> compatibility: <rte_os.h> and a networking shim (socket headers).
> The latter conflicts with system headers and should not exist.
> Exposing the former from EAL can break consumer own POSIX compatibility
> layer and is against standards in general. Hide these symbols from
> external consumers, while keeping them available for DPDK code.
>
> v9:
>      * Fix missing <rte_os_shim.h> include in rte_common_log.c.
>        (This will happen again. Going to add a checkpatch test
>         after this series is merged.)
> v8:
>      * Drop rte_thread_sleep API, use rte_delay_us_sleep (Morten Brørup).
>
> Dmitry Kozlyuk (4):
>    eal/windows: hide asprintf() shim
>    eal: make OS shims internal
>    net: work around s_addr macro on Windows
>    net: provide IP-related API on any OS
>
> <Snip>

The change to remove the networking shim breaks l2fwd compilation on 
Windows, since l2fwd/main.c includes netinet/in.h explicitly.

How do you propose we fix this, only for Windows?

ranjit m.
  
Dmitry Kozlyuk April 13, 2021, 7 a.m. UTC | #2
Hi Ranjit,

2021-04-12 21:46 (UTC-0700), Ranjit Menon:
[...]
> The change to remove the networking shim breaks l2fwd compilation on 
> Windows, since l2fwd/main.c includes netinet/in.h explicitly.
> 
> How do you propose we fix this, only for Windows?

This include is redundant for this file on all platforms, it can be removed.
Since -Dexamples=all doesn't work on Windows because of missing dependencies,
I wonder which of them need fixing.
  
Thomas Monjalon April 14, 2021, 9:12 p.m. UTC | #3
13/04/2021 09:00, Dmitry Kozlyuk:
> Hi Ranjit,
> 
> 2021-04-12 21:46 (UTC-0700), Ranjit Menon:
> [...]
> > The change to remove the networking shim breaks l2fwd compilation on 
> > Windows, since l2fwd/main.c includes netinet/in.h explicitly.
> > 
> > How do you propose we fix this, only for Windows?
> 
> This include is redundant for this file on all platforms, it can be removed.
> Since -Dexamples=all doesn't work on Windows because of missing dependencies,
> I wonder which of them need fixing.

Let's fix the examples which are supported on Windows.
Other examples may require more updates anyway.
  
Menon, Ranjit April 14, 2021, 9:34 p.m. UTC | #4
On 4/14/2021 2:12 PM, Thomas Monjalon wrote:
> 13/04/2021 09:00, Dmitry Kozlyuk:
>> Hi Ranjit,
>>
>> 2021-04-12 21:46 (UTC-0700), Ranjit Menon:
>> [...]
>>> The change to remove the networking shim breaks l2fwd compilation on
>>> Windows, since l2fwd/main.c includes netinet/in.h explicitly.
>>>
>>> How do you propose we fix this, only for Windows?
>> This include is redundant for this file on all platforms, it can be removed.
>> Since -Dexamples=all doesn't work on Windows because of missing dependencies,
>> I wonder which of them need fixing.
> Let's fix the examples which are supported on Windows.
> Other examples may require more updates anyway.
>
Thanks, Thomas. For now, this is only required in l2fwd.

Dmitry, can you please include this in your patch 4/4?

thanks,

ranjit m.

>
  
Thomas Monjalon April 14, 2021, 9:42 p.m. UTC | #5
14/04/2021 23:34, Ranjit Menon:
> On 4/14/2021 2:12 PM, Thomas Monjalon wrote:
> > 13/04/2021 09:00, Dmitry Kozlyuk:
> >> Hi Ranjit,
> >>
> >> 2021-04-12 21:46 (UTC-0700), Ranjit Menon:
> >> [...]
> >>> The change to remove the networking shim breaks l2fwd compilation on
> >>> Windows, since l2fwd/main.c includes netinet/in.h explicitly.
> >>>
> >>> How do you propose we fix this, only for Windows?
> >> This include is redundant for this file on all platforms, it can be removed.
> >> Since -Dexamples=all doesn't work on Windows because of missing dependencies,
> >> I wonder which of them need fixing.
> > Let's fix the examples which are supported on Windows.
> > Other examples may require more updates anyway.
> >
> Thanks, Thomas. For now, this is only required in l2fwd.

Only l2fwd is supported on Windows?

> Dmitry, can you please include this in your patch 4/4?

Ranjit, if you tell me what exactly is needed, I can do it
and merge the series quickly.
  
Menon, Ranjit April 14, 2021, 9:47 p.m. UTC | #6
On 4/14/2021 2:42 PM, Thomas Monjalon wrote:
> 14/04/2021 23:34, Ranjit Menon:
>> On 4/14/2021 2:12 PM, Thomas Monjalon wrote:
>>> 13/04/2021 09:00, Dmitry Kozlyuk:
>>>> Hi Ranjit,
>>>>
>>>> 2021-04-12 21:46 (UTC-0700), Ranjit Menon:
>>>> [...]
>>>>> The change to remove the networking shim breaks l2fwd compilation on
>>>>> Windows, since l2fwd/main.c includes netinet/in.h explicitly.
>>>>>
>>>>> How do you propose we fix this, only for Windows?
>>>> This include is redundant for this file on all platforms, it can be removed.
>>>> Since -Dexamples=all doesn't work on Windows because of missing dependencies,
>>>> I wonder which of them need fixing.
>>> Let's fix the examples which are supported on Windows.
>>> Other examples may require more updates anyway.
>>>
>> Thanks, Thomas. For now, this is only required in l2fwd.
> Only l2fwd is supported on Windows?
>
>> Dmitry, can you please include this in your patch 4/4?
> Ranjit, if you tell me what exactly is needed, I can do it
> and merge the series quickly.

Sure, Thomas. In l2wfd/main.c, all we need to do is remove the #include 
<netinet/in.h> line.

This include file will not exist on Windows anymore, and Dmitry 
determined that this include is not required in l2fwd on all platforms.


ranjit m.

>
  
Dmitry Kozlyuk April 14, 2021, 10:08 p.m. UTC | #7
2021-04-14 14:47 (UTC-0700), Ranjit Menon:
> On 4/14/2021 2:42 PM, Thomas Monjalon wrote:
> > 14/04/2021 23:34, Ranjit Menon:  
> >> On 4/14/2021 2:12 PM, Thomas Monjalon wrote:  
> >>> 13/04/2021 09:00, Dmitry Kozlyuk:  
> >>>> Hi Ranjit,
> >>>>
> >>>> 2021-04-12 21:46 (UTC-0700), Ranjit Menon:
> >>>> [...]  
> >>>>> The change to remove the networking shim breaks l2fwd compilation on
> >>>>> Windows, since l2fwd/main.c includes netinet/in.h explicitly.
> >>>>>
> >>>>> How do you propose we fix this, only for Windows?  
> >>>> This include is redundant for this file on all platforms, it can be removed.
> >>>> Since -Dexamples=all doesn't work on Windows because of missing dependencies,
> >>>> I wonder which of them need fixing.  
> >>> Let's fix the examples which are supported on Windows.
> >>> Other examples may require more updates anyway.
> >>>  
> >> Thanks, Thomas. For now, this is only required in l2fwd.  
> > Only l2fwd is supported on Windows?
> >  
> >> Dmitry, can you please include this in your patch 4/4?  
> > Ranjit, if you tell me what exactly is needed, I can do it
> > and merge the series quickly.  

I've just sent v10 with all required fixes.

> Sure, Thomas. In l2wfd/main.c, all we need to do is remove the #include 
> <netinet/in.h> line.
> 
> This include file will not exist on Windows anymore, and Dmitry 
> determined that this include is not required in l2fwd on all platforms.

For the reference, complete list of examples that can build for Windows:

	helloworld
	cmdline
	flow_filtering
	ipv4_multicast
	l2fwd
	link_status_interrupt
	qos_meter
	rxtx_callbacks (-Wformat with clang)
	service_cores
	skeleton
  
Thomas Monjalon April 14, 2021, 10:58 p.m. UTC | #8
15/04/2021 00:08, Dmitry Kozlyuk:
> 2021-04-14 14:47 (UTC-0700), Ranjit Menon:
> > On 4/14/2021 2:42 PM, Thomas Monjalon wrote:
> > > 14/04/2021 23:34, Ranjit Menon:  
> > >> On 4/14/2021 2:12 PM, Thomas Monjalon wrote:  
> > >>> 13/04/2021 09:00, Dmitry Kozlyuk:  
> > >>>> Hi Ranjit,
> > >>>>
> > >>>> 2021-04-12 21:46 (UTC-0700), Ranjit Menon:
> > >>>> [...]  
> > >>>>> The change to remove the networking shim breaks l2fwd compilation on
> > >>>>> Windows, since l2fwd/main.c includes netinet/in.h explicitly.
> > >>>>>
> > >>>>> How do you propose we fix this, only for Windows?  
> > >>>> This include is redundant for this file on all platforms, it can be removed.
> > >>>> Since -Dexamples=all doesn't work on Windows because of missing dependencies,
> > >>>> I wonder which of them need fixing.  
> > >>> Let's fix the examples which are supported on Windows.
> > >>> Other examples may require more updates anyway.
> > >>>  
> > >> Thanks, Thomas. For now, this is only required in l2fwd.  
> > > Only l2fwd is supported on Windows?
> > >  
> > >> Dmitry, can you please include this in your patch 4/4?  
> > > Ranjit, if you tell me what exactly is needed, I can do it
> > > and merge the series quickly.  
> 
> I've just sent v10 with all required fixes.

Thank you
Some acks were removed from this v10, I am re-adding them.

> > Sure, Thomas. In l2wfd/main.c, all we need to do is remove the #include 
> > <netinet/in.h> line.
> > 
> > This include file will not exist on Windows anymore, and Dmitry 
> > determined that this include is not required in l2fwd on all platforms.
> 
> For the reference, complete list of examples that can build for Windows:
> 
> 	helloworld
> 	cmdline
> 	flow_filtering
> 	ipv4_multicast
> 	l2fwd
> 	link_status_interrupt
> 	qos_meter
> 	rxtx_callbacks (-Wformat with clang)
> 	service_cores
> 	skeleton

devtools/test-meson-builds.sh should be updated to compile these examples
with MinGW.