DPDK patches and discussions
 help / color / mirror / Atom feed
From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
To: Nick Connolly <nick.connolly@mayadata.io>
Cc: Ferruh Yigit <ferruh.yigit@intel.com>,
	dev@dpdk.org, Tyler Retzlaff <roretzla@microsoft.com>,
	Mike Wells <mike.wells@telchemy.com>,
	Narcisa Ana Maria Vasile <navasile@linux.microsoft.com>,
	Dmitry Malloy <dmitrym@microsoft.com>,
	Pallavi Kadam <pallavi.kadam@intel.com>,
	Jie Zhou <jizh@linux.microsoft.com>
Subject: Re: [dpdk-dev] [PATCH v2 4/6] net/pcap: add libpcap wrappers
Date: Fri, 26 Feb 2021 02:10:01 +0300	[thread overview]
Message-ID: <20210226021001.73829588@sovereign> (raw)
In-Reply-To: <ac14b4fc-7e0a-f3f5-a75d-65bc6e8f90fa@mayadata.io>

2021-02-25 20:31, Nick Connolly:
> > Your comment made me revise Windows EAL networking shims. Surprisingly, if
> > changed to expose Windows networking headers (specifically, <ws2tcpip.h>) with
> > some additions (but no hacks), they create no issues to any existing code.
> >
> > The only workaround remaining is `#undef s_addr` in <rte_ether.h>.
> >
> > So maybe this commit can be dropped, if Windows EAL networking headers be
> > reworked in the following way:
> >
> > 	#if defined min
> > 	#define USER_EXPLICITLY_WANTS_WINDOWS_H
> > 	#endif
> >
> > 	#include <ws2tcpip.h>
> >
> > 	/* hide definitions that break portable code,
> > 	 * e.g. it had to be done once already for i40e
> > 	 */
> > 	#ifndef USER_EXPLICITLY_WANTS_WINDOWS_H
> > 	#undef min, max, ...
> > 	#endif
> >
> > 	#define what's missing from Windows headers, e.g. IPPROTO_SCTP
> >
> > + Windows maintainers, Nick Connolly, and Jie Zhou to discuss.  
> In my opinion, there are long term maintenance issues either way round.
> 
> Wrapping the system headers hides the details and keeps the code 'clean',
> but has to work with multiple compiler environments and versions of the
> headers - not always straightforward. 

You're probably right.
We had an exchange with Tyler, let me quote:

[DmitryK]
> I'm pretty sure no DPDK header really needs anything beyond standard C,
> but one exception: intrinsic functions for vector instructions.
> Struct in_addr? An oversight, there's rte_ether_addr, should be rte_ip_addr.

Here's a complete list of offending files included from public headers,
with comments on usage:

POSIX

    netinet/in.h
    netinet/ip6.h
    netinet/ip.h
	cmdline:  struct in_addr, struct in6_addr, 6 constants
	net:      ditto
	security: ditto
    pthread.h
	eal:	not used
	ethdev:	pthread_mutex_t flow_ops_mutex;
    sched.h
	eal:       cpu_set_t
	telemetry: rte_cpuset_t (not used?)
    sys/types.h
	ehtdev:   not needed (FILE?)
	net:      not needed
	mbuf:     ditto
	pci:      ditto
	security: ditto
	sched:    ditto

Unix

    sys/queue.h
	(acl, eal, hash, lpm, mempool, pci, ring, table,
	 bus/{fslmc,pci,vdev,vmbus}, net/{memif,tap})
		multiprocessing

Thread-related stuff will go away as rte_thread.h expands.
Network headers are not much needed, as you can see above.
Other headers mostly not needed or can be replaced with standard ones.

Replacing `in_addr` with and `rte_ip_addr` will break API, but not ABI,
because it's essentially the same thing. We can define new types
conditionally, as it was done for struct cmdline, if need be.

