Skip to content

Sockets dimensioned by Num queues #2000

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.

Sign up for GitHub

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

Closed
JasMetzger wants to merge 7 commits into from
Closed

Sockets dimensioned by Num queues #2000

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9d030cd
Sockets dimensioned by Num queues
Oct 20, 2025
09f8145
Bug with sizeof
Oct 20, 2025
7fc2796
Bug with sizeof II
Oct 20, 2025
79b06f5
Defined Socket class encapsulating items
Oct 20, 2025
b565c74
Maintain backward compatibility. Separate XdpSocket. Implement other …
Oct 21, 2025
943f7bd
Merge branch 'dev' into dev
JasMetzger Oct 21, 2025
739fd67
Fix Doxygen errors and one from getConfig
Oct 21, 2025
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
46 changes: 28 additions & 18 deletions Pcap++/header/XdpDevice.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
/// @
namespace pcpp
{

// used to dimension sockets
#define MAXIMUM_NUMBER_QUEUES 8
Copy link
Copy Markdown
Collaborator

@Dimi1010 Dimi1010 Oct 20, 2025

Choose a reason for hiding this comment

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

For hard constants, prefer using C++ const / constexpr variables.

If macros are needed in public headers, prefix them with PCPP_ so they don't conflict with other macros the users might have.

Copy link
Copy Markdown
Contributor Author

@JasMetzger JasMetzger Oct 20, 2025

Choose a reason for hiding this comment

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

ok---perhaps with the std::array I don't need this definition any more


/// @class XdpDevice
/// A class wrapping the main functionality of using AF_XDP (XSK) sockets
/// which are optimized for high performance packet processing.
Expand Down Expand Up @@ -47,6 +51,10 @@ namespace pcpp
/// AF_XDP operation mode
AttachMode attachMode;

/// number of queues. Should be less than or equal to the number of hardware queues supported by the device
// the queue ids are inferred as consecutive starting at zero
uint32_t numQueues;

/// UMEM is a region of virtual contiguous memory, divided into equal-sized frames.
/// This parameter determines the number of frames that will be allocated as pert of the UMEM.
uint16_t umemNumFrames;
Expand Down Expand Up @@ -89,7 +97,7 @@ namespace pcpp
explicit XdpDeviceConfiguration(AttachMode attachMode = AutoMode, uint16_t umemNumFrames = 0,
uint16_t umemFrameSize = 0, uint32_t fillRingSize = 0,
uint32_t completionRingSize = 0, uint32_t rxSize = 0, uint32_t txSize = 0,
uint16_t rxTxBatchSize = 0)
uint16_t rxTxBatchSize = 0, uint32_t numQueues = 0)
{
this->attachMode = attachMode;
this->umemNumFrames = umemNumFrames;
Expand All @@ -99,6 +107,7 @@ namespace pcpp
this->rxSize = rxSize;
this->txSize = txSize;
this->rxTxBatchSize = rxTxBatchSize;
this->numQueues = numQueues;
}
};

Expand Down Expand Up @@ -196,11 +205,11 @@ namespace pcpp
/// ms
/// @return True if stopped receiving packets because stopReceivePackets() was called or because timeoutMS
/// passed, or false if an error occurred.
bool receivePackets(OnPacketsArrive onPacketsArrive, void* onPacketsArriveUserCookie, int timeoutMS = 5000);
bool receivePackets(OnPacketsArrive onPacketsArrive, void* onPacketsArriveUserCookie, int timeoutMS = 5000, uint32_t queueid = 0);

/// Stop receiving packets. Call this method from within the callback passed to receivePackets() whenever you
/// want to stop receiving packets.
void stopReceivePackets();
void stopReceivePackets(uint32_t queueid = 0);
Copy link
Copy Markdown
Collaborator

@Dimi1010 Dimi1010 Oct 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

How would the user call the correct queue id from inside the receive loop (OnPacketsArrive callback)? 🤔
The users are expected to call stopReceivePackets inside the receive callback for their own queue.

Of course, they can save it externally or in the user cookie, but that seems like a code smell to me, as it should be provided to them in the callback.

@seladb IMO, it would be useful if the callback signature be changed to have a param XdpReceiveContext&, instead of having a pointer to XdpDevice. It would allow some methods to be contextualized for the receive op. Access to the device can also be through the context. For example in the current case, XdpReceiveContext::stopReceive(), that automatically cancels further receives. It would also allow cleaner addition and removal of parameters to the callback. Downside is, that it is a breaking change.

