From fb497b3b94c774c20c8ff126b99ea1ad024bcada Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Wed, 3 Jan 2024 10:33:01 +0100 Subject: [PATCH] link: use struct BridgeId instead of tuple and fix its endianess MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bridge ID defined by attributes BRIDGE_ID and ROOT_ID, both in bridge and bridge_port, contains a priority field and the MAC address. Use a named struct to show the meaning of these fields instead of using a tuple. Also, fix the endianess of the priority field. The kernel uses a `u8 prio[2]` field instead of `u16`, with the first byte being the highest order byte. See: https://elixir.bootlin.com/linux/v6.6.9/source/net/bridge/br_netlink.c#L1633 Note: to make the file bridge.rs to be well organized, moved the definition of BRIDGE_QUERIER_* close to BridgeQuerierState where it's used. Signed-off-by: Íñigo Huguet --- src/link/link_info/bridge.rs | 91 ++++++++++++++++++++----------- src/link/link_info/bridge_port.rs | 48 +++++----------- src/link/link_info/mod.rs | 4 +- src/link/mod.rs | 13 +++-- src/link/tests/bridge.rs | 43 ++++++++------- 5 files changed, 105 insertions(+), 94 deletions(-) diff --git a/src/link/link_info/bridge.rs b/src/link/link_info/bridge.rs index 59dba27f..b09430ae 100644 --- a/src/link/link_info/bridge.rs +++ b/src/link/link_info/bridge.rs @@ -14,14 +14,6 @@ use netlink_packet_utils::{ DecodeError, }; -const BRIDGE_QUERIER_IP_ADDRESS: u16 = 1; -const BRIDGE_QUERIER_IP_PORT: u16 = 2; -const BRIDGE_QUERIER_IP_OTHER_TIMER: u16 = 3; -// const BRIDGE_QUERIER_PAD: u16 = 4; -const BRIDGE_QUERIER_IPV6_ADDRESS: u16 = 5; -const BRIDGE_QUERIER_IPV6_PORT: u16 = 6; -const BRIDGE_QUERIER_IPV6_OTHER_TIMER: u16 = 7; - const IFLA_BR_FORWARD_DELAY: u16 = 1; const IFLA_BR_HELLO_TIME: u16 = 2; const IFLA_BR_MAX_AGE: u16 = 3; @@ -98,8 +90,8 @@ pub enum InfoBridge { Priority(u16), VlanProtocol(u16), GroupFwdMask(u16), - RootId((u16, [u8; 6])), - BridgeId((u16, [u8; 6])), + RootId(BridgeId), + BridgeId(BridgeId), RootPort(u16), VlanDefaultPvid(u16), VlanFiltering(u8), @@ -219,10 +211,8 @@ impl Nla for InfoBridge { Self::VlanProtocol(value) => BigEndian::write_u16(buffer, *value), - Self::RootId((priority, ref address)) - | Self::BridgeId((priority, ref address)) => { - NativeEndian::write_u16(buffer, *priority); - buffer[2..].copy_from_slice(&address[..]); + Self::RootId(bridge_id) | Self::BridgeId(bridge_id) => { + bridge_id.emit(buffer) } Self::GroupAddr(value) => buffer.copy_from_slice(&value[..]), @@ -423,25 +413,14 @@ impl<'a, T: AsRef<[u8]> + ?Sized> Parseable> for InfoBridge { parse_u16(payload) .context("invalid IFLA_BR_GROUP_FWD_MASK value")?, ), - IFLA_BR_ROOT_ID | IFLA_BR_BRIDGE_ID => { - if payload.len() != 8 { - return Err( - "invalid IFLA_BR_ROOT_ID or IFLA_BR_BRIDGE_ID value" - .into(), - ); - } - - let priority = NativeEndian::read_u16(&payload[..2]); - let address = parse_mac(&payload[2..]).context( - "invalid IFLA_BR_ROOT_ID or IFLA_BR_BRIDGE_ID value", - )?; - - match buf.kind() { - IFLA_BR_ROOT_ID => Self::RootId((priority, address)), - IFLA_BR_BRIDGE_ID => Self::BridgeId((priority, address)), - _ => unreachable!(), - } - } + IFLA_BR_ROOT_ID => Self::RootId( + BridgeId::parse(&BridgeIdBuffer::new(payload)) + .context("invalid IFLA_BR_ROOT_ID value")?, + ), + IFLA_BR_BRIDGE_ID => Self::BridgeId( + BridgeId::parse(&BridgeIdBuffer::new(payload)) + .context("invalid IFLA_BR_BRIDGE_ID value")?, + ), IFLA_BR_GROUP_ADDR => Self::GroupAddr( parse_mac(payload) .context("invalid IFLA_BR_GROUP_ADDR value")?, @@ -536,6 +515,52 @@ impl<'a, T: AsRef<[u8]> + ?Sized> Parseable> for InfoBridge { } } +const BRIDGE_ID_LEN: usize = 8; + +#[derive(Debug, PartialEq, Eq, Clone)] +pub struct BridgeId { + pub priority: u16, + pub address: [u8; 6], +} + +buffer!(BridgeIdBuffer(BRIDGE_ID_LEN) { + priority: (u16, 0..2), + address: (slice, 2..BRIDGE_ID_LEN) +}); + +impl + ?Sized> Parseable> for BridgeId { + fn parse(buf: &BridgeIdBuffer<&T>) -> Result { + // Priority is encoded in big endian. From kernel's + // net/bridge/br_netlink.c br_fill_info(): + // u16 priority = (br->bridge_id.prio[0] << 8) | br->bridge_id.prio[1]; + Ok(Self { + priority: u16::from_be(buf.priority()), + address: parse_mac(buf.address()) + .context("invalid MAC address in BridgeId buffer")?, + }) + } +} + +impl Emitable for BridgeId { + fn buffer_len(&self) -> usize { + BRIDGE_ID_LEN + } + + fn emit(&self, buffer: &mut [u8]) { + let mut buffer = BridgeIdBuffer::new(buffer); + buffer.set_priority(self.priority.to_be()); + buffer.address_mut().copy_from_slice(&self.address[..]); + } +} + +const BRIDGE_QUERIER_IP_ADDRESS: u16 = 1; +const BRIDGE_QUERIER_IP_PORT: u16 = 2; +const BRIDGE_QUERIER_IP_OTHER_TIMER: u16 = 3; +// const BRIDGE_QUERIER_PAD: u16 = 4; +const BRIDGE_QUERIER_IPV6_ADDRESS: u16 = 5; +const BRIDGE_QUERIER_IPV6_PORT: u16 = 6; +const BRIDGE_QUERIER_IPV6_OTHER_TIMER: u16 = 7; + #[derive(Debug, Clone, Eq, PartialEq)] #[non_exhaustive] pub enum BridgeQuerierState { diff --git a/src/link/link_info/bridge_port.rs b/src/link/link_info/bridge_port.rs index d121fe58..35c818cf 100644 --- a/src/link/link_info/bridge_port.rs +++ b/src/link/link_info/bridge_port.rs @@ -1,10 +1,11 @@ // SPDX-License-Identifier: MIT +use crate::link::{BridgeId, BridgeIdBuffer}; use anyhow::Context; use byteorder::{ByteOrder, NativeEndian}; use netlink_packet_utils::{ nla::{DefaultNla, Nla, NlaBuffer}, - parsers::{parse_mac, parse_u16, parse_u32, parse_u64, parse_u8}, - traits::Parseable, + parsers::{parse_u16, parse_u32, parse_u64, parse_u8}, + traits::{Emitable, Parseable}, DecodeError, }; @@ -68,8 +69,8 @@ pub enum InfoBridgePort { UnicastFlood(bool), ProxyARP(bool), ProxyARPWifi(bool), - RootId((u16, [u8; 6])), - BridgeId((u16, [u8; 6])), + RootId(BridgeId), + BridgeId(BridgeId), DesignatedPort(u16), DesignatedCost(u16), PortId(u16), @@ -199,11 +200,8 @@ impl Nla for InfoBridgePort { | InfoBridgePort::HoldTimer(value) => { NativeEndian::write_u64(buffer, *value) } - InfoBridgePort::RootId((prio, addr)) - | InfoBridgePort::BridgeId((prio, addr)) => { - NativeEndian::write_u16(buffer, *prio); - buffer[2..].copy_from_slice(&addr[..]); - } + InfoBridgePort::RootId(bridge_id) + | InfoBridgePort::BridgeId(bridge_id) => bridge_id.emit(buffer), InfoBridgePort::State(state) => buffer[0] = (*state).into(), InfoBridgePort::MulticastRouter(mcast_router) => { buffer[0] = (*mcast_router).into() @@ -339,32 +337,16 @@ impl<'a, T: AsRef<[u8]> + ?Sized> Parseable> format!("invalid IFLA_BRPORT_PROXYARP_WIFI {payload:?}") })? > 0, ), - IFLA_BRPORT_ROOT_ID => { - if payload.len() != 8 { - return Err(format!( - "invalid IFLA_BRPORT_ROOT_ID {payload:?}" - ) - .into()); - } - let prio = NativeEndian::read_u16(&payload[0..2]); - let addr = parse_mac(&payload[2..]).with_context(|| { + IFLA_BRPORT_ROOT_ID => Self::RootId( + BridgeId::parse(&BridgeIdBuffer::new(payload)).with_context(|| { format!("invalid IFLA_BRPORT_ROOT_ID {payload:?}") - })?; - InfoBridgePort::RootId((prio, addr)) - } - IFLA_BRPORT_BRIDGE_ID => { - if payload.len() != 8 { - return Err(format!( - "invalid IFLA_BRPORT_BRIDGE_ID {payload:?}" - ) - .into()); - } - let prio = NativeEndian::read_u16(&payload[0..2]); - let addr = parse_mac(&payload[2..]).with_context(|| { + })?, + ), + IFLA_BRPORT_BRIDGE_ID => Self::BridgeId( + BridgeId::parse(&BridgeIdBuffer::new(payload)).with_context(|| { format!("invalid IFLA_BRPORT_BRIDGE_ID {payload:?}") - })?; - InfoBridgePort::BridgeId((prio, addr)) - } + })?, + ), IFLA_BRPORT_DESIGNATED_PORT => InfoBridgePort::DesignatedPort( parse_u16(payload).with_context(|| { format!("invalid IFLA_BRPORT_DESIGNATED_PORT {payload:?}") diff --git a/src/link/link_info/mod.rs b/src/link/link_info/mod.rs index c32bac73..2b00a69f 100644 --- a/src/link/link_info/mod.rs +++ b/src/link/link_info/mod.rs @@ -29,7 +29,9 @@ mod xstats; pub use self::bond::{BondAdInfo, InfoBond}; pub use self::bond_port::{BondPortState, InfoBondPort, MiiStatus}; -pub use self::bridge::{BridgeQuerierState, InfoBridge}; +pub use self::bridge::{ + BridgeId, BridgeIdBuffer, BridgeQuerierState, InfoBridge, +}; pub use self::bridge_port::{ BridgePortMulticastRouter, BridgePortState, InfoBridgePort, }; diff --git a/src/link/mod.rs b/src/link/mod.rs index 18e91936..e2247a27 100644 --- a/src/link/mod.rs +++ b/src/link/mod.rs @@ -38,12 +38,13 @@ pub use self::ext_mask::LinkExtentMask; pub use self::header::{LinkHeader, LinkMessageBuffer}; pub use self::link_flag::LinkFlag; pub use self::link_info::{ - BondAdInfo, BondPortState, BridgePortMulticastRouter, BridgePortState, - BridgeQuerierState, HsrProtocol, InfoBond, InfoBondPort, InfoBridge, - InfoBridgePort, InfoData, InfoGreTap, InfoGreTap6, InfoGreTun, InfoGreTun6, - InfoGtp, InfoHsr, InfoIpVlan, InfoIpoib, InfoKind, InfoMacSec, InfoMacVlan, - InfoMacVtap, InfoPortData, InfoPortKind, InfoSitTun, InfoTun, InfoVeth, - InfoVlan, InfoVrf, InfoVti, InfoVxlan, InfoXfrm, LinkInfo, LinkXstats, + BondAdInfo, BondPortState, BridgeId, BridgeIdBuffer, + BridgePortMulticastRouter, BridgePortState, BridgeQuerierState, + HsrProtocol, InfoBond, InfoBondPort, InfoBridge, InfoBridgePort, InfoData, + InfoGreTap, InfoGreTap6, InfoGreTun, InfoGreTun6, InfoGtp, InfoHsr, + InfoIpVlan, InfoIpoib, InfoKind, InfoMacSec, InfoMacVlan, InfoMacVtap, + InfoPortData, InfoPortKind, InfoSitTun, InfoTun, InfoVeth, InfoVlan, + InfoVrf, InfoVti, InfoVxlan, InfoXfrm, LinkInfo, LinkXstats, MacSecCipherId, MacSecOffload, MacSecValidate, MiiStatus, VlanQosMapping, }; pub use self::link_layer_type::LinkLayerType; diff --git a/src/link/tests/bridge.rs b/src/link/tests/bridge.rs index 24122edf..ab40b6cd 100644 --- a/src/link/tests/bridge.rs +++ b/src/link/tests/bridge.rs @@ -7,11 +7,12 @@ use netlink_packet_utils::{ use crate::link::{ af_spec::VecAfSpecBridge, AfSpecBridge, AfSpecInet, AfSpecInet6, - AfSpecUnspec, BridgePortMulticastRouter, BridgePortState, BridgeVlanInfo, - Inet6CacheInfo, Inet6DevConf, Inet6IfaceFlag, Inet6IfaceFlags, InetDevConf, - InfoBridge, InfoBridgePort, InfoData, InfoKind, InfoPortData, InfoPortKind, - LinkAttribute, LinkFlag, LinkHeader, LinkInfo, LinkLayerType, LinkMessage, - LinkMessageBuffer, LinkXdp, Map, State, Stats, Stats64, XdpAttached, + AfSpecUnspec, BridgeId, BridgePortMulticastRouter, BridgePortState, + BridgeVlanInfo, Inet6CacheInfo, Inet6DevConf, Inet6IfaceFlag, + Inet6IfaceFlags, InetDevConf, InfoBridge, InfoBridgePort, InfoData, + InfoKind, InfoPortData, InfoPortKind, LinkAttribute, LinkFlag, LinkHeader, + LinkInfo, LinkLayerType, LinkMessage, LinkMessageBuffer, LinkXdp, Map, + State, Stats, Stats64, XdpAttached, }; use crate::AddressFamily; @@ -260,14 +261,14 @@ fn test_parse_link_bridge_no_extention_mask() { InfoBridge::Priority(32768), InfoBridge::VlanFiltering(0), InfoBridge::GroupFwdMask(0), - InfoBridge::BridgeId(( - 0x0080, // iproute is showing it as 0x8000 - [0x00, 0x23, 0x45, 0x67, 0x89, 0x1c], - )), - InfoBridge::RootId(( - 0x0080, // iproute is showing it as 0x8000 - [0x00, 0x23, 0x45, 0x67, 0x89, 0x1c], - )), + InfoBridge::BridgeId(BridgeId { + priority: 0x8000, + address: [0x00, 0x23, 0x45, 0x67, 0x89, 0x1c], + }), + InfoBridge::RootId(BridgeId { + priority: 0x8000, + address: [0x00, 0x23, 0x45, 0x67, 0x89, 0x1c], + }), InfoBridge::RootPort(0), InfoBridge::RootPathCost(0), InfoBridge::TopologyChange(0), @@ -513,14 +514,14 @@ fn test_bridge_port_link_info() { InfoBridgePort::BroadcastFlood(true), InfoBridgePort::ProxyARP(false), InfoBridgePort::ProxyARPWifi(false), - InfoBridgePort::RootId(( - 0x0080, - [0x52, 0x54, 0x00, 0xde, 0x0d, 0x2e], - )), - InfoBridgePort::BridgeId(( - 0x0080, - [0x52, 0x54, 0x00, 0xde, 0x0d, 0x2e], - )), + InfoBridgePort::RootId(BridgeId { + priority: 0x8000, + address: [0x52, 0x54, 0x00, 0xde, 0x0d, 0x2e], + }), + InfoBridgePort::BridgeId(BridgeId { + priority: 0x8000, + address: [0x52, 0x54, 0x00, 0xde, 0x0d, 0x2e], + }), InfoBridgePort::DesignatedPort(32769), InfoBridgePort::DesignatedCost(0), InfoBridgePort::PortId(32769),