Skip to content

Refactor reentrant flag to be set during channel creation. #1738

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

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

Refactor reentrant flag to be set during channel creation. #1738

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5e7a3d4
Renamed private overload of `openSingleRxChannel` to `openSingleRxCha…
Dimi1010 Mar 23, 2025
1678405
Changed `m_ReentrantMode` to be set when opening the channels instead…
Dimi1010 Mar 23, 2025
3123107
Lint
Dimi1010 Apr 2, 2025
cc59707
Merge branch 'dev' into refactor/pf-ring-reenterant-flag
Dimi1010 Apr 2, 2025
60b353e
Merge 'dev' into 'refactor/pf-ring-reenterant-flag'
Dimi1010 Apr 4, 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
2 changes: 1 addition & 1 deletion Pcap++/header/PfRingDevice.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ namespace pcpp
bool initCoreConfigurationByCoreMask(CoreMask coreMask);
void captureThreadMain(std::shared_ptr<StartupBlock> startupBlock);

int openSingleRxChannel(const char* deviceName, pfring** ring);
int openSingleRxChannelImpl(const char* deviceName, pfring** ring, bool useReenterant = false);

bool getIsHwClockEnable()
{
Expand Down
26 changes: 17 additions & 9 deletions Pcap++/src/PfRingDevice.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,13 @@ namespace pcpp
m_NumOfOpenedRxChannels = 0;

PCPP_LOG_DEBUG("Trying to open device [" << m_DeviceName << "]");
int res = openSingleRxChannel(m_DeviceName.c_str(), &m_PfRingDescriptors[0]);
int res = openSingleRxChannelImpl(m_DeviceName.c_str(), &m_PfRingDescriptors[0]);
if (res == 0)
{
PCPP_LOG_DEBUG("Succeeded opening device [" << m_DeviceName << "]");
m_NumOfOpenedRxChannels = 1;
// Set reentrant mode to false as the channel is opened without the PF_RING_REENTRANT flag.
m_ReentrantMode = false;
m_DeviceOpened = true;
return true;
}
Expand All @@ -92,7 +94,7 @@ namespace pcpp
return openMultiRxChannels(channelIds, 1);
}

int PfRingDevice::openSingleRxChannel(const char* deviceName, pfring** ring)
int PfRingDevice::openSingleRxChannelImpl(const char* deviceName, pfring** ring, bool useReenterant)
{
if (m_DeviceOpened)
{
Expand All @@ -101,6 +103,11 @@ namespace pcpp
}

uint32_t flags = PF_RING_PROMISC | PF_RING_HW_TIMESTAMP | PF_RING_DNA_SYMMETRIC_RSS;
if (useReenterant)
{
flags |= PF_RING_REENTRANT;
}

*ring = pfring_open(deviceName, DEFAULT_PF_RING_SNAPLEN, flags);

if (*ring == nullptr)
Expand Down Expand Up @@ -176,7 +183,9 @@ namespace pcpp
std::string ringName = ringNameStream.str();
PCPP_LOG_DEBUG("Trying to open device [" << m_DeviceName << "] on channel [" << channelId
<< "]. Channel name [" << ringName << "]");
int res = openSingleRxChannel(ringName.c_str(), &m_PfRingDescriptors[i]);
// todo: Shouldn't we use the reentrant mode here? We are opening multiple channels?
// todo: Potentially only open in reenterant mode if creating N > 1 channels?
int res = openSingleRxChannelImpl(ringName.c_str(), &m_PfRingDescriptors[i]);
Copy link
Collaborator Author

@Dimi1010 Dimi1010 Mar 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@seladb Can you please check this code?

Why is the implementation opening multiple Rx channels in non-reentrant mode here, when the other multiple channel function opens them in reentrant mode?

if (res == 0)
{
PCPP_LOG_DEBUG("Succeeded opening device [" << m_DeviceName << "] on channel [" << channelId
Expand All @@ -198,7 +207,7 @@ namespace pcpp
{
// if an error occurred, close all rings from index=0 to index=m_NumOfOpenedRxChannels-1
// there's no need to close m_PfRingDescriptors[m_NumOfOpenedRxChannels] because it has already been
// closed by openSingleRxChannel
// closed by openSingleRxChannelImpl
for (int i = 0; i < m_NumOfOpenedRxChannels - 1; i++)
{
pfring_close(m_PfRingDescriptors[i]);
Expand All @@ -208,6 +217,8 @@ namespace pcpp
return false;
}

// Set reentrant mode to false as the channels are opened without the PF_RING_REENTRANT flag.
m_ReentrantMode = false;
m_DeviceOpened = true;

return true;
Expand Down Expand Up @@ -329,7 +340,8 @@ namespace pcpp
}

m_NumOfOpenedRxChannels = ringsOpen;

// Set reentrant mode to true as the channels are opened with the PF_RING_REENTRANT flag.
m_ReentrantMode = true;
m_DeviceOpened = true;
return true;
}
Expand Down Expand Up @@ -478,8 +490,6 @@ namespace pcpp
if (!m_CoreConfiguration[coreId].IsInUse)
continue;

m_ReentrantMode = true;

m_OnPacketsArriveCallback = onPacketsArrive;
m_OnPacketsArriveUserCookie = onPacketsArriveUserCookie;

Expand Down Expand Up @@ -538,8 +548,6 @@ namespace pcpp

m_StopThread = false;

m_ReentrantMode = false;

std::shared_ptr<StartupBlock> startupBlock = std::make_shared<StartupBlock>();

m_CoreConfiguration[0].IsInUse = true;
Expand Down
Loading