Skip to content

Add function to get neighbours on bridge#160

Open
Kriegsameise wants to merge 1 commit intorust-netlink:mainfrom
Kriegsameise:main
Open

Add function to get neighbours on bridge#160
Kriegsameise wants to merge 1 commit intorust-netlink:mainfrom
Kriegsameise:main

Conversation

@Kriegsameise
Copy link
Copy Markdown

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

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

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.

Comment thread src/neighbour/get.rs

use crate::{Error, Handle, IpVersion};

const NTF_SELF: u8 = 0x02;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
const NTF_SELF: u8 = 0x02;
#[cfg(not(target_os = "freebsd"))]
const NTF_SELF: u8 = 0x02;

Comment thread src/neighbour/get.rs
Comment on lines +31 to +39
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 }
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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 }
    }

Comment thread src/neighbour/handle.rs
Comment on lines +23 to +26
/// List fdb entries (equivalent to `bridge fdw show`)
pub fn get_bridge(&self) -> NeighbourGetRequest {
NeighbourGetRequest::new_bridge(self.0.clone())
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

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.

Suggested change
/// 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())
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant