bond: align FromStr/Display strings with iproute2#264
Conversation
- Change BondXmitHashPolicy Display/FromStr strings to match iproute2: "layer34"->"layer3+4", "layer23"->"layer2+3", "encap23"->"encap2+3", "encap34"->"encap3+4", "vlan-src-mac"->"vlan+srcmac" - Add BOND_AD_ACTOR_PORT_PRIO constant and ActorPortPrio variant to BondAdSelect with Display/FromStr support Unit tests for both BondXmitHashPolicy and BondAdSelect FromStr are updated. Signed-off-by: Gris Ge <cnfourt@gmail.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #264 +/- ##
==========================================
+ Coverage 68.10% 68.14% +0.03%
==========================================
Files 144 146 +2
Lines 10103 10420 +317
==========================================
+ Hits 6881 7101 +220
- Misses 3222 3319 +97 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request updates the string representations of several BondXmitHashPolicy variants (e.g., changing "layer34" to "layer3+4") and adds support for the ActorPortPrio variant in the BondAdSelect enum. The review feedback recommends maintaining backward compatibility in FromStr parsing for the old BondXmitHashPolicy string formats to prevent breaking existing configurations, and suggests adding corresponding test assertions to verify this compatibility.
| "layer3+4" => Self::Layer34, | ||
| "layer2+3" => Self::Layer23, | ||
| "encap2+3" => Self::Encap23, | ||
| "encap3+4" => Self::Encap34, | ||
| "vlan+srcmac" => Self::VlanSrcMac, |
There was a problem hiding this comment.
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
- 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)
- When converting from a string slice (&str) to a type where the conversion can fail, implement std::str::FromStr instead of From<&str>.
| 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() | ||
| ); |
There was a problem hiding this comment.
Add assertions to verify that the old string representations are still parsed correctly for backward compatibility.
| 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() | |
| ); |
Add FromStr and Display trait implementations for BondLacpRate using "slow"/"fast" strings. Unit test included. Signed-off-by: Gris Ge <cnfourt@gmail.com>
"layer34"->"layer3+4", "layer23"->"layer2+3", "encap23"->"encap2+3",
"encap34"->"encap3+4", "vlan-src-mac"->"vlan+srcmac"
BondAdSelect with Display/FromStr support
Unit tests for both BondXmitHashPolicy and BondAdSelect FromStr are
updated.