Afpacket improvements/v13 #6551
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add tpacket v2 ring diag code that dumps the ring if we appear stuck for a long time.
The Suricata AF_PACKET code opens a socket per thread, then after some minor setup enters a loop where the socket is poll()'d with a timeout. When the poll() call returns a non zero positive value, the AF_PACKET ring will be processed. The ringbuffer processing logic has a pointer into the ring where we last checked the ring. From this position we will inspect each frame until we find a frame with tp_status == TP_STATUS_KERNEL (so essentially 0). This means the frame is currently owned by the kernel. There is a special case handling for starting the ring processing but finding a TP_STATUS_KERNEL immediately. This logic then skip to the next frame, rerun the check, etc until it either finds an initialized frame or the last frame of the ringbuffer. The problem was, however, that the initial uninitialized frame was possibly (likely?) still being initialized by the kernel. A data race between the notification through the socket (the poll()) and the updating of the `tp_status` field in the frame could lead to a valid frame getting skipped. Of note is that for example libpcap does not do frame scanning. Instead it simply exits it ring processing loop. Also interesting is that libpcap uses atomic loads and stores on the tp_status field. This skipping of frames had 2 bad side effects: 1. in most cases, the buffer would be full enough that the frame would be processed in the next pass of the ring, but now the frame would out of order. This might have lead to packets belong to the same flow getting processed in the wrong order. 2. more severe is the soft lockup case. The skipped frame sits at ring buffer index 0. The rest of the ring has been cleared, after the initial frame was skipped. As our pass of the ring stops at the end of the ring (ptv->frame_offset + 1 == ptv->req.v2.tp_frame_nr) the code exits the ring processing loop at goes back to poll(). However, poll() will not indicate that there is more data, as the stale frame in the ring blocks the kernel from populating more frames beyond it. This is now a dead lock, as the kernel waits for Suricata and Suricata never touches the ring until it hears from the kernel. The scan logic will scan the whole ring at most once, so it won't reconsider the stale frame either. This patch addresses the issues in several ways: 1. the startup "discard" logic was fixed to not skip over kernel frames. Doing so would get us in a bad state at start up. 2. Instead of scanning the ring, we now enter a busy wait loop when encountering a kernel frame where we didn't expect one. This means that if we got a > 0 poll() result, we'll busy wait until we get at least one frame. 3. Error handling is unified and cleaned up. Any frame error now returns the frame to the kernel and progresses the frame pointer. 4. If we find a frame that is owned by us (TP_STATUS_USER_BUSY) we yield to poll() immediately, as the next expected status of that frame is TP_STATUS_KERNEL. 5. the ring is no longer processed until the "end" of the ring (so highest index), but instead we process at most one full ring size per run. 6. Work with a copy of `tp_status` instead of accessing original touched also by the kernel. Bug: OISF#4785.
Avoid colision of TP_STATUS_USER_BUSY with TP_STATUS_TS_RAW_HARDWARE, both were using bit 31.
Flag was always set for tpacket v2 and v3, which meant the check for it in the packet setup paths were useless.
Ticket: OISF#4796. V2 (for IDS and IPS) and V3 (for IDS) are widely supported. V2 was introduced in 2008, so we can safely assume that all systems can run V2+.
Leave no runtime checks for bypass/ebpf/xdp if not compiled in.
victorjulien
commented
Oct 31, 2021
| * hasn't been released by a worker thread. | ||
| * | ||
| * We use bits 29, 30, 31. 29 and 31 are software and hardware | ||
| * timestamps. 30 should not be used. Combined they should never |
There was a problem hiding this comment.
kernel says it should never set this
#define TP_STATUS_TS_SYS_HARDWARE (1 << 30) /* deprecated, never set */
|
Flow counter commit doesn't belong here. First afpacket commit should start with |
Codecov Report
@@ Coverage Diff @@
## master #6551 +/- ##
==========================================
- Coverage 77.12% 77.07% -0.05%
==========================================
Files 613 613
Lines 186750 186714 -36
==========================================
- Hits 144028 143909 -119
- Misses 42722 42805 +83
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
WARNING:
Pipeline 4676 |
|
Replaced by #6553 |
regit
added a commit
to regit/suricata
that referenced
this pull request
Nov 18, 2023
victorjulien
pushed a commit
to victorjulien/suricata
that referenced
this pull request
Nov 19, 2023
victorjulien
pushed a commit
to victorjulien/suricata
that referenced
this pull request
Jan 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Continuation of #6544 work.
https://redmine.openinfosecfoundation.org/issues/4785
https://redmine.openinfosecfoundation.org/issues/4796
https://redmine.openinfosecfoundation.org/issues/4800