Re: [dpdk-dev] [PATCH v2 4/6] net/pcap: add libpcap wrappers

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

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?