Refactor reentrant flag to be set during channel creation. #1738
Conversation
…nnelImpl` to remove ambiguity between public and private calls.
… of when starting capture as it is determined by the PF_RING_REENTRANT flag. - Added ability to choose if `openSingleChannelImpl` will open w/reenterant flag.
Pcap++/src/PfRingDevice.cpp
Outdated
| 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]); |
There was a problem hiding this comment.
@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?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #1738 +/- ##
==========================================
- Coverage 83.21% 83.21% -0.01%
==========================================
Files 282 282
Lines 48741 48741
Branches 10314 10334 +20
==========================================
- Hits 40560 40558 -2
+ Misses 7051 7050 -1
- Partials 1130 1133 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@Dimi1010 this code was written many years ago so I don't remember all the details, but I think the The problem with the current implementation is that when opening the channels we don't know how many threads will read from them:
In addition - if opening in reentrant mode then zero copy can't be used, hence the We may need a bigger refactor to allow the user to choose when opening the device whether multiple threads will read from each channel (and set the flag accordingly), and when start capturing verify that we don't allow multiple threads to read from the same channel. But this is something that needs further testing with a PF_RING environment to test in - do you have such an environment? |
This would work. I think a bool in the
Not fully. I have a WSL to confirm compilation, but not sure if that will work for actual capture testing. |
Yes, something like that could work
I'm not sure it'll work well with WSL, you may need to set up an Ubuntu VM. I use VirtualBox on Windows... |
Split of #1668
This PR aims to standardize
m_ReentrantModeto be set during the channel opening phase instead of the capture initialization phase as it is determined by the opening flags. The currently used implementation essentially assumes the what the reentrant mode is based on the function being called to start the capture, instead of recording the correct value.