-
Notifications
You must be signed in to change notification settings - Fork 28
LLT-6769: add tp lite stats collection #1648
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
base: main
Are you sure you want to change the base?
Conversation
4cec59b to
ba678a2
Compare
6e24aab to
a6c769b
Compare
ba18143 to
43c943a
Compare
e7d365c to
4627210
Compare
4627210 to
9a6685a
Compare
9a6685a to
b006869
Compare
| })?; | ||
| { | ||
| let mut old_cb = Box::new(collect_stats_cb); | ||
| let mut cb = self.tp_lite_stats_cb.callback.write(); |
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.
I don't think we need the old value, so a simple assignment should be enough - no need for swap.
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.
I added a comment to explain it, let me know if it clears it up or if you want me to change something
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.
Could we change the order of those two operations to make it simpler, and avoid this temporal coupling? First call libfw_enable_tp_lite_stats_collection and only if it succeeds, do we update the callback field?
Paintree
left a comment
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.
+0.5
b006869 to
28fbe4e
Compare
28fbe4e to
50bf5a2
Compare
50bf5a2 to
67dce12
Compare
67dce12 to
e5bb73f
Compare
|
I can already give +1, regardless of #1648 (comment) |
e5bb73f to
eb1cc25
Compare
eb1cc25 to
bc74e7e
Compare
Problem
We want to add TP-Lite stats collection to libtelio
Solution
The stats collection is implemented in libfirewall so we just need to configure that. Some inbound packets will be rewritten slightly which requires the packet buffer for that to be mutable, which requires changes to neptun. And tests need a newer version of libfirewall which means that libtelio-build needs to be updated as well
☑️ Definition of Done checklist