diff --git a/dpd-api/src/lib.rs b/dpd-api/src/lib.rs index 7441178e..bea3e935 100644 --- a/dpd-api/src/lib.rs +++ b/dpd-api/src/lib.rs @@ -9,11 +9,12 @@ use std::collections::BTreeMap; use std::net::{Ipv4Addr, Ipv6Addr}; +use dpd_types_versions::v14::CmdAck; use dpd_types_versions::{latest, v1, v4, v7}; use dropshot::{ - EmptyScanParams, HttpError, HttpResponseCreated, HttpResponseDeleted, - HttpResponseOk, HttpResponseUpdatedNoContent, PaginationParams, Path, - Query, RequestContext, ResultsPage, TypedBody, + EmptyScanParams, ErrorStatusCode, HttpError, HttpResponseCreated, + HttpResponseDeleted, HttpResponseOk, HttpResponseUpdatedNoContent, + PaginationParams, Path, Query, RequestContext, ResultsPage, TypedBody, }; use dropshot_api_manager_types::api_versions; @@ -29,6 +30,7 @@ api_versions!([ // | example for the next person. // v // (next_int, IDENT), + (14, ADDRESS_TAGS), (13, ALLOW_DDM_TRAFFIC), (12, PRBS_ERROR_TRACKING), (11, WALLCLOCK_HISTORY), @@ -1045,7 +1047,9 @@ pub trait DpdApi { async fn link_ipv4_list( rqctx: RequestContext, path: Path, - query: Query>, + query: Query< + PaginationParams>, + >, ) -> Result>, HttpError>; /// Add an IPv4 address to a link. @@ -1063,22 +1067,64 @@ pub trait DpdApi { #[endpoint { method = DELETE, path = "/ports/{port_id}/links/{link_id}/ipv4", + versions = ..VERSION_ADDRESS_TAGS }] async fn link_ipv4_reset( rqctx: RequestContext, path: Path, - ) -> Result; + ) -> Result { + let _ = rqctx; + let _ = path; + + Err(HttpError { + status_code: ErrorStatusCode::GONE, + error_code: None, + external_message: ADDR_RESET_EMSG.to_string(), + internal_message: ADDR_RESET_EMSG.to_string(), + headers: None, + }) + } - /// Remove an IPv4 address from a link. + /// Remove an IPv4 address from this link. + /// + /// If a tag is provided, the address will only be deleted + /// if it belongs to the given tag. + /// + /// - Returns 404 NOT_FOUND if the address is not registered + /// on the switch. + /// - Returns 409 CONFLICT if the address exists on the switch + /// but is owned by something else (link, loopback, tag, etc). #[endpoint { method = DELETE, path = "/ports/{port_id}/links/{link_id}/ipv4/{address}", + versions = VERSION_ADDRESS_TAGS.. }] async fn link_ipv4_delete( rqctx: RequestContext, path: Path, + tag: Query>, ) -> Result; + /// This version's replacement allows users to specify a tag + /// scope for deletion. That allows callers to cooperate more easily. + #[endpoint { + method = DELETE, + path = "/ports/{port_id}/links/{link_id}/ipv4/{address}", + versions = ..VERSION_ADDRESS_TAGS, + operation_id = "link_ipv4_delete" + }] + async fn link_ipv4_delete_v1( + rqctx: RequestContext, + path: Path, + ) -> Result { + Self::link_ipv4_delete( + rqctx, + path, + latest::misc::MaybeTagged::default().into(), + ) + .await + } + /// List the IPv6 addresses associated with a link. #[endpoint { method = GET, @@ -1087,7 +1133,9 @@ pub trait DpdApi { async fn link_ipv6_list( rqctx: RequestContext, path: Path, - query: Query>, + query: Query< + PaginationParams>, + >, ) -> Result>, HttpError>; /// Add an IPv6 address to a link. @@ -1105,22 +1153,63 @@ pub trait DpdApi { #[endpoint { method = DELETE, path = "/ports/{port_id}/links/{link_id}/ipv6", + versions = ..VERSION_ADDRESS_TAGS }] async fn link_ipv6_reset( rqctx: RequestContext, path: Path, - ) -> Result; + ) -> Result { + let _ = rqctx; + let _ = path; + + Err(HttpError { + status_code: ErrorStatusCode::GONE, + error_code: None, + external_message: ADDR_RESET_EMSG.to_string(), + internal_message: ADDR_RESET_EMSG.to_string(), + headers: None, + }) + } - /// Remove an IPv6 address from a link. + /// Remove an IPv6 address from this link. + /// + /// If a tag is provided, the address will only be deleted + /// if it belongs to the given tag. + /// + /// - Returns 404 NOT_FOUND if the address is not registered + /// on the switch. + /// - Returns 409 CONFLICT if the address exists on the switch + /// but is owned by something else (link, loopback, tag, etc). #[endpoint { method = DELETE, path = "/ports/{port_id}/links/{link_id}/ipv6/{address}", + versions = VERSION_ADDRESS_TAGS.., }] async fn link_ipv6_delete( rqctx: RequestContext, path: Path, + tag: Query>, ) -> Result; + // Replaced for the same reason as [`link_ipv4_delete_v1`]. + #[endpoint { + method = DELETE, + path = "/ports/{port_id}/links/{link_id}/ipv6/{address}", + versions = ..VERSION_ADDRESS_TAGS, + operation_id = "link_ipv6_delete" + }] + async fn link_ipv6_delete_v1( + rqctx: RequestContext, + path: Path, + ) -> Result { + Self::link_ipv6_delete( + rqctx, + path, + latest::misc::MaybeTagged::default().into(), + ) + .await + } + /// Get a link's MAC address. #[endpoint { method = GET, @@ -1236,29 +1325,75 @@ pub trait DpdApi { rqctx: RequestContext, ) -> Result>, HttpError>; - /** - * Add a loopback IPv4. - */ + /// Creates an IPv4 loopback address if one does not yet exist. + /// + /// Returns CONFLICT 409 if this address already belongs to another + /// interface. #[endpoint { method = POST, path = "/loopback/ipv4", + versions = VERSION_ADDRESS_TAGS.., }] async fn loopback_ipv4_create( rqctx: RequestContext, val: TypedBody, - ) -> Result; + ) -> Result, HttpError>; /** - * Remove one loopback IPv4 address. + * Add a loopback IPv4. */ + #[endpoint { + method = POST, + path = "/loopback/ipv4", + versions = ..VERSION_ADDRESS_TAGS, + operation_id = "loopback_ipv4_create" + }] + async fn loopback_ipv4_create_v1( + rqctx: RequestContext, + val: TypedBody, + ) -> Result { + Self::loopback_ipv4_create(rqctx, val).await?; + Ok(HttpResponseUpdatedNoContent()) + } + + /// Remove a loopback IPv4 address. + /// + /// If a tag is provided, the address will only be deleted + /// if it belongs to the given tag. + /// + /// Returns 200 for any exit condition in which this address + /// does not exist on loopback, even if the address still + /// exists on another interface. #[endpoint { method = DELETE, path = "/loopback/ipv4/{ipv4}", + versions = VERSION_ADDRESS_TAGS.. }] async fn loopback_ipv4_delete( rqctx: RequestContext, path: Path, - ) -> Result; + tag: Query>, + ) -> Result, HttpError>; + + // Replaced for the same reason as [`link_ipv4_delete_v1`]. + #[endpoint { + method = DELETE, + path = "/loopback/ipv4/{ipv4}", + versions = ..VERSION_ADDRESS_TAGS, + operation_id = "loopback_ipv4_delete" + }] + async fn loopback_ipv4_delete_v1( + rqctx: RequestContext, + path: Path, + ) -> Result { + Self::loopback_ipv4_delete( + rqctx, + path, + latest::misc::MaybeTagged::default().into(), + ) + .await?; + Ok(HttpResponseDeleted()) + } /** * Get loopback IPv6 addresses. @@ -1271,29 +1406,75 @@ pub trait DpdApi { rqctx: RequestContext, ) -> Result>, HttpError>; - /** - * Add a loopback IPv6. - */ + /// Creates an IPv6 loopback address if one does not yet exist. + /// + /// Returns CONFLICT 409 if this address already belongs to another + /// interface. #[endpoint { method = POST, path = "/loopback/ipv6", + versions = VERSION_ADDRESS_TAGS.. }] async fn loopback_ipv6_create( rqctx: RequestContext, val: TypedBody, - ) -> Result; + ) -> Result, HttpError>; /** - * Remove one loopback IPv6 address. + * Add a loopback IPv6. */ + #[endpoint { + method = POST, + path = "/loopback/ipv6", + versions = ..VERSION_ADDRESS_TAGS, + operation_id = "loopback_ipv6_create" + }] + async fn loopback_ipv6_create_v1( + rqctx: RequestContext, + val: TypedBody, + ) -> Result { + Self::loopback_ipv6_create(rqctx, val).await?; + Ok(HttpResponseUpdatedNoContent()) + } + + /// Remove a loopback IPv6 address. + /// + /// If a tag is provided, the address will only be deleted + /// if it belongs to the given tag. + /// + /// Returns 200 for any exit condition in which this address + /// does not exist on loopback, even if the address still + /// exists on another interface. #[endpoint { method = DELETE, path = "/loopback/ipv6/{ipv6}", + versions = VERSION_ADDRESS_TAGS.., }] async fn loopback_ipv6_delete( rqctx: RequestContext, path: Path, - ) -> Result; + tag: Query>, + ) -> Result, HttpError>; + + // Replaced for the same reason as [`link_ipv4_delete_v1`]. + #[endpoint { + method = DELETE, + path = "/loopback/ipv6/{ipv6}", + versions = ..VERSION_ADDRESS_TAGS, + operation_id = "loopback_ipv6_delete" + }] + async fn loopback_ipv6_delete_v1( + rqctx: RequestContext, + path: Path, + ) -> Result { + Self::loopback_ipv6_delete( + rqctx, + path, + latest::misc::MaybeTagged::default().into(), + ) + .await?; + Ok(HttpResponseDeleted()) + } /** * Get all of the external addresses in use for NAT mappings. @@ -1646,26 +1827,61 @@ pub trait DpdApi { HttpError, >; - /** - * Apply port settings atomically. - * - * These settings will be applied holistically, and to the extent possible - * atomically to a given port. In the event of a failure a rollback is - * attempted. If the rollback fails there will be inconsistent state. This - * failure mode returns the error code "rollback failure". For more details see - * the docs on the [`PortSettings`] type. - */ + /// Apply port settings atomically. + /// + /// These settings will be applied holistically, and to the extent possible + /// atomically to a given port. In the event of a failure, a rollback is + /// attempted. If the rollback fails, there will be inconsistent state. This + /// failure mode returns the error code "rollback failure". For more details see + /// the docs on the [`PortSettings`] type. + /// + /// Modifications within a link respect address tags. However, this may still + /// delete a link even if it contains addresses belonging to other tags. #[endpoint { method = POST, - versions = VERSION_ALLOW_DDM_TRAFFIC.., - path = "/port/{port_id}/settings" + versions = VERSION_ADDRESS_TAGS.., + path = "/port/{port_id}/settings/{tag}" }] async fn port_settings_apply( + rqctx: RequestContext, + path: Path>, + body: TypedBody, + ) -> Result; + + /// This was replaced by a version that requires a tag. + /// + /// - DPD tags all addresses, so allowing a `None` tag just used + /// an implicit empty string tag. That's confusing and unnecessary. + /// - This method ostensibly only succeeds if all settings were + /// idempotently applied. So returning those same settings + /// isn't super useful. And doing so implicates awkward + /// async reconciler assumptions. + #[endpoint { + method = POST, + versions = VERSION_ALLOW_DDM_TRAFFIC..VERSION_ADDRESS_TAGS, + path = "/port/{port_id}/settings", + operation_id = "port_settings_apply" + }] + async fn port_settings_apply_v2( rqctx: RequestContext, path: Path, query: Query, body: TypedBody, - ) -> Result, HttpError>; + ) -> Result, HttpError> { + let tagged_port = path.map(|port| latest::misc::Tagged { + tag: query.into_inner().tag.unwrap_or_default(), + value: port, + }); + + let mut settings = latest::port::PortSettings::default(); + let body = body.map(|s| { + settings = s.clone(); + s + }); + + Self::port_settings_apply(rqctx, tagged_port, body).await?; + Ok(HttpResponseOk(settings)) + } /** * Apply port settings atomically. @@ -1688,24 +1904,49 @@ pub trait DpdApi { query: Query, body: TypedBody, ) -> Result, HttpError> { - Self::port_settings_apply(rqctx, path, query, body.map(Into::into)) + Self::port_settings_apply_v2(rqctx, path, query, body.map(Into::into)) .await .map(|resp| resp.map(Into::into)) } - /** - * Clear port settings atomically. - */ + /// Clears port settings atomically. + /// + /// This is similar to calling link_delete on all links of + /// a port, however it also attempts to roll back to the + /// previous state if something fails. #[endpoint { method = DELETE, - versions = VERSION_ALLOW_DDM_TRAFFIC.., + versions = VERSION_ADDRESS_TAGS.., path = "/port/{port_id}/settings" }] async fn port_settings_clear( rqctx: RequestContext, path: Path, + ) -> Result; + + /// This was replaced due to two issues: + /// - Tags have no effect on link deletion, and requiring + /// an unused tag parameter was misleading. + /// - This returns PortSettings. It's unclear whether that's the previous + /// settings or the resulting settings. The implementation chose the + /// latter, which is pointless because a successful clear definitionally + /// produces empty settings. + #[endpoint { + method = DELETE, + versions = VERSION_ALLOW_DDM_TRAFFIC..VERSION_ADDRESS_TAGS, + path = "/port/{port_id}/settings", + operation_id = "port_settings_clear" + }] + async fn port_settings_clear_v2( + rqctx: RequestContext, + path: Path, query: Query, - ) -> Result, HttpError>; + ) -> Result, HttpError> { + let _ = query; + Self::port_settings_clear(rqctx, path) + .await + .map(|_| HttpResponseOk(latest::port::PortSettings::default())) + } /** * Clear port settings atomically. @@ -1721,24 +1962,49 @@ pub trait DpdApi { path: Path, query: Query, ) -> Result, HttpError> { - Self::port_settings_clear(rqctx, path, query) + Self::port_settings_clear_v2(rqctx, path, query) .await .map(|resp| resp.map(Into::into)) } - /** - * Get port settings atomically. - */ + /// Retrieves the settings for all links on this port. + /// + /// Among tagged resources, only those matching the given + /// tag are returned. + /// + /// Use link_ipv*_list if you want all addresses on a link + /// regardless of tag. #[endpoint { method = GET, - versions = VERSION_ALLOW_DDM_TRAFFIC.., - path = "/port/{port_id}/settings" + versions = VERSION_ADDRESS_TAGS.., + path = "/port/{port_id}/settings/{tag}" }] async fn port_settings_get( + rqctx: RequestContext, + path: Path>, + ) -> Result, HttpError>; + + /// The version replacing this makes tag a required parameter. + /// Reconciler usage doesn't want tag `None` to return all addresses, + /// which forced dpd to implicitly unwrap tag `None` to a default `""`. + #[endpoint { + method = GET, + versions = VERSION_ALLOW_DDM_TRAFFIC..VERSION_ADDRESS_TAGS, + path = "/port/{port_id}/settings", + operation_id = "port_settings_get" + }] + async fn port_settings_get_v2( rqctx: RequestContext, path: Path, query: Query, - ) -> Result, HttpError>; + ) -> Result, HttpError> { + let tagged_port = path.map(|port| latest::misc::Tagged { + tag: query.into_inner().tag.unwrap_or_default(), + value: port, + }); + + Self::port_settings_get(rqctx, tagged_port).await + } /** * Get port settings atomically. @@ -1754,7 +2020,7 @@ pub trait DpdApi { path: Path, query: Query, ) -> Result, HttpError> { - Self::port_settings_get(rqctx, path, query) + Self::port_settings_get_v2(rqctx, path, query) .await .map(|resp| resp.map(Into::into)) } @@ -1802,11 +2068,30 @@ pub trait DpdApi { #[endpoint { method = GET, path = "/links/tfport_data", + versions = VERSION_ADDRESS_TAGS.. }] async fn tfport_data( rqctx: RequestContext, ) -> Result>, HttpError>; + /// Deprecated for a new version that returns + /// oxnet::UnicastLinkLocalIpv6Addr. + #[endpoint { + method = GET, + path = "/links/tfport_data", + versions = ..VERSION_ADDRESS_TAGS, + operation_id = "tfport_data" + }] + async fn tfport_data_v1( + rqctx: RequestContext, + ) -> Result>, HttpError> { + Self::tfport_data(rqctx).await.map(|res| { + res.map(|tfports| { + tfports.into_iter().map(v1::link::TfportData::from).collect() + }) + }) + } + /** * Get NAT generation number */ @@ -2790,3 +3075,11 @@ pub trait DpdApi { HttpError, >; } + +const ADDR_RESET_EMSG: &str = " +This endpoint was deprecated because it provides redundant functionality and interacts poorly with ownership tags. The following endpoints may also be used to delete addresses: + +- port_settings_apply +- port_settings_clear +- link_ipv*_list followed by link_ipv*_delete +"; diff --git a/dpd-client/Cargo.toml b/dpd-client/Cargo.toml index 8e0c4bf0..4f24058e 100644 --- a/dpd-client/Cargo.toml +++ b/dpd-client/Cargo.toml @@ -11,6 +11,7 @@ tofino_asic = ["asic/tofino_asic"] [dependencies] async-trait = "0.1.89" +anyhow.workspace = true chrono.workspace = true futures.workspace = true http.workspace = true diff --git a/dpd-client/src/lib.rs b/dpd-client/src/lib.rs index 9e86eeee..06ec5803 100644 --- a/dpd-client/src/lib.rs +++ b/dpd-client/src/lib.rs @@ -39,7 +39,7 @@ progenitor::generate_api!( PortId = {derives = [PartialOrd, Ord, PartialEq, Eq, Hash, Clone] }, }, crates = { - "oxnet" = "0.1.0", + "oxnet" = "0.1.7", }, ); @@ -418,6 +418,31 @@ impl fmt::Display for types::MediaInterfaceId { } } +impl types::CmdAck { + /// A test helper that returns err if `self` is not [`CmdAck::Updated`]. + pub fn verify_updated(&self, msg: impl AsRef) -> anyhow::Result<()> { + self.verify_state(msg, Self::Updated) + } + + /// A test helper that returns err if `self` is not [`CmdAck::NoChange`]. + pub fn verify_no_change(&self, msg: impl AsRef) -> anyhow::Result<()> { + self.verify_state(msg, Self::NoChange) + } + + fn verify_state( + &self, + msg: impl AsRef, + want: types::CmdAck, + ) -> anyhow::Result<()> { + anyhow::ensure!( + self == &want, + "Expectation failed, {self:?} != {want:?}: {}", + msg.as_ref() + ); + Ok(()) + } +} + #[cfg(test)] mod tests { use super::types::SwitchIdentifiers; diff --git a/dpd-client/tests/chaos_tests/port_settings.rs b/dpd-client/tests/chaos_tests/port_settings.rs index cf4b3000..41615d3e 100644 --- a/dpd-client/tests/chaos_tests/port_settings.rs +++ b/dpd-client/tests/chaos_tests/port_settings.rs @@ -26,12 +26,14 @@ use dpd_client::types::{ PortId, PortSettings, PortSpeed, }; use dpd_client::{Client, ROLLBACK_FAILURE_ERROR_CODE}; +use futures::{StreamExt, TryStreamExt}; use http::status::StatusCode; use pretty_assertions::{Comparison, assert_eq}; use rand::{Rng, RngCore}; use std::borrow::Cow; use std::collections::{BTreeSet, HashMap}; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; +use std::num::NonZeroU32; use std::sync::atomic::{AtomicU8, Ordering}; use std::sync::{Arc, LazyLock}; use tokio::time::Duration; @@ -108,7 +110,7 @@ async fn test_port_settings_addr_fail_1() -> anyhow::Result<()> { ); let err = client - .port_settings_apply(&QSFP0, Some("chaos"), &settings) + .port_settings_apply(&QSFP0, "chaos", &settings) .await .expect_err("Expected error on port settings apply"); @@ -137,7 +139,7 @@ async fn test_port_settings_addr_success_1() -> anyhow::Result<()> { }, ); - client.port_settings_apply(&QSFP0, Some("chaos"), &settings).await?; + client.port_settings_apply(&QSFP0, "chaos", &settings).await?; let addrs = link_list_ipv4(&client, "qsfp0", "0").await.unwrap(); @@ -163,7 +165,7 @@ async fn test_port_settings_addr_success_multi() -> anyhow::Result<()> { }, ); - client.port_settings_apply(&QSFP0, Some("chaos"), &settings).await?; + client.port_settings_apply(&QSFP0, "chaos", &settings).await?; let addrs = link_list_ipv4(&client, "qsfp0", "0").await.unwrap(); assert_eq!(addrs.len(), 1); @@ -186,7 +188,7 @@ async fn test_port_settings_addr_success_multi() -> anyhow::Result<()> { }, ); - client.port_settings_apply(&QSFP0, Some("chaos"), &settings).await?; + client.port_settings_apply(&QSFP0, "chaos", &settings).await?; let addrs = link_list_ipv4(&client, "qsfp0", "0").await.unwrap(); assert_eq!(addrs.len(), 2); @@ -209,7 +211,7 @@ async fn test_port_settings_addr_success_multi() -> anyhow::Result<()> { }, ); - client.port_settings_apply(&QSFP0, Some("chaos"), &settings).await?; + client.port_settings_apply(&QSFP0, "chaos", &settings).await?; let addrs = link_list_ipv4(&client, "qsfp0", "0").await.unwrap(); assert_eq!(addrs.len(), 1); @@ -219,7 +221,7 @@ async fn test_port_settings_addr_success_multi() -> anyhow::Result<()> { // Clear all settings - client.port_settings_clear(&QSFP0, Some("chaos")).await?; + client.port_settings_clear(&QSFP0).await?; // The addresses are all cleared synchronously, but the link deletion is // async. We pause briefly to give it a chance to complete. The subsequent @@ -279,47 +281,40 @@ async fn test_port_settings_txn_sweep() -> anyhow::Result<()> { let target = random_port_settings(); print!("current/target: {}", Comparison::new(¤t, &target)); - match client.port_settings_apply(&QSFP0, Some("chaos"), &target).await { - Ok(mut returned) => { - sort_addrs(&mut returned); - // Verify that what the server attempted to configure matches - // what we asked them to configure. - assert_eq!(target, returned.into_inner()); - - // While attempting to apply our requested config on the server - // side, some operations will be async - as will cleanup after - // any errors. We retry the test operation for a few seconds - // waiting for that to happen. - retry::retry_op(RETRY_INTERVAL, RETRY_MAX, || async { - match current_port_settings(&client, &QSFP0).await { - Err(e) => Err(retry::ReturnCode::Fatal(e.to_string())), - Ok(new) => { - if new == target { - Ok(()) - } else { - Err(retry::ReturnCode::Retry(format!( - "desired settings: {target:#?}\ncurrent settings: {new:#?}" - ))) - } - } - } - }) - .await?; - print!("operation succeeded, settings changed as expected"); - success += 1; - } - Err(e) => { - if is_rollback_error(&e) { - rollback_fail += 1; - continue; + let Err(e) = client.port_settings_apply(&QSFP0, "chaos", &target).await + else { + // While attempting to apply our requested config on the server + // side, some operations will be async - as will cleanup after + // any errors. We retry the test operation for a few seconds + // waiting for that to happen. + retry::retry_op(RETRY_INTERVAL, RETRY_MAX, || async { + let new = current_port_settings(&client, &QSFP0) + .await + .map_err(|e| ReturnCode::Fatal(e.to_string()))?; + if new == target { + return Ok(()); } - expect_random_chaos!(e); - let new = current_port_settings(&client, &QSFP0).await?; - assert_eq!(new, current); - print!("operation failed, settings remained as expected"); - fail += 1; - } + Err(retry::ReturnCode::Retry(format!( + " +desired settings: {target:#?} +current settings: {new:#?}" + ))) + }) + .await?; + print!("operation succeeded, settings changed as expected"); + success += 1; + continue; + }; + + if is_rollback_error(&e) { + rollback_fail += 1; + continue; } + expect_random_chaos!(e); + let new = current_port_settings(&client, &QSFP0).await?; + assert_eq!(new, current); + print!("operation failed, settings remained as expected"); + fail += 1; } println!("SUCCESS: {}", success); @@ -331,10 +326,9 @@ async fn test_port_settings_txn_sweep() -> anyhow::Result<()> { // This is a transaction sweep test that is more or less the same as the one // above, except it runs the loop in parallel. Because of this we cannot -// meaningfully check current state after modifying. The only thing we can check -// is that the return value for the updated state is exactly what we asked for. +// meaningfully check current state after modifying. // This test is useful to ensure that a concurrent barrage of transaction -// requests cannot corrupt each other. +// requests cannot corrupt link settings. #[tokio::test] async fn test_port_settings_txn_par_sweep() -> anyhow::Result<()> { let config = AsicConfig::uniform_set(TESTING_RADIX, OPERATION_FAILURE_RATE); @@ -355,34 +349,69 @@ async fn test_port_settings_txn_par_sweep() -> anyhow::Result<()> { let j = tokio::spawn(async move { let target = random_port_settings(); - match client - .port_settings_apply(&QSFP0, Some("chaos"), &target) - .await - { - Ok(mut returned) => { - sort_addrs(&mut returned); - assert_eq!(target, returned.into_inner()); - success.fetch_add(1, Ordering::Relaxed); - } - Err(e) => { - // TODO return current state on error so we can check - // transaction properties here? - if is_rollback_error(&e) { - rollback_fail.fetch_add(1, Ordering::Relaxed); - return; - } - //expect_random_chaos!(e); - fail.fetch_add(1, Ordering::Relaxed); - } + let Err(e) = + client.port_settings_apply(&QSFP0, "chaos", &target).await + else { + success.fetch_add(1, Ordering::Relaxed); + return; + }; + + // TODO return current state on error so we can check + // transaction properties here? + if is_rollback_error(&e) { + rollback_fail.fetch_add(1, Ordering::Relaxed); + return; } + //expect_random_chaos!(e); + fail.fetch_add(1, Ordering::Relaxed); }); joins.push(j); } for j in joins { - j.await?; + j.await.context("task failed")?; } + // After all that, the link should still behave properly and accept new settings. + let target = random_port_settings(); + retry::retry_op(Duration::ZERO, RETRY_MAX, async || { + let Err(e) = client.port_settings_apply(&QSFP0, "chaos", &target).await + else { + return Ok(()); + }; + Err(ReturnCode::Retry(format!( + "port_settings_apply failed: {e:?}. Target: {target:?}" + ))) + }) + .await + .context("final port_settings_apply")?; + + retry::retry_op(Duration::ZERO, RETRY_MAX, async || { + let mut settings = match client.port_settings_get(&QSFP0, "chaos").await + { + Err(e) => { + return Err(ReturnCode::Retry(format!( + "port_settings_get failed: {e:?}" + ))); + } + Ok(s) => s.into_inner(), + }; + + self::sort_addrs(&mut settings); + if settings == target { + return Ok(()); + } + + Err(retry::ReturnCode::Retry(format!( + " +Mismatched settings in final check. +desired settings: {target:#?} +current settings: {settings:#?}" + ))) + }) + .await + .context("final_port_settings_get")?; + println!("SUCCESS: {}", success.load(Ordering::Relaxed)); println!("FAIL: {}", fail.load(Ordering::Relaxed)); println!("ROLLBACK FAIL: {}", rollback_fail.load(Ordering::Relaxed)); @@ -405,9 +434,9 @@ fn is_rollback_error(e: &dpd_client::Error) -> bool { async fn current_port_settings( client: &Client, port: &PortId, -) -> anyhow::Result { +) -> Result> { let mut settings = - client.port_settings_get(port, Some("chaos")).await?.into_inner(); + client.port_settings_get(port, "chaos").await?.into_inner(); sort_addrs(&mut settings); Ok(settings) } @@ -505,7 +534,7 @@ Reconciliation retries: for _ in 0..3 { retry::retry_op(RETRY_INTERVAL, RETRY_MAX, async || { let Err(e) = - client.port_settings_apply(&QSFP0, Some(TAG1), &settings).await + client.port_settings_apply(&QSFP0, TAG1, &settings).await else { return Ok(()); }; @@ -517,8 +546,7 @@ Reconciliation retries: .context("Timeout in port_settings_apply")?; retry::retry_op(RETRY_INTERVAL, RETRY_MAX, async || { - let Err(e) = client.port_settings_clear(&QSFP0, Some(TAG1)).await - else { + let Err(e) = client.port_settings_clear(&QSFP0).await else { return Ok(()); }; clear += 1; @@ -529,7 +557,7 @@ Reconciliation retries: .context("Timeout in port_settings_clear")?; retry::retry_op(RETRY_INTERVAL, RETRY_MAX, async || { - let err = match client.port_settings_get(&QSFP0, Some(TAG1)).await { + let err = match client.port_settings_get(&QSFP0, TAG1).await { Ok(s) if s.links.is_empty() => return Ok(()), Ok(s) => Err(ReturnCode::Retry(format!( "Link settings should be empty. Found {s:?}" @@ -569,11 +597,7 @@ async fn settings_apply_respects_tags() -> anyhow::Result<()> { tag2.verify_addrs_exist(Verify::NonExhaustive).await?; client - .port_settings_apply( - &QSFP0, - Some(TAG2), - &TestAddrs::empty_settings(link_id), - ) + .port_settings_apply(&QSFP0, TAG2, &TestAddrs::empty_settings(link_id)) .await?; tag1.verify_addrs_exist(Verify::Exhaustive).await?; @@ -703,10 +727,7 @@ impl AddrCheck { let link_is_empty = addrs .cxt .client - .port_settings_get( - &addrs.cxt.port_id, - Some(&addrs.v4_entry.tag), - ) + .port_settings_get(&addrs.cxt.port_id, &addrs.v4_entry.tag) .await? .into_inner() .links @@ -751,10 +772,20 @@ async fn address_cmds_respect_tags() -> anyhow::Result<()> { ]) .await?; - client.link_ipv4_delete(&QSFP0, &LINK0, &tag1.v4_entry.addr).await?; - client.link_ipv6_delete(&QSFP0, &LINK0, &tag1.v6_entry.addr).await?; + client + .link_ipv4_delete(&QSFP0, &LINK0, &tag1.v4_entry.addr, Some(TAG2)) + .await + .expect_err("this v4 entry doesn't exist") + .expect_status(StatusCode::NOT_FOUND); + client + .link_ipv6_delete(&QSFP0, &LINK0, &tag1.v6_entry.addr, Some(TAG2)) + .await + .expect_err("this v6 entry doesn't exist") + .expect_status(StatusCode::NOT_FOUND); cxt.ensure_addrs([ + (tag1.tag(), tag1.v4_entry.addr.into()), + (tag1.tag(), tag1.v6_entry.addr.into()), (tag1.tag(), tag2.v4_entry.addr.into()), (tag2.tag(), tag2.v4_entry.addr.into()), (tag2.tag(), tag2.v6_entry.addr.into()), @@ -762,37 +793,203 @@ async fn address_cmds_respect_tags() -> anyhow::Result<()> { .await?; client - .link_ipv4_delete(&QSFP0, &LINK0, &tag1.v4_entry.addr) + .link_ipv4_delete(&QSFP0, &LINK0, &tag1.v4_entry.addr, Some(TAG1)) + .await?; + client + .link_ipv6_delete(&QSFP0, &LINK0, &tag1.v6_entry.addr, Some(TAG1)) + .await?; + + client + .link_ipv4_delete(&QSFP0, &LINK0, &tag1.v4_entry.addr, Some(TAG1)) .await .expect_err("v4 address was deleted, should see 404") .expect_status(StatusCode::NOT_FOUND); client - .link_ipv6_delete(&QSFP0, &LINK0, &tag1.v6_entry.addr) + .link_ipv6_delete(&QSFP0, &LINK0, &tag1.v6_entry.addr, Some(TAG1)) .await .expect_err("v6 address was deleted, should see 404") .expect_status(StatusCode::NOT_FOUND); - client.link_ipv4_delete(&QSFP0, &LINK0, &tag2.v4_entry.addr).await?; - cxt.ensure_addrs([(tag2.tag(), tag2.v6_entry.addr.into())]).await?; + cxt.ensure_addrs([ + (tag1.tag(), tag2.v4_entry.addr.into()), + (tag2.tag(), tag2.v4_entry.addr.into()), + (tag2.tag(), tag2.v6_entry.addr.into()), + ]) + .await?; + + client + .link_ipv4_delete(&QSFP0, &LINK0, &tag2.v4_entry.addr, Some(TAG2)) + .await?; + + cxt.ensure_addrs([ + (tag1.tag(), tag2.v4_entry.addr.into()), + (tag2.tag(), tag2.v6_entry.addr.into()), + ]) + .await?; tag2.apply_addrs().await?; cxt.ensure_addrs([ + (tag1.tag(), tag2.v4_entry.addr.into()), (tag2.tag(), tag2.v4_entry.addr.into()), (tag2.tag(), tag2.v6_entry.addr.into()), ]) .await?; - client.link_ipv4_delete(&QSFP0, &LINK0, &tag2.v4_entry.addr).await?; + client.link_ipv4_delete(&QSFP0, &LINK0, &tag2.v4_entry.addr, None).await?; cxt.ensure_addrs([(tag2.tag(), tag2.v6_entry.addr.into())]).await?; Ok(()) } -/// Verifies the current known behavior of port_settings_clear, which -/// is to delete the link and all its config from the port. +/// Verifies tagged loopback address isolation against a sequence +/// of CRUD commands. +/// +/// Return code for link and loopback CRUD is not obvious. Say a +/// caller tries creating or deleting an address that belongs to +/// another interface. That could reasonably be a 409, 404, or 200. +/// Since the target audience is reconcilers, this is the policy I +/// chose: "Return 200 unless some condition prevents the target +/// state from being reached". +/// +/// Say, for example, `ifaceB` has an address `addr`. `delete(ifaceA, addr)` +/// returns 200 NoChange because the end state has `addr` not belonging +/// to `ifaceA`. But `create(ifaceA, addr)` returns 409 because addr already +/// belongs to ifaceB, and the end state of this address + ifaceA cannot +/// be attained. +/// +/// As of writing, these semantics are only added to the loopback API. Link +/// is a larger project with its own surface area. +#[tokio::test] +async fn loopback_respects_tags() -> anyhow::Result<()> { + let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.); + let (_guard, client) = + harness::init_harness("loopback_respects_tags", &no_failures); + + let mut rng = IpRng::new(fname_hash!()); + + let loop4 = rng.unique_ipv4(); + let loop6 = rng.unique_ipv6(); + + let cxt = TestContext::new(&client, QSFP0.clone(), LINK0); + let mut link = TestAddrs::new(&cxt, &mut rng, TAG1.to_string()); + link.apply_addrs().await?; + + client + .loopback_ipv4_create(&Ipv4Entry { addr: loop4, tag: TAG1.to_string() }) + .await? + .verify_updated("New v4")?; + client + .loopback_ipv6_create(&Ipv6Entry { addr: loop6, tag: TAG1.to_string() }) + .await? + .verify_updated("New v6")?; + + client + .loopback_ipv4_delete(&loop4, Some(TAG2)) + .await? + .verify_no_change("v4 under this tag shouldn't exist")?; + client + .loopback_ipv6_delete(&loop6, Some(TAG2)) + .await? + .verify_no_change("v6 under this tag doesn't exist")?; + + client + .loopback_ipv4_create(&Ipv4Entry { addr: loop4, tag: TAG2.to_string() }) + .await? + .verify_updated("New v4 tag")?; + client + .loopback_ipv6_create(&Ipv6Entry { addr: loop6, tag: TAG2.to_string() }) + .await? + .verify_updated("New v6 tag")?; + + self::try_steal(&mut link, loop4.into(), TAG1.to_string()) + .await + .expect_err("Link must not steal v4 loopback addr"); + self::try_steal(&mut link, loop6.into(), TAG1.to_string()) + .await + .expect_err("Link must not steal v6 loopback addr"); + + link.verify_addrs_exist(Verify::Exhaustive).await?; + + client + .loopback_ipv4_delete(&link.v4_entry.addr, None) + .await? + .verify_no_change("v4 addr doesn't exist on loopback")?; + client + .loopback_ipv6_delete(&link.v6_entry.addr, None) + .await? + .verify_no_change("v6 addr doesn't exist on loopback")?; + + cxt.ensure_loopbacks([ + (Cow::Borrowed(TAG1), loop4.to_ip()), + (Cow::Borrowed(TAG2), loop4.to_ip()), + (Cow::Borrowed(TAG1), loop6.to_ip()), + (Cow::Borrowed(TAG2), loop6.to_ip()), + ]) + .await?; + + let extra4 = rng.unique_ipv4(); + let extra6 = rng.unique_ipv6(); + + client + .loopback_ipv4_create(&Ipv4Entry { + addr: extra4, + tag: TAG2.to_string(), + }) + .await? + .verify_updated("Extra v4")?; + client + .loopback_ipv6_create(&Ipv6Entry { + addr: extra6, + tag: TAG2.to_string(), + }) + .await? + .verify_updated("Extra v6")?; + + client.loopback_ipv4_delete(&loop4, Some(TAG1)).await?; + client.loopback_ipv6_delete(&loop6, Some(TAG1)).await?; + + client + .loopback_ipv4_delete(&loop4, Some(TAG1)) + .await? + .verify_no_change("v4 addr shouldn't exist")?; + + client + .loopback_ipv6_delete(&loop6, Some(TAG1)) + .await? + .verify_no_change("v6 addr shouldn't exist")?; + + cxt.ensure_loopbacks([ + (Cow::Borrowed(TAG2), loop4.to_ip()), + (Cow::Borrowed(TAG2), extra4.to_ip()), + (Cow::Borrowed(TAG2), loop6.to_ip()), + (Cow::Borrowed(TAG2), extra6.to_ip()), + ]) + .await?; + + client + .loopback_ipv4_delete(&extra4, None) + .await? + .verify_updated("v4 delete should succeed")?; + client + .loopback_ipv6_delete(&extra6, None) + .await? + .verify_updated("v6 delete should succeed")?; + + cxt.ensure_loopbacks([ + (Cow::Borrowed(TAG2), loop4.to_ip()), + (Cow::Borrowed(TAG2), loop6.to_ip()), + ]) + .await?; + + Ok(()) +} + +/// Verifies port_settings_clear, which deletes the link +/// and all its config from the port regardless of tag. /// -/// This test isn't necessarily endorsing the implementation, but -/// it does seek to track API's current behavior. +/// Ignoring seems like the best option for now. There's not +/// an obvious alternative short of making tag ownership a +/// property of the link itself. #[tokio::test] async fn settings_clear_ignores_tags() -> anyhow::Result<()> { let no_failures = AsicConfig::uniform_set(TESTING_RADIX, 0.0); @@ -812,8 +1009,7 @@ async fn settings_clear_ignores_tags() -> anyhow::Result<()> { tag1.verify_addrs_exist(Verify::NonExhaustive).await?; tag2.verify_addrs_exist(Verify::NonExhaustive).await?; - // Tag2 means nothing here - client.port_settings_clear(&cxt.port_id, Some(TAG2)).await?; + client.port_settings_clear(&cxt.port_id).await?; tag1.verify_addrs_exist(Verify::Exhaustive).await.expect_err( "Addresses do not exist because we cleared the port settings.", @@ -908,22 +1104,35 @@ async fn deleted_address_must_exist() -> anyhow::Result<()> { tag1.apply_addrs().await?; client - .link_ipv4_delete(&QSFP0, &LINK0, &rng.unique_ipv4()) + .link_ipv4_delete(&QSFP0, &LINK0, &rng.unique_ipv4(), None) .await .expect_err("Deleting a non-existent IPv4 addr fails") .expect_status(StatusCode::NOT_FOUND); client - .link_ipv6_delete(&QSFP0, &LINK0, &rng.unique_ipv6()) + .link_ipv6_delete(&QSFP0, &LINK0, &rng.unique_ipv6(), None) .await .expect_err("Deleting a non-existent IPv6 addr fails") .expect_status(StatusCode::NOT_FOUND); - // Deletion is not currently tagged, so there's no way/reason to test - // tag conflicts. + client + .link_ipv4_delete(&QSFP0, &LINK0, &tag1.v4_entry.addr, Some(TAG2)) + .await + .expect_err("Must not delete an IPv4 address belonging to another tag") + .expect_status(StatusCode::NOT_FOUND); + client + .link_ipv6_delete(&QSFP0, &LINK0, &tag1.v6_entry.addr, Some(TAG2)) + .await + .expect_err("Must not delete an IPv6 address belonging to another tag") + .expect_status(StatusCode::NOT_FOUND); + tag1.verify_addrs_exist(Verify::NonExhaustive).await?; - client.link_ipv4_delete(&QSFP0, &LINK0, &tag1.v4_entry.addr).await?; - client.link_ipv6_delete(&QSFP0, &LINK0, &tag1.v6_entry.addr).await?; + client + .link_ipv4_delete(&QSFP0, &LINK0, &tag1.v4_entry.addr, Some(TAG1)) + .await?; + client + .link_ipv6_delete(&QSFP0, &LINK0, &tag1.v6_entry.addr, Some(TAG1)) + .await?; tag1.verify_addrs_exist(Verify::NonExhaustive) .await .expect_err("Addresses were deleted"); @@ -989,37 +1198,39 @@ async fn apply_allows_multiple_tags() -> anyhow::Result<()> { ]) .await?; - client.link_ipv4_delete(&QSFP0, &LINK0, &tag1.v4_entry.addr).await?; + client + .link_ipv4_delete(&QSFP0, &LINK0, &tag1.v4_entry.addr, Some(TAG2)) + .await?; cxt.ensure_addrs([ + (tag1.tag(), tag1.ipv4()), (tag1.tag(), tag1.ipv6()), - // (tag2.tag(), tag2.ipv4()), requires tagged deletion - (tag2.tag(), tag2.ipv6()), + (tag2.tag(), tag1.ipv6()), ]) .await?; - tag1.apply_addrs().await?; + tag2.apply_addrs().await?; cxt.ensure_addrs([ (tag1.tag(), tag1.ipv4()), (tag1.tag(), tag1.ipv6()), + (tag2.tag(), tag1.ipv4()), (tag2.tag(), tag1.ipv6()), ]) .await?; - tag2.apply_addrs().await?; cxt.client - .port_settings_apply( - &QSFP0, - Some(TAG1), - &TestAddrs::empty_settings(LINK0), - ) + .port_settings_apply(&QSFP0, TAG1, &TestAddrs::empty_settings(LINK0)) .await?; cxt.ensure_addrs([(tag2.tag(), tag1.ipv4()), (tag2.tag(), tag1.ipv6())]) .await?; - cxt.client.link_ipv4_delete(&QSFP0, &LINK0, &tag2.v4_entry.addr).await?; - cxt.client.link_ipv6_delete(&QSFP0, &LINK0, &tag2.v6_entry.addr).await?; + cxt.client + .link_ipv4_delete(&QSFP0, &LINK0, &tag2.v4_entry.addr, Some(TAG2)) + .await?; + cxt.client + .link_ipv6_delete(&QSFP0, &LINK0, &tag2.v6_entry.addr, Some(TAG2)) + .await?; cxt.ensure_addrs([]).await?; @@ -1059,7 +1270,7 @@ async fn partial_failures_are_recoverable() -> anyhow::Result<()> { ); client - .port_settings_clear(&QSFP0, Some(TAG1)) + .port_settings_clear(&QSFP0) .await .expect_err("Port couldn't be cleared because IPv4 addr is stuck"); @@ -1069,7 +1280,7 @@ async fn partial_failures_are_recoverable() -> anyhow::Result<()> { } client - .port_settings_apply(&"qsfp1".parse()?, Some(TAG2), &v4_only) + .port_settings_apply(&"qsfp1".parse()?, TAG2, &v4_only) .await .expect_err("Stuck entry cannot be stolen by another tag."); @@ -1077,11 +1288,13 @@ async fn partial_failures_are_recoverable() -> anyhow::Result<()> { link.addrs.push(rng.routable_ipv4().into()); } - let mut applied = client - .port_settings_apply(&QSFP0, Some(TAG1), &v4_only) + client + .port_settings_apply(&QSFP0, TAG1, &v4_only) .await .context("Apply should succeed because we can at least overwrite the IPv4 table entry")? .into_inner(); + let mut applied = + client.port_settings_get(&QSFP0, TAG1).await?.into_inner(); self::sort_addrs(&mut v4_only); self::sort_addrs(&mut applied); @@ -1235,13 +1448,9 @@ async fn deletion_doesnt_leak_table_entries() -> anyhow::Result<()> { tag1.apply_addrs().await?; let msg = "Link should not be dropped because that would orphan the stuck table entry"; - client.port_settings_clear(&QSFP0, None).await.expect_err(msg); + client.port_settings_clear(&QSFP0).await.expect_err(msg); client - .port_settings_apply( - &QSFP0, - Some(TAG1), - &TestAddrs::empty_settings(LINK0), - ) + .port_settings_apply(&QSFP0, TAG1, &TestAddrs::empty_settings(LINK0)) .await .expect_err("Addr cannot be implicitly deleted via apply"); tag1.verify_addrs_exist(Verify::Exhaustive).await?; @@ -1281,7 +1490,7 @@ async fn deletion_prevails() -> anyhow::Result<()> { let applied = client .port_settings_apply( &QSFP0, - Some(TAG1), + TAG1, &TestAddrs::empty_settings(LINK0), ) .await; @@ -1297,7 +1506,7 @@ async fn deletion_prevails() -> anyhow::Result<()> { .context("Timeout trying to successfully clear addresses")?; let cleared_addrs = client - .port_settings_get(&QSFP0, Some(TAG1)) + .port_settings_get(&QSFP0, TAG1) .await? .links .get(&LINK0.to_string()) @@ -1460,32 +1669,29 @@ async fn basic_multiple_tag_crud() -> anyhow::Result<()> { "fetched addrs should match what was registered" ); - // TODO::cory: revive this once dpd api supports tags in page tokens // Verify pagination works identically. - // let one = NonZeroU32::new(1).unwrap(); - // let mut fetched_stream: BTreeSet<(Cow<'static, str>, IpAddr)> = cxt - // .client - // .link_ipv4_list_stream(&cxt.port_id, &cxt.link_id, Some(one)) - // .map_ok(|entry| (entry.tag.into(), entry.addr.into())) - // .try_collect() - // .await?; - - // let mut v6s = cxt.client.link_ipv6_list_stream( - // &cxt.port_id, - // &cxt.link_id, - // Some(one), - // ); - // while let Some(entry) = v6s.next().await { - // let entry = entry?; - // fetched_stream.insert((entry.tag.into(), entry.addr.into())); - // } - // assert_eq!( - // fetched, fetched_stream, - // "addr fetch must be identical with and without stream" - // ); - } + let one = NonZeroU32::new(1).unwrap(); + let mut fetched_stream: BTreeSet<(Cow<'static, str>, IpAddr)> = cxt + .client + .link_ipv4_list_stream(&cxt.port_id, &cxt.link_id, Some(one)) + .map_ok(|entry| (entry.tag.into(), entry.addr.into())) + .try_collect() + .await?; - // TODO::cory: finish this once we support tagged deletion. + let mut v6s = cxt.client.link_ipv6_list_stream( + &cxt.port_id, + &cxt.link_id, + Some(one), + ); + while let Some(entry) = v6s.next().await { + let entry = entry?; + fetched_stream.insert((entry.tag.into(), entry.addr.into())); + } + assert_eq!( + fetched, fetched_stream, + "addr fetch must be identical with and without stream" + ); + } Ok(()) } @@ -1609,7 +1815,7 @@ async fn link_local_crud() -> anyhow::Result<()> { .link_ipv6_create(&cxt.port_id, &cxt.link_id, &entry) .await .expect("New link local creation should succeed"); - assert_eq!(client.tfport_data().await?[0].link_local, Some(entry.addr)); + assert_eq!(client.tfport_data().await?[0].link_local, Some(ll)); client .link_ipv6_create( @@ -1628,12 +1834,14 @@ async fn link_local_crud() -> anyhow::Result<()> { conf.verify_addrs_exist(Verify::NonExhaustive).await?; assert!(cxt.fetch_all_addrs().await?.contains(&(TAG1.into(), ll.into()))); - assert_eq!(client.tfport_data().await?[0].link_local, Some(entry.addr)); + assert_eq!(client.tfport_data().await?[0].link_local, Some(ll)); - client.link_ipv6_delete(&cxt.port_id, &cxt.link_id, &entry.addr).await?; + client + .link_ipv6_delete(&cxt.port_id, &cxt.link_id, &entry.addr, Some(TAG1)) + .await?; assert_eq!(client.tfport_data().await?[0].link_local, None); client - .link_ipv6_delete(&cxt.port_id, &cxt.link_id, &entry.addr) + .link_ipv6_delete(&cxt.port_id, &cxt.link_id, &entry.addr, Some(TAG1)) .await .expect_err("Addr shouldn't exist") .expect_status(StatusCode::NOT_FOUND); @@ -1683,7 +1891,7 @@ async fn port_settings_disallows_link_local() -> anyhow::Result<()> { .expect_err("Address should already exist"); let mut link_addrs = client - .port_settings_get(&cxt.port_id, Some(TAG1)) + .port_settings_get(&cxt.port_id, TAG1) .await? .into_inner() .links @@ -1698,7 +1906,7 @@ async fn port_settings_disallows_link_local() -> anyhow::Result<()> { expect_addrs.sort(); assert_eq!(&link_addrs, &expect_addrs); - client.port_settings_clear(&cxt.port_id, None).await?; + client.port_settings_clear(&cxt.port_id).await?; assert!(client.tfport_data().await?.is_empty()); Ok(()) @@ -1736,6 +1944,24 @@ expected: {expected:#?}" Ok(()) } + /// [`Self::ensure_addrs`] but for loopback addresses. + async fn ensure_loopbacks<'b>( + &self, + expected: impl IntoIterator, IpAddr)>, + ) -> anyhow::Result<()> { + let fetched = self.fetch_loopbacks().await?; + let expected = expected.into_iter().collect(); + + anyhow::ensure!( + fetched == expected, + "Loopback addresses do not match expectation: +fetched: {fetched:#?} +expected: {expected:#?}" + ); + + Ok(()) + } + /// Returns all addresses belonging to this link regardless of tag. async fn fetch_all_addrs( &self, @@ -1761,6 +1987,30 @@ expected: {expected:#?}" Ok(fetched) } + + /// Returns the switch's loopback addresses as a [`BTreeSet`]. + async fn fetch_loopbacks( + &self, + ) -> anyhow::Result, IpAddr)>> { + let mut fetched: BTreeSet<_> = self + .client + .loopback_ipv4_list() + .await? + .into_inner() + .into_iter() + .map(|entry| (Cow::Owned(entry.tag), entry.addr.to_ip())) + .collect(); + fetched.extend( + self.client + .loopback_ipv6_list() + .await? + .into_inner() + .into_iter() + .map(|entry| (Cow::Owned(entry.tag), entry.addr.to_ip())), + ); + + Ok(fetched) + } } /// This struct simplifies repetitive CRUD operations @@ -1828,7 +2078,7 @@ impl<'a> TestAddrs<'a> { .client .port_settings_apply( &self.cxt.port_id, - Some(&self.v4_entry.tag), + &self.v4_entry.tag, &self.settings(), ) .await?; @@ -1879,7 +2129,7 @@ impl<'a> TestAddrs<'a> { let mut psget_addrs = self .cxt .client - .port_settings_get(&self.cxt.port_id, Some(&self.v4_entry.tag)) + .port_settings_get(&self.cxt.port_id, &self.v4_entry.tag) .await? .links .remove(&self.cxt.link_id.to_string()) diff --git a/dpd-client/tests/integration_tests/loopback.rs b/dpd-client/tests/integration_tests/loopback.rs index c5e90b40..b272be57 100644 --- a/dpd-client/tests/integration_tests/loopback.rs +++ b/dpd-client/tests/integration_tests/loopback.rs @@ -14,6 +14,8 @@ use crate::integration_tests::common::prelude::*; #[tokio::test] #[ignore] async fn test_api() -> TestResult { + const TAG: &str = "test_api"; + let lo4: Ipv4Addr = "200.0.213.10".parse().unwrap(); let lo6: Ipv6Addr = "2001:db8::10".parse().unwrap(); @@ -21,13 +23,13 @@ async fn test_api() -> TestResult { switch .client - .loopback_ipv4_create(&Ipv4Entry { tag: "test".into(), addr: lo4 }) + .loopback_ipv4_create(&Ipv4Entry { tag: TAG.to_string(), addr: lo4 }) .await .expect("Should be able to create IPv4 loopback addr"); switch .client - .loopback_ipv6_create(&Ipv6Entry { tag: "test".into(), addr: lo6 }) + .loopback_ipv6_create(&Ipv6Entry { tag: TAG.to_string(), addr: lo6 }) .await .expect("Should be able to create IPv6 loopback addr"); @@ -47,13 +49,13 @@ async fn test_api() -> TestResult { switch .client - .loopback_ipv4_create(&Ipv4Entry { tag: "test".into(), addr: lo4 }) + .loopback_ipv4_create(&Ipv4Entry { tag: TAG.to_string(), addr: lo4 }) .await .expect("IPv4 loopback add should be idempotent"); switch .client - .loopback_ipv6_create(&Ipv6Entry { tag: "test".into(), addr: lo6 }) + .loopback_ipv6_create(&Ipv6Entry { tag: TAG.to_string(), addr: lo6 }) .await .expect("IPv6 loopback add should be idempotent"); @@ -77,7 +79,7 @@ async fn test_api() -> TestResult { switch .client - .loopback_ipv4_delete(&lo4) + .loopback_ipv4_delete(&lo4, Some(TAG)) .await .expect("delete v4 loopback once"); @@ -92,13 +94,13 @@ async fn test_api() -> TestResult { switch .client - .loopback_ipv4_delete(&lo4) + .loopback_ipv4_delete(&lo4, Some(TAG)) .await .expect("delete v4 loopback twice"); switch .client - .loopback_ipv6_delete(&lo6) + .loopback_ipv6_delete(&lo6, Some(TAG)) .await .expect("delete v6 loopback once"); @@ -113,7 +115,7 @@ async fn test_api() -> TestResult { switch .client - .loopback_ipv6_delete(&lo6) + .loopback_ipv6_delete(&lo6, Some(TAG)) .await .expect("delete v6 loopback twice"); diff --git a/dpd-client/tests/integration_tests/port_api.rs b/dpd-client/tests/integration_tests/port_api.rs index c580d6d9..152c7ef7 100644 --- a/dpd-client/tests/integration_tests/port_api.rs +++ b/dpd-client/tests/integration_tests/port_api.rs @@ -191,9 +191,10 @@ async fn test_ipv4_clear() -> TestResult { .await .unwrap(); - switch.client.link_ipv4_delete(&port_id, &link_id, &a).await.unwrap(); - switch.client.link_ipv4_delete(&port_id, &link_id, &b).await.unwrap(); - switch.client.link_ipv4_delete(&port_id, &link_id, &c).await.unwrap(); + let tag = Some(switch.client.inner().tag.as_str()); + switch.client.link_ipv4_delete(&port_id, &link_id, &a, tag).await.unwrap(); + switch.client.link_ipv4_delete(&port_id, &link_id, &b, None).await.unwrap(); + switch.client.link_ipv4_delete(&port_id, &link_id, &c, tag).await.unwrap(); let l = switch .client .link_ipv4_list_stream(&port_id, &link_id, None) @@ -214,6 +215,7 @@ async fn test_ipv4_delete() -> TestResult { let b: Ipv4Addr = "10.10.5.2".parse().unwrap(); let c: Ipv4Addr = "10.10.5.3".parse().unwrap(); let (port_id, link_id) = switch.link_id(PhysPort(11)).unwrap(); + let tag = Some(switch.client.inner().tag.as_str()); switch .client @@ -231,7 +233,7 @@ async fn test_ipv4_delete() -> TestResult { .await .unwrap(); - switch.client.link_ipv4_delete(&port_id, &link_id, &b).await.unwrap(); + switch.client.link_ipv4_delete(&port_id, &link_id, &b, tag).await.unwrap(); let l = switch .client .link_ipv4_list_stream(&port_id, &link_id, None) @@ -241,7 +243,7 @@ async fn test_ipv4_delete() -> TestResult { .unwrap(); addr_compare(vec![a, c], l).unwrap(); - switch.client.link_ipv4_delete(&port_id, &link_id, &a).await.unwrap(); + switch.client.link_ipv4_delete(&port_id, &link_id, &a, tag).await.unwrap(); let l = switch .client .link_ipv4_list_stream(&port_id, &link_id, None) @@ -261,7 +263,7 @@ async fn test_ipv6_set() -> TestResult { let a = rng.host_underlay(); let b = rng.host_underlay(); - let c = rng.link_local_ipv6(); + let c = rng.link_local_ipv6().into_addr(); let (port_id, link_id) = switch.link_id(PhysPort(11)).unwrap(); switch @@ -316,7 +318,7 @@ async fn test_ipv6_clear() -> TestResult { let a = rng.host_underlay(); let b = rng.host_underlay(); - let c = rng.link_local_ipv6(); + let c = rng.link_local_ipv6().into_addr(); let (port_id, link_id) = switch.link_id(PhysPort(11)).unwrap(); switch @@ -335,9 +337,10 @@ async fn test_ipv6_clear() -> TestResult { .await .unwrap(); - switch.client.link_ipv6_delete(&port_id, &link_id, &a).await.unwrap(); - switch.client.link_ipv6_delete(&port_id, &link_id, &b).await.unwrap(); - switch.client.link_ipv6_delete(&port_id, &link_id, &c).await.unwrap(); + let tag = Some(switch.client.inner().tag.as_str()); + switch.client.link_ipv6_delete(&port_id, &link_id, &a, tag).await.unwrap(); + switch.client.link_ipv6_delete(&port_id, &link_id, &b, None).await.unwrap(); + switch.client.link_ipv6_delete(&port_id, &link_id, &c, tag).await.unwrap(); assert!( switch .client @@ -358,8 +361,9 @@ async fn test_ipv6_delete() -> TestResult { let a = rng.host_underlay(); let b = rng.host_underlay(); - let c = rng.link_local_ipv6(); + let c = rng.link_local_ipv6().into_addr(); let (port_id, link_id) = switch.link_id(PhysPort(11)).unwrap(); + let tag = Some(switch.client.inner().tag.as_str()); switch .client @@ -377,7 +381,7 @@ async fn test_ipv6_delete() -> TestResult { .await .unwrap(); - switch.client.link_ipv6_delete(&port_id, &link_id, &b).await.unwrap(); + switch.client.link_ipv6_delete(&port_id, &link_id, &b, tag).await.unwrap(); let l = switch .client .link_ipv6_list_stream(&port_id, &link_id, None) @@ -387,7 +391,7 @@ async fn test_ipv6_delete() -> TestResult { .unwrap(); addr_compare(vec![a, c], l).unwrap(); - switch.client.link_ipv6_delete(&port_id, &link_id, &a).await.unwrap(); + switch.client.link_ipv6_delete(&port_id, &link_id, &a, tag).await.unwrap(); let l = switch .client .link_ipv6_list_stream(&port_id, &link_id, None) diff --git a/dpd-client/tests/integration_tests/table_tests.rs b/dpd-client/tests/integration_tests/table_tests.rs index f0d4c4b8..b2c38a19 100644 --- a/dpd-client/tests/integration_tests/table_tests.rs +++ b/dpd-client/tests/integration_tests/table_tests.rs @@ -174,9 +174,10 @@ impl TableTest for types::Ipv4Entry { async fn delete_entry(switch: &Switch, idx: usize) -> OpResult<()> { let (port_id, link_id) = switch.link_id(PhysPort(11)).unwrap(); + let tag = Some(switch.client.inner().tag.as_str()); switch .client - .link_ipv4_delete(&port_id, &link_id, &gen_ipv4_addr(idx)) + .link_ipv4_delete(&port_id, &link_id, &gen_ipv4_addr(idx), tag) .await } @@ -217,9 +218,10 @@ impl TableTest for types::Ipv6Entry { async fn delete_entry(switch: &Switch, idx: usize) -> OpResult<()> { let (port_id, link_id) = switch.link_id(PhysPort(11)).unwrap(); + let tag = Some(switch.client.inner().tag.as_str()); switch .client - .link_ipv6_delete(&port_id, &link_id, &gen_ipv6_addr(idx)) + .link_ipv6_delete(&port_id, &link_id, &gen_ipv6_addr(idx), tag) .await } diff --git a/dpd-types/versions/src/address_tags/link.rs b/dpd-types/versions/src/address_tags/link.rs new file mode 100644 index 00000000..fce2dbe6 --- /dev/null +++ b/dpd-types/versions/src/address_tags/link.rs @@ -0,0 +1,44 @@ +// 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/ +// +// Copyright 2026 Oxide Computer Company + +use std::net::Ipv6Addr; + +use oxnet::UnicastLinkLocalIpv6Addr; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use crate::v1::{self, link::LinkId, network::MacAddr, port::PortId}; + +/// The per-link data consumed by tfportd +#[derive(Clone, Debug, Deserialize, JsonSchema, Serialize)] +pub struct TfportData { + /// The switch port ID for this link. + pub port_id: PortId, + /// The link ID for this link. + pub link_id: LinkId, + /// The lower-level ASIC ID used to refer to this object in the switch + /// driver software. + pub asic_id: u16, + /// The MAC address for the link. + pub mac: MacAddr, + /// Is ipv6 enabled for this link + pub ipv6_enabled: bool, + /// The IPv6 link-local address of the link, if it exists. + pub link_local: Option, +} + +impl From for v1::link::TfportData { + fn from(value: TfportData) -> Self { + v1::link::TfportData { + port_id: value.port_id, + link_id: value.link_id, + asic_id: value.asic_id, + mac: value.mac, + ipv6_enabled: value.ipv6_enabled, + link_local: value.link_local.map(Ipv6Addr::from), + } + } +} diff --git a/dpd-types/versions/src/address_tags/mod.rs b/dpd-types/versions/src/address_tags/mod.rs new file mode 100644 index 00000000..013b6cb4 --- /dev/null +++ b/dpd-types/versions/src/address_tags/mod.rs @@ -0,0 +1,84 @@ +// 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/ +// +// Copyright 2026 Oxide Computer Company + +use std::borrow::Cow; + +use schemars::JsonSchema; +use serde::Deserialize; +use serde::Serialize; + +pub mod link; + +/// Conveys whether a successful API call caused a change of state. +#[derive( + Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema, +)] +pub enum CmdAck { + /// The action caused some some change in internal state. + Updated, + + /// The action caused no change in internal state. What was + /// requested matches what already exists. + NoChange, +} + +/// Associates a resource `T` with a namespace tag. +/// CRUD operations on this resource are then scoped +/// to the tag's domain. +#[derive(Debug, Serialize, Deserialize, JsonSchema)] +pub struct Tagged { + pub tag: String, + #[serde(flatten)] + pub value: T, +} + +/// Variant of [`Tagged`] where tag is optional. +/// If None, the action applies to all tags. +#[derive(Debug, Serialize, Deserialize, JsonSchema, Default)] +pub struct MaybeTagged { + #[schemars(with = "Option")] + pub tag: OptTag<'static>, + #[serde(flatten)] + pub value: T, +} + +/// A pagination token for listing tagged IPv4 and IPv6 addresses. +#[derive(Debug, Deserialize, Serialize, JsonSchema)] +pub struct IpToken { + pub ip: A, + #[serde(default)] + pub tag: String, +} + +/// Clients may use DPD tags to cooperatively namespace CRUD +/// operations and avoid disrupting eachother. +/// +/// This type may be used in APIs where tag usage is optional. +#[derive(Debug, Deserialize, JsonSchema, Clone, PartialEq, Eq, Default)] +#[serde(from = "Option")] +pub enum OptTag<'a> { + /// The action or result applies only to resources with this tag. + Tag(Cow<'a, str>), + + /// The action or result applies to this resource regardless of tag(s). + #[default] + Any, +} + +impl Serialize for OptTag<'static> { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.tag().serialize(serializer) + } +} + +impl From> for OptTag<'static> { + fn from(t: Option) -> Self { + t.map_or(OptTag::Any, |s| OptTag::Tag(s.into())) + } +} diff --git a/dpd-types/versions/src/impls/misc.rs b/dpd-types/versions/src/impls/misc.rs new file mode 100644 index 00000000..6bbe2d1f --- /dev/null +++ b/dpd-types/versions/src/impls/misc.rs @@ -0,0 +1,31 @@ +// 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/ +// +// Copyright 2026 Oxide Computer Company + +use crate::latest::misc::OptTag; + +impl<'a> OptTag<'a> { + /// Constructs a new [`OptTag`] selector matching only + /// the given tag string. + pub const fn exact(tag: &'a str) -> Self { + Self::Tag(std::borrow::Cow::Borrowed(tag)) + } + + /// Returns true if this tag applies to the given tag str. + pub fn matches(&self, other: &str) -> bool { + match self { + Self::Any => true, + Self::Tag(tag) => *tag == other, + } + } + + /// Returns the inner tag if one is specified. + pub fn tag(&self) -> Option<&str> { + match self { + Self::Tag(s) => Some(s), + Self::Any => None, + } + } +} diff --git a/dpd-types/versions/src/impls/mod.rs b/dpd-types/versions/src/impls/mod.rs index 285e642f..15e80031 100644 --- a/dpd-types/versions/src/impls/mod.rs +++ b/dpd-types/versions/src/impls/mod.rs @@ -9,6 +9,7 @@ mod attached_subnet; mod link; pub(crate) mod mcast; +mod misc; mod nat; pub(crate) mod network; pub(crate) mod oxstats; diff --git a/dpd-types/versions/src/latest.rs b/dpd-types/versions/src/latest.rs index 3b56eb39..cc5ce340 100644 --- a/dpd-types/versions/src/latest.rs +++ b/dpd-types/versions/src/latest.rs @@ -49,7 +49,6 @@ pub mod link { pub use crate::v1::link::LinkPath; pub use crate::v1::link::LinkState; pub use crate::v1::link::LinkUpCounter; - pub use crate::v1::link::TfportData; pub use crate::v11::link::LinkHistory; @@ -57,6 +56,9 @@ pub mod link { pub use crate::v12::link::MsDuration; pub use crate::v13::link::LinkCreate; + + pub use crate::v14::IpToken; + pub use crate::v14::link::TfportData; } pub mod loopback { @@ -96,6 +98,11 @@ pub mod mcast { pub mod misc { pub use crate::v1::misc::BuildInfo; pub use crate::v1::misc::TagPath; + + pub use crate::v14::CmdAck; + pub use crate::v14::MaybeTagged; + pub use crate::v14::OptTag; + pub use crate::v14::Tagged; } pub mod nat { diff --git a/dpd-types/versions/src/lib.rs b/dpd-types/versions/src/lib.rs index 617a58c7..1b799a88 100644 --- a/dpd-types/versions/src/lib.rs +++ b/dpd-types/versions/src/lib.rs @@ -43,6 +43,8 @@ pub mod v11; pub mod v12; #[path = "allow_ddm_traffic/mod.rs"] pub mod v13; +#[path = "address_tags/mod.rs"] +pub mod v14; #[path = "attached_subnets/mod.rs"] pub mod v3; #[path = "v4_over_v6_routes/mod.rs"] diff --git a/dpd/src/addr.rs b/dpd/src/addr.rs index 08b38eac..d10dd380 100644 --- a/dpd/src/addr.rs +++ b/dpd/src/addr.rs @@ -7,7 +7,6 @@ //! This module defines the mapping by which DPD tracks //! Address + Interface relations. -use std::borrow::Cow; use std::collections::BTreeSet; use std::collections::HashMap; use std::collections::hash_map; @@ -25,6 +24,8 @@ use common::addr::IpAddrLike; use common::addr::RouterIfaceIpAddr; use common::ports::PortId; use dpd_types::link::LinkId; +use dpd_types::misc::CmdAck; +use dpd_types::misc::OptTag; use oxnet::UnicastLinkLocalIpv6Addr; use crate::DpdError; @@ -672,46 +673,6 @@ impl Display for AddrIface { } } -/// Clients may use DPD tags to cooperatively namespace CRUD -/// operations and avoid disrupting eachother. -/// -/// This type may be used in APIs where tag usage is optional. -#[derive(Clone, Debug, PartialEq, Eq)] -pub enum OptTag<'a> { - /// The action or result applies only to resources with this tag. - Tag(Cow<'a, str>), - - /// The action or result applies to this resource regardless of tag(s). - Any, -} - -impl<'a> OptTag<'a> { - /// Constructs a new [`OptTag`] selector matching only - /// the given tag string. - pub const fn exact(tag: &'a str) -> Self { - Self::Tag(Cow::Borrowed(tag)) - } - - /// Returns true if this tag applies to the given tag str. - pub fn matches(&self, other: &str) -> bool { - match self { - Self::Any => true, - Self::Tag(tag) => *tag == other, - } - } -} - -/// Conveys whether a successful API call caused a change of state. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum CmdAck { - /// The action caused some some change in internal state. - Updated, - - /// The action caused no change in internal state. What was - /// requested matches what already exists. - NoChange, -} - /// Maps a [`Bound`] of type `In` to one of type `Out`. /// Returns `None` if `TryFrom` fails. fn map_bound(bound: Bound) -> Option> diff --git a/dpd/src/api_server.rs b/dpd/src/api_server.rs index e054b88d..408e6574 100644 --- a/dpd/src/api_server.rs +++ b/dpd/src/api_server.rs @@ -28,8 +28,9 @@ use dpd_types::counters::{ }; use dpd_types::fault::{Fault, FaultCondition}; use dpd_types::link::{ - LinkCreate, LinkFilter, LinkFsmCounters, LinkHistory, LinkId, LinkIpv4Path, - LinkIpv6Path, LinkPath, LinkUpCounter, LinkView, MsDuration, TfportData, + IpToken, LinkCreate, LinkFilter, LinkFsmCounters, LinkHistory, LinkId, + LinkIpv4Path, LinkIpv6Path, LinkPath, LinkUpCounter, LinkView, MsDuration, + TfportData, }; use dpd_types::loopback::{LoopbackIpv4Path, LoopbackIpv6Path}; #[cfg(feature = "multicast")] @@ -42,14 +43,15 @@ use dpd_types::mcast::{ MulticastGroupUpdateUnderlayEntry, MulticastTagPath, MulticastUnderlayGroupIpParam, }; -use dpd_types::misc::{BuildInfo, TagPath}; +use dpd_types::misc::CmdAck; +use dpd_types::misc::{BuildInfo, MaybeTagged, OptTag, TagPath, Tagged}; use dpd_types::nat::{ NatIpv4Path, NatIpv4PortPath, NatIpv4RangePath, NatIpv6Path, NatIpv6PortPath, NatIpv6RangePath, NatToken, }; use dpd_types::oxstats::OximeterMetadata; use dpd_types::port::{ - FreeChannels, LinkSettings, PortIdPathParams, PortSettings, PortSettingsTag, + FreeChannels, LinkSettings, PortIdPathParams, PortSettings, }; use dpd_types::port_map::BackplaneLink; use dpd_types::route::{ @@ -101,7 +103,6 @@ use common::ports::TxEq; use common::ports::TxEqSwHw; use crate::addr::AsicAddrIface; -use crate::addr::OptTag; use crate::addr::SyncAddrMap; use crate::addr::TableIp; use crate::attached_subnet; @@ -1117,7 +1118,7 @@ impl DpdApi for DpdApiImpl { async fn link_ipv4_list( rqctx: RequestContext>, path: Path, - query: Query>, + query: Query>>, ) -> Result>, HttpError> { let switch: &Switch = rqctx.context(); let path = path.into_inner(); @@ -1131,10 +1132,11 @@ impl DpdApi for DpdApiImpl { ); }; - // TODO::cory: include tag in pagination token. let addr = match &pagination.page { WhichPage::First(..) => None, - WhichPage::Next(Ipv4Token { ip }) => Some((IpAddr::V4(*ip), "")), + WhichPage::Next(IpToken { ip, tag }) => { + Some((ip.to_ip(), tag.as_str())) + } }; let entries = switch.list_ip_addresses( port_id, @@ -1146,7 +1148,10 @@ impl DpdApi for DpdApiImpl { ResultsPage::new( entries, &EmptyScanParams {}, - |entry: &Ipv4Entry, _| Ipv4Token { ip: entry.addr }, + |entry: &Ipv4Entry, _| IpToken { + ip: entry.addr, + tag: entry.tag.clone(), + }, ) .map(HttpResponseOk) } @@ -1170,36 +1175,19 @@ impl DpdApi for DpdApiImpl { Ok(HttpResponseUpdatedNoContent()) } - async fn link_ipv4_reset( - rqctx: RequestContext>, - path: Path, - ) -> Result { - let switch: &Switch = rqctx.context(); - let path = path.into_inner(); - let port_id = path.port_id; - let link_id = path.link_id; - switch - .reset_addresses::(port_id, link_id) - .map(|_| HttpResponseUpdatedNoContent()) - .map_err(|e| e.into()) - } - async fn link_ipv4_delete( rqctx: RequestContext>, path: Path, + tag: Query>, ) -> Result { let switch: &Switch = rqctx.context(); let LinkIpv4Path { port_id, link_id, address } = path.into_inner(); let address = address.to_ip().try_into()?; + let tag = tag.into_inner().tag; switch.link_update(port_id, link_id, |link| { let iface_id = link.asic_addr_iface(); - switch.addrs.delete_addr( - switch, - iface_id, - address, - &OptTag::Any, - )?; + switch.addrs.delete_addr(switch, iface_id, address, &tag)?; Ok(()) })?; @@ -1209,7 +1197,7 @@ impl DpdApi for DpdApiImpl { async fn link_ipv6_list( rqctx: RequestContext>, path: Path, - query: Query>, + query: Query>>, ) -> Result>, HttpError> { let switch: &Switch = rqctx.context(); let path = path.into_inner(); @@ -1222,10 +1210,12 @@ impl DpdApi for DpdApiImpl { DpdError::Invalid("Invalid page limit".to_string()).into() ); }; - // TODO::cory: include tag in pagination token. + let addr = match &pagination.page { WhichPage::First(..) => None, - WhichPage::Next(Ipv6Token { ip }) => Some((IpAddr::V6(*ip), "")), + WhichPage::Next(IpToken { ip, tag }) => { + Some((ip.to_ip(), tag.as_str())) + } }; let entries = switch.list_ip_addresses( port_id, @@ -1238,7 +1228,10 @@ impl DpdApi for DpdApiImpl { ResultsPage::new( entries, &EmptyScanParams {}, - |entry: &Ipv6Entry, _| Ipv6Token { ip: entry.addr }, + |entry: &Ipv6Entry, _| IpToken { + ip: entry.addr, + tag: entry.tag.clone(), + }, ) .map(HttpResponseOk) } @@ -1262,36 +1255,19 @@ impl DpdApi for DpdApiImpl { Ok(HttpResponseUpdatedNoContent()) } - async fn link_ipv6_reset( - rqctx: RequestContext>, - path: Path, - ) -> Result { - let switch: &Switch = rqctx.context(); - let path = path.into_inner(); - let port_id = path.port_id; - let link_id = path.link_id; - switch - .reset_addresses::(port_id, link_id) - .map(|_| HttpResponseUpdatedNoContent()) - .map_err(|e| e.into()) - } - async fn link_ipv6_delete( rqctx: RequestContext>, path: Path, + tag: Query>, ) -> Result { let switch: &Switch = rqctx.context(); let LinkIpv6Path { port_id, link_id, address } = path.into_inner(); let address = address.to_ip().try_into()?; + let tag = tag.into_inner().tag; switch.link_update(port_id, link_id, |link| { let iface_id = link.asic_addr_iface(); - switch.addrs.delete_addr( - switch, - iface_id, - address, - &OptTag::Any, - )?; + switch.addrs.delete_addr(switch, iface_id, address, &tag)?; Ok(()) })?; @@ -1395,36 +1371,34 @@ impl DpdApi for DpdApiImpl { async fn loopback_ipv4_create( rqctx: RequestContext>, val: TypedBody, - ) -> Result { + ) -> Result, HttpError> { let switch: &Switch = rqctx.context(); let entry = val.into_inner(); - switch.addrs.set_addr( + Ok(HttpResponseOk(switch.addrs.set_addr( switch, AsicAddrIface::Loopback, TableIp::try_from(IpAddr::V4(entry.addr))?, entry.tag, - )?; - - Ok(HttpResponseUpdatedNoContent {}) + )?)) } async fn loopback_ipv4_delete( rqctx: RequestContext>, path: Path, - ) -> Result { + tag: Query>, + ) -> Result, HttpError> { let switch = rqctx.context(); + let tag = tag.into_inner().tag; let addr = RouterIfaceIpAddr::try_new(path.into_inner().ipv4.to_ip()) .map_err(DpdError::from)?; - switch.addrs.clear_addr( + Ok(HttpResponseOk(switch.addrs.clear_addr( switch, AsicAddrIface::Loopback, addr.into(), - &OptTag::Any, - )?; - - Ok(HttpResponseDeleted()) + &tag, + )?)) } async fn loopback_ipv6_list( @@ -1449,36 +1423,34 @@ impl DpdApi for DpdApiImpl { async fn loopback_ipv6_create( rqctx: RequestContext>, val: TypedBody, - ) -> Result { + ) -> Result, HttpError> { let switch: &Switch = rqctx.context(); let entry = val.into_inner(); - switch.addrs.set_addr( + Ok(HttpResponseOk(switch.addrs.set_addr( switch, AsicAddrIface::Loopback, TableIp::try_from(IpAddr::V6(entry.addr))?, entry.tag, - )?; - - Ok(HttpResponseUpdatedNoContent {}) + )?)) } async fn loopback_ipv6_delete( rqctx: RequestContext>, path: Path, - ) -> Result { + tag: Query>, + ) -> Result, HttpError> { let switch = rqctx.context(); + let tag = tag.into_inner().tag; let addr = RouterIfaceIpAddr::try_new(path.into_inner().ipv6.to_ip()) .map_err(DpdError::from)?; - switch.addrs.clear_addr( + Ok(HttpResponseOk(switch.addrs.clear_addr( switch, AsicAddrIface::Loopback, addr.into(), - &OptTag::Any, - )?; - - Ok(HttpResponseDeleted()) + &tag, + )?)) } async fn nat_ipv6_addresses_list( @@ -1908,54 +1880,45 @@ impl DpdApi for DpdApiImpl { } async fn port_settings_apply( - rqctx: RequestContext>, - path: Path, - query: Query, + rqctx: RequestContext, + path: Path>, body: TypedBody, - ) -> Result, HttpError> { + ) -> Result { let switch = rqctx.context(); - let path = path.into_inner(); - let query = query.into_inner(); - let port_id = path.port_id; - let settings = body.into_inner(); - let tag = query.tag.as_deref().unwrap_or(""); + let Tagged { tag, value: PortIdPathParams { port_id } } = + path.into_inner(); switch - .apply_port_settings(port_id, settings, tag) + .apply_port_settings(port_id, body.into_inner(), &tag) .await - .map(HttpResponseOk) + .map(|()| HttpResponseUpdatedNoContent()) .map_err(HttpError::from) } async fn port_settings_clear( rqctx: RequestContext>, path: Path, - _query: Query, - ) -> Result, HttpError> { + ) -> Result { let switch = rqctx.context(); - let path = path.into_inner(); - let port_id = path.port_id; + let port_id = path.into_inner().port_id; switch .clear_port_settings(port_id) .await - .map(HttpResponseOk) + .map(|()| HttpResponseDeleted()) .map_err(HttpError::from) } async fn port_settings_get( rqctx: RequestContext>, - path: Path, - query: Query, + path: Path>, ) -> Result, HttpError> { let switch = rqctx.context(); - let path = path.into_inner(); - let query = query.into_inner(); - let port_id = path.port_id; - let tag = query.tag.as_deref().unwrap_or(""); + let Tagged { tag, value: PortIdPathParams { port_id } } = + path.into_inner(); switch - .get_port_settings(port_id, tag) + .get_port_settings(port_id, &tag) .await .map(HttpResponseOk) .map_err(HttpError::from) diff --git a/dpd/src/link.rs b/dpd/src/link.rs index 19772220..33e3163e 100644 --- a/dpd/src/link.rs +++ b/dpd/src/link.rs @@ -8,7 +8,6 @@ use crate::MacAddr; use crate::Switch; -use crate::addr::OptTag; use crate::addr::SyncAddrMap; use crate::fault::AutonegTracker; use crate::fault::Faultable; @@ -43,6 +42,7 @@ use dpd_types::link::LinkState; use dpd_types::link::LinkUpCounter; use dpd_types::link::LinkView; use dpd_types::link::TfportData; +use dpd_types::misc::OptTag; use dpd_types::serdes::Ber; use slog::debug; use slog::error; @@ -54,7 +54,6 @@ use std::collections::BTreeSet; use std::collections::btree_map; use std::collections::btree_map::Entry; use std::net::IpAddr; -use std::net::Ipv6Addr; use std::ops::Bound; use std::sync::Arc; use std::sync::Mutex; @@ -521,8 +520,7 @@ impl Link { .get() .read() .unwrap() - .get_link_local(self.asic_addr_iface()) - .map(Ipv6Addr::from), + .get_link_local(self.asic_addr_iface()), } } } @@ -1033,7 +1031,7 @@ impl Switch { let (addr_bounds, tag_bounds) = if let Some((addr, tag)) = last_entry { start_addr = Some(addr); - let addr = (Bound::Excluded(addr), Bound::Unbounded); + let addr = (Bound::Included(addr), Bound::Unbounded); let tag = (Bound::Excluded(tag), Bound::Unbounded); (addr, tag) } else { @@ -1064,23 +1062,6 @@ impl Switch { }) } - /// Delete all addresses of the given type on the specified link. - /// - /// Returns [`DpdError::AddrClear`] if any addresses could not be deleted. - pub fn reset_addresses( - &self, - port_id: PortId, - link_id: LinkId, - ) -> DpdResult<()> { - self.link_update(port_id, link_id, |link| { - self.addrs.clear_addrs::( - self, - link.asic_addr_iface(), - &OptTag::Any, - ) - }) - } - /// Return a link's configured MAC address. pub fn link_mac_address( &self, diff --git a/dpd/src/port_settings.rs b/dpd/src/port_settings.rs index f2720987..d7cf4220 100644 --- a/dpd/src/port_settings.rs +++ b/dpd/src/port_settings.rs @@ -7,7 +7,6 @@ use crate::DpdError; use crate::DpdResult; use crate::Switch; -use crate::addr::OptTag; use crate::addr::SyncAddrMap; use crate::addr::TableIp; use crate::link::Link; @@ -19,6 +18,7 @@ use common::ports::PortId; use common::ports::PortSpeed; use common::ports::TxEq; use dpd_types::link::LinkId; +use dpd_types::misc::OptTag; use dpd_types::port::LinkSettings; use dpd_types::port::PortSettings; use slog::Logger; @@ -652,21 +652,18 @@ impl Switch { port_id: PortId, settings: PortSettings, tag: &str, - ) -> DpdResult { + ) -> DpdResult<()> { let mut ctx = context!(port_id, self, tag); let mut diff = PortSettingsDiff::calculate(&mut ctx, &settings)?; trace!(self.log, "port settings diff: {:#?}", diff); diff.execute(&mut ctx)?; - Self::get_port_settings_locked(&mut ctx, true) + Ok(()) } /// Clear port settings as an atomic transaction. - pub async fn clear_port_settings( - &self, - port_id: PortId, - ) -> DpdResult { + pub async fn clear_port_settings(&self, port_id: PortId) -> DpdResult<()> { // Doesn't matter because we're deleting all links anyway. let tag = ""; let mut ctx = context!(port_id, self, tag); @@ -676,55 +673,26 @@ impl Switch { trace!(self.log, "port settings diff: {:#?}", diff); diff.execute(&mut ctx)?; - Self::get_port_settings_locked(&mut ctx, true) + Ok(()) } /// Get port settings as an atomic transaction. pub async fn get_port_settings( &self, - port_id: PortId, + get_port_id: PortId, tag: &str, ) -> DpdResult { - let mut ctx = context!(port_id, self, tag); - Self::get_port_settings_locked(&mut ctx, false) - } - - fn get_port_settings_locked( - ctx: &mut Context<'_>, - ignore_deleting: bool, - ) -> DpdResult { - let links = ctx - .link_map + let links = self + .links + .lock() + .unwrap() .get_links() .iter() .filter_map(|((port_id, link_id), link_lock)| { - if *port_id == ctx.port_id { + if *port_id == get_port_id { let link = link_lock.lock().unwrap(); - - // This is an awkward mechanism to work around the - // asynchrony of link deletion. When we return the - // PortSettings as a side effect of the apply() and clear() - // operations, we assume that any requested link deletion - // will succeed - as indeed it almost certainly will. This - // optimism is primarily to allow the chaos tests to pass, - // since they have no mechanism to wait for this async - // operation to complete. Arguably, we should simply stop - // returning anything other than success or failure on these - // operations. - // - // If we get an explicit request for the current settings, - // we will return the actual state of any link marked for - // deletion. This allows the chaos tests to run correctly - // even in the presence of repeated artificial errors. More - // importantly, it also ensures that a real consumer of the - // port_settings API will be operating on the correct - // information. - if ignore_deleting && link.config.delete_me { - None - } else { - let conf = link.settings(&ctx.switch.addrs, ctx.tag); - Some(((*link_id).into(), conf)) - } + let conf = link.settings(&self.addrs, tag); + Some(((*link_id).into(), conf)) } else { None } diff --git a/openapi/dpd/dpd-13.0.0-5db8bd.json.gitstub b/openapi/dpd/dpd-13.0.0-5db8bd.json.gitstub new file mode 100644 index 00000000..c886f19e --- /dev/null +++ b/openapi/dpd/dpd-13.0.0-5db8bd.json.gitstub @@ -0,0 +1 @@ +2df9101e95757037f2cc4e31e8bf00fd9e79efc7:openapi/dpd/dpd-13.0.0-5db8bd.json diff --git a/openapi/dpd/dpd-13.0.0-5db8bd.json b/openapi/dpd/dpd-14.0.0-c1f40d.json similarity index 98% rename from openapi/dpd/dpd-13.0.0-5db8bd.json rename to openapi/dpd/dpd-14.0.0-c1f40d.json index 22694237..cecc8e71 100644 --- a/openapi/dpd/dpd-13.0.0-5db8bd.json +++ b/openapi/dpd/dpd-14.0.0-c1f40d.json @@ -7,7 +7,7 @@ "url": "https://oxide.computer", "email": "api@oxide.computer" }, - "version": "13.0.0" + "version": "14.0.0" }, "paths": { "/all-settings": { @@ -1112,7 +1112,8 @@ } }, "post": { - "summary": "Add a loopback IPv4.", + "summary": "Creates an IPv4 loopback address if one does not yet exist.", + "description": "Returns CONFLICT 409 if this address already belongs to another interface.", "operationId": "loopback_ipv4_create", "requestBody": { "content": { @@ -1125,8 +1126,15 @@ "required": true }, "responses": { - "204": { - "description": "resource updated" + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CmdAck" + } + } + } }, "4XX": { "$ref": "#/components/responses/Error" @@ -1139,7 +1147,8 @@ }, "/loopback/ipv4/{ipv4}": { "delete": { - "summary": "Remove one loopback IPv4 address.", + "summary": "Remove a loopback IPv4 address.", + "description": "If a tag is provided, the address will only be deleted if it belongs to the given tag.\n\nReturns 200 for any exit condition in which this address does not exist on loopback, even if the address still exists on another interface.", "operationId": "loopback_ipv4_delete", "parameters": [ { @@ -1150,11 +1159,26 @@ "type": "string", "format": "ipv4" } + }, + { + "in": "query", + "name": "tag", + "schema": { + "nullable": true, + "type": "string" + } } ], "responses": { - "204": { - "description": "successful deletion" + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CmdAck" + } + } + } }, "4XX": { "$ref": "#/components/responses/Error" @@ -1193,7 +1217,8 @@ } }, "post": { - "summary": "Add a loopback IPv6.", + "summary": "Creates an IPv6 loopback address if one does not yet exist.", + "description": "Returns CONFLICT 409 if this address already belongs to another interface.", "operationId": "loopback_ipv6_create", "requestBody": { "content": { @@ -1206,8 +1231,15 @@ "required": true }, "responses": { - "204": { - "description": "resource updated" + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CmdAck" + } + } + } }, "4XX": { "$ref": "#/components/responses/Error" @@ -1220,7 +1252,8 @@ }, "/loopback/ipv6/{ipv6}": { "delete": { - "summary": "Remove one loopback IPv6 address.", + "summary": "Remove a loopback IPv6 address.", + "description": "If a tag is provided, the address will only be deleted if it belongs to the given tag.\n\nReturns 200 for any exit condition in which this address does not exist on loopback, even if the address still exists on another interface.", "operationId": "loopback_ipv6_delete", "parameters": [ { @@ -1231,11 +1264,26 @@ "type": "string", "format": "ipv6" } + }, + { + "in": "query", + "name": "tag", + "schema": { + "nullable": true, + "type": "string" + } } ], "responses": { - "204": { - "description": "successful deletion" + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/CmdAck" + } + } + } }, "4XX": { "$ref": "#/components/responses/Error" @@ -2370,9 +2418,10 @@ } }, "/port/{port_id}/settings": { - "get": { - "summary": "Get port settings atomically.", - "operationId": "port_settings_get", + "delete": { + "summary": "Clears port settings atomically.", + "description": "This is similar to calling link_delete on all links of a port, however it also attempts to roll back to the previous state if something fails.", + "operationId": "port_settings_clear", "parameters": [ { "in": "path", @@ -2382,27 +2431,11 @@ "schema": { "$ref": "#/components/schemas/PortId" } - }, - { - "in": "query", - "name": "tag", - "description": "Restrict operations on this port to the provided tag.", - "schema": { - "nullable": true, - "type": "string" - } } ], "responses": { - "200": { - "description": "successful operation", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/PortSettings" - } - } - } + "204": { + "description": "successful deletion" }, "4XX": { "$ref": "#/components/responses/Error" @@ -2411,11 +2444,13 @@ "$ref": "#/components/responses/Error" } } - }, - "post": { - "summary": "Apply port settings atomically.", - "description": "These settings will be applied holistically, and to the extent possible atomically to a given port. In the event of a failure a rollback is attempted. If the rollback fails there will be inconsistent state. This failure mode returns the error code \"rollback failure\". For more details see the docs on the [`PortSettings`] type.", - "operationId": "port_settings_apply", + } + }, + "/port/{port_id}/settings/{tag}": { + "get": { + "summary": "Retrieves the settings for all links on this port.", + "description": "Among tagged resources, only those matching the given tag are returned.\n\nUse link_ipv*_list if you want all addresses on a link regardless of tag.", + "operationId": "port_settings_get", "parameters": [ { "in": "path", @@ -2427,25 +2462,14 @@ } }, { - "in": "query", + "in": "path", "name": "tag", - "description": "Restrict operations on this port to the provided tag.", + "required": true, "schema": { - "nullable": true, "type": "string" } } ], - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/PortSettings" - } - } - }, - "required": true - }, "responses": { "200": { "description": "successful operation", @@ -2465,9 +2489,10 @@ } } }, - "delete": { - "summary": "Clear port settings atomically.", - "operationId": "port_settings_clear", + "post": { + "summary": "Apply port settings atomically.", + "description": "These settings will be applied holistically, and to the extent possible atomically to a given port. In the event of a failure, a rollback is attempted. If the rollback fails, there will be inconsistent state. This failure mode returns the error code \"rollback failure\". For more details see the docs on the [`PortSettings`] type.\n\nModifications within a link respect address tags. However, this may still delete a link even if it contains addresses belonging to other tags.", + "operationId": "port_settings_apply", "parameters": [ { "in": "path", @@ -2479,26 +2504,28 @@ } }, { - "in": "query", + "in": "path", "name": "tag", - "description": "Restrict operations on this port to the provided tag.", + "required": true, "schema": { - "nullable": true, "type": "string" } } ], - "responses": { - "200": { - "description": "successful operation", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/PortSettings" - } + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PortSettings" } } }, + "required": true + }, + "responses": { + "204": { + "description": "resource updated" + }, "4XX": { "$ref": "#/components/responses/Error" }, @@ -3340,46 +3367,12 @@ "$ref": "#/components/responses/Error" } } - }, - "delete": { - "summary": "Clear all IPv4 addresses from a link.", - "operationId": "link_ipv4_reset", - "parameters": [ - { - "in": "path", - "name": "link_id", - "description": "The link in the switch port on which to operate.", - "required": true, - "schema": { - "$ref": "#/components/schemas/LinkId" - } - }, - { - "in": "path", - "name": "port_id", - "description": "The switch port on which to operate.", - "required": true, - "schema": { - "$ref": "#/components/schemas/PortId" - } - } - ], - "responses": { - "204": { - "description": "resource updated" - }, - "4XX": { - "$ref": "#/components/responses/Error" - }, - "5XX": { - "$ref": "#/components/responses/Error" - } - } } }, "/ports/{port_id}/links/{link_id}/ipv4/{address}": { "delete": { - "summary": "Remove an IPv4 address from a link.", + "summary": "Remove an IPv4 address from this link.", + "description": "If a tag is provided, the address will only be deleted if it belongs to the given tag.\n\n- Returns 404 NOT_FOUND if the address is not registered on the switch. - Returns 409 CONFLICT if the address exists on the switch but is owned by something else (link, loopback, tag, etc).", "operationId": "link_ipv4_delete", "parameters": [ { @@ -3409,6 +3402,14 @@ "schema": { "$ref": "#/components/schemas/PortId" } + }, + { + "in": "query", + "name": "tag", + "schema": { + "nullable": true, + "type": "string" + } } ], "responses": { @@ -3534,46 +3535,12 @@ "$ref": "#/components/responses/Error" } } - }, - "delete": { - "summary": "Clear all IPv6 addresses from a link.", - "operationId": "link_ipv6_reset", - "parameters": [ - { - "in": "path", - "name": "link_id", - "description": "The link in the switch port on which to operate.", - "required": true, - "schema": { - "$ref": "#/components/schemas/LinkId" - } - }, - { - "in": "path", - "name": "port_id", - "description": "The switch port on which to operate.", - "required": true, - "schema": { - "$ref": "#/components/schemas/PortId" - } - } - ], - "responses": { - "204": { - "description": "resource updated" - }, - "4XX": { - "$ref": "#/components/responses/Error" - }, - "5XX": { - "$ref": "#/components/responses/Error" - } - } } }, "/ports/{port_id}/links/{link_id}/ipv6/{address}": { "delete": { - "summary": "Remove an IPv6 address from a link.", + "summary": "Remove an IPv6 address from this link.", + "description": "If a tag is provided, the address will only be deleted if it belongs to the given tag.\n\n- Returns 404 NOT_FOUND if the address is not registered on the switch. - Returns 409 CONFLICT if the address exists on the switch but is owned by something else (link, loopback, tag, etc).", "operationId": "link_ipv6_delete", "parameters": [ { @@ -3603,6 +3570,14 @@ "schema": { "$ref": "#/components/schemas/PortId" } + }, + { + "in": "query", + "name": "tag", + "schema": { + "nullable": true, + "type": "string" + } } ], "responses": { @@ -6099,6 +6074,25 @@ "rev_num" ] }, + "CmdAck": { + "description": "Conveys whether a successful API call caused a change of state.", + "oneOf": [ + { + "description": "The action caused some some change in internal state.", + "type": "string", + "enum": [ + "Updated" + ] + }, + { + "description": "The action caused no change in internal state. What was requested matches what already exists.", + "type": "string", + "enum": [ + "NoChange" + ] + } + ] + }, "CmisDatapath": { "description": "A datapath in a CMIS module.\n\nIn contrast to SFF-8636, CMIS makes first-class the concept of a datapath: a set of lanes and all the associated machinery involved in the transfer of data. This includes:\n\n- The \"application descriptor\" which is the host and media interfaces, and the lanes on each side used to transfer data; - The state of the datapath in a well-defined finite state machine (see CMIS 5.0 section 6.3.3); - The flags indicating how the datapath components are operating, such as receiving an input Rx signal or whether the transmitter is disabled.", "type": "object", @@ -10428,8 +10422,11 @@ "link_local": { "nullable": true, "description": "The IPv6 link-local address of the link, if it exists.", - "type": "string", - "format": "ipv6" + "allOf": [ + { + "$ref": "#/components/schemas/UnicastLinkLocalIpv6Addr" + } + ] }, "mac": { "description": "The MAC address for the link.", @@ -10611,6 +10608,19 @@ "type": "string", "format": "ipv6" }, + "UnicastLinkLocalIpv6Addr": { + "example": "fe80::1", + "title": "A unicast link-local IPv6 address", + "description": "An IPv6 address in fe80::/10", + "x-rust-type": { + "crate": "oxnet", + "path": "oxnet::UnicastLinkLocalIpv6Addr", + "version": "0.1.7" + }, + "type": "string", + "format": "ipv6", + "pattern": "^[fF][eE][89aAbB][0-9a-fA-F]:" + }, "Vendor": { "description": "Vendor-specific information about a transceiver module.", "type": "object", diff --git a/openapi/dpd/dpd-latest.json b/openapi/dpd/dpd-latest.json index d1a5660e..a8cf6f79 120000 --- a/openapi/dpd/dpd-latest.json +++ b/openapi/dpd/dpd-latest.json @@ -1 +1 @@ -dpd-13.0.0-5db8bd.json \ No newline at end of file +dpd-14.0.0-c1f40d.json \ No newline at end of file diff --git a/swadm/src/addr.rs b/swadm/src/addr.rs index 9fb5a267..984f8ce8 100644 --- a/swadm/src/addr.rs +++ b/swadm/src/addr.rs @@ -390,12 +390,12 @@ async fn addr_del( ) -> anyhow::Result<()> { match addr { IpAddr::V4(addr) => client - .link_ipv4_delete(&link.port_id, &link.link_id, &addr) + .link_ipv4_delete(&link.port_id, &link.link_id, &addr, None) .await .context("failed to delete IPv4 address") .map(|_| ()), IpAddr::V6(addr) => client - .link_ipv6_delete(&link.port_id, &link.link_id, &addr) + .link_ipv6_delete(&link.port_id, &link.link_id, &addr, None) .await .context("failed to delete IPv6 address") .map(|_| ()), @@ -408,12 +408,12 @@ async fn addr_del_loopback( ) -> anyhow::Result<()> { match addr { IpAddr::V4(addr) => client - .loopback_ipv4_delete(&addr) + .loopback_ipv4_delete(&addr, None) .await .context("failed to delete IPv4 address") .map(|_| ()), IpAddr::V6(addr) => client - .loopback_ipv6_delete(&addr) + .loopback_ipv6_delete(&addr, None) .await .context("failed to delete IPv6 address") .map(|_| ()), diff --git a/swadm/src/link.rs b/swadm/src/link.rs index 914f4ac5..c6b211ef 100644 --- a/swadm/src/link.rs +++ b/swadm/src/link.rs @@ -2181,7 +2181,7 @@ pub async fn link_cmd(client: &Client, link: Link) -> anyhow::Result<()> { }, ); client - .port_settings_apply(port_id, Some(tag.as_str()), &body) + .port_settings_apply(port_id, tag.as_str(), &body) .await .context("port settings apply failed")?; } diff --git a/tfportd/src/linklocal.rs b/tfportd/src/linklocal.rs index 2ebc419c..e8e3d93b 100644 --- a/tfportd/src/linklocal.rs +++ b/tfportd/src/linklocal.rs @@ -5,10 +5,10 @@ // Copyright 2025 Oxide Computer Company use std::collections::BTreeMap; -use std::net::Ipv6Addr; use anyhow::Result; use anyhow::anyhow; +use oxnet::UnicastLinkLocalIpv6Addr; use slog::debug; use slog::error; @@ -20,7 +20,9 @@ use common::illumos::IPV6_LINK_LOCAL_NAME; // address. This function returns an error if the ipadm command fails or the // output is malformed. If it finds a valid line of data that is not a // link-local address, it returns None. -fn parse_ipadm_line(line: &str) -> Result> { +fn parse_ipadm_line( + line: &str, +) -> Result> { // parse ipadm's "addrobj:address" into distinct components let Some((addrobj, addr)) = line.split_once(':') else { return Err(anyhow!("malformed ipadm output: {line}")); @@ -48,7 +50,7 @@ fn parse_ipadm_line(line: &str) -> Result> { // The parseable output of `ipadm show-addr` uses `:` as the field // separator, so the `:` punctuating the IPv6 address octets is escaped. - match addr.parse::() { + match addr.parse::() { Ok(addr) => Ok(Some((iface.to_string(), addr))), Err(_) => Err(anyhow!( "ipadm returned invalid ipv6 address for {addrobj}: {addr}" @@ -57,7 +59,7 @@ fn parse_ipadm_line(line: &str) -> Result> { } /// Return a map of all the per-tfport link-local addresses known to ipadm. -pub async fn get_all() -> Result> { +pub async fn get_all() -> Result> { let args = vec!["show-addr", "-p", "-o", "addrobj,addr"]; let mut rval = BTreeMap::new(); @@ -87,7 +89,8 @@ pub async fn create(g: &Global, iface: &str) -> anyhow::Result<()> { #[test] fn test_parse_ipadm() -> Result<()> { - let good_addr: Ipv6Addr = "fe80::aa40:25ff:fe04:392".parse().unwrap(); + let good_addr: UnicastLinkLocalIpv6Addr = + "fe80::aa40:25ff:fe04:392".parse().unwrap(); // test a valid line let (addrobj, addr) = parse_ipadm_line( diff --git a/tfportd/src/ports.rs b/tfportd/src/ports.rs index 8005d598..85a2a00a 100644 --- a/tfportd/src/ports.rs +++ b/tfportd/src/ports.rs @@ -31,13 +31,13 @@ use std::collections::BTreeMap; use std::collections::BTreeSet; use std::fmt; -use std::net::Ipv6Addr; use std::sync::Arc; use anyhow::Context; use anyhow::Result; use dpd_client::ClientInfo; use dpd_client::types; +use oxnet::UnicastLinkLocalIpv6Addr; use slog::debug; use slog::error; use slog::info; @@ -64,14 +64,14 @@ pub struct LinkInfo { /// Is this link configured to support IPv6? pub ipv6_enabled: bool, /// The IPv6 link-local address of the link in dpd, if it exists. - pub dpd_link_local: Option, + pub dpd_link_local: Option, /// The name of the `tfport` device, if it exists. pub tfport: Option, /// The index of the `tfport` interface, if it exists. pub tfport_ifindex: Option, /// The IPv6 link-local address of the tfport in illumos, if it exists. - pub tfport_link_local: Option, + pub tfport_link_local: Option, } impl fmt::Display for LinkInfo { @@ -239,12 +239,14 @@ async fn illumos_port_update( // happen if the mac address changes which, as noted in dpd_port_update(), // would also be very weird. async fn ensure_address_match(g: &Global, link: &LinkInfo) -> Result<()> { + let tag = g.client.inner().tag.as_str(); + if let Some(addr) = link.dpd_link_local && link.dpd_link_local != link.tfport_link_local { warn!(g.log, "deleting stale dpd address: {addr}"); g.client - .link_ipv6_delete(&link.port_id, &link.link_id, &addr) + .link_ipv6_delete(&link.port_id, &link.link_id, &addr, None) .await .context("deleting stale link-local address")?; } @@ -257,7 +259,7 @@ async fn ensure_address_match(g: &Global, link: &LinkInfo) -> Result<()> { .link_ipv6_create( &link.port_id, &link.link_id, - &types::Ipv6Entry { tag: g.client.inner().tag.clone(), addr }, + &types::Ipv6Entry { tag: tag.to_string(), addr: addr.into() }, ) .await .context("sending new link-local address")?; diff --git a/tfportd/src/simport.rs b/tfportd/src/simport.rs index 81920ce7..9e8dbc13 100644 --- a/tfportd/src/simport.rs +++ b/tfportd/src/simport.rs @@ -4,19 +4,19 @@ // // Copyright 2026 Oxide Computer Company -use std::collections::HashSet; -use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; +use std::net::IpAddr; use std::str::FromStr; use std::sync::Arc; -use futures::TryStreamExt; +use dpd_client::types::Ipv6Entry; +use oxnet::UnicastLinkLocalIpv6Addr; use slog::{debug, error, warn}; use crate::Global; use crate::oxstats::link; use crate::poll_interval; -use common::illumos; -use dpd_client::types; +use common::{addr::IpAddrLike, illumos}; +use dpd_client::{ClientInfo, types}; async fn simnet_tfport_get() -> anyhow::Result> { // dladm show-simnet seems to be broken when not in the global zone. @@ -28,33 +28,29 @@ async fn simnet_tfport_get() -> anyhow::Result> { }) } -async fn ipadm_addrs() -> anyhow::Result> { - illumos::ipadm(&["show-addr", "-p", "-o", "addrobj,addr"]) - .await - .map_err(|e| e.into()) - .map(|lines| { - lines - .iter() - .filter_map(|s| { - // divide the address object and address fields - let (aobj, addr) = s.split_once(':')?; - // Clean up the ":" escapes added by ipadm - let addr = addr.replace('\\', ""); - // Drop any interface suffix - let addr = addr - .split('%') - .next() - .expect("a split must return at least one item"); - // Drop any subnet suffix - let addr = addr - .split('/') - .next() - .expect("a split must return at least one item"); - let addr: IpAddr = addr.parse().ok()?; - Some((aobj.to_owned(), addr)) - }) - .collect() - }) +async fn ipadm_addrs() -> anyhow::Result> +{ + Ok(illumos::ipadm(&["show-addr", "-p", "-o", "addrobj,addr"]) + .await? + .into_iter() + .filter_map(|s| { + // divide the address object and address fields + let (aobj, addr) = s.split_once(':')?; + // Clean up the ":" escapes added by ipadm + let addr = addr.replace('\\', ""); + // Drop any interface suffix + let addr = addr + .split('%') + .next() + .expect("a split must return at least one item"); + // Drop any subnet suffix + let addr = addr + .split('/') + .next() + .expect("a split must return at least one item"); + let addr: IpAddr = addr.parse().ok()?; + Some((aobj.to_owned(), addr)) + })) } pub async fn simnet_loop(g: Arc) { @@ -68,39 +64,46 @@ pub async fn simnet_loop(g: Arc) { debug!(g.log, "simnet loop exiting"); } +/// Reconciles the IPv6 link local address from illumos to DPD +/// for all links DPD is aware of. async fn simnet_process(g: &Global) -> anyhow::Result<()> { let simports = simnet_tfport_get().await?; debug!(g.log, "found simports {:#?}", simports); - let addrs = ipadm_addrs().await?; + + let addrs = ipadm_addrs() + .await? + .filter_map(|(name, addr)| { + let addr = UnicastLinkLocalIpv6Addr::new(addr.v6()?).ok()?; + Some((name, addr)) + }) + .collect::>(); + let tag = g.client.inner().tag.as_str(); + + let dpd_ports = g.client.tfport_data().await?.into_inner(); for p in &simports { if let Err(e) = illumos::iface_ensure(p).await { warn!(g.log, "{e}"); continue; } - let p_addrs = - addrs - .iter() - .filter_map(|(name, addr)| { - if name.starts_with(p) { Some(*addr) } else { None } - }) - .collect::>(); - - if p_addrs.is_empty() { - warn!(g.log, "{p} has no addrs"); + + // breakouts not a thing yet, just assume the 0th link + let link_id = types::LinkId(0); + let link_local = addrs + .iter() + .find_map(|(name, addr)| name.starts_with(p).then_some(addr)); + + let Some(link_local) = link_local.copied() else { + warn!(g.log, "{p} has no IPv6 unicast link local addr(s)"); continue; - } else { - debug!(g.log, "sync {p} addrs {:?}", p_addrs); - } + }; + debug!(g.log, "sync {p} addr {link_local:?}"); // need to go from tfportM_N to port_id=M link_id=N - let port_name = match p.strip_prefix("tfport") { - Some(name) => name, - None => continue, + let Some(port_name) = p.strip_prefix("tfport") else { + continue; }; let port_name = port_name.split('_').next().unwrap(); - // breakouts not a thing yet, just assume the 0th link - let link_id = types::LinkId(0); let port_id = match types::PortId::from_str(port_name) { Ok(name) => name, Err(e) => { @@ -108,28 +111,16 @@ async fn simnet_process(g: &Global) -> anyhow::Result<()> { continue; } }; - // ensure the link for the tfport exists - match g.client.link_get(&port_id, &link_id).await { - Ok(_) => {} - Err(_) => { - // this is for softnpu environments which do not currently care - // about these parameters, so just pick some - let params = types::LinkCreate { - lane: None, - speed: types::PortSpeed::Speed100G, - fec: Some(types::PortFec::None), - autoneg: false, - kr: false, - tx_eq: None, - allow_ddm_traffic: false, - }; - if let Err(e) = g.client.link_create(&port_id, ¶ms).await { - error!( - g.log, - "failed to create link for tfport {port_name}: {e}" - ); - } - } + + let Some(conf) = dpd_ports + .iter() + .find(|entry| entry.port_id == port_id && entry.link_id == link_id) + else { + error!( + g.log, + "Cannot reconcile {port_id}/{link_id}. Not found in tfport_data()" + ); + continue; }; // track the metrics for the simport link @@ -137,93 +128,31 @@ async fn simnet_process(g: &Global) -> anyhow::Result<()> { error!(g.log, "failed to track link {p}: {e}"); } - // sync addresses to the ASIC - let asic_v4_addrs: HashSet = match g - .client - .link_ipv4_list_stream(&port_id, &link_id, None) - .map_ok(|entry| entry.addr) - .try_collect() - .await + if conf.link_local == Some(link_local) { + continue; + } + + if let Some(addr) = conf.link_local + && let Err(e) = g + .client + .link_ipv6_delete(&port_id, &link_id, &addr, Some(tag)) + .await { - Ok(addrs) => addrs, - Err(e) => { - error!( - g.log, - "failed to collect stream of ipv4 addresses: {e}" - ); - continue; - } - }; + error!(g.log, "failed to delete v6 address {addr}: {e}"); + } - let asic_v6_addrs: HashSet = match g + if let Err(e) = g .client - .link_ipv6_list_stream(&port_id, &link_id, None) - .map_ok(|entry| entry.addr) - .try_collect() + .link_ipv6_create( + &port_id, + &link_id, + &Ipv6Entry { addr: link_local.into(), tag: tag.to_string() }, + ) .await { - Ok(addrs) => addrs, - Err(e) => { - error!( - g.log, - "failed to collect stream of ipv6 addresses: {e}" - ); - continue; - } - }; - - let mut port_v4_addrs: HashSet = HashSet::new(); - let mut port_v6_addrs: HashSet = HashSet::new(); - for a in &p_addrs { - match a { - IpAddr::V4(a) => { - port_v4_addrs.insert(*a); - } - IpAddr::V6(a) => { - port_v6_addrs.insert(*a); - } - } - } - - let to_add_v4 = port_v4_addrs.difference(&asic_v4_addrs); - let to_del_v4 = asic_v4_addrs.difference(&port_v4_addrs); - - let to_add_v6 = port_v6_addrs.difference(&asic_v6_addrs); - let to_del_v6 = asic_v6_addrs.difference(&port_v6_addrs); - - for a in to_add_v4 { - let entry = g.client.ipv4_entry(*a); - if let Err(e) = - g.client.link_ipv4_create(&port_id, &link_id, &entry).await - { - error!(g.log, "failed to add v4 address {a}: {e}"); - } - } - - for a in to_del_v4 { - if let Err(e) = - g.client.link_ipv4_delete(&port_id, &link_id, a).await - { - error!(g.log, "failed to delete v4 address {a}: {e}"); - } - } - - for a in to_add_v6 { - let entry = g.client.ipv6_entry(*a); - if let Err(e) = - g.client.link_ipv6_create(&port_id, &link_id, &entry).await - { - error!(g.log, "failed to add v6 address {a}: {e}"); - } - } - - for a in to_del_v6 { - if let Err(e) = - g.client.link_ipv6_delete(&port_id, &link_id, a).await - { - error!(g.log, "failed to delete v6 address {a}: {e}"); - } + error!(g.log, "failed to create v6 address {link_local}: {e}"); } } + Ok(()) } diff --git a/tfportd/src/tfport.rs b/tfportd/src/tfport.rs index 753aebd6..7637605e 100644 --- a/tfportd/src/tfport.rs +++ b/tfportd/src/tfport.rs @@ -5,12 +5,12 @@ // Copyright 2026 Oxide Computer Company use std::collections::BTreeMap; -use std::net::Ipv6Addr; use anyhow::Context; use anyhow::Result; use anyhow::anyhow; use anyhow::bail; +use oxnet::UnicastLinkLocalIpv6Addr; use slog::debug; use slog::error; use slog::info; @@ -38,7 +38,7 @@ pub struct TfportInfo { /// index of the interface created by `ipadm` pub ifindex: Option, /// link-local address assigned to the tfport interface by Illumos - pub link_local: Option, + pub link_local: Option, } // Parse a single line of dladm output to extract the 3 different expected diff --git a/tfportd/src/vlans.rs b/tfportd/src/vlans.rs index 7dbea3d3..da20ffd4 100644 --- a/tfportd/src/vlans.rs +++ b/tfportd/src/vlans.rs @@ -87,7 +87,7 @@ async fn vlans_get(tfport: &str) -> anyhow::Result> { let link = fields[0].to_string(); let vid = fields[1].parse::().context("invalid vlan_id")?; let ifindex = crate::netsupport::get_ifindex(&link); - let link_local = link_locals.get(&link).copied(); + let link_local = link_locals.get(&link).copied().map(Ipv6Addr::from); let vlan = VlanInfo { name: link, vid, ifindex, link_local }; // NOTE: We previously used a BTreeMap here, and ignored any duplicates.