Skip to content

Commit

Permalink
link: use struct BridgeId instead of tuple and fix its endianess
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@ihuguet @cathay4t
ihuguet authored and cathay4t committed Jan 26, 2024
1 parent faffa52 commit fb497b3
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 94 deletions.
91 changes: 58 additions & 33 deletions src/link/link_info/bridge.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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[..]),
Expand Down Expand Up @@ -423,25 +413,14 @@ impl<'a, T: AsRef<[u8]> + ?Sized> Parseable<NlaBuffer<&'a T>> 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")?,
Expand Down Expand Up @@ -536,6 +515,52 @@ impl<'a, T: AsRef<[u8]> + ?Sized> Parseable<NlaBuffer<&'a T>> 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<T: AsRef<[u8]> + ?Sized> Parseable<BridgeIdBuffer<&T>> for BridgeId {
fn parse(buf: &BridgeIdBuffer<&T>) -> Result<Self, DecodeError> {
// 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 {
Expand Down
48 changes: 15 additions & 33 deletions src/link/link_info/bridge_port.rs
View file
Original file line number Diff line number Diff line change
@@ -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,
};

Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -339,32 +337,16 @@ impl<'a, T: AsRef<[u8]> + ?Sized> Parseable<NlaBuffer<&'a T>>
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:?}")
Expand Down
4 changes: 3 additions & 1 deletion src/link/link_info/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
13 changes: 7 additions & 6 deletions src/link/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
43 changes: 22 additions & 21 deletions src/link/tests/bridge.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down

0 comments on commit fb497b3

Please sign in to comment.