Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 11 additions & 11 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -621,8 +621,8 @@ ntp-admin-client = { path = "clients/ntp-admin-client" }
ntp-admin-v1-client = { path = "clients/ntp-admin-v1-client" }
ntp-admin-types = { path = "ntp-admin/types" }
ntp-admin-types-versions = { path = "ntp-admin/types/versions" }
mg-admin-client = { git = "https://github.com/oxidecomputer/maghemite", rev = "dc84a6ce494a9d004568b67e22add2285a04d887" }
ddm-admin-client = { git = "https://github.com/oxidecomputer/maghemite", rev = "dc84a6ce494a9d004568b67e22add2285a04d887" }
mg-admin-client = { git = "https://github.com/oxidecomputer/maghemite", rev = "0d06cd1533cb8c376ac4ac30a278815c02e2f0e1" }
ddm-admin-client = { git = "https://github.com/oxidecomputer/maghemite", rev = "0d06cd1533cb8c376ac4ac30a278815c02e2f0e1" }
multimap = "0.10.1"
nexus-auth = { path = "nexus/auth" }
nexus-background-task-interface = { path = "nexus/background-task-interface" }
Expand Down Expand Up @@ -761,7 +761,7 @@ rats-corim = { git = "https://github.com/oxidecomputer/rats-corim.git", rev = "f
raw-cpuid = { git = "https://github.com/oxidecomputer/rust-cpuid.git", rev = "a4cf01df76f35430ff5d39dc2fe470bcb953503b" }
rayon = "1.10"
rcgen = { version = "0.12.1", default-features = false, features = ["aws_lc_rs", "pem"] }
mg-api-types = { git = "https://github.com/oxidecomputer/maghemite", rev = "dc84a6ce494a9d004568b67e22add2285a04d887" }
mg-api-types = { git = "https://github.com/oxidecomputer/maghemite", rev = "0d06cd1533cb8c376ac4ac30a278815c02e2f0e1" }
reconfigurator-cli = { path = "dev-tools/reconfigurator-cli" }
reedline = "0.40.0"
ref-cast = "1.0"
Expand Down
3 changes: 3 additions & 0 deletions nexus/external-api/output/nexus_tags.txt
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,9 @@ networking_bgp_exported GET /v1/system/networking/bgp-expo
networking_bgp_imported GET /v1/system/networking/bgp-imported
networking_bgp_message_history GET /v1/system/networking/bgp-message-history
networking_bgp_status GET /v1/system/networking/bgp-status
networking_bgp_unnumbered_interface_list GET /v1/system/networking/bgp/unnumbered/interfaces
networking_bgp_unnumbered_interface_view GET /v1/system/networking/bgp/unnumbered/interfaces/{switch_slot}/{interface_name}
networking_bgp_unnumbered_manager_status GET /v1/system/networking/bgp/unnumbered/manager

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that we have a great pattern here, but these stand out in being the only ones to nest something under bgp. Maybe all of them should look like /bgp/*, but for now, for consistency maybe it should be /v1/system/networking/bgp-unnumbered-interfaces?

networking_bfd_disable POST /v1/system/networking/bfd-disable
networking_bfd_enable POST /v1/system/networking/bfd-enable
networking_bfd_status GET /v1/system/networking/bfd-status
networking_bgp_announce_set_delete DELETE /v1/system/networking/bgp-announce-set/{announce_set}
networking_bgp_announce_set_list GET /v1/system/networking/bgp-announce-set
networking_bgp_announce_set_update PUT /v1/system/networking/bgp-announce-set
networking_bgp_announcement_list GET /v1/system/networking/bgp-announce-set/{announce_set}/announcement
networking_bgp_config_create POST /v1/system/networking/bgp
networking_bgp_config_delete DELETE /v1/system/networking/bgp
networking_bgp_config_list GET /v1/system/networking/bgp
networking_bgp_config_update PUT /v1/system/networking/bgp
networking_bgp_exported GET /v1/system/networking/bgp-exported
networking_bgp_imported GET /v1/system/networking/bgp-imported
networking_bgp_message_history GET /v1/system/networking/bgp-message-history
networking_bgp_status GET /v1/system/networking/bgp-status
networking_bgp_unnumbered_interface_list GET /v1/system/networking/bgp/unnumbered/interfaces
networking_bgp_unnumbered_interface_view GET /v1/system/networking/bgp/unnumbered/interfaces/{switch_slot}/{interface_name}
networking_bgp_unnumbered_manager_status GET /v1/system/networking/bgp/unnumbered/manager

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can shift over to that convention for consistency. Thanks!

networking_inbound_icmp_update PUT /v1/system/networking/inbound-icmp
networking_inbound_icmp_view GET /v1/system/networking/inbound-icmp
networking_loopback_address_create POST /v1/system/networking/loopback-address
Expand Down
44 changes: 44 additions & 0 deletions nexus/external-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ api_versions!([
// | date-based version should be at the top of the list.
// v
// (next_yyyy_mm_dd_nn, IDENT),
(2026_06_25_00, BGP_UNNUMBERED_STATUS),
(2026_06_10_00, BGP_CONFIGURATION_UPDATE),
(2026_06_08_00, INSTANCE_CPU_TYPE_TURIN_V2),
(2026_06_05_00, EXTERNAL_JUMBO_FRAMES),
Expand Down Expand Up @@ -6004,6 +6005,49 @@ pub trait NexusExternalApi {
})
}

/// Get BGP Unnumbered manager state
#[endpoint {
method = GET,
path = "/v1/system/networking/bgp/unnumbered/manager",
tags = ["system/networking"],
versions = VERSION_BGP_UNNUMBERED_STATUS..,
}]
async fn networking_bgp_unnumbered_manager_status(
rqctx: RequestContext<Self::Context>,
) -> Result<
HttpResponseOk<Vec<latest::networking::SwitchUnnumberedManagerState>>,
HttpError,
>;

/// List BGP Unnumbered interfaces
#[endpoint {
method = GET,
path = "/v1/system/networking/bgp/unnumbered/interfaces",
tags = ["system/networking"],
versions = VERSION_BGP_UNNUMBERED_STATUS..,
}]
async fn networking_bgp_unnumbered_interface_list(
rqctx: RequestContext<Self::Context>,
) -> Result<
HttpResponseOk<Vec<latest::networking::SwitchUnnumberedInterface>>,
HttpError,
>;

/// Get BGP Unnumbered interface state
#[endpoint {
method = GET,
path = "/v1/system/networking/bgp/unnumbered/interfaces/{switch_slot}/{interface_name}",
tags = ["system/networking"],
versions = VERSION_BGP_UNNUMBERED_STATUS..,
}]
async fn networking_bgp_unnumbered_interface_view(
rqctx: RequestContext<Self::Context>,
path_params: Path<latest::networking::UnnumberedInterfacePath>,
) -> Result<
HttpResponseOk<latest::networking::SwitchUnnumberedInterface>,
HttpError,
>;

/// Get user-facing services IP allowlist
#[endpoint {
method = GET,
Expand Down
1 change: 1 addition & 0 deletions nexus/src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ mod switch_port;
mod system_networking;
pub mod test_interfaces;
mod trust_quorum;
mod unnumbered;
mod update;
mod utilization;
mod volume;
Expand Down
125 changes: 125 additions & 0 deletions nexus/src/app/unnumbered.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

use nexus_db_queries::context::OpContext;
use nexus_types::external_api::networking::{
SwitchUnnumberedInterface, SwitchUnnumberedManagerState,
};
use omicron_common::api::external::Error;
use sled_agent_types::early_networking::SwitchSlot;
use strum::IntoEnumIterator;

fn maghemite_interface_name(interface_name: &str) -> String {
format!("tfport{interface_name}_0")
}

impl super::Nexus {
pub async fn bgp_unnumbered_manager_status(
&self,
_opctx: &OpContext,
) -> Result<Vec<SwitchUnnumberedManagerState>, Error> {
// Ask each switch about the BGP unnumbered interfaces it manages.
let mg_clients = self.mg_clients().await.map_err(|err| {
Error::internal_error(&format!("failed to get mg clients: {err}"))
})?;
let mut result = Vec::new();
for switch_slot in SwitchSlot::iter() {
// Log an error if we only have one scrimlet, but keep going.
// We still want to return anything we're able to collect.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not guaranteed to have any clients here; from the docs on resolve_mgd_clients():

/// # Errors
///
/// Fails if we cannot resolve MGD in DNS.
///
/// For any MGD instance we resolve via DNS, if the MGD instance does not know
/// its own switch slot, the switch slot -> client mapping for that instance
/// will be omitted from the returned map. Callers must not assume an `Ok(_)`
/// return value contains any client.

As written, that means if we get back an empty map from self.mg_clients(), we'll return an Ok(Vec::new()) result. Is that okay? I wonder if instead of returning a Vec, we should return something like a BTreeMap<SwitchSlot, UnnumberedManagerStateOrError>? Then we could return something logically like

{ "switch0": "no client found: blah blah error", "switch1": "SomeValidStatus" }

let Some(mg_client) = mg_clients.get(&switch_slot) else {
warn!(
self.log, "no mgd client found for switch slot";
"switch-slot" => ?switch_slot,
);
continue;
};
let status = mg_client
.get_bgp_unnumbered_manager_state()
.await
.map_err(|e| {
Error::internal_error(&format!(
"maghemite get BGP unnumbered manager state: {e}"
))
})?
.into_inner();

result.push(SwitchUnnumberedManagerState {
switch_slot,
state: status.into(),
});
}
Ok(result)
}

pub async fn bgp_unnumbered_interfaces(
&self,
_opctx: &OpContext,
) -> Result<Vec<SwitchUnnumberedInterface>, Error> {
// Ask each switch about the BGP unnumbered interfaces it manages.
let mg_clients = self.mg_clients().await.map_err(|err| {
Error::internal_error(&format!("failed to get mg clients: {err}"))
})?;
let mut result = Vec::new();
for switch_slot in SwitchSlot::iter() {
// Log an error if we only have one scrimlet, but keep going.
// We still want to return anything we're able to collect.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same note here about maybe not having clients, and maybe returning a map keyed by switch slot so we can give back partial success?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially thought about returning a map keyed on switch slot, but it looked like other endpoints for switch-specific queries (e.g. lldp) all used the approach of returning a vec of wrapper structs that have switch slot as a member.

If we do go with a map, I think we could get rid of the wrapper structs SwitchUnnumberedManagerState and SwitchUnnumberedInterface since all they do for UnnumberedManagerState and UnnumberedInterface is embed the switch slot.

I don't work in this codebase very often, so I'm happy to go whichever way is preferable to the folks who live here

let Some(mg_client) = mg_clients.get(&switch_slot) else {
warn!(
self.log, "no mgd client found for switch slot";
"switch-slot" => ?switch_slot,
);
continue;
};
let interfaces = mg_client
.get_bgp_unnumbered_interfaces()
.await
.map_err(|e| {
Error::internal_error(&format!(
"maghemite get BGP unnumbered interfaces: {e}"
))
})?
.into_inner();

for interface in interfaces {
result.push(SwitchUnnumberedInterface {
switch_slot,
interface: interface.into(),
});
}
}
Ok(result)
}

pub async fn bgp_unnumbered_interface(
&self,
_opctx: &OpContext,
switch_slot: SwitchSlot,
interface_name: String,
) -> Result<SwitchUnnumberedInterface, Error> {
let mg_clients = self.mg_clients().await.map_err(|err| {
Error::internal_error(&format!("failed to get mg clients: {err}"))
})?;
let mg_client = mg_clients.get(&switch_slot).ok_or_else(|| {
Error::internal_error(&format!(
"no mgd client found for switch slot {switch_slot:?}"
))
})?;

let interface_name = maghemite_interface_name(&interface_name);
let interface = mg_client
.get_bgp_unnumbered_interface_detail(&interface_name)
.await
.map_err(|e| {
Error::internal_error(&format!(
"maghemite get BGP unnumbered interface detail: {e}"
))
})?
.into_inner();

Ok(SwitchUnnumberedInterface {
switch_slot,
interface: interface.into(),
})
}
}
Loading
Loading