-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
af-xdp: detach XDP program early v8 #9384
Conversation
To mitigate a bug with AF_XDP sockets in high traffic scenarios, the XDP program must be detatched before the sockets are closed. This issue happens when large ammounts of traffic are sent to suricata and the XDP program is not removed before AF_XDP sockets are closed. I believe this is a race condition bug as detailed here: https://bugzilla.kernel.org/show_bug.cgi?id=217712 Further investigation shows this may be a bug exclusive to the driver/AMD processor combination. This commit addresses the bug by ensuring the first thread to run the deinit function removes the XDP program, which fixes the bug as detailed in the bugzilla link. Bug OISF#6238
NOTE: This PR may contain new authors:
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #9384 +/- ##
==========================================
- Coverage 82.42% 82.17% -0.25%
==========================================
Files 968 968
Lines 274011 274198 +187
==========================================
- Hits 225845 225326 -519
- Misses 48166 48872 +706
Flags with carried forward coverage won't be shown. Click here to find out more. |
@regit is this your domain of expertise ? |
I ve looked at af_xdp but I don't have a big experience on it. I can look at MR if you want to. |
Or is it @lukashino who knows best ? |
@rmcconnell-r7 @giannitedesco could you have another look? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. There's a missing newline at end of file introduced but apart from that.
I'm unable to speak to the bug itself since we don't have AMD to repro, but the workaround looks reasonable.
@joser12345678 can you make sure to sign the CLA with the email address you use as the git author? Waiting for that, but we're ready to go after that. Thanks! |
@victorjulien done! |
Merged in #9847, thanks! |
Thanks for your contribution @joser12345678 🎉 |
To mitigate a bug with AF_XDP sockets in high traffic scenarios, the XDP program must be detatched before the sockets are closed. This issue happens when large ammounts of traffic are sent to suricata and the XDP program is not removed before AF_XDP sockets are closed. I believe this is a race condition bug as detailed here: https://bugzilla.kernel.org/show_bug.cgi?id=217712
Further investigation shows this may be a bug exclusive to the driver/AMD processor combination.
This commit addresses the bug by ensuring the first thread to run the deinit function removes the XDP program, which fixes the bug as detailed in the bugzilla link.
Bug #6238
Make sure these boxes are signed before submitting your Pull Request -- thank you.
Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/6238
Describe changes:
Added a RunModeAFXDPRemoveProg function to remove an AF_XDP program before closing sockets as this prevents the crash when receiving high traffic volume and shutting down suricata. Also fixed error for building and an error with an unread return value.
previous PR #9369
Provide values to any of the below to override the defaults.
To use a pull request use a branch name like
pr/N
whereN
is thepull request number.
Alternatively,
SV_BRANCH
may also be a link to anOISF/suricata-verify pull-request.