Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[target.x86_64-unknown-linux-gnu]
runner = 'sudo'
10 changes: 9 additions & 1 deletion .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,16 @@ jobs:
- name: Install necessary dependencies
run: |
sudo apt update
sudo apt install "linux-modules-extra-$(uname -r)"
sudo apt install -y "linux-modules-extra-$(uname -r)"
sudo modprobe vrf
- name: Install iproute2 lastest git build
run: |
sudo apt-get -y remove iproute2
git clone https://git.kernel.org/pub/scm/network/iproute2/iproute2.git
cd iproute2
./configure --prefix=/usr
make -j5
sudo make install

- name: Install Rust Stable
run: |
Expand Down
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ tokio = { version = "1.30", features = ["rt", "net", "time", "macros"] }

[dev-dependencies]
pretty_assertions = "1.4.1"
ctor = "0.5.0"

[patch.crates-io.rtnetlink]
git = "https://github.com/rust-netlink/rtnetlink"
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
check:
cargo build;
env CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="sudo" \
cargo test -- --test-threads=1 --show-output $(WHAT) ;
cargo test -- --show-output;
3 changes: 2 additions & 1 deletion src/color.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ impl CliColor {
false
}
} else {
false
// iproute assume dark color if unknown
true
}
})
}
Expand Down
115 changes: 69 additions & 46 deletions src/ip/link/ifaces/bridge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,11 @@

use iproute_rs::mac_to_string;
use rtnetlink::packet_route::link::{
BridgePortState, InfoBridge, InfoBridgePort, VlanProtocol,
BridgeBooleanOptionFlags as BoolOptFlags, BridgePortState, InfoBridge,
InfoBridgePort, VlanProtocol,
};
use serde::Serialize;

// Additional bridge constants not yet in netlink-packet-route
const IFLA_BR_FDB_N_LEARNED: u16 = 48;
const IFLA_BR_FDB_MAX_LEARNED: u16 = 49;
const IFLA_BR_NO_LL_LEARN: u16 = 51;
const IFLA_BR_VLAN_MCAST_SNOOPING: u16 = 52;
const IFLA_BR_MST_ENABLED: u16 = 53;

