-
Notifications
You must be signed in to change notification settings - Fork 11
Conntrack get netlink netfilter message types #12
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
eaaa88f to
c7c6939
Compare
c7c6939 to
568f8d5
Compare
f89a9cd to
e171eca
Compare
|
Thanks for the review @cathay4t . I've updated the tests and moved the conntrack constants to be private as you suggested. I noticed the nflog module still exposes public constants. Let me know if you want those refactored as well to hide the implementation details. Happy to do it in a follow up PR. I also added one more conntrack get UDP IPv6 test. |
cathay4t
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.
All public data type should be protected by #[non_exhaustive] unless you are sure it will never changes in the future of Linux kernel.
e171eca to
40c4c4f
Compare
cathay4t
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.
The netlink-packet-routes has changed nlas to attributes for better understanding.
The nlas is for hard for new developer to understand.
40c4c4f to
62cb27e
Compare
Implemented the following attributes required to successfully construct a conntrack get request: * iptuple * protoinfo * protoinfotcp * prototuple * tcp_flags * tuple Signed-off-by: Shivang K Raghuvanshi <shivangraghuvanshi2005@gmail.com>
2d15bc3 to
f002e81
Compare
This refactors the crate to use type-safe enums for netfilter subsystems and message types, for a safer and more idiomatic API. - Introduces a `Subsystem` enum to replace raw `u8` identifiers for `NfLog` and `Conntrack` subsystems. - Introduces `NfLogMessageType` and `ConntrackMessageType` enums to provide type safety for messages within each subsystem. - Makes the top-level `NetfilterMessage::message_type()` function private to guide users towards the safer pattern of matching on `NetfilterMessageInner`. - Updates the internal parsing logic in `buffer.rs` to use the new `Subsystem` enum. Signed-off-by: Shivang K Raghuvanshi <shivangraghuvanshi2005@gmail.com>
f002e81 to
c9d7dc3
Compare
|
Hi @cathay4t, I've addressed all of the feedback. NOTE: a normal crate user would not have to do this type of stuff. |
| } | ||
|
|
||
| const NFNL_SUBSYS_CTNETLINK: u8 = libc::NFNL_SUBSYS_CTNETLINK as u8; | ||
| const NFNL_SUBSYS_ULOG: u8 = libc::NFNL_SUBSYS_ULOG as u8; |
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.
do not use libc constant, use hard coded number instead. I have to check libc code and kernel code to make sure your are correct.
| impl From<u8> for Subsystem { | ||
| fn from(value: u8) -> Self { | ||
| match value { | ||
| NFNL_SUBSYS_ULOG => Self::NfLog, |
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.
A proper explanation command is required for this inconsistency on naming.
|
|
||
| #[derive(Clone, Debug, PartialEq, Eq)] | ||
| #[non_exhaustive] | ||
| pub enum ConntrackNla { |
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.
how about ContrackAttribute like LinkAttribute in netlink-packet-route.
Depends on #11
Implemented the following types required to successfully construct a conntrack get request:
Also wrote tests to construct a conntrack get and dump request.