Complete removal of non-standard dependencies in headers is within a grasp.
Then we can remove shims and include whatever needed.
Thoughts?

  reply	other threads:[~2021-02-25 23:10 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-14  1:20 [dpdk-dev] [PATCH 0/6] net/pcap: build on Windows Dmitry Kozlyuk
2021-02-14  1:20 ` [dpdk-dev] [PATCH 1/6] eal: add internal API for current time Dmitry Kozlyuk
2021-03-01 22:31   ` Nick Connolly
2021-03-01 22:36     ` Nick Connolly
2021-02-14  1:20 ` [dpdk-dev] [PATCH 2/6] net/pcap: fix format string Dmitry Kozlyuk
2021-03-01 22:33   ` Nick Connolly
2021-02-14  1:20 ` [dpdk-dev] [PATCH 3/6] net/pcap: move OS-dependent code to separate files Dmitry Kozlyuk
2021-02-14  1:20 ` [dpdk-dev] [PATCH 4/6] net/pcap: add libpcap wrappers Dmitry Kozlyuk
2021-02-14  1:20 ` [dpdk-dev] [PATCH 5/6] config: discover libpcap on Windows Dmitry Kozlyuk
2021-02-14  1:20 ` [dpdk-dev] [PATCH 6/6] net/pcap: build " Dmitry Kozlyuk
2021-02-14  2:16 ` [dpdk-dev] [PATCH v2 0/6] " Dmitry Kozlyuk
2021-02-14  2:16   ` [dpdk-dev] [PATCH v2 1/6] eal: add internal API for current time Dmitry Kozlyuk
2021-03-01 22:39     ` Nick Connolly
2021-03-05 17:50     ` Jie Zhou
2021-03-16  9:18     ` Thomas Monjalon
2021-03-16 18:59     ` Stephen Hemminger
2021-03-16 20:07       ` Dmitry Kozlyuk
2021-03-17  9:50         ` Morten Brørup
2021-02-14  2:16   ` [dpdk-dev] [PATCH v2 2/6] net/pcap: fix format string Dmitry Kozlyuk
2021-02-25 14:45     ` Ferruh Yigit
2021-03-02 11:48       ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2021-02-14  2:16   ` [dpdk-dev] [PATCH v2 3/6] net/pcap: move OS-dependent code to separate files Dmitry Kozlyuk
2021-02-25 14:51     ` Ferruh Yigit
2021-02-25 16:05       ` Dmitry Kozlyuk
2021-02-14  2:16   ` [dpdk-dev] [PATCH v2 4/6] net/pcap: add libpcap wrappers Dmitry Kozlyuk
2021-02-25 14:59     ` Ferruh Yigit
2021-02-25 19:04       ` Dmitry Kozlyuk
2021-02-25 20:31         ` Nick Connolly
2021-02-25 23:10           ` Dmitry Kozlyuk [this message]
2021-03-01 21:43             ` Nick Connolly
2021-03-01 23:05               ` Dmitry Kozlyuk
2021-03-01 23:23                 ` Dmitry Kozlyuk
2021-03-02 11:22                 ` Nick Connolly
2021-03-03 16:32                   ` Dmitry Kozlyuk
2021-03-03 16:47                     ` Ferruh Yigit
2021-03-03 18:19                       ` Dmitry Kozlyuk
2021-03-03 19:30                         ` Ferruh Yigit
2021-03-03 23:03                           ` Dmitry Kozlyuk
2021-02-14  2:16   ` [dpdk-dev] [PATCH v2 5/6] config: discover libpcap on Windows Dmitry Kozlyuk
2021-02-25 15:02     ` Ferruh Yigit
2021-02-25 16:04       ` Dmitry Kozlyuk
2021-02-25 16:33         ` Bruce Richardson
2021-02-25 17:42           ` Dmitry Kozlyuk
2021-03-16  9:16             ` Thomas Monjalon
2021-03-16  9:37               ` Dmitry Kozlyuk
2021-02-14  2:16   ` [dpdk-dev] [PATCH v2 6/6] net/pcap: build " Dmitry Kozlyuk
2021-03-24  0:50   ` [dpdk-dev] [PATCH v3 0/3] " Dmitry Kozlyuk
2021-03-24  0:50     ` [dpdk-dev] [PATCH v3 1/3] eal/windows: add timespec_get shim for MinGW Dmitry Kozlyuk
2021-03-24  0:50     ` [dpdk-dev] [PATCH v3 2/3] net/pcap: move OS-dependent code to separate files Dmitry Kozlyuk
2021-03-24  0:50     ` [dpdk-dev] [PATCH v3 3/3] net/pcap: build on Windows Dmitry Kozlyuk
2021-04-09 10:51     ` [dpdk-dev] [PATCH v3 0/3] " Ferruh Yigit
2021-04-09 11:03       ` Dmitry Kozlyuk
2021-04-09 11:24         ` Ferruh Yigit
2021-04-15 22:10     ` [dpdk-dev] [PATCH v4 " Dmitry Kozlyuk
2021-04-15 22:10       ` [dpdk-dev] [PATCH v4 1/3] eal/windows: add timespec_get shim for MinGW Dmitry Kozlyuk
2021-04-15 22:10       ` [dpdk-dev] [PATCH v4 2/3] net/pcap: move OS-dependent code to separate files Dmitry Kozlyuk
2021-04-15 22:10       ` [dpdk-dev] [PATCH v4 3/3] net/pcap: build on Windows Dmitry Kozlyuk
2021-04-19 21:05         ` Tyler Retzlaff
2021-04-16 17:22       ` [dpdk-dev] [PATCH v4 0/3] " Ferruh Yigit
2021-04-20 22:20       ` Thomas Monjalon
2021-04-21 14:53         ` Dmitry Kozlyuk
2021-04-21 18:12           ` Thomas Monjalon
2021-04-21 19:33       ` [dpdk-dev] [PATCH v5 " Dmitry Kozlyuk
2021-04-21 19:33         ` [dpdk-dev] [PATCH v5 1/3] eal: add timespec_get shim Dmitry Kozlyuk
2021-04-21 19:33         ` [dpdk-dev] [PATCH v5 2/3] net/pcap: move OS-dependent code to separate files Dmitry Kozlyuk
2021-04-21 19:33         ` [dpdk-dev] [PATCH v5 3/3] net/pcap: build on Windows Dmitry Kozlyuk
2021-04-21 21:54         ` [dpdk-dev] [PATCH v5 0/3] " Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210226021001.73829588@sovereign \
    --to=dmitry.kozliuk@gmail.com \
    --cc=dev@dpdk.org \
    --cc=dmitrym@microsoft.com \
    --cc=ferruh.yigit@intel.com \
    --cc=jizh@linux.microsoft.com \
    --cc=mike.wells@telchemy.com \
    --cc=navasile@linux.microsoft.com \
    --cc=nick.connolly@mayadata.io \
    --cc=pallavi.kadam@intel.com \
    --cc=roretzla@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).