the-tcpdump-group / libpcap

the LIBpcap interface to various kernel packet capture mechanism
https://www.tcpdump.org/
Other
2.64k stars 839 forks source link

Compiling errors after updating dpdk to version higher then v21.11-rc1 #1159

Open Emanuesson opened 1 year ago

Emanuesson commented 1 year ago

Due to changes in the shipped headers by DPDK, especially within the following commit, libpcap became incompatible to compile against dpdk: https://github.com/DPDK/dpdk/commit/295968d1740760337e16b0d7914875c5cac52850

How to reproduce:

mkdir build && cd build
cmake .. && make

Reproduced with the following versions:

$ git describe --tags --always
 libpcap-1.10-bp-695-g8cb3bac2
$ uname -a
 Linux pc1 6.1.8-arch1-1 #1 SMP PREEMPT_DYNAMIC Tue, 24 Jan 2023 21:07:04 +0000 x86_64 GNU/Linux
$ gcc -v
 gcc version 12.2.1 20230111 (GCC) 
$ cmake --version
 cmake version 3.25.2
$ pkg-config --modversion libdpdk
 22.11.1

The following compiler-errors are visible during the build:

/libpcap/pcap-dpdk.c:202:18: error: ‘struct rte_eth_rxmode’ has no member named ‘split_hdr_size’
  202 |                 .split_hdr_size = 0,
      |                  ^~~~~~~~~~~~~~
/libpcap/pcap-dpdk.c:205:28: error: ‘ETH_MQ_TX_NONE’ undeclared here (not in a function); did you mean ‘RTE_ETH_MQ_TX_NONE’?
  205 |                 .mq_mode = ETH_MQ_TX_NONE,
      |                            ^~~~~~~~~~~~~~
      |                            RTE_ETH_MQ_TX_NONE
/libpcap/pcap-dpdk.c: In function ‘check_link_status’:
/libpcap/pcap-dpdk.c:502:38: error: ‘ETH_LINK_UP’ undeclared (first use in this function); did you mean ‘RTE_ETH_LINK_UP’?
  502 |         return plink->link_status == ETH_LINK_UP;
      |                                      ^~~~~~~~~~~
      |                                      RTE_ETH_LINK_UP
/libpcap/pcap-dpdk.c:502:38: note: each undeclared identifier is reported only once for each function it appears in
/libpcap/pcap-dpdk.c: In function ‘pcap_dpdk_activate’:
/libpcap/pcap-dpdk.c:838:48: error: ‘DEV_TX_OFFLOAD_MBUF_FAST_FREE’ undeclared (first use in this function); did you mean ‘RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE’?
  838 |                 if (dev_info.tx_offload_capa & DEV_TX_OFFLOAD_MBUF_FAST_FREE)
      |                                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                                RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE
In file included from /libpcap/pcap-dpdk.c:97:
/libpcap/pcap-dpdk.c:975:62: error: ‘ETH_LINK_FULL_DUPLEX’ undeclared (first use in this function); did you mean ‘RTE_ETH_LINK_FULL_DUPLEX’?
  975 |                                         (link.link_duplex == ETH_LINK_FULL_DUPLEX) ?
      |                                                              ^~~~~~~~~~~~~~~~~~~~
make[2]: *** [CMakeFiles/pcap.dir/build.make:398: CMakeFiles/pcap.dir/pcap-dpdk.c.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:161: CMakeFiles/pcap.dir/all] Error 2
make: *** [Makefile:136: all] Error 2
infrastation commented 1 year ago

"Add 'RTE_ETH' namespace to all enums & macros in a backward compatible way."

A more correct way to describe this change would be "use a prefix in names", not "add a namespace". But anyway. It might help to understand which particular part of this change is backward-compatible.

Emanuesson commented 1 year ago

I was also surprised by such an extensive API-change in a project like DPDK. However, I'm just "affected" as well and have no real explanation or intention available why this change was necessary or what incompatibilities come along with this change. The DPDK-Changelog may help here.

guyharris commented 1 year ago

Add 'RTE_ETH' namespace to all enums & macros in a backward compatible way. The macros for backward compatibility can be removed in next LTS. Also updated some struct names to have 'rte_eth' prefix.

I guess part of the contract between DPDK developers and developers of code that uses DPDK "APIs" is "the DPDK developers reserve the right to make incompatible changes, or to make "compatible" changes and then remove the compatibility in a future release, and the developers will say "thank you, sir, may I have another?" and change their code every time we decide to change out "APIs"."

This is not a contract that I would sign if I had a choice about it; I guess the DPDK developers don't want developers unwilling to put up with that. (The DPDK code in libpcap was dropped in from a helicopter; we're now kinda stuck with it. At this point, I view it as a strong argument for providing a plugin mechanism in libpcap - albeit one that limits the locations from which plugins can be loaded, for security reasons, given that, unfortunately, a number of capture mechanisms requite root privilege - and leave it up to Somebody Else, possibly the DPDK developers themselves, to keep it working.)

(The libpcap contract, at least to the extent that I can enforce it, includes "the only way in which we will break the API or the ABI intentionally are maybe to drop older interfaces that have been marked as deprecated for a long time"; so far, I don't think we've dropped any older interfaces yet.)

Corgile commented 8 months ago

solutions?

guyharris commented 8 months ago

solutions?

Somebody with DPDK experience (hint: not me) and who has an interest in DPDK support (hint: not me) modifies it or rewrites it? At this point, I'm sorry we picked this code up; it would perhaps have been better had the DPDK developers maintained their own version of libpcap with DPDK support.