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

Conversation

hoxnox
Copy link

@hoxnox hoxnox commented Feb 1, 2025

Added FindXDP.cmake, corrected CMakeLists and changed bpf/xsk.h to xdp/xsk.h.
This is fix for #1635.

@hoxnox hoxnox requested a review from seladb as a code owner February 1, 2025 07:26
Copy link

codecov bot commented Feb 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.58%. Comparing base (3af7383) to head (c654d4d).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1703      +/-   ##
==========================================
+ Coverage   83.13%   83.58%   +0.45%     
==========================================
  Files         279      275       -4     
  Lines       48395    47763     -632     
  Branches    10246    10074     -172     
==========================================
- Hits        40231    39924     -307     
+ Misses       7042     6735     -307     
+ Partials     1122     1104      -18     
Flag Coverage Δ
alpine320 75.10% <ø> (+<0.01%) ⬆️
fedora40 75.15% <ø> (ø)
macos-13 80.61% <ø> (+<0.01%) ⬆️
macos-14 80.61% <ø> (ø)
macos-15 80.58% <ø> (ø)
mingw32 70.82% <ø> (-0.03%) ⬇️
mingw64 70.78% <ø> (-0.03%) ⬇️
npcap 85.21% <ø> (-0.02%) ⬇️
rhel94 74.98% <ø> (+<0.01%) ⬆️
ubuntu2004 58.59% <ø> (ø)
ubuntu2004-zstd 58.71% <ø> (ø)
ubuntu2204 74.89% <ø> (ø)
ubuntu2204-icpx 61.27% <ø> (ø)
ubuntu2404 75.13% <ø> (+<0.01%) ⬆️
unittest 83.58% <ø> (+0.45%) ⬆️
windows-2019 85.23% <ø> (-0.02%) ⬇️
windows-2022 85.27% <ø> (-0.01%) ⬇️
winpcap 85.24% <ø> (ø)
xdp ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants