Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #1635 - xsk.h should be taken from libxdp #1703

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,10 @@ if(PCAPPP_USE_XDP)
if(NOT BPF_FOUND)
message(FATAL_ERROR "libbpf not found!")
endif()
find_package(XDP)
if(NOT XDP_FOUND)
message(FATAL_ERROR "libxdp not found!")
endif()
add_definitions(-DUSE_XDP)
endif()

Expand Down
1 change: 1 addition & 0 deletions Pcap++/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ target_link_libraries(
$<$<BOOL:${PCAPPP_USE_PF_RING}>:PF_RING::PF_RING>
$<$<BOOL:${PCAPPP_USE_DPDK}>:DPDK::DPDK>
$<$<BOOL:${PCAPPP_USE_XDP}>:BPF::BPF>
$<$<BOOL:${PCAPPP_USE_XDP}>:XDP::XDP>
PCAP::PCAP
Threads::Threads
)
Expand Down
2 changes: 1 addition & 1 deletion Pcap++/src/XdpDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
#include "Logger.h"
#include "Packet.h"
#include <bpf/libbpf.h>
#include <bpf/xsk.h>
#include <xdp/xsk.h>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why we need "xdp" instead of "bpf" here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why we need "xdp" instead of "bpf" here?

On my systems there is no such header. Same is here: #1635.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hoxnox I think we may need to add some conditions here instead of changing the code directly. What's your system? Maybe just add a special case for handling your system?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hoxnox I think we may need to add some conditions here instead of changing the code directly. What's your system? Maybe just add a special case for handling your system?

I don't understand why in your system "xsk.h" present. There is no such at https://github.com/libbpf/libbpf and no such in kernel tree https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/bpf?h=v6.14-rc1

Copy link
Collaborator

@tigercosmos tigercosmos Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might based on how and where you install it. I saw people using <bpf/xsk.h>, <xdp/xsk.h>, and <linux/xsk.h>. Maybe we need a better way to find the corret one for the user system.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might based on how and where you install it. I saw people using <bpf/xsk.h>, <xdp/xsk.h>, and <linux/xsk.h>. Maybe we need a better way to find the corret one for the user system.

I'll try to figure it out and return with more accurate patch.

Copy link
Author

@hoxnox hoxnox Feb 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Between 5.15.178 and 6.1.128 xsk.h was moved from tools/lib/bpf/xsk.h to tools/testing/selftests/bpf/xsk.h and it is not part of linux-headers. So regular user with kernel 6+ doesn't have that header.

#include <linux/if_link.h>
#include <net/if.h>
#include <sys/mman.h>
Expand Down
34 changes: 34 additions & 0 deletions cmake/modules/FindXDP.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# ~~~
# - Try to find libxdp
#
# Once done this will define
#
# XDP_FOUND - System has libxdp
# XDP_INCLUDE_DIRS - The libxdp include directories
# XDP_LIBRARIES - The libraries needed to use libxdp
# ~~~

find_package(PkgConfig QUIET)
pkg_check_modules(PC_LIBBPF libxdp)

find_path(
XDP_INCLUDE_DIRS
NAMES xdp/xsk.h
HINTS ${PC_LIBXDP_INCLUDE_DIRS})

find_library(
XDP
NAMES xdp
HINTS ${PC_LIBXDP_LIBRARY_DIRS})

include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(
XDP
REQUIRED_VARS XDP XDP_INCLUDE_DIRS
VERSION_VAR XDP_VERSION
FAIL_MESSAGE "libxdp not found!")

if(XDP_FOUND AND NOT TARGET XDP::XDP)
add_library(XDP::XDP INTERFACE IMPORTED)
set_target_properties(XDP::XDP PROPERTIES INTERFACE_LINK_LIBRARIES "${XDP_LIBRARIES}" INTERFACE_INCLUDE_DIRECTORIES "${XDP_INCLUDE_DIRS}")
endif()
Loading