Specify queue id as a configuration parameter by JasMetzger · Pull Request #2009 · seladb/PcapPlusPlus

JasMetzger · 2025-10-27T20:03:57Z

The queue id can be specified as an additional configuration parameter. It is checked against the maximum queues available by the device using a utility provided as a static function. In the open() function, the configured queue id is used in place of the hardwired zero value that was used before. Added a convenience function to obtain the queue id from device.

Community may consider renaming the XdpDevice to XdpSocketDevice as each instance of the XdpDevice class pertains to a single queue id / socket.

codecov · 2025-10-27T20:34:14Z

Codecov Report

❌ Patch coverage is 75.75758% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.46%. Comparing base (a789e12) to head (ca25dcc).
⚠️ Report is 9 commits behind head on dev.

Files with missing lines	Patch %	Lines
Pcap++/src/XdpDevice.cpp	74.19%	5 Missing and 3 partials ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##              dev    #2009    +/-   ##
========================================
  Coverage   83.46%   83.46%            
========================================
  Files         311      312     +1     
  Lines       54556    54606    +50     
  Branches    11491    11821   +330     
========================================
+ Hits        45534    45576    +42     
- Misses       7795     7798     +3     
- Partials     1227     1232     +5

Flag	Coverage Δ
alpine320	`75.90% <ø> (-0.01%)`	⬇️
fedora42	`75.46% <ø> (+<0.01%)`	⬆️
macos-14	`81.58% <ø> (+<0.01%)`	⬆️
macos-15	`81.57% <ø> (-0.01%)`	⬇️
mingw32	`70.00% <ø> (-0.03%)`	⬇️
mingw64	`69.96% <ø> (+0.07%)`	⬆️
npcap	`?`
rhel94	`75.47% <ø> (-0.01%)`	⬇️
ubuntu2004	`59.48% <ø> (-0.01%)`	⬇️
ubuntu2004-zstd	`59.57% <ø> (-0.02%)`	⬇️
ubuntu2204	`75.40% <ø> (-0.03%)`	⬇️
ubuntu2204-icpx	`57.85% <ø> (+0.01%)`	⬆️
ubuntu2404	`75.51% <ø> (-0.01%)`	⬇️
ubuntu2404-arm64	`75.57% <ø> (+<0.01%)`	⬆️
unittest	`83.46% <75.75%> (+<0.01%)`	⬆️
windows-2022	`85.43% <ø> (+0.17%)`	⬆️
windows-2025	`85.46% <ø> (+0.13%)`	⬆️
winpcap	`85.46% <ø> (-0.07%)`	⬇️
xdp	`52.99% <75.75%> (-0.02%)`	⬇️

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.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Dimi1010

Looks good overall. Some minor notes.

XDP CI is failing tho. If you could look into that. 🤔

Dimi1010 · 2025-10-31T21:54:53Z

Pcap++/header/XdpDevice.h

 		XdpDeviceStats getStatistics();

+		/// @return Return queue identifier for underlying socket
+		uint32_t getQueueId()


nit: I think this method can be marked as const?

Dimi1010 · 2025-10-31T21:56:14Z

Pcap++/src/XdpDevice.cpp

+	uint32_t XdpDevice::getNumQueues(const std::string& iface, bool tx)
+	{
+		// returns number of hardware queues associated with the device
+		uint32_t rxtxqueues = 0;


nit: can we rename this to numQueues?

Dimi1010 · 2025-10-31T21:58:09Z

Pcap++/src/XdpDevice.cpp

+		{
+			std::regex rxtx_regex("^" + prefix + "[0-9]+$");
+
+			struct dirent* entry;


nit: Curious why the struct dirent*? Can't we just do dirent*?

seladb · 2025-11-06T06:19:39Z

@JasMetzger, please notice that clang-format fails in CI

JasMetzger · 2025-11-09T15:25:52Z

Sorry for the delay--- I was out of town. Where is this test failing?

seladb · 2025-11-09T21:08:29Z

Sorry for the delay--- I was out of town. Where is this test failing?

@JasMetzger you can see the pre-commit build that failed: https://github.com/seladb/PcapPlusPlus/actions/runs/19214326473/job/54921310413?pr=2009

There are trailing white-spaces and formatting issues. To fix formatting, you should run clang-format as mentioned in https://github.com/seladb/PcapPlusPlus/blob/master/CONTRIBUTING.md

JasMetzger · 2025-11-10T10:02:29Z

When I run the pre-commit, there are errors in files that I did not modify. Shall I address those?

seladb · 2025-11-13T08:19:42Z

When I run the pre-commit, there are errors in files that I did not modify. Shall I address those?

@JasMetzger there shouldn't be errors in files you did not modify, please avoid updating them in this PR

seladb · 2025-12-07T07:07:21Z

@JasMetzger should we move this PR to DRAFT until it's ready for review?
The CI is currently failing...

JasMetzger · 2025-12-07T09:39:07Z

When I do a clean make, there are warnings in files I haven't touched, so not sure what to do about that code. The first warning is freebsd running bash but I don't understand how it is failing BSD or what it is doing. Do I need to create a FreeBSD environment? Is it compiling it? I think I need some help with the CI. I got it to pass clang.

seladb · 2025-12-08T04:14:25Z

There might be issues with the FreeBSD tests, I need to look into it... 😕

seladb · 2025-12-08T07:59:17Z

Pcap++/header/XdpDevice.h

 /// @
 namespace pcpp
 {
+#define XDP_MAX_RXTX_QUEUES 16


This const isn't used anywhere, should we remove it?

I will remove. I think at first I was simply setting a hard limit before I created a function that can detect it. If the function cannot detect it, it returns 0 which may not be desirable. Maybe it should return 1 if nothing else. That will be the identical behavior in the release version.

I made the default number 1. So a qid of zero, the default, and as things worked before, will pass the test.

seladb · 2025-12-08T08:03:01Z

Pcap++/src/XdpDevice.cpp

+		xskConfig.rx_size = m_Config->rxSize;
+		xskConfig.tx_size = m_Config->txSize;


This was already fixed in #2030

Will that be a problem if I leave it like this? A merge issue?

seladb · 2025-12-08T08:13:45Z

Pcap++/header/XdpDevice.h

+		/// @param[in] interfaceName The interface name to use to detect hardware queues
+		/// @param[in] tx  If true, return TX queues, otherwise RX. Default is false
+		/// @return The number of hardware queues associated with the device.
+		static uint32_t numQueues(const std::string& interfaceName, bool tx = false);


Why is this a public method? Shouldn't it be private?

It may be useful for configuring the user space, such as limiting the number of threads and resources.

seladb · 2025-12-08T08:14:29Z

Pcap++/src/XdpDevice.cpp

 			return false;
 		}

+		unsigned int nhwqueues = numQueues(m_InterfaceName);


nit: let's rename to numOfHardwareQueues?

JasMetzger · 2025-12-09T16:34:38Z

Redirecting a packet to a particular rxqueue and associated socket requires a "map" between kernel and userspace. Entries in the map are created on the user space side using the socket's file descriptor. Unless there is a different way, I am considering a function in XdpDevice that passes in a pointer to int for the file descriptor to be written. The function returns a bool, true if the fd was set ok, false otherwise.

seladb · 2025-12-12T07:16:58Z

Redirecting a packet to a particular rxqueue and associated socket requires a "map" between kernel and userspace. Entries in the map are created on the user space side using the socket's file descriptor. Unless there is a different way, I am considering a function in XdpDevice that passes in a pointer to int for the file descriptor to be written. The function returns a bool, true if the fd was set ok, false otherwise.

@JasMetzger do you have a code example somewhere that shows how it should be done in AF_XDP?
How should this map be created? How do you pass it to the kernel?
I'm not familiar with this area, so providing more context would definitely help.

james.metzger added 3 commits October 24, 2025 12:38

Configure queue id for XDP Device (socket)

c7b00f5

Utility to find HW queues for device

fe735fe

Remove old check for queueid

db23ec3

JasMetzger requested a review from seladb as a code owner October 27, 2025 20:03

james.metzger added 2 commits October 27, 2025 16:08

Fix doxygen error

a0906cf

Fix doxygen error

e6a1e9b

Dimi1010 reviewed Oct 31, 2025

View reviewed changes

seladb mentioned this pull request Nov 7, 2025

Sockets dimensioned by Num queues #2000

Closed

Merge branch 'dev' into dev

7ab1f01

james.metzger added 2 commits November 10, 2025 11:39

Formatting and bug

a8d97bf

Minor adjustments recommended

cb5e47e

seladb linked an issue Nov 30, 2025 that may be closed by this pull request

Xdp Rx Queue #1992

Open

james.metzger added 2 commits December 4, 2025 14:13

Frame headroom size configuration

16c0da9

Clang compliance

3b8c2d5

seladb reviewed Dec 8, 2025

View reviewed changes

Default number of hw queues is one

ca25dcc

		xskConfig.rx_size = m_Config->rxSize;
		xskConfig.tx_size = m_Config->txSize;

Conversation

JasMetzger commented Oct 27, 2025

Uh oh!

codecov bot commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Dimi1010 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seladb commented Nov 6, 2025

Uh oh!

JasMetzger commented Nov 9, 2025

Uh oh!

seladb commented Nov 9, 2025

Uh oh!

JasMetzger commented Nov 10, 2025

Uh oh!

seladb commented Nov 13, 2025

Uh oh!

seladb commented Dec 7, 2025

Uh oh!

JasMetzger commented Dec 7, 2025

Uh oh!

seladb commented Dec 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JasMetzger commented Dec 9, 2025

Uh oh!

seladb commented Dec 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Oct 27, 2025 •

edited

Loading

Dimi1010 left a comment •

edited

Loading