[dpdk-dev] [PATCH v2 4/6] net/pcap: add libpcap wrappers
Ferruh Yigit
ferruh.yigit at intel.com
Wed Mar 3 20:30:11 CET 2021
On 3/3/2021 6:19 PM, Dmitry Kozlyuk wrote:
> 2021-03-03 16:47, Ferruh Yigit:
>> On 3/3/2021 4:32 PM, Dmitry Kozlyuk wrote:
> [...]
>>> If we can't help including <ws2tcpip.h>/<netinet/ip.h> from public headers,
>>> might as well use `struct in_addr`, just replace `#include <netinet/ip.h>`
>>> with `#include <rte_ip.h>` everywhere.
>>>
>>> The only remaining issue will be `s_addr` macro on Windows that conflicts with
>>> `struct rte_ether_addr` field `s_addr`. I don't know a better solution than
>>> renaming `s_addr` to `src_addr` in DPDK (OTOH, it will be consistent with
>>> `struct rte_ipvX_hdr` field naming).
>>>
>>> Ferruh, what do you think?
>>>
>>
>> No problem on the chosen name, that will work fine, but the 'struct
>> rte_eth_addr' is public struct, we can't rename it without breaking the user
>> applications.
>>
>> Still it is possible to rename public struct, but it is a long process, need to
>> send a deprecation notice and wait for next LTS, 21.11.
>>
>> Ethernet header already has following, doesn't it help:
>> #undef s_addr /* Defined in winsock2.h included in windows.h. */
>
> It helps until you do the following:
>
> #include <winsock2.h> /* #define s_addr S_un.S_addr */
> #include <rte_ether.h> /* #undef s_addr */
>
> struct in_addr a;
> a.s_addr = 0; /* ERROR, `s_addr` has been defined to do that */
>
> Or the following:
>
> #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 */
>
> If you swap include order in each case, it compiles, i.e . code is fragile.
> Shims redefine `struct in_addr` and we're trying to get rid of them anyway.
>
Got it, thanks for clarification.
>
> Here's a solution that always works, however ugly it looks. It defines
> `struct rte_ether_hdr` for Windows such that `s_addr` macro works for it if
> defined. We can keep it until 21.11, then remove it and rename fields. In
> return, we can remove networking shims and workarounds immediately.
>
Agree it looks ugly :), but I am OK with above plan.
You need to send a deprecation notice for this. We can discuss there if 'd_addr'
also should be renamed for consistency, or leave with the workaround instead of
rename ...
> `S_un.S_addr` part of `struct in_addr` is documented, and thus is unlikely to
> ever change:
>
> https://docs.microsoft.com/en-us/windows/win32/api/winsock2/ns-winsock2-in_addr
>
>
> diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
> index 060b63fc9..67d340bb2 100644
> --- a/lib/librte_net/rte_ether.h
> +++ b/lib/librte_net/rte_ether.h
> @@ -23,10 +23,6 @@ extern "C" {
> #include <rte_mbuf.h>
> #include <rte_byteorder.h>
>
> -#ifdef RTE_EXEC_ENV_WINDOWS /* Workaround conflict with rte_ether_hdr. */
> -#undef s_addr /* Defined in winsock2.h included in windows.h. */
> -#endif
> -
> #define RTE_ETHER_ADDR_LEN 6 /**< Length of Ethernet address. */
> #define RTE_ETHER_TYPE_LEN 2 /**< Length of Ethernet type field. */
> #define RTE_ETHER_CRC_LEN 4 /**< Length of Ethernet CRC. */
> @@ -257,6 +253,8 @@ __rte_experimental
> int
> rte_ether_unformat_addr(const char *str, struct rte_ether_addr *eth_addr);
>
> +#if !defined RTE_EXEC_ENV_WINDOWS || defined __DOXYGEN__
> +
> /**
> * Ethernet header: Contains the destination address, source address
> * and frame type.
> @@ -267,6 +265,28 @@ struct rte_ether_hdr {
> uint16_t ether_type; /**< Frame type. */
> } __rte_aligned(2);
>
> +#else
> +
> +#pragma push_macro("s_addr")
> +#ifdef s_addr
> +#undef s_addr
> +#endif
I guess this needs to be as following, in case this header included before the
windows header:
#ifdef s_addr
#pragma push_macro("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;
> + } S_addr;
> + };
> + uint16_t ether_type; /**< Frame type. */
> +} __rte_aligned(2);
> +
> +#pragma pop_macro("s_addr")
> +#endif
> +
> /**
> * Ethernet VLAN Header.
> * Contains the 16-bit VLAN Tag Control Identifier and the Ethernet type
>
More information about the dev
mailing list