Skip to content

bond: align FromStr/Display strings with iproute2 #264

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.

Sign up for GitHub

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

Merged
cathay4t merged 2 commits into from
Jun 1, 2026
Merged

bond: align FromStr/Display strings with iproute2 #264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d94142
bond: align FromStr/Display strings with iproute2
cathay4t Jun 1, 2026
099ba90
bond: add FromStr/Display for BondLacpRate
cathay4t Jun 1, 2026
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
52 changes: 42 additions & 10 deletions src/link/link_info/bond.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,11 +436,11 @@ impl std::fmt::Display for BondXmitHashPolicy {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let kernel_name = match self {
BondXmitHashPolicy::Layer2 => "layer2",
BondXmitHashPolicy::Layer34 => "layer34",
BondXmitHashPolicy::Layer23 => "layer23",
BondXmitHashPolicy::Encap23 => "encap23",
BondXmitHashPolicy::Encap34 => "encap34",
BondXmitHashPolicy::VlanSrcMac => "vlan-src-mac",
BondXmitHashPolicy::Layer34 => "layer3+4",
BondXmitHashPolicy::Layer23 => "layer2+3",
BondXmitHashPolicy::Encap23 => "encap2+3",
BondXmitHashPolicy::Encap34 => "encap3+4",
BondXmitHashPolicy::VlanSrcMac => "vlan+srcmac",
BondXmitHashPolicy::Other(d) => {
return write!(f, "unknown-variant ({d})")
}
Expand All @@ -455,11 +455,11 @@ impl std::str::FromStr for BondXmitHashPolicy {
fn from_str(s: &str) -> Result<Self, Self::Err> {
Ok(match s {
"layer2" => Self::Layer2,
"layer34" => Self::Layer34,
"layer23" => Self::Layer23,
"encap23" => Self::Encap23,
"encap34" => Self::Encap34,
"vlan-src-mac" => Self::VlanSrcMac,
"layer3+4" => Self::Layer34,
"layer2+3" => Self::Layer23,
"encap2+3" => Self::Encap23,
"encap3+4" => Self::Encap34,
"vlan+srcmac" => Self::VlanSrcMac,
Comment on lines +458 to +462

@gemini-code-assist gemini-code-assist Bot Jun 1, 2026

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

To prevent breaking backward compatibility for existing configurations or scripts that use the old string representations, it is highly recommended to continue supporting the old strings (layer34, layer23, encap23, encap34, and vlan-src-mac) in FromStr parsing, while using the new format for Display. Additionally, to adhere to the project's line length limits, use a block expression for the longer match arm.

            "layer3+4" | "layer34" => Self::Layer34,
            "layer2+3" | "layer23" => Self::Layer23,
            "encap2+3" | "encap23" => Self::Encap23,
            "encap3+4" | "encap34" => Self::Encap34,
            "vlan+srcmac" | "vlan-src-mac" => {
                Self::VlanSrcMac
            }
References
  1. Code formatting must adhere to the project's rustfmt configuration, especially the maximum line length. This may require using block expressions for what would otherwise be single-line match arms to stay within the configured limit. (link)
  2. When converting from a string slice (&str) to a type where the conversion can fail, implement std::str::FromStr instead of From<&str>.

_ => {
return Err(DecodeError::from(format!(
"unknown bond xmit hash policy: {s}"
Expand Down Expand Up @@ -720,16 +720,44 @@ impl From<BondLacpRate> for u8 {
}
}

impl std::fmt::Display for BondLacpRate {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
BondLacpRate::Slow => f.write_str("slow"),
BondLacpRate::Fast => f.write_str("fast"),
BondLacpRate::Other(v) => write!(f, "{v}"),
}
}
}

impl std::str::FromStr for BondLacpRate {
type Err = DecodeError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
Ok(match s {
"slow" => Self::Slow,
"fast" => Self::Fast,
_ => {
return Err(DecodeError::from(format!(
"unknown bond lacp rate: {s}"
)))
}
})
}
}

const BOND_AD_STABLE: u8 = 0;
const BOND_AD_BANDWIDTH: u8 = 1;
const BOND_AD_COUNT: u8 = 2;
const BOND_AD_ACTOR_PORT_PRIO: u8 = 3;

#[derive(Debug, PartialEq, Eq, Clone, Copy)]
#[non_exhaustive]
pub enum BondAdSelect {
Stable,
Bandwidth,
Count,
ActorPortPrio,
Other(u8),
}

Expand All @@ -739,6 +767,7 @@ impl From<u8> for BondAdSelect {
BOND_AD_STABLE => Self::Stable,
BOND_AD_BANDWIDTH => Self::Bandwidth,
BOND_AD_COUNT => Self::Count,
BOND_AD_ACTOR_PORT_PRIO => Self::ActorPortPrio,
_ => Self::Other(d),
}
}
Expand All @@ -750,6 +779,7 @@ impl From<BondAdSelect> for u8 {
BondAdSelect::Stable => BOND_AD_STABLE,
BondAdSelect::Bandwidth => BOND_AD_BANDWIDTH,
BondAdSelect::Count => BOND_AD_COUNT,
BondAdSelect::ActorPortPrio => BOND_AD_ACTOR_PORT_PRIO,
BondAdSelect::Other(d) => d,
}
}
Expand All @@ -761,6 +791,7 @@ impl std::fmt::Display for BondAdSelect {
BondAdSelect::Stable => "stable",
BondAdSelect::Bandwidth => "bandwidth",
BondAdSelect::Count => "count",
BondAdSelect::ActorPortPrio => "actor_port_prio",
BondAdSelect::Other(d) => {
return write!(f, "unknown-variant ({d})")
}
Expand All @@ -777,6 +808,7 @@ impl std::str::FromStr for BondAdSelect {
"stable" => Self::Stable,
"bandwidth" => Self::Bandwidth,
"count" => Self::Count,
"actor_port_prio" => Self::ActorPortPrio,
_ => {
return Err(DecodeError::from(format!(
"unknown bond ad select: {s}"
Expand Down
22 changes: 17 additions & 5 deletions src/link/tests/bond.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,13 +276,13 @@ fn test_bond_primary_reselect_from_str() {
fn test_bond_xmit_hash_policy_from_str() {
use std::str::FromStr;
assert_eq!(BondXmitHashPolicy::Layer2, "layer2".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Layer34, "layer34".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Layer23, "layer23".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Encap23, "encap23".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Encap34, "encap34".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Layer34, "layer3+4".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Layer23, "layer2+3".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Encap23, "encap2+3".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Encap34, "encap3+4".parse().unwrap());
assert_eq!(
BondXmitHashPolicy::VlanSrcMac,
"vlan-src-mac".parse().unwrap()
"vlan+srcmac".parse().unwrap()
);
Comment on lines +279 to 286

@gemini-code-assist gemini-code-assist Bot Jun 1, 2026

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

Add assertions to verify that the old string representations are still parsed correctly for backward compatibility.

Suggested change
assert_eq!(BondXmitHashPolicy::Layer34, "layer3+4".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Layer23, "layer2+3".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Encap23, "encap2+3".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Encap34, "encap3+4".parse().unwrap());
assert_eq!(
BondXmitHashPolicy::VlanSrcMac,
"vlan-src-mac".parse().unwrap()
"vlan+srcmac".parse().unwrap()
);
assert_eq!(BondXmitHashPolicy::Layer34, "layer3+4".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Layer34, "layer34".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Layer23, "layer2+3".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Layer23, "layer23".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Encap23, "encap2+3".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Encap23, "encap23".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Encap34, "encap3+4".parse().unwrap());
assert_eq!(BondXmitHashPolicy::Encap34, "encap34".parse().unwrap());
assert_eq!(
BondXmitHashPolicy::VlanSrcMac,
"vlan+srcmac".parse().unwrap()
);
assert_eq!(
BondXmitHashPolicy::VlanSrcMac,
"vlan-src-mac".parse().unwrap()
);

assert!(BondXmitHashPolicy::from_str("bogus").is_err());
}
Expand Down Expand Up @@ -310,5 +310,17 @@ fn test_bond_ad_select_from_str() {
assert_eq!(BondAdSelect::Stable, "stable".parse().unwrap());
assert_eq!(BondAdSelect::Bandwidth, "bandwidth".parse().unwrap());
assert_eq!(BondAdSelect::Count, "count".parse().unwrap());
assert_eq!(
BondAdSelect::ActorPortPrio,
"actor_port_prio".parse().unwrap()
);
assert!(BondAdSelect::from_str("bogus").is_err());
}

#[test]
fn test_bond_lacp_rate_from_str() {
use std::str::FromStr;
assert_eq!(BondLacpRate::Slow, "slow".parse().unwrap());
assert_eq!(BondLacpRate::Fast, "fast".parse().unwrap());
assert!(BondLacpRate::from_str("bogus").is_err());
}
Loading