When performing the database inserts for a SwitchPortSettingsCreate request, we assemble an in-memory map keyed by IP address:
|
let mut peer_by_addr: BTreeMap<IpAddr, &networking::BgpPeer> = |
|
BTreeMap::new(); |
For unnumbered peers, we use the sentinel value 0.0.0.0 as the key:
|
// Track peers for policy lookup. For unnumbered peers (addr is None), |
|
// use UNSPECIFIED as the sentinel value. |
|
let lookup_addr = |
|
p.addr.unwrap_or(IpAddr::V4(Ipv4Addr::UNSPECIFIED)); |
After performing the inserts (which do record the correct data for each peer - this bug only affects the in-memory result we return), we reassemble the communities and import/export policies by matching up the inserted row against its corresponding entry in peer_by_addr:
|
// Lookup policies and communities for peers. For unnumbered peers (addr |
|
// is None), use UNSPECIFIED as the lookup key. |
|
let lookup_addr = |
|
p.addr.map(|a| a.ip()).unwrap_or(IpAddr::V4(Ipv4Addr::UNSPECIFIED)); |
|
let (allowed_import, allowed_export, communities) = ( |
|
peer_by_addr |
|
.get(&lookup_addr) |
|
.map(|x| x.allowed_import.clone()) |
|
.unwrap_or(ImportExportPolicy::NoFiltering), |
|
peer_by_addr |
|
.get(&lookup_addr) |
|
.map(|x| x.allowed_export.clone()) |
|
.unwrap_or(ImportExportPolicy::NoFiltering), |
|
peer_by_addr |
|
.get(&lookup_addr) |
|
.map(|x| x.communities.clone()) |
|
.unwrap_or(Vec::new()), |
|
); |
|
result.bgp_peers.push( |
|
BgpPeerFromDbBuilder { |
|
peer_config: &p, |
|
communities, |
|
allowed_import, |
|
allowed_export, |
|
} |
|
.build(), |
|
); |
However, unnumbered peers all share the same 0.0.0.0 key, so we only have one possible value, and we reuse that for all unnumbered peers. If we insert two different unnumbered peers associated with two different links and those two peers have different communities or import/export policies, the result we return will be incorrect: we'll return the same communities and import/export policies for all unnumbered peers (the exact value being whichever was inserted into the peer_by_addr map last).
This came up during cleanup work for #9832, when reworking the database representations for unnumbered peers to get rid of the need for a sentinel value. The upcoming PR that does that rework will fix this.
When performing the database inserts for a
SwitchPortSettingsCreaterequest, we assemble an in-memory map keyed by IP address:omicron/nexus/db-queries/src/db/datastore/switch_port.rs
Lines 1410 to 1411 in eb0bb82
For unnumbered peers, we use the sentinel value
0.0.0.0as the key:omicron/nexus/db-queries/src/db/datastore/switch_port.rs
Lines 1416 to 1419 in eb0bb82
After performing the inserts (which do record the correct data for each peer - this bug only affects the in-memory result we return), we reassemble the communities and import/export policies by matching up the inserted row against its corresponding entry in
peer_by_addr:omicron/nexus/db-queries/src/db/datastore/switch_port.rs
Lines 1535 to 1561 in eb0bb82
However, unnumbered peers all share the same
0.0.0.0key, so we only have one possible value, and we reuse that for all unnumbered peers. If we insert two different unnumbered peers associated with two different links and those two peers have different communities or import/export policies, the result we return will be incorrect: we'll return the same communities and import/export policies for all unnumbered peers (the exact value being whichever was inserted into thepeer_by_addrmap last).This came up during cleanup work for #9832, when reworking the database representations for unnumbered peers to get rid of the need for a sentinel value. The upcoming PR that does that rework will fix this.