feat(conntrack): bpf code for conntrack + conntrack GC #610
Conversation
nddq
added
type/enhancement
Signed-off-by: Quang Nguyen <[email protected]>
| * @arg observation_point The point in the network stack where the packet is observed. | ||
| * Returns true if the packet should be report to userspace. False otherwise. | ||
| */ | ||
| static __always_inline __attribute__((unused)) bool ct_process_packet(struct ct_v4_key key, __u8 flags, __u8 observation_point) { |
There was a problem hiding this comment.
key can be passed as a pointer like you do in other functions to avoid copying
There was a problem hiding this comment.
actually, my initial plan was to pass in the pointer to the packet into conntrack and then construct the key within ct_process_packet. That way we don't have to do the subsequent function call to get the traffic_direction and is_reply
and instead, we can do it within ct_process_packet. However, we will need to relocate the packet struct to conntrack since cycle dependency is not allowed, i think. Curious about your opinion
There was a problem hiding this comment.
There was a problem hiding this comment.
I didn't understand - why cannot we pass a pointer to they input key here - this function on;y seems to be accessing its values.
Where would we have a cyclical dependency? (Do you mean the eBPF verifier fails on that?)
There was a problem hiding this comment.
IIRC, and i could be wrong, there was some issue with the verifier when passing the key pointer from packetparser to conntrack, i can test it again to verify.
The cyclical dependency is applied to the statement where i wanted to pass the packet pointer from packetparser to conntrack. Since packetparser is already importing conntrack, conntrack can't import packetparser back to get the packet struct def. The reason why I think we should be passing the packet pointer to conntrack is because so that we don't have to do subsequent map look up for is_reply and traffic_direction, but instead set in immediately in ct_process_packet
# Description Part 2 of #610 ## Related Issue If this pull request is related to any issue, please mention it here. Additionally, make sure that the issue is assigned to you before submitting this pull request. ## Checklist - [ ] I have read the [contributing documentation](https://retina.sh/docs/contributing). - [ ] 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. - [ ] 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 Output from debug CLI tool:  Hubble flow logs:  ## 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: Quang Nguyen <[email protected]>
Description
Part of #283, broken down for ease of review. Testing results will be added in #624
Related Issue
If this pull request is related to any issue, please mention it here. Additionally, make sure that the issue is assigned to you before submitting this pull request.
Checklist
git commit -S -s ...). See this documentation on signing commits.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.