Skip to content
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

Ring buffer block not automatically released without call to nextPacket() #33

Closed
4 tasks done
fako1024 opened this issue May 8, 2023 · 0 comments · Fixed by #34
Closed
4 tasks done

Ring buffer block not automatically released without call to nextPacket() #33

fako1024 opened this issue May 8, 2023 · 0 comments · Fixed by #34
Assignees
Labels
bug Something isn't working

Comments

@fako1024
Copy link
Owner

fako1024 commented May 8, 2023

els0r/goProbe#88 revealed yet another small bug: As it turns out the current logic in afring.go does not release the current ring buffer packet back to the kernel as soon as the last packet has been consumed, but instead upon the subsequent call to nextPacket(). In practice this probably won't be a problem, but especially when testing we quite often known exactly how many packets have been process and hence don't even try to perform said call to nextPacket() just for the sake of releasing the resource. Consequently, any logic that checks e.g. if all blocks have been consumed will fail in such scenarios.
In addition, the E2E test did reveal yet another data race regarding that buffer (my analysis in #24 was not accurate - it just happens very rarely) that needs to be addressed (which in turn requires the aforementioned bug to be fixed).

DoD

  • Ensure that blocks are released to the kernel as soon as all packets have been read
  • Fix race condition in block status field
  • Fix race condition in Pipe() / run() methods (we must not return from there until all blocks have been released back to the "kernel", i.e. the mock)
  • Add better way of assessing if source is closed (using syscall.Fcntl)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant