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

fix(conntrack): delete keys in eBPF instead of user space #831

Merged
merged 8 commits into from
Oct 9, 2024

Conversation

SRodi
Copy link
Member

@SRodi SRodi commented Oct 8, 2024

Description

Delete TCP connections in retina_conntrack map directly in the eBPF layer instead of relying on the userspace process to delete it later when the connection is closing and has exceeded its lifetime.

  • remove is_closing flag from retina_conntrack map, update userspace and bpf program accordingly
  • delete connection from retina_conntrack map if connection is timed out or when FIN or RST flags are set
  • invoke bpf_map_delete_elem in_ct_should_report_packet and remove update seen_flags and last_report

Related Issue

fix #807

Checklist

  • I have read the contributing documentation.
  • I signed and signed-off the commits (git commit -S -s ...). See this documentation on signing commits.
  • I have correctly attributed the author(s) of the code.
  • I have tested the changes locally.
  • I have followed the project's style guidelines.
  • I have updated the documentation, if necessary.
  • I have added tests, if applicable.

Screenshots (if applicable) or Testing Completed

Please add any relevant screenshots or GIFs to showcase the changes made.

Additional Notes

Add any additional notes or context about the pull request here.


Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.

Signed-off-by: Simone Rodigari <srodigari@microsoft.com>
@SRodi SRodi added type/enhancement New feature or request area/ebpf lang/c The C Programming Language scope/S Change is Small labels Oct 8, 2024
@SRodi SRodi requested a review from a team as a code owner October 8, 2024 15:30
@SRodi SRodi requested review from mainred and alexcastilio October 8, 2024 15:30
@SRodi SRodi marked this pull request as draft October 8, 2024 15:30
@SRodi SRodi requested a review from nddq October 8, 2024 15:32
@SRodi SRodi marked this pull request as ready for review October 8, 2024 17:10
Copy link
Member

@timraymond timraymond left a comment

Choose a reason for hiding this comment

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

This looks pretty reasonable to me, but I don't feel confident approving it. @nddq should take a look.

Signed-off-by: Simone Rodigari <srodigari@microsoft.com>
SRodi and others added 2 commits October 9, 2024 07:20
@SRodi SRodi requested a review from nddq October 9, 2024 06:37
SRodi added 2 commits October 9, 2024 16:46
Signed-off-by: Simone Rodigari <srodigari@microsoft.com>
Signed-off-by: Simone Rodigari <srodigari@microsoft.com>
@nddq nddq enabled auto-merge October 9, 2024 16:05
@nddq nddq added this pull request to the merge queue Oct 9, 2024
Merged via the queue into microsoft:main with commit 3983a57 Oct 9, 2024
22 checks passed
vakalapa pushed a commit that referenced this pull request Oct 9, 2024
# Description

Delete TCP connections in `retina_conntrack` map directly in the eBPF
layer instead of relying on the userspace process to delete it later
when the connection is closing and has exceeded its lifetime.

* remove `is_closing` flag from `retina_conntrack` map, update userspace
and bpf program accordingly
* delete connection from `retina_conntrack` map if connection is timed
out or when FIN or RST flags are set
* invoke `bpf_map_delete_elem` in`_ct_should_report_packet` and remove
update `seen_flags` and `last_report`

## Related Issue

fix #807

## Checklist

- [x] I have read the [contributing
documentation](https://retina.sh/docs/contributing).
- [x] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [x] I have correctly attributed the author(s) of the code.
- [x] I have tested the changes locally.
- [x] I have followed the project's style guidelines.
- [x] I have updated the documentation, if necessary.
- [x] I have added tests, if applicable.

## Screenshots (if applicable) or Testing Completed

Please add any relevant screenshots or GIFs to showcase the changes
made.

## Additional Notes

Add any additional notes or context about the pull request here.

---

Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more
information on how to contribute to this project.

---------

Signed-off-by: Simone Rodigari <srodigari@microsoft.com>
@SRodi SRodi deleted the fix/conntrackbpf branch October 10, 2024 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ebpf lang/c The C Programming Language scope/S Change is Small type/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[conntrack] improving GC by removing an entry directly in bpf
3 participants