Alternative is to have the callback require a bool "continue receiving" return value, which is easier for simpler cases, but does not help with future maintenance if other changes are needed.

XdpReceiveContext version sample:

Unsure if the raw packets should be added to the context. There are pros and cons to both approaches:

  • Packets inside context pros:
    • More consistent stable API signature. No need to change signature if packets structure changes.
      Each param change is breaking, while adding and deprecating accessors is non-breaking.
  • Packets as separate params pros:
    • Better focus on the packet variables. They are generally the primary parameters the users want.
    • Independent lifetime can allow reuse of the context from the previous callback, as packets change more often than the rest of the context values.
void handlePacket(RawPacket packets[], uint32_t packetCount, XdpReceiveContext& ctx)
{
  XdpDevice* device = ctx.getDevice(); // Get the device if needed.
  
  void* userCookie = ctx.getUserCookie();
  T* userCookie = ctx.getUserCookieAs<T>(); // Utility method to reinterpret_cast the cookie to T?
  
  // Assume foo is a future value that is added later. Notice how it does not modify the callback signature
  // and keeps backwards compatibility. Extra parameter would have required all callbacks to be modified.
  Foo* futureValue = ctx.getFoo();

   /* handle packet */
   
   // Stop operation sample.
   if(stopCondition)
   {
     ctx.requestStopReceiving();
   }
}

"Continue receiving" bool version sample:

bool handlePacket2(RawPacket packets[], uint32_t packetCount, XdpDevice* device, void* userCookie)
{
  /* handle packet */

  if (shouldStopCondition)
  {
    return false;
  }

  return true;
}

Copy link
Copy Markdown
Collaborator

@Dimi1010 Dimi1010 Oct 20, 2025

Choose a reason for hiding this comment

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

PS: IMO, the context idea is relevant to all device type callback signatures, not just XdpDevice.

Copy link
Copy Markdown
Contributor Author

@JasMetzger JasMetzger Oct 20, 2025

Choose a reason for hiding this comment

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

I agree that the OnPacketArrive should indicate the queue they are arriving on. This and any other information should passback to the callback.


/// Send a vector of packet pointers.
/// @param[in] packets A vector of packet pointers to send
Expand All @@ -212,7 +221,7 @@ namespace pcpp
/// @return True if all packets were sent, or if waitForTxCompletion is true - all sent packets were confirmed.
/// Returns false if an error occurred or if poll timed out.
bool sendPackets(const RawPacketVector& packets, bool waitForTxCompletion = false,
int waitForTxCompletionTimeoutMS = 5000);
int waitForTxCompletionTimeoutMS = 5000, uint32_t queueid = 0);

/// Send an array of packets.
/// @param[in] packets An array of raw packets to send
Expand All @@ -225,7 +234,7 @@ namespace pcpp
/// @return True if all packets were sent, or if waitForTxCompletion is true - all sent packets were confirmed.
/// Returns false if an error occurred or if poll timed out.
bool sendPackets(RawPacket packets[], size_t packetCount, bool waitForTxCompletion = false,
int waitForTxCompletionTimeoutMS = 5000);
int waitForTxCompletionTimeoutMS = 5000, uint32_t queueid = 0);

/// @return A pointer to the current device configuration. If the device is not open this method returns nullptr
XdpDeviceConfiguration* getConfig() const
Expand All @@ -234,7 +243,7 @@ namespace pcpp
}

/// @return Current device statistics
XdpDeviceStats getStatistics();
XdpDeviceStats getStatistics(uint32_t queueid = 0);

private:
class XdpUmem
Expand Down Expand Up @@ -294,21 +303,22 @@ namespace pcpp

std::string m_InterfaceName;
XdpDeviceConfiguration* m_Config;
bool m_ReceivingPackets;
XdpUmem* m_Umem;
void* m_SocketInfo;
XdpDeviceStats m_Stats;
XdpPrevDeviceStats m_PrevStats;
uint32_t m_NumQueues;
bool m_ReceivingPackets[MAXIMUM_NUMBER_QUEUES];
XdpUmem* m_Umem[MAXIMUM_NUMBER_QUEUES];
void* m_SocketInfo[MAXIMUM_NUMBER_QUEUES];
XdpDeviceStats m_Stats[MAXIMUM_NUMBER_QUEUES];
XdpPrevDeviceStats m_PrevStats[MAXIMUM_NUMBER_QUEUES];
Copy link
Copy Markdown
Collaborator

@Dimi1010 Dimi1010 Oct 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We prefer std::array<T, N> over raw arrays in new code. It gives better API for basically no extra cost.

IMO, the main benefit is the addition of std::array<T, N>::size() which makes it easy to get the size of the array without external variables.

Copy link
Copy Markdown
Contributor Author

@JasMetzger JasMetzger Oct 20, 2025

Choose a reason for hiding this comment

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

Yes. I was just seeing how things propagate with the added dimension. I will make this change on my fork.

Copy link
Copy Markdown
Collaborator

@Dimi1010 Dimi1010 Oct 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Additionally, I am debating on the viability of encapsulating a single open socket in XdpSocket.
It would allow simpler methods on a per socket basis. The XdpDevice can store std::vector<XdpSocket> m_sockets.

The current API can either forward to the selected socket (via queueId), or remain unchanged and be made to operate on all open sockets (except receive and send, those default to queue 0 for backwards compat). If option 2 is taken, XdpDevice can expose XdpSocket* getSocket() (WIP name), to allow per socket configurations.

The idea is because most elements are accessed on a "per thread queue" basis instead of "per array type" and would somewhat clean up the [queueId]s sprinkled everywhere. It would also allow easier addition (if needed) of sharing Umem regions between sockets as they aren't required to be on a 1-1 relationship with the open sockets.

Any thoughts?

Copy link
Copy Markdown
Contributor Author

@JasMetzger JasMetzger Oct 20, 2025

Choose a reason for hiding this comment

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

I think so. Would the Xdp hold the socket info, queueid, and the umem parts? I am still groping around with XDP but it seems to be a natural progression.

Copy link
Copy Markdown
Collaborator

@Dimi1010 Dimi1010 Oct 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Essentially, yes. The XdpSocket holds:

  • queue id
  • socket info
  • stats
  • prev stats
  • bool receivingPackets
  • any other per socket variables
  • (if needed) a pointer to the parent xdp device.

It can be a POD struct that just holds them for the device to use, or it can be a full class with its own methods (essentially some of the current XdpDevice methods that operate on a single socket) so it can be exposed to the user as a full XdpSocket object.

I somewhat like the full object solution, as it would provide simpler methods in the API.
You get the socket object from the device, store a reference to it somewhere, and then operate on it. There is no need to keep track of queue ids after fetching the open socket from the device for simple operations that run on a single queue.

Copy link
Copy Markdown
Contributor Author

@JasMetzger JasMetzger Oct 20, 2025

Choose a reason for hiding this comment

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

I would make it a class subordinate to the XdpDevice namespace. I can start there and see how it pans out. For now might we define the Umem region part of the XdpSocket too since it's dimensioned that way currently,

Copy link
Copy Markdown
Contributor Author

@JasMetzger JasMetzger Oct 20, 2025

Choose a reason for hiding this comment

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

I pushed something that encapsulates in a Socket class.


bool sendPackets(const std::function<RawPacket(uint32_t)>& getPacketAt,
const std::function<uint32_t()>& getPacketCount, bool waitForTxCompletion = false,
int waitForTxCompletionTimeoutMS = 5000);
bool populateFillRing(uint32_t count, uint32_t rxId = 0);
bool populateFillRing(const std::vector<uint64_t>& addresses, uint32_t rxId);
uint32_t checkCompletionRing();
bool configureSocket();
bool initUmem();
int waitForTxCompletionTimeoutMS = 5000, uint32_t queueid = 0);
bool populateFillRing(uint32_t count, uint32_t rxId = 0, uint32_t queueid = 0);
bool populateFillRing(const std::vector<uint64_t>& addresses, uint32_t rxId, uint32_t queueid = 0);
uint32_t checkCompletionRing(uint32_t queueid = 0);
bool configureSocket(uint32_t queueid = 0);
bool initUmem(uint32_t queueid = 0);
bool initConfig();
bool getSocketStats();
bool getSocketStats(uint32_t queueid = 0);
};
} // namespace pcpp
Loading
Loading