Add function to get neighbours on bridge#160
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to list bridge Forwarding Database (FDB) entries by adding a new_bridge constructor to NeighbourGetRequest and a get_bridge method to NeighbourHandle. The review feedback suggests adding platform-specific gating (#[cfg(not(target_os = "freebsd"))]) to the new constant and methods for consistency with existing bridge-related code. It also points out a typo in the documentation and suggests a more idiomatic way to initialize flags using unwrap_or_else.
|
|
||
| use crate::{Error, Handle, IpVersion}; | ||
|
|
||
| const NTF_SELF: u8 = 0x02; |
There was a problem hiding this comment.
This constant is only used in the Linux-specific new_bridge function. It should be gated with #[cfg(not(target_os = "freebsd"))] to maintain consistency with other bridge-related functionality in this crate.
| const NTF_SELF: u8 = 0x02; | |
| #[cfg(not(target_os = "freebsd"))] | |
| const NTF_SELF: u8 = 0x02; |
| pub(crate) fn new_bridge(handle: Handle) -> Self { | ||
| let mut message = NeighbourMessage::default(); | ||
| message.header.family = AddressFamily::Bridge; | ||
| message.header.flags = match NeighbourFlags::from_bits(NTF_SELF) { | ||
| Some(f) => f, | ||
| None => NeighbourFlags::empty(), | ||
| }; | ||
| NeighbourGetRequest { handle, message } | ||
| } |
There was a problem hiding this comment.
This function should be gated with #[cfg(not(target_os = "freebsd"))] for consistency with other bridge-related neighbor operations (like NeighbourAddRequest::new_bridge). Additionally, the flag initialization can be simplified using unwrap_or_else.
#[cfg(not(target_os = "freebsd"))]
pub(crate) fn new_bridge(handle: Handle) -> Self {
let mut message = NeighbourMessage::default();
message.header.family = AddressFamily::Bridge;
message.header.flags = NeighbourFlags::from_bits(NTF_SELF).unwrap_or_else(NeighbourFlags::empty);
NeighbourGetRequest { handle, message }
}| /// List fdb entries (equivalent to `bridge fdw show`) | ||
| pub fn get_bridge(&self) -> NeighbourGetRequest { | ||
| NeighbourGetRequest::new_bridge(self.0.clone()) | ||
| } |
There was a problem hiding this comment.
There is a typo in the documentation (fdw should be fdb). Furthermore, this method should be gated with #[cfg(not(target_os = "freebsd"))] to match the platform support of other bridge-related functions in this handle, such as add_bridge on line 35.
| /// List fdb entries (equivalent to `bridge fdw show`) | |
| pub fn get_bridge(&self) -> NeighbourGetRequest { | |
| NeighbourGetRequest::new_bridge(self.0.clone()) | |
| } | |
| #[cfg(not(target_os = "freebsd"))] | |
| /// List fdb entries (equivalent to bridge fdb show) | |
| pub fn get_bridge(&self) -> NeighbourGetRequest { | |
| NeighbourGetRequest::new_bridge(self.0.clone()) | |
| } |
|
Hi, |
|
Let's mark |
|
Please squash the commits into single one and fix CI failure. |
deprecated NeighbourGetRequest::set_family(IpVersion) in favor of NeighbourGetRequest::set_address_family(AddressFamily) added example to get neighbours on bridge interfaces
|
I squashed the commits, the ci failure wasn't related but fixed now |
|
Merged. Thank you for the contribution! |
2 participants
For a project of mine I needed the option to find the neighbours on bridge interfaces, specifically to find VXLAN remotes and came up with this solution for the problem and figured I'd contribute it back here.
Hope this helps and let me know if something needs to be changed