#[derive(Serialize)]
pub(crate) struct CliLinkInfoDataBridge {
forward_delay: u32,
Expand Down Expand Up @@ -48,9 +42,16 @@ pub(crate) struct CliLinkInfoDataBridge {
#[serde(skip_serializing_if = "String::is_empty")]
group_addr: String,
mcast_snooping: u8,
no_linklocal_learn: u8,
mcast_vlan_snooping: u8,
mst_enabled: u8,
#[serde(skip_serializing_if = "Option::is_none")]
no_linklocal_learn: Option<u8>,
#[serde(skip_serializing_if = "Option::is_none")]
mcast_vlan_snooping: Option<u8>,
#[serde(skip_serializing_if = "Option::is_none")]
mst_enabled: Option<u8>,
#[serde(skip_serializing_if = "Option::is_none")]
mdb_offload_fail_notification: Option<u8>,
#[serde(skip_serializing_if = "Option::is_none")]
fdb_local_vlan_0: Option<u8>,
mcast_router: u8,
mcast_query_use_ifaddr: u8,
mcast_querier: u8,
Expand Down Expand Up @@ -124,9 +125,11 @@ impl From<&[InfoBridge]> for CliLinkInfoDataBridge {
let mut mcast_mld_version = None;
let mut fdb_n_learned = None;
let mut fdb_max_learned = None;
let mut no_linklocal_learn = 0;
let mut mcast_vlan_snooping = 0;
let mut mst_enabled = 0;
let mut no_linklocal_learn = None;
let mut mcast_vlan_snooping = None;
let mut mst_enabled = None;
let mut mdb_offload_fail_notification = None;
let mut fdb_local_vlan_0 = None;

for nla in info {
match nla {
Expand Down Expand Up @@ -223,35 +226,41 @@ impl From<&[InfoBridge]> for CliLinkInfoDataBridge {
InfoBridge::MulticastMldVersion(v) => {
mcast_mld_version = Some(*v)
}
InfoBridge::Other(nla) => {
use rtnetlink::packet_core::Nla;
match nla.kind() {
IFLA_BR_FDB_N_LEARNED => {
let mut val = [0u8; 4];
nla.emit_value(&mut val);
fdb_n_learned = Some(u32::from_ne_bytes(val));
}
IFLA_BR_FDB_MAX_LEARNED => {
let mut val = [0u8; 4];
nla.emit_value(&mut val);
fdb_max_learned = Some(u32::from_ne_bytes(val));
}
IFLA_BR_NO_LL_LEARN => {
let mut val = [0u8; 1];
nla.emit_value(&mut val);
no_linklocal_learn = val[0];
}
IFLA_BR_VLAN_MCAST_SNOOPING => {
let mut val = [0u8; 1];
nla.emit_value(&mut val);
mcast_vlan_snooping = val[0];
}
IFLA_BR_MST_ENABLED => {
let mut val = [0u8; 1];
nla.emit_value(&mut val);
mst_enabled = val[0];
}
_ => (),
InfoBridge::FdbNLearned(v) => fdb_n_learned = Some(*v),
InfoBridge::FdbMaxLearned(v) => fdb_max_learned = Some(*v),
InfoBridge::MultiBoolOpt(opts) => {
if opts.mask.contains(BoolOptFlags::NoLinkLocalLearn) {
no_linklocal_learn = Some(
opts.value
.contains(BoolOptFlags::NoLinkLocalLearn)
.into(),
);
}
if opts.mask.contains(BoolOptFlags::VlanMulticastSnooping) {
mcast_vlan_snooping = Some(
opts.value
.contains(BoolOptFlags::VlanMulticastSnooping)
.into(),
);
}
if opts.mask.contains(BoolOptFlags::MstEnable) {
mst_enabled = Some(
opts.value.contains(BoolOptFlags::MstEnable).into(),
);
}
if opts.mask.contains(BoolOptFlags::MdbOffloadFailNotif) {
mdb_offload_fail_notification = Some(
opts.value
.contains(BoolOptFlags::MdbOffloadFailNotif)
.into(),
);
}
if opts.mask.contains(BoolOptFlags::FdbLocalVlan0) {
fdb_local_vlan_0 = Some(
opts.value
.contains(BoolOptFlags::FdbLocalVlan0)
.into(),
);
}
}
Comment on lines +231 to 265

Choose a reason for hiding this comment

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

medium

The logic for handling MultiBoolOpt is repeated for each flag. This could be simplified by extracting it into a helper function to reduce code duplication and improve readability.

For example, you could introduce a helper function like this:

fn get_bool_opt(opts: &rtnetlink::packet_route::link::BridgeMultiBooleanOption, flag: BoolOptFlags) -> Option<u8> {
    if opts.mask.contains(flag) {
        Some(opts.value.contains(flag).into())
    } else {
        None
    }
}

And then use it like so:

no_linklocal_learn = get_bool_opt(opts, BoolOptFlags::NoLinkLocalLearn);
mcast_vlan_snooping = get_bool_opt(opts, BoolOptFlags::VlanMulticastSnooping);
// ... and so on

_ => (),
Expand Down Expand Up @@ -290,6 +299,8 @@ impl From<&[InfoBridge]> for CliLinkInfoDataBridge {
no_linklocal_learn,
mcast_vlan_snooping,
mst_enabled,
mdb_offload_fail_notification,
fdb_local_vlan_0,
mcast_router,
mcast_query_use_ifaddr,
mcast_querier,
Expand Down Expand Up @@ -368,9 +379,21 @@ impl std::fmt::Display for CliLinkInfoDataBridge {
write!(f, "group_address {} ", self.group_addr)?;
}
write!(f, "mcast_snooping {} ", self.mcast_snooping)?;
write!(f, "no_linklocal_learn {} ", self.no_linklocal_learn)?;
write!(f, "mcast_vlan_snooping {} ", self.mcast_vlan_snooping)?;
write!(f, "mst_enabled {} ", self.mst_enabled)?;
if let Some(v) = self.no_linklocal_learn {
write!(f, "no_linklocal_learn {v} ")?;
}
if let Some(v) = self.mcast_vlan_snooping {
write!(f, "mcast_vlan_snooping {v} ")?;
}
if let Some(v) = self.mst_enabled {
write!(f, "mst_enabled {v} ")?;
}
if let Some(v) = self.mdb_offload_fail_notification {
write!(f, "mdb_offload_fail_notification {v} ")?;
}
if let Some(v) = self.fdb_local_vlan_0 {
write!(f, "fdb_local_vlan_0 {v} ")?;
}
write!(f, "mcast_router {} ", self.mcast_router)?;
write!(f, "mcast_query_use_ifaddr {} ", self.mcast_query_use_ifaddr)?;
write!(f, "mcast_querier {} ", self.mcast_querier)?;
Expand Down
13 changes: 8 additions & 5 deletions src/ip/link/show.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,7 @@ pub(crate) async fn handle_show(

tokio::spawn(connection);

let mut link_get_handle = handle.link().get();

if let Some(iface_name) = opts.first() {
link_get_handle = link_get_handle.match_name(iface_name.to_string());
}
let link_get_handle = handle.link().get();

let mut links = link_get_handle.execute();
let mut ifaces: Vec<CliLinkInfo> = Vec::new();
Expand All @@ -150,6 +146,13 @@ pub(crate) async fn handle_show(
resolve_controller_and_link_names(&mut ifaces);
resolve_netns_names(&mut ifaces).await?;

// In order to resolved interface index to interface name and netns name,
// we cannot use kernel side interface filter, but need to dump everything,
// then filter here
if let Some(iface_name) = opts.first() {
ifaces.retain(|i| i.ifname.as_str() == *iface_name);
}
Comment on lines +152 to +154

Choose a reason for hiding this comment

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

high

This change moves the interface filtering from the kernel-side (handle.link().get().match_name(...)) to be client-side after fetching all links. While this might offer more flexibility, it can be significantly less performant if there are many network interfaces on the system, as it now fetches all of them over the netlink socket before filtering. Was this change intentional to support a use case that match_name doesn't cover? If not, I'd recommend reverting to the more efficient kernel-side filtering.

Copy link
Member Author

Choose a reason for hiding this comment

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

Explained with comment.


Ok(ifaces)
}

Expand Down
Loading