From 7de86184ec2aa79048165e8b4d1613d1f57a960a Mon Sep 17 00:00:00 2001 From: brian Date: Sun, 9 Aug 2026 23:59:27 -0400 Subject: [PATCH] fix(api-core,dpf): Add BF3 and generic BF4 intercept bridging parity for DPF --- crates/api-core/src/cfg/README.md | 41 +- crates/api-core/src/cfg/file.rs | 305 +++++- crates/api-core/src/dpf_services.rs | 224 +++- crates/api-core/src/handlers/instance.rs | 26 +- crates/api-core/src/instance/mod.rs | 132 ++- crates/api-core/src/setup.rs | 236 ++++- .../src/test_support/default_config.rs | 49 + crates/api-core/src/tests/instance.rs | 184 ++++ crates/dpf/README.md | 4 +- crates/dpf/src/flavor.rs | 802 ++++++++++++++- crates/dpf/src/lib.rs | 21 +- crates/dpf/src/sdk.rs | 953 ++++++++++++++++-- crates/dpf/src/test/sdk_initialization.rs | 593 ++++++++++- crates/dpf/src/types.rs | 632 +++++++++++- crates/machine-controller/src/dpf.rs | 23 +- crates/rpc/src/errors.rs | 2 +- .../rpc/src/model/instance/config/network.rs | 24 +- 17 files changed, 3996 insertions(+), 255 deletions(-) diff --git a/crates/api-core/src/cfg/README.md b/crates/api-core/src/cfg/README.md index 6ed6e3b505..48b8a50e90 100644 --- a/crates/api-core/src/cfg/README.md +++ b/crates/api-core/src/cfg/README.md @@ -509,16 +509,16 @@ Extends `StateControllerConfig` with: | Field | Type | Default | Description | |-------|------|---------|-------------| -| `allow_instance_vf` | `bool` | `true` | Allow VFs on instance creation. | +| `allow_instance_vf` | `bool` | `true` | Global instance-VF admission switch for creation and network updates. `false` rejects every VF. With DPF intercept topology, `true` additionally requires every requested VF ID to be explicitly selected by that topology. Without DPF intercept topology, the historical boolean-only admission behavior is preserved. | | `hbn_reps` | `Option` | — | Select which representors from the configured VF population HBN is expected to use during DPU provisioning. When omitted, HBN uses its default representor selection. | -| `bridging` | `Option` | — | Provisioning-time topology for bridges inserted between host representors and HBN. | +| `bridging` | `Option` | — | Provisioning-time topology for bridges inserted between host representors and HBN or DPF's `br-sfc`. Under DPF, a present `vmaas_config` makes this map the complete configurable PF/VF inventory and requires exactly one PF entry. | ### `HostRepresentorBridgingConfig` | Field | Type | Default | Description | |-------|------|---------|-------------| | `hbn_bridge` | `String` | `"br-hbn"` | HBN/SFC bridge that host-representor patch ports attach to during BlueField provisioning. | -| `host_representor_intercept_bridging` | `HashMap` | `{}` | Host-owned PF/VF representor bridge layout keyed by representor name. Non-skipped entries are sent to BlueField provisioning as `::`. | +| `host_representor_intercept_bridging` | `HashMap` | `{}` | Host-owned PF/VF representor bridge layout keyed by the legacy representor name. Non-skipped entries are sent to pre-DPF BlueField provisioning as `::`. When DPF is enabled, this map replaces the static PF/VF inventory; an absent map, empty map, or VF-only map is rejected because FMDS requires the selected PF. | ### `HostInterceptBridging` @@ -527,6 +527,27 @@ Extends `StateControllerConfig` with: | `bridge` | `String` | **required** | Bridge that sits between the host PF/VF representor and br-hbn or br-sfc. | | `patch_port` | `String` | **required** | Patch port on this bridge that connects it toward HBN or SFC. | | `skip_create` | `bool` | `false` | When true, the entry is omitted from provisioning-time bridge creation. | +| `dpf_interface` | `Option` | — | Typed DPF PF/VF selection. Optional and ignored by pre-DPF provisioning, but required for every map entry when DPF is enabled. The surrounding legacy map key is never parsed as DPF identity. | + +When DPF is enabled, `skip_create=true` is rejected. `bridge` is a Linux netdev name and must contain 1–15 lowercase ASCII letters, digits, or hyphens and start with a letter. `patch_port` is an OVS patch-interface name rather than a Linux netdev: it must be non-empty, start with a lowercase ASCII letter, and contain only lowercase ASCII letters, digits, hyphens, or underscores. NICo does not apply the Linux 15-character limit to patch ports. Both names must be unique across the rendered OVS topology. DPF requires exactly one configured PF and also rejects duplicate identities, generated-name collisions, BF3 raw-representor collisions, configurations spanning more than one selected controller/PF parent, hardware VF counts above 126, and VFs whose `vf_id` is greater than 15 or greater than or equal to `dpu_config.num_of_vfs`. PF selection is independent of that VF count. + +Pre-DPF configurations retain their existing behavior: `dpf_interface` may be +omitted, `skip_create=true` remains valid, `hbn_bridge` still defaults to +`br-hbn`, and the sorted provisioning value remains exactly +`::`. DPF ignores `hbn_reps`, derives +`br-sfc` internally, and uses the typed identity rather than the map key. + +With a configured DPF intercept inventory, the topology is exclusively the complete VMaaS-managed PF/VF inventory. NICo retains the fixed `p0` and `p1` physical interfaces and assigns both to HBN. Every configured PF or VF becomes a Patch interface between `br-sfc` and its configured intermediate bridge and is assigned to HBN and DHCP; only the selected PF is also assigned to FMDS. The selected hardware PF is exposed inside HBN as `pf0hpf_if`, and VF identity `vf_id` as `pf0vf{vf_id}_if`, regardless of the configured controller and PF identifiers. DHCP exposes them as `d_pf0hpf_if` and `d_pf0vf{vf_id}_if`; FMDS exposes the selected PF as `f_pf0hpf_if`. For `P` configured PFs and `V` configured VFs, NICo manages `2 + P + V` HBN endpoints, `P + V` DHCP endpoints, and `P` FMDS endpoints, for `2 + 3P + 2V` total SF-backed service endpoints. The one-selected-PF contract requires `P = 1`; together with the VF15 bound, it permits at most 19 generated HBN interfaces. The generated HBN inventory may not exceed 32 interfaces. With `allow_instance_vf=true`, instance creation and network updates admit only the explicitly configured VF IDs; a PF-only topology therefore admits no instance VFs. + +Without configured DPF intercept topology, NICo deliberately preserves the established static VF0–VF13 inventory, its VF0–VF7 DHCP subset, and historical instance admission behavior. In this mode, `pf_total_sf_reserved` is the complete `PF_TOTAL_SF`; it defaults to `30`, and explicit operator overrides remain supported. Reconciling the inventory, DHCP, or admission surfaces would change existing ServiceInterfaces and the hashed DPUFlavor, so it is deferred to a separately planned cleanup and DPU re-ingestion migration. + +### `DpfInterfaceIdentity` + +| Field | Type | Default | Description | +|-------|------|---------|-------------| +| `controller_id` | `u8` | **required** | DPF controller number (`0..=255`) containing the selected PF or VF. | +| `pf_id` | `u8` | **required** | PF identifier (`0..=255`) on the selected controller. | +| `vf_id` | `Option` | — | VF identifier. Omission selects the PF. Under DPF intercept topology, presence selects that VMaaS VF and requires both `vf_id <= 15` and `vf_id < dpu_config.num_of_vfs`; pre-DPF provisioning ignores this typed field. | ### `DpuConfig` @@ -538,7 +559,7 @@ Extends `StateControllerConfig` with: | `dpu_models` | `HashMap` | *(BF2+BF3 defaults)* | DPU model firmware definitions. | | `dpu_nic_firmware_update_versions` | `Vec` | *(BF2+BF3 NIC versions)* | DPU NIC firmware version strings. | | `dpu_enable_secure_boot` | `bool` | `false` | Enable secure boot flow for DPU provisioning via Redfish. | -| `num_of_vfs` | `u32` | `16` | Number of VFs configured per DPU PF during BlueField provisioning. Max `126`. | +| `num_of_vfs` | `u32` | `16` | Number of hardware VFs configured per DPU PF during BlueField provisioning. Max `126`. Under DPF, changing this value changes the immutable BF3/generic-BF4 flavor and requires a carbide-api restart and DPU reprovisioning. Reducing it below the static inventory's previous effective VF count also removes desired VF ServiceInterfaces; because NICo does not prune them, operators must stop NICo, remove the omitted NICo ServiceInterfaces, re-ingest the DPUs, and restart. Configured intercept inventories remain valid only while every selected `vf_id` is both lower than this value and no greater than 15. | | `restart_ovs_on_use_admin_network_change` | `bool` | `false` | Restart OVS on DPU-OS agents when host `use_admin_network` changes. Containerized agents skip the local service restart and still ACK the network config. | To use `embedded`, build a site-specific BFB with an explicit @@ -659,17 +680,19 @@ events, so consumers handle them identically. | Field | Type | Default | Description | |-------|------|---------|-------------| | `enabled` | `bool` | `false` | Enable DPF Kubernetes deployment. | +| `deployment_scoped_service_interfaces` | `bool` | `false` | Opt the complete DPF namespace into deployment-scoped `-bf3`, `-bf4`, and `-astra` DPUServiceInterfaces. Each resource selects Nodes in the remote DPU cluster through DPF's propagated `svc.dpu.nvidia.com/owned-by-dpudeployment=_` ownership label; management-cluster DPUNode deployment labels are not used for this selector. Enabling or disabling is a planned migration: stop NICo, remove old-mode NICo ServiceInterfaces in both transition directions, perform DPU re-ingestion, and restart. NICo neither detects nor deletes old-mode resources; skipping cleanup can leave competing interface generations active. Astra requires this setting. | +| `pf_total_sf_reserved` | `u32` | `30` | SF capacity reserved beyond the NICo-managed HBN, DHCP, and FMDS endpoints when an intercept-bridging inventory is configured. NICo sets `PF_TOTAL_SF` to the effective inventory's endpoint count plus this value for BF3 and generic BF4. Without configured intercept bridging, this value is the complete `PF_TOTAL_SF`, preserving the legacy default of `30`; BF4 Astra retains its fixed flavor and ignores this setting. Changing this value changes the BF3/generic-BF4 flavor. Every intercept-inventory change requires controlled ServiceInterface cleanup and DPU re-ingestion, even when the serialized flavor and its hash remain unchanged. Operators must select a value compatible with their platform's SF and BAR capacity. With configured intercept bridging, startup rejects configurations whose managed endpoint count plus reserve exceeds `u32::MAX`. | | `dpu_agent_bootstrap_ca` | `DpfDpuAgentBootstrapCa` | `legacy_download` | Bootstrap trust for the containerized DPU agent. Supports `legacy_download` and `mounted`, as described in the following examples. | | `services` | `Box` | built-in mandatory-service defaults | Helm chart, image, pull-secret, and `extra_helm_values` settings for the six mandatory DPF services. | | `docker_image_pull_secret` | `Option` | — | Override for the Kubernetes `imagePullSecrets` entry used to pull mandatory-service images (applied to every mandatory service except `dts` and `doca_hbn`, which take a pull secret only from their per-service config). | | `proxy` | `Option` | — | Proxy configuration for the DPU. When set, containerd on the DPU routes outbound HTTPS traffic through it. | | `deployments` | `DpfDeploymentsConfig` | *(default)* | Per-generation DPUDeployment configurations. BF3 is always present with defaults; BF4 variants are opt-in. BF4 Astra gets default Weave DHCP agent, Weave flow controller, and Xplane services; `extra_services` can replace any of those definitions. | -Each entry under `[dpf.services]` accepts a chart-native `extra_helm_values` table. NICo -deep-merges it over generated `DPUServiceTemplate` values. Nested scalars and arrays -replace generated values. DPF applies NICo's deployment-specific -`DPUServiceConfiguration` values after the template values. -Top-level and per-deployment service fields both overlay the service's built-in defaults. +Every active DPF deployment must use distinct `deployment_name`, `flavor_name`, and `node_label_key` values. A deployment `node_label_key` must not be `feature.node.kubernetes.io/dpu-enabled`, which marks every DPF-managed node, or `carbide.nvidia.com/host-bmc-ip`, whose per-node contextual value is the host BMC address. These checks use the local configuration and do not query or modify cluster resources. + +Each entry under `[dpf.services]` accepts a chart-native `extra_helm_values` table. NICo deep-merges it over generated `DPUServiceTemplate` values. Nested scalars and arrays replace generated values. DPF applies NICo's deployment-specific `DPUServiceConfiguration` values after the template values. Top-level and per-deployment service fields both overlay the service's built-in defaults. + +The topology-derived `[dpf.services.doca_hbn.extra_helm_values.resources]` value `nvidia.com/bf_sf` is reserved: NICo restores the effective HBN interface count after applying operator overrides so the SF request, interface assignment, and startup configuration cannot diverge. ```toml [dpf.services.dpu_agent.extra_helm_values.fmds] diff --git a/crates/api-core/src/cfg/file.rs b/crates/api-core/src/cfg/file.rs index a87ceddce2..115724846e 100644 --- a/crates/api-core/src/cfg/file.rs +++ b/crates/api-core/src/cfg/file.rs @@ -71,9 +71,23 @@ use serde::{Deserialize, Deserializer, Serialize}; use crate::CarbideError; -pub(crate) const DEFAULT_DPU_NUM_OF_VFS: u32 = 16; +pub(crate) const DEFAULT_DPU_NUM_OF_VFS: u32 = carbide_dpf::DEFAULT_DPU_NUM_OF_VFS; pub(crate) const MAX_DPU_NUM_OF_VFS: u32 = 126; +// Deployment selectors must never reuse labels whose values NICo supplies independently. +// The shared marker would make every deployment select every DPUNode, while the contextual +// host-BMC label is overwritten with an address during registration and would match none. +const RESERVED_DPF_DEPLOYMENT_NODE_LABELS: [(&str, &str); 2] = [ + ( + carbide_dpf::DPU_ENABLED_NODE_LABEL, + "the shared DPF-enabled node marker", + ), + ( + carbide_machine_controller::dpf::HOST_BMC_IP_LABEL, + "the per-node host BMC address", + ), +]; + /// Parses an optional duration ("30d", "12h", ...; absent = `None`) into /// `Option`. Hand-rolled because `duration_str` deprecated /// its own Option variant -- we do NOT use the deprecated function. @@ -1494,11 +1508,25 @@ fn default_dpf_bootstrap_ca_key() -> String { "ca.crt".to_string() } -#[derive(Clone, Debug, Serialize, Default, Deserialize)] +/// Returns the legacy PF_TOTAL_SF population used when the operator omits the reserve. +fn default_dpf_pf_total_sf_reserved() -> u32 { + carbide_dpf::DEFAULT_PF_TOTAL_SF_RESERVED +} + +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct DpfConfig { /// Enables DPF deployment. #[serde(default)] pub enabled: bool, + /// Opts the DPF namespace into deployment-scoped DPUServiceInterfaces. + /// Changing modes requires operators to remove old-mode NICo resources and + /// re-ingest DPUs; NICo neither detects nor deletes those resources. + #[serde(default)] + pub deployment_scoped_service_interfaces: bool, + /// SF capacity reserved beyond configured NICo-managed service endpoints. + /// Without intercept bridging, this remains the complete legacy `PF_TOTAL_SF` value. + #[serde(default = "default_dpf_pf_total_sf_reserved")] + pub pf_total_sf_reserved: u32, /// Optional override for the Kubernetes `imagePullSecrets` entry used to pull the /// docker images of the mandatory services. When set, it is applied to every /// mandatory service except `dts` and `doca_hbn`, which take a pull secret only @@ -1522,7 +1550,36 @@ pub struct DpfConfig { pub deployments: DpfDeploymentsConfig, } +impl Default for DpfConfig { + fn default() -> Self { + Self { + enabled: false, + deployment_scoped_service_interfaces: false, + pf_total_sf_reserved: default_dpf_pf_total_sf_reserved(), + docker_image_pull_secret: None, + dpu_agent_bootstrap_ca: DpfDpuAgentBootstrapCa::default(), + services: Box::default(), + proxy: None, + deployments: DpfDeploymentsConfig::default(), + } + } +} + impl DpfConfig { + /// Rejects Astra unless deployment-scoped ServiceInterfaces are enabled. + /// + /// Astra is supported only by the BF4+CX9 deployment class and has a + /// different interface inventory. Legacy global ServiceInterfaces cannot + /// safely distinguish it from BF3 or generic BF4 nodes. + pub(crate) fn validate_service_interface_scoping(&self) -> eyre::Result<()> { + eyre::ensure!( + self.deployments.bf4_astra.is_none() || self.deployment_scoped_service_interfaces, + "dpf.deployments.bf4_astra requires \ + dpf.deployment_scoped_service_interfaces=true" + ); + Ok(()) + } + /// Returns the top-level mandatory services with the optional /// [`Self::docker_image_pull_secret`] override applied. The override affects every /// mandatory service except `dts` and `doca_hbn`, which take a pull secret only @@ -1798,7 +1855,8 @@ pub struct DpfBlueFieldSoftwareConfig { } /// Named DPUDeployment configurations under `[dpf.deployments]`. -/// Each entry creates its own BFB, DPUFlavor, and DPUDeployment CR at startup. +/// Each entry creates its own provisioning source, DPUFlavor, and DPUDeployment +/// CR at startup. #[derive(Clone, Debug, Default, Serialize)] pub struct DpfDeploymentsConfig { /// BF3 deployment. Present by default with sensible values; override individual @@ -1877,9 +1935,8 @@ impl DpfDeploymentsConfig { v } - /// Validates that no two active deployments share a `deployment_name`, - /// `flavor_name`, or `node_label_key`. Returns an error listing every - /// conflict so the operator can fix them all in one pass. + /// Validates that identifiers are unique and deployment label keys are not reserved. + /// Returns every conflict so the operator can fix them all in one pass. pub fn validate_unique_identifiers(&self) -> eyre::Result<()> { let deployments = self.all(); let mut errors: Vec = Vec::new(); @@ -1912,11 +1969,24 @@ impl DpfDeploymentsConfig { } } + // This is intentionally a local configuration check. Querying current DPUNode labels + // cannot establish safety: these keys have fixed NICo semantics before any node exists. + for (deployment, label_key) in &label_vals { + if let Some((_, purpose)) = RESERVED_DPF_DEPLOYMENT_NODE_LABELS + .iter() + .find(|(reserved, _)| label_key == reserved) + { + errors.push(format!( + "node_label_key {label_key:?} for deployment {deployment:?} is reserved for {purpose}" + )); + } + } + if errors.is_empty() { Ok(()) } else { Err(eyre::eyre!( - "DPF deployment configuration has conflicting identifiers:\n - {}", + "DPF deployment configuration has invalid identifiers:\n - {}", errors.join("\n - ") )) } @@ -3662,6 +3732,22 @@ pub struct HostInterceptBridging { /// By default, we expect to create these bridges. #[serde(default)] pub skip_create: bool, + + /// Typed PF/VF identity used only when DPF is enabled. + pub dpf_interface: Option, +} + +/// Typed DPF identity for a configured host PF or VF. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize)] +pub struct DpfInterfaceIdentity { + /// DPF controller number that owns the selected PF or VF. + pub controller_id: u8, + + /// Physical-function identifier on the selected controller. + pub pf_id: u8, + + /// Virtual-function identifier. Omission selects the PF itself. + pub vf_id: Option, } impl HostRepresentorBridgingConfig { @@ -3695,8 +3781,8 @@ mod tests { use carbide_authn::config::CertComponent; use carbide_network::virtualization::VpcVirtualizationType; use carbide_site_explorer::config::SiteExplorerExploreMode; - use carbide_test_support::Outcome::Yields; - use carbide_test_support::{Check, check_values, scenarios}; + use carbide_test_support::Outcome::{Fails, Yields}; + use carbide_test_support::{Check, check_values, scenarios, value_scenarios}; use chrono::Datelike; use figment::Figment; use figment::providers::{Env, Format, Toml}; @@ -3713,6 +3799,96 @@ mod tests { const TEST_DATA_DIR: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/src/cfg/test_data"); + /// Verifies legacy entries remain valid while typed DPF identities require and preserve their + /// complete controller, PF, and optional VF selection. + #[test] + fn host_intercept_bridging_deserializes_optional_dpf_identity() { + scenarios!( + run = |config: &str| { + toml::from_str::(config) + .map(|bridging| bridging.dpf_interface) + .map_err(drop) + }; + "legacy compatibility" { + // Existing pre-DPF entries do not need typed identity. + "bridge = 'br-host'\npatch_port = 'p-host'" => Yields(None), + } + + "PF identity" { + // Omitting vf_id selects the complete configured PF identity. + "bridge = 'br-host'\npatch_port = 'p-host'\ndpf_interface = { controller_id = 2, pf_id = 3 }" => Yields(Some(DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: None, + })), + } + + "VF identity" { + // Including vf_id preserves that VF under the complete configured PF identity. + "bridge = 'br-host'\npatch_port = 'p-host'\ndpf_interface = { controller_id = 2, pf_id = 3, vf_id = 4 }" => Yields(Some(DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: Some(4), + })), + } + + "missing controller identity" { + // A PF number without its controller cannot select DPF hardware unambiguously. + "bridge = 'br-host'\npatch_port = 'p-host'\ndpf_interface = { pf_id = 3 }" => Fails, + } + + "missing PF identity" { + // A controller without its PF cannot select the required parent interface. + "bridge = 'br-host'\npatch_port = 'p-host'\ndpf_interface = { controller_id = 2 }" => Fails, + } + ); + } + + /// Verifies typed DPF identity cannot alter the legacy sorted `bf.cfg` value. + #[test] + fn dpf_identity_does_not_change_legacy_bridging_provisioning_config() { + // Build equivalent legacy entries with and without a typed DPF identity. + let make_config = |dpf_interface| HostRepresentorBridgingConfig { + hbn_bridge: default_hbn_bridge(), + host_representor_intercept_bridging: HashMap::from([ + ( + "pf0vf1".to_string(), + HostInterceptBridging { + bridge: "br-vf1".to_string(), + patch_port: "p-vf1".to_string(), + skip_create: false, + dpf_interface, + }, + ), + ( + "pf0vf0".to_string(), + HostInterceptBridging { + bridge: "br-vf0".to_string(), + patch_port: "p-vf0".to_string(), + skip_create: false, + dpf_interface, + }, + ), + ]), + }; + let typed_identity = Some(DpfInterfaceIdentity { + controller_id: 1, + pf_id: 0, + vf_id: Some(3), + }); + + // Both variants must render the exact historical ordering and wire format. + let expected = Some("pf0vf0:br-vf0:p-vf0,pf0vf1:br-vf1:p-vf1".to_string()); + assert_eq!( + make_config(None).host_representor_intercept_bridging_provisioning_config(), + expected + ); + assert_eq!( + make_config(typed_identity).host_representor_intercept_bridging_provisioning_config(), + expected + ); + } + fn vpc_config( routing_profile_type: Option<&str>, routing_profile_overrides: Option, @@ -6121,6 +6297,72 @@ object_kind = "secret" assert!(error.to_string().contains("unknown field `object_kind`")); } + /// Verifies deployment scoping is opt-in and is mandatory for BF4+CX9 Astra. + #[test] + fn dpf_service_interface_scoping_gates_astra() { + // Serde omission must preserve the legacy namespace-wide mode. + assert!( + !toml::from_str::("") + .unwrap() + .deployment_scoped_service_interfaces + ); + + value_scenarios!( + run = |(deployment_scoped_service_interfaces, has_astra)| { + let mut config = DpfConfig { + deployment_scoped_service_interfaces, + ..Default::default() + }; + config.deployments.bf4_astra = has_astra.then(DpfDeploymentConfig::default); + config.validate_service_interface_scoping().is_ok() + }; + "legacy default" { + // Omission preserves existing global ServiceInterfaces without a scoping migration. + (false, false) => true, + } + + "explicit scoped mode" { + // Operators opt into the migration independently of Astra enablement. + (true, false) => true, + } + + "unsafe Astra mode" { + // Astra's distinct inventory cannot be represented by legacy global resources. + (false, true) => false, + } + + "scoped Astra mode" { + // The BF4+CX9 deployment is valid only after opting into isolated resources. + (true, true) => true, + } + ); + } + + /// Verifies the reserved SF setting preserves the legacy pool by default while allowing an + /// operator to reserve additional platform-specific capacity. + #[test] + fn dpf_pf_total_sf_reserved_defaults_and_deserializes() { + // Exercise omission and an explicit override through the operator-facing TOML contract. + value_scenarios!( + run = |input| toml::from_str::(input).unwrap().pf_total_sf_reserved; + "omitted reserve" { + // Omission must retain the existing PF_TOTAL_SF value for inventory-free sites. + "" => carbide_dpf::DEFAULT_PF_TOTAL_SF_RESERVED, + } + + "explicit reserve" { + // Operators may size headroom for their DPF and firmware consumers. + "pf_total_sf_reserved = 47" => 47, + } + ); + + // Programmatic defaults must match deserialization defaults used by production config. + assert_eq!( + DpfConfig::default().pf_total_sf_reserved, + carbide_dpf::DEFAULT_PF_TOTAL_SF_RESERVED + ); + } + #[test] fn empty_dpf_service_uses_its_defaults() { let config = toml::from_str::("[services.dpu_agent]").unwrap(); @@ -6820,6 +7062,51 @@ node_label_key = "carbide.nvidia.com/astra" } } + /// Verifies deployment selectors remain distinct from each other and NICo-owned labels. + #[test] + fn validate_dpf_deployment_node_label_keys() { + // Build the smallest two-deployment configuration needed to exercise selector overlap. + let deployments = |bf3_label: &str, bf4_label: &str| { + let bf3 = DpfDeploymentConfig { + node_label_key: bf3_label.to_string(), + ..Default::default() + }; + let bf4 = DpfDeploymentConfig { + node_label_key: bf4_label.to_string(), + ..bf4_config(None, None) + }; + DpfDeploymentsConfig { + bf3, + bf4_generic: Some(bf4), + bf4_astra: None, + } + }; + + value_scenarios!( + run = |(bf3_label, bf4_label)| deployments(bf3_label, bf4_label) + .validate_unique_identifiers() + .is_ok(); + "distinct deployment labels" { + ("carbide.nvidia.com/bf3", "carbide.nvidia.com/bf4") => true, + } + + "duplicate deployment labels" { + ("carbide.nvidia.com/dpu", "carbide.nvidia.com/dpu") => false, + } + + "shared DPF marker" { + (carbide_dpf::DPU_ENABLED_NODE_LABEL, "carbide.nvidia.com/bf4") => false, + } + + "contextual host BMC label" { + ( + "carbide.nvidia.com/bf4", + carbide_machine_controller::dpf::HOST_BMC_IP_LABEL, + ) => false, + } + ); + } + #[test] fn validate_provisioning_sources_accepts_exactly_one() { // bf3 default has bfb_url; bf4 has bluefield_software with one PSID. diff --git a/crates/api-core/src/dpf_services.rs b/crates/api-core/src/dpf_services.rs index 6350a81098..bbbcc88213 100644 --- a/crates/api-core/src/dpf_services.rs +++ b/crates/api-core/src/dpf_services.rs @@ -20,11 +20,11 @@ use std::collections::BTreeMap; use std::fmt::Write; -use carbide_dpf::sdk::build_dpu_interfaces_vec; use carbide_dpf::types::{ DHCP_SERVER_SERVICE_NAME, DOCA_HBN_SERVICE_NAME, DOCA_WEAVE_DHCP_AGENT_SERVICE_NAME, DOCA_WEAVE_FLOW_CONTROLLER_SERVICE_NAME, DOCA_XPLANE_SERVICE_NAME, DPU_AGENT_SERVICE_NAME, - DTS_SERVICE_NAME, FMDS_SERVICE_NAME, OTEL_COLLECTOR_SERVICE_NAME, + DTS_SERVICE_NAME, DpuServiceInterfaceTemplateDefinition, FMDS_SERVICE_NAME, + OTEL_COLLECTOR_SERVICE_NAME, }; use carbide_dpf::{ IntOrString, ServiceDefinition, ServiceInterface, ServiceNAD, ServiceNADResourceType, @@ -117,26 +117,41 @@ pub(crate) const COMPILE_TIME_IMAGE_TAG: &str = match option_env!("CARBIDE_BUILD None => "", }; -fn doca_hbn_service_interfaces() -> Vec { - dpu_service_interfaces(DOCA_HBN_SERVICE_NAME, DOCA_HBN_SERVICE_NETWORK) +fn doca_hbn_service_interfaces( + interfaces: &[DpuServiceInterfaceTemplateDefinition], +) -> Vec { + dpu_service_interfaces(interfaces, DOCA_HBN_SERVICE_NAME, DOCA_HBN_SERVICE_NETWORK) } -fn dhcp_server_service_interfaces() -> Vec { - dpu_service_interfaces(DHCP_SERVER_SERVICE_NAME, DHCP_SERVER_SERVICE_NAD_NAME) +fn dhcp_server_service_interfaces( + interfaces: &[DpuServiceInterfaceTemplateDefinition], +) -> Vec { + dpu_service_interfaces( + interfaces, + DHCP_SERVER_SERVICE_NAME, + DHCP_SERVER_SERVICE_NAD_NAME, + ) } -fn fmds_service_interfaces() -> Vec { - dpu_service_interfaces(FMDS_SERVICE_NAME, FMDS_SERVICE_NAD_NAME) +fn fmds_service_interfaces( + interfaces: &[DpuServiceInterfaceTemplateDefinition], +) -> Vec { + dpu_service_interfaces(interfaces, FMDS_SERVICE_NAME, FMDS_SERVICE_NAD_NAME) } -fn dpu_service_interfaces(service_name: &str, network: &str) -> Vec { - build_dpu_interfaces_vec() - .into_iter() +fn dpu_service_interfaces( + interfaces: &[DpuServiceInterfaceTemplateDefinition], + service_name: &str, + network: &str, +) -> Vec { + // Service definitions consume only endpoints declared by the shared effective inventory. + interfaces + .iter() .filter_map(|iface| { - iface.chained_svc_if.and_then(|chains| { + iface.chained_svc_if.as_ref().and_then(|chains| { chains - .into_iter() + .iter() .find_map(|(chained_service_name, interface_name)| { (chained_service_name == service_name).then(|| ServiceInterface { - name: interface_name, + name: interface_name.clone(), network: network.to_string(), }) }) @@ -339,9 +354,28 @@ fn apply_helm_values(helm_values: &mut serde_json::Value, cfg: &DpfServiceConfig } } +/// Restores the topology-derived HBN SF request after applying operator Helm overrides. +fn set_hbn_sf_count(helm_values: &mut serde_json::Value, sf_count: usize) { + let resources = helm_values + .as_object_mut() + .expect("generated Helm values must be an object") + .entry("resources") + .or_insert_with(|| serde_json::json!({})); + if !resources.is_object() { + *resources = serde_json::json!({}); + } + resources + .as_object_mut() + .expect("normalized HBN resources must be an object") + .insert("nvidia.com/bf_sf".to_string(), serde_json::json!(sf_count)); +} + /// DOCA HBN service definition. -pub(crate) fn doca_hbn_service(cfg: &DpfServiceConfig) -> ServiceDefinition { - let interfaces = doca_hbn_service_interfaces(); +pub(crate) fn doca_hbn_service( + cfg: &DpfServiceConfig, + dpu_interfaces: &[DpuServiceInterfaceTemplateDefinition], +) -> ServiceDefinition { + let interfaces = doca_hbn_service_interfaces(dpu_interfaces); let mut helm_values = serde_json::json!({ "image": { "repository": cfg.docker_repo_url, @@ -363,6 +397,9 @@ pub(crate) fn doca_hbn_service(cfg: &DpfServiceConfig) -> ServiceDefinition { } }); apply_helm_values(&mut helm_values, cfg); + // Interface assignments, startup YAML, and the requested SF population are one contract. + // An operator Helm overlay may customize other chart values but must not split that contract. + set_hbn_sf_count(&mut helm_values, interfaces.len()); ServiceDefinition { helm_values: Some(helm_values), @@ -486,7 +523,10 @@ pub(crate) fn dpu_agent_service( } /// Forge DHCP Server service definition. -pub(crate) fn dhcp_server_service(cfg: &DpfServiceConfig) -> ServiceDefinition { +pub(crate) fn dhcp_server_service( + cfg: &DpfServiceConfig, + dpu_interfaces: &[DpuServiceInterfaceTemplateDefinition], +) -> ServiceDefinition { let mut helm_values = serde_json::json!({ "image": { "repository": cfg.docker_repo_url, @@ -497,7 +537,7 @@ pub(crate) fn dhcp_server_service(cfg: &DpfServiceConfig) -> ServiceDefinition { ServiceDefinition { helm_values: Some(helm_values), - interfaces: dhcp_server_service_interfaces(), + interfaces: dhcp_server_service_interfaces(dpu_interfaces), service_daemon_set_annotations: Some(BTreeMap::new()), @@ -519,7 +559,10 @@ pub(crate) fn dhcp_server_service(cfg: &DpfServiceConfig) -> ServiceDefinition { } /// Forge FMDS service definition. -pub(crate) fn fmds_service(cfg: &DpfServiceConfig) -> ServiceDefinition { +pub(crate) fn fmds_service( + cfg: &DpfServiceConfig, + dpu_interfaces: &[DpuServiceInterfaceTemplateDefinition], +) -> ServiceDefinition { let mut helm_values = serde_json::json!({ "image": { "repository": cfg.docker_repo_url, @@ -530,7 +573,7 @@ pub(crate) fn fmds_service(cfg: &DpfServiceConfig) -> ServiceDefinition { ServiceDefinition { helm_values: Some(helm_values), - interfaces: fmds_service_interfaces(), + interfaces: fmds_service_interfaces(dpu_interfaces), service_daemon_set_annotations: Some(BTreeMap::new()), @@ -719,13 +762,14 @@ pub(crate) fn doca_xplane_service(cfg: &DpfServiceConfig) -> ServiceDefinition { pub(crate) fn mandatory_services( resolved: &DpfResolvedMandatoryServicesConfig, bootstrap_ca: &DpfDpuAgentBootstrapCa, + interfaces: &[DpuServiceInterfaceTemplateDefinition], ) -> Vec { let mut service_vec = vec![ dts_service(&resolved.base.dts), - doca_hbn_service(&resolved.base.doca_hbn), - dhcp_server_service(&resolved.base.dhcp_server), + doca_hbn_service(&resolved.base.doca_hbn, interfaces), + dhcp_server_service(&resolved.base.dhcp_server, interfaces), dpu_agent_service(&resolved.base.dpu_agent, bootstrap_ca), - fmds_service(&resolved.base.fmds), + fmds_service(&resolved.base.fmds, interfaces), otelcol_service(&resolved.base.otel), ]; @@ -746,8 +790,11 @@ pub(crate) fn mandatory_services( #[cfg(test)] mod tests { - use carbide_dpf::sdk::build_dpu_interfaces_vec; - use carbide_dpf::types::DpuServiceInterfaceTemplateType; + use carbide_dpf::sdk::{build_dpu_interfaces_vec, build_effective_dpu_interfaces}; + use carbide_dpf::types::{ + DpfInterceptBridge, DpfInterceptBridging, DpfInterfaceIdentity, + DpuServiceInterfaceTemplateType, + }; use carbide_dpf::{ build_service_configuration, build_service_interface, build_service_template, }; @@ -758,6 +805,92 @@ mod tests { const TEST_NS: &str = "dpf-operator-system"; + /// Verifies every service definition consumes the same configured effective inventory. + #[test] + fn configured_inventory_drives_hbn_dhcp_and_fmds_definitions() { + // Build a complete replacement inventory containing one PF and one VF. + let topology = DpfInterceptBridging::new( + vec![ + DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: None, + }, + "br-pf3", + "p-pf3", + ), + DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: Some(4), + }, + "br-vf4", + "p-vf4", + ), + ], + 16, + ) + .expect("configured service inventory fixture must be valid"); + let interfaces = build_effective_dpu_interfaces(16, Some(&topology)); + + // HBN receives p0, p1, the PF, and the VF; its SF count and startup YAML agree. + let hbn = doca_hbn_service(&default_doca_hbn_service(), &interfaces); + assert_eq!(hbn.interfaces.len(), 4); + assert_eq!( + hbn.helm_values.as_ref().unwrap()["resources"]["nvidia.com/bf_sf"], + 4 + ); + let startup_yaml = hbn.config_values.as_ref().unwrap()["configuration"]["startupYAMLJ2"] + .as_str() + .unwrap(); + assert!(startup_yaml.contains("pf0hpf_if:") && startup_yaml.contains("pf0vf4_if:")); + + // DHCP receives both configured entries, while FMDS receives only the PF. + let dhcp = dhcp_server_service(&default_dhcp_server_service(), &interfaces); + let fmds = fmds_service(&default_fmds_service(), &interfaces); + assert_eq!( + dhcp.interfaces + .iter() + .map(|interface| interface.name.as_str()) + .collect::>(), + ["d_pf0hpf_if", "d_pf0vf4_if"] + ); + assert_eq!( + fmds.interfaces + .iter() + .map(|interface| interface.name.as_str()) + .collect::>(), + ["f_pf0hpf_if"] + ); + } + + /// Verifies operator Helm values cannot disconnect HBN's SF request from its interfaces. + #[test] + fn hbn_sf_count_remains_topology_derived() { + // Attempt to replace the generated SF count while customizing another resource value. + let mut config = default_doca_hbn_service(); + config.extra_helm_values = serde_json::json!({ + "resources": { + "memory": "8Gi", + "nvidia.com/bf_sf": 1, + } + }) + .as_object() + .cloned(); + let interfaces = build_dpu_interfaces_vec(); + + // Ordinary resource overrides remain effective, while the SF count follows inventory. + let hbn = doca_hbn_service(&config, &interfaces); + let helm_values = hbn.helm_values.unwrap(); + assert_eq!(helm_values["resources"]["memory"], "8Gi"); + assert_eq!( + helm_values["resources"]["nvidia.com/bf_sf"], + interfaces.len() + ); + } + #[test] fn helm_value_tables_merge_recursively() { let mut values = serde_json::json!({ @@ -865,7 +998,8 @@ mod tests { #[test] fn hbn_and_dts_omit_image_pull_secrets_by_default() { // HBN and DTS pull from the public DOCA registry: no imagePullSecrets unless configured. - let hbn = doca_hbn_service(&default_doca_hbn_service()); + let interfaces = build_dpu_interfaces_vec(); + let hbn = doca_hbn_service(&default_doca_hbn_service(), &interfaces); assert!( hbn.helm_values.unwrap().get("imagePullSecrets").is_none(), "HBN must not emit imagePullSecrets without a configured secret" @@ -881,11 +1015,12 @@ mod tests { #[test] fn hbn_and_dts_emit_image_pull_secrets_when_configured() { let expected = serde_json::json!([{ "name": "private-pull-secret" }]); + let interfaces = build_dpu_interfaces_vec(); let mut hbn_cfg = default_doca_hbn_service(); hbn_cfg.docker_image_pull_secret = Some("private-pull-secret".to_string()); assert_eq!( - doca_hbn_service(&hbn_cfg).helm_values.unwrap()["imagePullSecrets"], + doca_hbn_service(&hbn_cfg, &interfaces).helm_values.unwrap()["imagePullSecrets"], expected ); @@ -1087,7 +1222,9 @@ mod tests { #[test] fn test_dpu_service_interfaces_hbn_uses_correct_network() { - let ifaces = dpu_service_interfaces(DOCA_HBN_SERVICE_NAME, DOCA_HBN_SERVICE_NETWORK); + let interfaces = build_dpu_interfaces_vec(); + let ifaces = + dpu_service_interfaces(&interfaces, DOCA_HBN_SERVICE_NAME, DOCA_HBN_SERVICE_NETWORK); assert!(!ifaces.is_empty(), "HBN should have at least one interface"); for iface in &ifaces { assert_eq!( @@ -1100,7 +1237,12 @@ mod tests { #[test] fn test_dpu_service_interfaces_dhcp_uses_correct_network() { - let ifaces = dpu_service_interfaces(DHCP_SERVER_SERVICE_NAME, DHCP_SERVER_SERVICE_NAD_NAME); + let interfaces = build_dpu_interfaces_vec(); + let ifaces = dpu_service_interfaces( + &interfaces, + DHCP_SERVER_SERVICE_NAME, + DHCP_SERVER_SERVICE_NAD_NAME, + ); assert!( !ifaces.is_empty(), "DHCP server should have at least one interface" @@ -1118,9 +1260,13 @@ mod tests { fn test_dpu_service_interfaces_derived_from_build_dpu_interfaces_vec() { // Every interface returned for HBN must originate from build_dpu_interfaces_vec. let all_ifaces = build_dpu_interfaces_vec(); - let hbn_ifaces = dpu_service_interfaces(DOCA_HBN_SERVICE_NAME, DOCA_HBN_SERVICE_NETWORK); - let dhcp_ifaces = - dpu_service_interfaces(DHCP_SERVER_SERVICE_NAME, DHCP_SERVER_SERVICE_NAD_NAME); + let hbn_ifaces = + dpu_service_interfaces(&all_ifaces, DOCA_HBN_SERVICE_NAME, DOCA_HBN_SERVICE_NETWORK); + let dhcp_ifaces = dpu_service_interfaces( + &all_ifaces, + DHCP_SERVER_SERVICE_NAME, + DHCP_SERVER_SERVICE_NAD_NAME, + ); let all_chained_names: Vec = all_ifaces .iter() @@ -1145,7 +1291,7 @@ mod tests { .find(|i| i.name == "p0") .expect("p0 must exist"); assert!(matches!( - p0.iface_type, + &p0.iface_type, DpuServiceInterfaceTemplateType::Physical )); let cr = build_service_interface(p0, TEST_NS); @@ -1168,7 +1314,7 @@ mod tests { .find(|i| i.name == "pf0hpf") .expect("pf0hpf must exist"); assert!(matches!( - pf0hpf.iface_type, + &pf0hpf.iface_type, DpuServiceInterfaceTemplateType::Pf )); let cr = build_service_interface(pf0hpf, TEST_NS); @@ -1177,6 +1323,12 @@ mod tests { template_spec.pf.is_some(), "pf spec must be set for Pf type" ); + let pf = template_spec + .pf + .as_ref() + .expect("pf spec must remain available for selector validation"); + // The public builder is the legacy unscoped path and must not reconcile a new selector. + assert!(pf.nic_selector.is_none()); assert!(template_spec.physical.is_none()); assert!(template_spec.vf.is_none()); } @@ -1189,7 +1341,7 @@ mod tests { .find(|i| i.name == "pf0vf0") .expect("pf0vf0 must exist"); assert!(matches!( - pf0vf0.iface_type, + &pf0vf0.iface_type, DpuServiceInterfaceTemplateType::Vf )); let cr = build_service_interface(pf0vf0, TEST_NS); @@ -1202,6 +1354,8 @@ mod tests { assert_eq!(vf.pf_id, 0); assert_eq!(vf.vf_id, 0); assert_eq!(vf.parent_interface_ref.as_deref(), Some("p0")); + // The public builder is the legacy unscoped path and must not reconcile a new selector. + assert!(vf.nic_selector.is_none()); assert!(template_spec.physical.is_none()); assert!(template_spec.pf.is_none()); } diff --git a/crates/api-core/src/handlers/instance.rs b/crates/api-core/src/handlers/instance.rs index 4afc233b80..c55bb81466 100644 --- a/crates/api-core/src/handlers/instance.rs +++ b/crates/api-core/src/handlers/instance.rs @@ -59,12 +59,13 @@ use sqlx::PgConnection; use tonic::{Request, Response, Status}; use crate::api::{Api, log_machine_id, log_request_data, log_tenant_organization_id}; -use crate::cfg::file::FnnConfig; +use crate::cfg::file::CarbideConfig; use crate::ethernet_virtualization::validate_instance_interface_routing_profiles; use crate::handlers::utils::convert_and_log_machine_id; use crate::instance::{ InstanceAllocationRequest, allocate_ib_port_guid, allocate_instance, allocate_network, - allocate_spx_port_mac, validate_ib_partition_ownership, validate_os_definition_usable, + allocate_spx_port_mac, validate_ib_partition_ownership, + validate_instance_vfs_against_dpf_topology, validate_os_definition_usable, validate_spx_partition_ownership, }; use crate::{CarbideError, CarbideResult}; @@ -1377,12 +1378,7 @@ pub(crate) async fn update_instance_config( } update_instance_network_config( - api.runtime_config - .vmaas_config - .as_ref() - .map(|vc| vc.allow_instance_vf) - .unwrap_or(true), - api.runtime_config.fnn.as_ref(), + &api.runtime_config, &instance, &mut config.network, &mh_snapshot, @@ -1445,8 +1441,7 @@ pub(crate) async fn update_instance_config( /// indicate the state machine to start updating network on DPUs. This function also increments /// network_config_version. async fn update_instance_network_config( - allow_instance_vf: bool, - fnn_config: Option<&FnnConfig>, + runtime_config: &CarbideConfig, instance: &InstanceSnapshot, network: &mut InstanceNetworkConfig, mh_snapshot: &ManagedHostStateSnapshot, @@ -1544,9 +1539,16 @@ async fn update_instance_network_config( // Resolve prefix-backed network resources before validating the generated segment IDs. allocate_network(network, &instance.config.tenant.tenant_organization_id, txn).await?; network - .validate(allow_instance_vf) + .validate( + runtime_config + .vmaas_config + .as_ref() + .map(|config| config.allow_instance_vf) + .unwrap_or(true), + ) .map_err(CarbideError::from)?; - validate_instance_interface_routing_profiles(txn, network, fnn_config).await?; + validate_instance_vfs_against_dpf_topology(network, runtime_config)?; + validate_instance_interface_routing_profiles(txn, network, runtime_config.fnn.as_ref()).await?; // Allocate IPs and add them to the network config let updated_network_config = db::instance_network_config::with_allocated_ips( diff --git a/crates/api-core/src/instance/mod.rs b/crates/api-core/src/instance/mod.rs index a8f3052b04..4c0f2d183f 100644 --- a/crates/api-core/src/instance/mod.rs +++ b/crates/api-core/src/instance/mod.rs @@ -61,7 +61,7 @@ use model::vpc_prefix::VpcPrefix; use sqlx::PgConnection; use crate::api::Api; -use crate::cfg::file::ComputeAllocationEnforcement; +use crate::cfg::file::{CarbideConfig, ComputeAllocationEnforcement}; use crate::ethernet_virtualization::validate_instance_interface_routing_profiles; use crate::network_segment::allocate::PrefixAllocator; @@ -132,6 +132,65 @@ async fn validate_zero_dpu_auto_vpc( Ok(vpc) } +/// Rejects instance VFs that DPF did not materialize from the configured replacement topology. +/// +/// The static no-topology path deliberately retains its historical admission behavior. DPF +/// startup owns normalization of the raw topology, so this request-time gate only projects its +/// already-validated VF identities from the immutable runtime configuration. +pub(crate) fn validate_instance_vfs_against_dpf_topology( + network: &InstanceNetworkConfig, + config: &CarbideConfig, +) -> CarbideResult<()> { + validate_vf_ids_against_dpf_topology( + network + .interfaces + .iter() + .filter_map(|interface| match &interface.function_id { + InterfaceFunctionId::Physical {} => None, + InterfaceFunctionId::Virtual { id } => Some(*id), + }), + config, + ) +} + +/// Applies exact topology membership to an already structurally validated VF sequence. +fn validate_vf_ids_against_dpf_topology( + vf_ids: impl IntoIterator, + config: &CarbideConfig, +) -> CarbideResult<()> { + let Some(selected_vfs) = dpf_topology_vf_ids(config) else { + return Ok(()); + }; + + if let Some(unselected_vf) = vf_ids + .into_iter() + .find(|vf_id| !selected_vfs.contains(vf_id)) + { + return Err(ConfigValidationError::InvalidValue(format!( + "virtual function VF{unselected_vf} is not selected by the configured DPF intercept-bridging topology" + )) + .into()); + } + + Ok(()) +} + +/// Returns the exact topology VF population, or `None` when legacy admission remains in effect. +fn dpf_topology_vf_ids(config: &CarbideConfig) -> Option> { + if !config.dpf.enabled { + return None; + } + + let topology = config.vmaas_config.as_ref()?.bridging.as_ref()?; + Some( + topology + .host_representor_intercept_bridging + .values() + .filter_map(|interface| interface.dpf_interface?.vf_id) + .collect(), + ) +} + /// Validates that an operating system definition referenced by ID exists, is active, /// and has status READY. Returns `Ok(())` when the OS variant is not /// `OperatingSystemId` (inline iPXE / OS image variants need no lookup). @@ -1768,6 +1827,7 @@ pub(crate) async fn batch_allocate_instances( .map(|vc| vc.allow_instance_vf) .unwrap_or(true), )?; + validate_instance_vfs_against_dpf_topology(&request.config.network, &api.runtime_config)?; validate_instance_interface_routing_profiles( &mut txn, &request.config.network, @@ -2314,7 +2374,7 @@ pub(crate) fn allocate_spx_port_mac( #[cfg(test)] mod tests { use carbide_test_support::Outcome::*; - use carbide_test_support::{Case, check_cases}; + use carbide_test_support::{Case, check_cases, value_scenarios}; use super::*; @@ -2353,6 +2413,74 @@ mod tests { ); } + /// Verifies instance admission uses exact topology membership without changing legacy mode. + #[test] + fn instance_vf_admission_follows_dpf_topology() { + #[derive(Clone, Copy)] + enum InventoryMode { + Topology(&'static [u8]), + Static, + DpfDisabled(&'static [u8]), + } + + value_scenarios!( + run = |(mode, requested_vfs)| { + let config = match mode { + InventoryMode::Topology(vf_ids) => { + crate::test_support::default_config::with_dpf_intercept_topology(vf_ids) + } + InventoryMode::Static => { + let mut config = crate::test_support::default_config::get(); + config.dpf.enabled = true; + config.vmaas_config = None; + config + } + InventoryMode::DpfDisabled(vf_ids) => { + let mut config = + crate::test_support::default_config::with_dpf_intercept_topology(vf_ids); + config.dpf.enabled = false; + config + } + }; + validate_vf_ids_against_dpf_topology(requested_vfs, &config).is_ok() + }; + "selected sparse VF" { + // An explicitly selected sparse VF is addressable. + (InventoryMode::Topology(&[7]), vec![7]) => true, + } + + "unselected sparse VF" { + // VF0 must not pass merely because the hardware provisions it. + (InventoryMode::Topology(&[7]), vec![0]) => false, + } + + "all requested VFs selected" { + // Multiple requested VFs must each belong to the replacement inventory. + (InventoryMode::Topology(&[4, 7]), vec![4, 7]) => true, + } + + "one requested VF omitted" { + // One unselected VF rejects the complete network configuration. + (InventoryMode::Topology(&[4, 7]), vec![4, 6]) => false, + } + + "PF-only topology" { + // A valid PF-only topology deliberately exposes no instance VFs. + (InventoryMode::Topology(&[]), vec![0]) => false, + } + + "static compatibility mode" { + // DPF without a replacement topology retains the historical admission behavior. + (InventoryMode::Static, vec![0]) => true, + } + + "DPF disabled" { + // Legacy non-DPF VMaaS configuration remains outside this DPF admission gate. + (InventoryMode::DpfDisabled(&[7]), vec![0]) => true, + } + ); + } + #[test] fn pending_boot_configuration_has_a_safe_allocation_error() { let machine_id = "fm100htes3rn1npvbtm5qd57dkilaag7ljugl1llmm7rfuq1ov50i0rpl30" diff --git a/crates/api-core/src/setup.rs b/crates/api-core/src/setup.rs index 1e2c9fd1ea..b558c9a684 100644 --- a/crates/api-core/src/setup.rs +++ b/crates/api-core/src/setup.rs @@ -73,6 +73,7 @@ use db::resource_pool::DefineResourcePoolError; use db::work_lock_manager::WorkLockManagerHandle; use eyre::WrapErr; use futures_util::TryFutureExt; +use itertools::Itertools; use librms::RackManagerClientPool; use model::attestation::spdm::VerifierImpl; use model::expected_machine::ExpectedMachine; @@ -93,7 +94,7 @@ use tokio_util::sync::CancellationToken; use crate::api::Api; use crate::api::metrics::ApiMetricsEmitter; -use crate::cfg::file::{CarbideConfig, InitialObjectsConfig, ListenMode}; +use crate::cfg::file::{CarbideConfig, InitialObjectsConfig, ListenMode, VmaasConfig}; use crate::cfg::load::all_configuration_files; use crate::dpa::handler::start_dpa_handler; use crate::dynamic_settings::DynamicSettings; @@ -204,14 +205,6 @@ pub(crate) async fn start_runtime( admin_ui_routes_builder: Option, cancel_token: CancellationToken, ) -> eyre::Result { - eyre::ensure!( - !matches!( - (carbide_config.dpf.enabled, &carbide_config.vmaas_config), - (true, Some(_)) - ), - "cannot enable both VMaaS and DPF; disable one in the configuration" - ); - let shared_redfish_pool = create_redfish_pool(&carbide_config, credential_manager.clone())?; let shared_nv_redfish_pool = carbide_redfish::nv_redfish::new_pool(carbide_config.site_explorer.bmc_proxy.clone()); @@ -528,6 +521,52 @@ pub(crate) async fn start_runtime( Ok(listen_address) } +/// Normalizes and validates DPF-only intercept-bridging topology without retaining legacy map keys. +fn normalize_dpf_intercept_bridging( + config: Option<&VmaasConfig>, + num_of_vfs: u32, +) -> eyre::Result> { + // Only complete VMaaS absence selects static inventory; present invalid maps fail below. + let Some(config) = config else { + return Ok(None); + }; + // Sort legacy keys only to make any per-entry validation error deterministic. + let entries = config + .bridging + .as_ref() + .map(|bridging| &bridging.host_representor_intercept_bridging); + let interfaces = entries + .into_iter() + .flatten() + .sorted_by(|(left, _), (right, _)| left.cmp(right)) + .map(|(legacy_key, interface)| { + eyre::ensure!( + !interface.skip_create, + "DPF intercept-bridging interface {legacy_key:?} cannot use skip_create=true" + ); + let identity = interface.dpf_interface.ok_or_else(|| { + eyre::eyre!( + "DPF intercept-bridging interface {legacy_key:?} is missing dpf_interface" + ) + })?; + Ok(carbide_dpf::DpfInterceptBridge::new( + carbide_dpf::DpfInterfaceIdentity { + controller_id: identity.controller_id, + pf_id: identity.pf_id, + vf_id: identity.vf_id, + }, + &interface.bridge, + &interface.patch_port, + )) + }) + .collect::>>()?; + + // DPF-local validation owns typed identity and all rendered-name constraints. + carbide_dpf::DpfInterceptBridging::new(interfaces, num_of_vfs) + .map(Some) + .map_err(|error| eyre::eyre!("invalid DPF intercept-bridging configuration: {error}")) +} + /// Initialize the DPF SDK and create all required Kubernetes CRs. /// /// Returns `None` (with a deprecation warning) when DPF is disabled. @@ -537,6 +576,11 @@ async fn initialize_dpf_sdk( db_pool: PgPool, join_set: &mut JoinSet<()>, ) -> eyre::Result>> { + // Astra is a BF4+CX9-only deployment with a distinct interface inventory. + // Reject unsafe global ServiceInterfaces even when DPF is disabled so a dormant Astra + // configuration cannot become unsafe merely by enabling DPF later. + carbide_config.dpf.validate_service_interface_scoping()?; + if !carbide_config.dpf.enabled { tracing::warn!( removed_in = "v2.1", @@ -561,6 +605,27 @@ async fn initialize_dpf_sdk( .validate() .map_err(|err| eyre::eyre!("invalid DPF bootstrap CA configuration: {err}"))?; + // Validate the complete site topology before constructing a repository or writing any CR. + let intercept_bridging = normalize_dpf_intercept_bridging( + carbide_config.vmaas_config.as_ref(), + carbide_config.dpu_config.num_of_vfs, + )?; + let effective_interfaces = carbide_dpf::build_effective_dpu_interfaces( + carbide_config.dpu_config.num_of_vfs, + intercept_bridging.as_ref(), + ); + + // SDK construction writes the shared BMC Secret, so capacity validation must remain on the + // pure configuration path and finish before Kubernetes repository construction. + carbide_dpf::calculate_pf_total_sf( + &effective_interfaces, + intercept_bridging.as_ref(), + carbide_config.dpf.pf_total_sf_reserved, + ) + .map_err(|error| eyre::eyre!("invalid DPF SF configuration: {error}"))?; + + let astra_interfaces = carbide_dpf::sdk::build_dpu_interfaces_vec(); + let repo = carbide_dpf::KubeRepository::new() .await .map_err(|e| eyre::eyre!("failed to create DPF repository: {e}"))?; @@ -602,15 +667,32 @@ async fn initialize_dpf_sdk( deployment_type: DpuDeploymentType, bluefield_software: Option| { let services = carbide_config.dpf.resolved_services_for(deployment); + let interfaces = match deployment_type { + DpuDeploymentType::Bf4Astra => &astra_interfaces, + DpuDeploymentType::Bf3 | DpuDeploymentType::Bf4Generic => &effective_interfaces, + }; carbide_dpf::InitDpfResourcesConfig { bfb_url: deployment.bfb_url.clone().unwrap_or_default(), bluefield_software, flavor_name: deployment.flavor_name.clone(), deployment_name: deployment.deployment_name.clone(), + deployment_scoped_service_interfaces: carbide_config + .dpf + .deployment_scoped_service_interfaces, services: crate::dpf_services::mandatory_services( &services, &carbide_config.dpf.dpu_agent_bootstrap_ca, + interfaces, ), + num_of_vfs: carbide_config.dpu_config.num_of_vfs, + pf_total_sf_reserved: carbide_config.dpf.pf_total_sf_reserved, + intercept_bridging: match deployment_type { + DpuDeploymentType::Bf4Astra => None, + DpuDeploymentType::Bf3 | DpuDeploymentType::Bf4Generic => { + intercept_bridging.clone() + } + }, + interfaces: interfaces.clone(), proxy: carbide_config.dpf.proxy.clone(), deployment_type, } @@ -684,7 +766,7 @@ fn build_deployment_type_labels( let make_labels = |key: &str| { std::collections::BTreeMap::from([ ( - "feature.node.kubernetes.io/dpu-enabled".to_string(), + carbide_dpf::DPU_ENABLED_NODE_LABEL.to_string(), "true".to_string(), ), (key.to_string(), "true".to_string()), @@ -1762,9 +1844,141 @@ mod tests { use model::resource_pool::define::ResourcePoolDef; use super::*; - use crate::cfg::file::{CarbideConfig, InitialObjectsConfig}; + use crate::cfg::file::{ + CarbideConfig, DpfInterfaceIdentity, HostInterceptBridging, HostRepresentorBridgingConfig, + InitialObjectsConfig, VmaasConfig, default_hbn_bridge, + }; use crate::cfg::load::{merged_carbide_config_figment, parse_carbide_config}; + /// Provides one intercept-bridging config entry for DPF normalization tests. + fn test_intercept_config(interface: HostInterceptBridging) -> VmaasConfig { + VmaasConfig { + allow_instance_vf: true, + hbn_reps: Some("legacy-only-value".to_string()), + bridging: Some(HostRepresentorBridgingConfig { + hbn_bridge: default_hbn_bridge(), + host_representor_intercept_bridging: HashMap::from([( + "legacy-map-key".to_string(), + interface, + )]), + }), + } + } + + /// Provides one typed PF or VF identity for DPF normalization tests. + fn test_dpf_identity(vf_id: Option) -> DpfInterfaceIdentity { + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id, + } + } + + /// Verifies static inventory remains available without VMaaS configuration while every + /// configured DPF replacement topology contains the PF required by single-interface FMDS. + #[test] + fn dpf_intercept_bridging_normalization_requires_configured_pf() { + // Absence retains the SDK's static PF/VF inventory mode. + assert!( + normalize_dpf_intercept_bridging(None, 16) + .unwrap() + .is_none() + ); + + value_scenarios!( + run = |config| normalize_dpf_intercept_bridging(Some(&config), 16).is_err(); + "missing bridging block" { + // A configured DPF replacement inventory must expose the PF used by FMDS. + VmaasConfig { + allow_instance_vf: false, + hbn_reps: None, + bridging: None, + } => true, + } + + "empty bridging map" { + // Fixed physical uplinks cannot replace the required host PF FMDS endpoint. + VmaasConfig { + allow_instance_vf: false, + hbn_reps: None, + bridging: Some(HostRepresentorBridgingConfig { + hbn_bridge: default_hbn_bridge(), + host_representor_intercept_bridging: HashMap::new(), + }), + } => true, + } + + "VF-only bridging map" { + // VFs share the selected parent but cannot supply FMDS's PF interface. + test_intercept_config(HostInterceptBridging { + bridge: "br-vf3".to_string(), + patch_port: "p-vf3".to_string(), + skip_create: false, + dpf_interface: Some(test_dpf_identity(Some(3))), + }) => true, + } + ); + } + + /// Verifies DPF normalization uses typed identity and ignores legacy-only values. + #[test] + fn dpf_intercept_bridging_normalization_ignores_legacy_identity_and_hbn_reps() { + // Build two configs differing only in legacy map key and HBN selection. + let interface = HostInterceptBridging { + bridge: "br-pf3".to_string(), + patch_port: "p-pf3".to_string(), + skip_create: false, + dpf_interface: Some(test_dpf_identity(None)), + }; + let first = test_intercept_config(interface.clone()); + let mut second = test_intercept_config(interface); + second.hbn_reps = Some("different-legacy-value".to_string()); + let bridging = second.bridging.as_mut().unwrap(); + let entry = bridging + .host_representor_intercept_bridging + .remove("legacy-map-key") + .unwrap(); + bridging + .host_representor_intercept_bridging + .insert("unrelated-key".to_string(), entry); + + // Only typed identity and normalized topology values may affect DPF output. + assert_eq!( + normalize_dpf_intercept_bridging(Some(&first), 16).unwrap(), + normalize_dpf_intercept_bridging(Some(&second), 16).unwrap() + ); + } + + /// Verifies incomplete or skipped entries are rejected only at the DPF boundary. + #[test] + fn dpf_intercept_bridging_normalization_rejects_missing_identity_and_skip_create() { + value_scenarios!( + run = |interface| normalize_dpf_intercept_bridging( + Some(&test_intercept_config(interface)), + 16, + ).is_err(); + "missing typed identity" { + // Legacy-only entries cannot select a DPF PF or VF safely. + HostInterceptBridging { + bridge: "br-host".to_string(), + patch_port: "p-host".to_string(), + skip_create: false, + dpf_interface: None, + } => true, + } + + "skipped entry" { + // Startup topology is declarative under DPF, so skipped entries are unsupported. + HostInterceptBridging { + bridge: "br-host".to_string(), + patch_port: "p-host".to_string(), + skip_create: true, + dpf_interface: Some(test_dpf_identity(None)), + } => true, + } + ); + } + #[test] fn firmware_object_redirects_require_same_origin() { value_scenarios!(run = |(initial, redirect)| { diff --git a/crates/api-core/src/test_support/default_config.rs b/crates/api-core/src/test_support/default_config.rs index 86f38b5bdf..f1042a291b 100644 --- a/crates/api-core/src/test_support/default_config.rs +++ b/crates/api-core/src/test_support/default_config.rs @@ -51,6 +51,55 @@ use crate::cfg::file::{ default_database_pool_max_lifetime, default_max_find_by_ids, default_max_site_prefixes_per_tenant, default_pxe_public_base_url, }; +#[cfg(test)] +use crate::cfg::file::{ + DpfInterfaceIdentity, HostInterceptBridging, HostRepresentorBridgingConfig, +}; + +/// Returns the default configuration with one selected PF and the requested sparse DPF VFs. +#[cfg(test)] +pub(crate) fn with_dpf_intercept_topology(selected_vfs: &[u8]) -> CarbideConfig { + let mut config = get(); + config.dpf.enabled = true; + + let mut interfaces = HashMap::from([( + "selected-pf".to_string(), + HostInterceptBridging { + bridge: "br-pf".to_string(), + patch_port: "p-pf".to_string(), + skip_create: false, + dpf_interface: Some(DpfInterfaceIdentity { + controller_id: 1, + pf_id: 0, + vf_id: None, + }), + }, + )]); + interfaces.extend(selected_vfs.iter().map(|vf_id| { + ( + format!("selected-vf{vf_id}"), + HostInterceptBridging { + bridge: format!("br-vf{vf_id}"), + patch_port: format!("p-vf{vf_id}"), + skip_create: false, + dpf_interface: Some(DpfInterfaceIdentity { + controller_id: 1, + pf_id: 0, + vf_id: Some(*vf_id), + }), + }, + ) + })); + config.vmaas_config = Some(VmaasConfig { + allow_instance_vf: true, + hbn_reps: None, + bridging: Some(HostRepresentorBridgingConfig { + hbn_bridge: "br-hbn".to_string(), + host_representor_intercept_bridging: interfaces, + }), + }); + config +} /// [`get`] with every `Option` config section populated. Used by tests that /// walk the *serialized* config shape — e.g. the admin-UI documentation diff --git a/crates/api-core/src/tests/instance.rs b/crates/api-core/src/tests/instance.rs index 6162dfdc1d..aaf4c906cc 100644 --- a/crates/api-core/src/tests/instance.rs +++ b/crates/api-core/src/tests/instance.rs @@ -7261,6 +7261,190 @@ async fn test_instance_without_vf_when_vf_disabled(_: PgPoolOptions, options: Pg assert!(instance_result.is_ok()); } +/// Verifies sparse DPF topology membership gates both allocation and network replacement. +#[crate::sqlx_test] +async fn test_dpf_topology_rejects_unselected_vf_on_create_and_update( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = PgPoolOptions::new().connect_with(options).await.unwrap(); + let config = crate::test_support::default_config::with_dpf_intercept_topology(&[7]); + let env = create_test_env_with_overrides(pool, TestEnvOverrides::with_config(config)).await; + let managed_host = create_managed_host(&env).await; + let segment_ids = env.create_vpc_and_tenant_segments(2).await; + + // VF0 is structurally valid and globally enabled, but the replacement topology selects VF7. + let mut network_with_unselected_vf = + single_interface_network_config_with_vfs(segment_ids.clone()); + network_with_unselected_vf.interfaces[1].virtual_function_id = Some(0); + let create_error = env + .api + .allocate_instance( + InstanceAllocationRequest::builder(false) + .machine_id(managed_host.id) + .config( + InstanceConfig::default_tenant_and_os() + .network(network_with_unselected_vf.clone()), + ) + .tonic_request(), + ) + .await + .expect_err("an unselected topology VF must not be allocated"); + assert!(create_error.message().contains( + "virtual function VF0 is not selected by the configured DPF intercept-bridging topology" + )); + + // A PF-only instance remains valid; once ready, adding the same unselected VF must fail at + // the network-update gate and leave no staged replacement request. + let instance = managed_host + .instance_builer(&env) + .single_interface_network_config(segment_ids[0]) + .build() + .await; + let update_error = env + .api + .update_instance_config(tonic::Request::new( + rpc::forge::InstanceConfigUpdateRequest { + instance_id: instance.rpc_instance().await.rpc_id(), + if_version_match: None, + config: Some(rpc::InstanceConfig { + tenant: Some(default_tenant_config()), + os: Some(default_os_config()), + network: Some(network_with_unselected_vf), + infiniband: None, + network_security_group_id: None, + dpu_extension_services: None, + nvlink: None, + spxconfig: None, + }), + metadata: Some(rpc::forge::Metadata { + name: "topology-update".to_string(), + description: String::new(), + labels: vec![], + }), + }, + )) + .await + .expect_err("an unselected topology VF must not be staged"); + assert!(update_error.message().contains( + "virtual function VF0 is not selected by the configured DPF intercept-bridging topology" + )); + + let mut txn = env.db_txn().await; + assert!( + instance + .db_instance(&mut txn) + .await + .update_network_config_request + .is_none() + ); + txn.rollback().await.unwrap(); +} + +/// Verifies raw protobuf VF identities cannot alias selected topology VFs during conversion. +#[crate::sqlx_test] +async fn test_public_instance_endpoints_reject_out_of_range_wire_vfs( + _: PgPoolOptions, + options: PgConnectOptions, +) { + let pool = PgPoolOptions::new().connect_with(options).await.unwrap(); + // Include the truncation targets for 256 (VF0) and 263 (VF7). Without wire-range validation, + // both malformed values would pass exact topology membership after a lossy u32-to-u8 cast. + let config = crate::test_support::default_config::with_dpf_intercept_topology(&[0, 7, 15]); + let env = create_test_env_with_overrides(pool, TestEnvOverrides::with_config(config)).await; + let create_host = create_managed_host(&env).await; + let update_host = create_managed_host(&env).await; + let segment_ids = env.create_vpc_and_tenant_segments(2).await; + let update_instance = update_host + .instance_builer(&env) + .single_interface_network_config(segment_ids[0]) + .build() + .await; + let update_instance_id = update_instance.rpc_instance().await.rpc_id(); + + // Build the protobuf directly so values wider than the model's u8 cannot be normalized by + // test helpers before they reach the public handlers. + let config_with_wire_vf = |wire_vf_id| { + let mut network = single_interface_network_config_with_vfs(segment_ids.clone()); + network.interfaces[1].virtual_function_id = Some(wire_vf_id); + rpc::InstanceConfig { + tenant: Some(default_tenant_config()), + os: Some(default_os_config()), + network: Some(network), + infiniband: None, + network_security_group_id: None, + dpu_extension_services: None, + nvlink: None, + spxconfig: None, + } + }; + let metadata = |operation: &str, wire_vf_id| rpc::forge::Metadata { + name: format!("{operation}-vf{wire_vf_id}"), + description: String::new(), + labels: vec![], + }; + + for wire_vf_id in [16, 256, 263] { + let create_error = env + .api + .allocate_instance(Request::new(rpc::forge::InstanceAllocationRequest { + machine_id: Some(create_host.id), + config: Some(config_with_wire_vf(wire_vf_id)), + instance_id: None, + instance_type_id: None, + metadata: Some(metadata("create", wire_vf_id)), + allow_unhealthy_machine: false, + })) + .await + .expect_err("out-of-range wire VF must be rejected during instance creation"); + assert_eq!(create_error.code(), tonic::Code::InvalidArgument); + assert!( + create_error + .message() + .contains(&format!("virtual function ID of value {wire_vf_id}")) + ); + + let update_error = env + .api + .update_instance_config(Request::new(rpc::forge::InstanceConfigUpdateRequest { + instance_id: update_instance_id, + if_version_match: None, + config: Some(config_with_wire_vf(wire_vf_id)), + metadata: Some(metadata("update", wire_vf_id)), + })) + .await + .expect_err("out-of-range wire VF must be rejected during instance update"); + assert_eq!(update_error.code(), tonic::Code::InvalidArgument); + assert!( + update_error + .message() + .contains(&format!("virtual function ID of value {wire_vf_id}")) + ); + } + + // VF15 is the inclusive boundary and must pass the same public create and update paths. + env.api + .allocate_instance(Request::new(rpc::forge::InstanceAllocationRequest { + machine_id: Some(create_host.id), + config: Some(config_with_wire_vf(15)), + instance_id: None, + instance_type_id: None, + metadata: Some(metadata("create", 15)), + allow_unhealthy_machine: false, + })) + .await + .expect("VF15 must remain valid during instance creation"); + env.api + .update_instance_config(Request::new(rpc::forge::InstanceConfigUpdateRequest { + instance_id: update_instance_id, + if_version_match: None, + config: Some(config_with_wire_vf(15)), + metadata: Some(metadata("update", 15)), + })) + .await + .expect("VF15 must remain valid during instance update"); +} + fn create_dpu_extension_service_data(name: &str) -> String { format!( "apiVersion: v1\nkind: Pod\nmetadata:\n name: {}\nspec:\n containers:\n - name: app\n image: nginx:1.27", diff --git a/crates/dpf/README.md b/crates/dpf/README.md index 68e1cfe334..9b4ea4b6b5 100644 --- a/crates/dpf/README.md +++ b/crates/dpf/README.md @@ -16,7 +16,9 @@ User-facing CRDs include: **BFB** (BlueField Boot image), **DPUFlavor** (hardwar This SDK provides a Rust interface for NICo to interact with the DPF operator. It maps to the four provisioning user flows from the [Component Description](https://docs.nvidia.com/networking/display/dpf25101/component-description). Details: `design-docs/DPF NICo SDK - DPF background.md`. 1. **Provision a DPU** (Component Description steps 2-5 + manual discovery + external node effect and reboot): - - Steps 2-5: `create_initialization_objects` creates BFB, DPUFlavor, and DPUDeployment (with `dpu_sets` referencing BFB and DPUFlavor). + - Steps 2-5: `create_initialization_objects` creates a BFB or + BlueFieldSoftware provisioning source, a DPUFlavor, and a DPUDeployment + referencing that source and flavor. - Manual discovery: `register_dpu_device` and `register_dpu_node` register DPUDevice/DPUNode CRDs (NICo uses manual registration instead of NFD auto-discovery). - Monitor flow (per design): watcher fires **MaintenanceNeeded** (DPU in NodeEffect) -> NICo calls `release_maintenance_hold`; then **RebootRequired** -> NICo reboots the host and calls `reboot_complete`; then **Ready**. diff --git a/crates/dpf/src/flavor.rs b/crates/dpf/src/flavor.rs index 24bd07e462..11382a7ec6 100644 --- a/crates/dpf/src/flavor.rs +++ b/crates/dpf/src/flavor.rs @@ -17,6 +17,9 @@ //! DPUFlavor configuration for HBN. +use std::collections::BTreeSet; +use std::fmt::Write; + use kube::core::ObjectMeta; use sha2::{Digest, Sha256}; @@ -27,10 +30,17 @@ use crate::crds::dpuflavors_generated::{ DpuFlavorEwNicConfigurationsSpectrumXOptimizedMultiplaneMode, DpuFlavorEwNicConfigurationsSpectrumXOptimizedOverlay, DpuFlavorGrub, DpuFlavorNvconfig, DpuFlavorNvconfigDevice, DpuFlavorOvs, DpuFlavorSpec, DpuFlavorSysctl, + DpuFlavorSystemdServices, DpuFlavorSystemdServicesOperation, +}; +use crate::types::{ + DEFAULT_DPU_NUM_OF_VFS, DEFAULT_PF_TOTAL_SF_RESERVED, DOCA_HBN_SERVICE_NAME, + DpfInterceptBridge, DpfInterceptBridging, DpfProxyDetails, DpuDeploymentType, + DpuServiceInterfaceTemplateDefinition, DpuServiceInterfaceTemplateType, }; -use crate::types::{DpfProxyDetails, DpuDeploymentType}; pub const DEFAULT_FLAVOR_NAME: &str = "dpu-flavor"; +const OVN_ENCAP_SERVICE_NAME: &str = "nico-ovn-encap-ip.service"; +const OVN_ENCAP_SCRIPT_PATH: &str = "/usr/local/sbin/nico-configure-ovn-encap-ip"; impl DPUFlavor { /// Returns `"{default_flavor_name}-{hash}"` where the hash is the first 8 bytes (16 hex chars) @@ -43,7 +53,7 @@ impl DPUFlavor { } } -fn get_default_ovs_defaults() -> String { +fn get_default_ovs_defaults_base() -> String { concat!( "_ovs-vsctl() {\n", "ovs-vsctl --timeout 15 \"$@\"\n", @@ -82,7 +92,7 @@ fn get_default_ovs_defaults() -> String { } /// OVS raw config script for the BF4 flavor. -fn get_bf4_ovs_defaults() -> String { +fn get_bf4_ovs_defaults_base() -> String { concat!( "_ovs-vsctl() {\n", " ovs-vsctl --timeout 15 \"$@\"\n", @@ -128,6 +138,156 @@ fn get_bf4_ovs_defaults() -> String { .to_string() } +/// Builds the BF3 OVS bootstrap with deterministic configured peer bridges. +fn get_default_ovs_defaults_with_topology(topology: Option<&DpfInterceptBridging>) -> String { + // Retain the BF3 base verbatim, then append normalized intercept-bridge state. + let mut script = get_default_ovs_defaults_base(); + if let Some(topology) = topology { + append_peer_bridge_bootstrap(&mut script, topology, |interface| { + format!("'{}'", interface.identity.bf3_raw_netdev_name()) + }); + } + append_ovn_encap_ip_bootstrap(&mut script); + script +} + +/// Builds the generic-BF4 OVS bootstrap after preflighting every configured PF. +fn get_bf4_ovs_defaults_with_topology(topology: Option<&DpfInterceptBridging>) -> String { + // Preflight is prepended so no inherited or configured OVS operation can run first. + let mut script = topology.map_or_else(String::new, render_bf4_pf_preflight); + script.push_str(&get_bf4_ovs_defaults_base()); + if let Some(topology) = topology { + append_peer_bridge_bootstrap(&mut script, topology, |interface| { + let variable = + bf4_pf_variable(interface.identity.controller_id, interface.identity.pf_id); + match interface.identity.vf_id { + Some(vf_id) => format!("\"${{{variable}}}vf{vf_id}\""), + None => format!("\"${{{variable}}}\""), + } + }); + } + append_ovn_encap_ip_bootstrap(&mut script); + script +} + +/// Appends the per-DPU OVN address update to provisioning-time OVS configuration. +fn append_ovn_encap_ip_bootstrap(script: &mut String) { + // Owner contract: DPF runs rawConfigScript after oob_net0 is configured. Set the value here + // so provisioning establishes it directly even when the installed DPF API prunes the retained + // systemdServices request. Keep this attempt best-effort because rawConfigScript must not fail + // provisioning when management addressing is not ready; the independently ordered oneshot + // executes the same body directly and retains its strict exit status. + script.push_str("# Configure the per-DPU OVN encapsulation address during OVS provisioning.\n"); + script.push_str("(\n"); + script.push_str(ovn_encap_ip_commands()); + script.push_str(") || true\n"); +} + +/// Appends idempotent bridge creation and tolerant raw-representor attachment. +fn append_peer_bridge_bootstrap( + script: &mut String, + topology: &DpfInterceptBridging, + raw_netdev: impl Fn(&DpfInterceptBridge) -> String, +) { + // Normalized topology order keeps the script stable across map iteration order. + for interface in topology.interfaces() { + writeln!(script, "host_representor={}", raw_netdev(interface)).ok(); + writeln!( + script, + "_ovs-vsctl --may-exist add-br '{}'", + interface.bridge + ) + .ok(); + writeln!( + script, + "_ovs-vsctl set bridge '{}' datapath_type=netdev", + interface.bridge + ) + .ok(); + writeln!( + script, + "_ovs-vsctl --if-exists del-port \"$host_representor\" -- --may-exist add-port '{}' \"$host_representor\" -- set interface \"$host_representor\" type=dpdk mtu_request=9216 external_ids='{{}}' || true", + interface.bridge + ) + .ok(); + writeln!( + script, + "_ovs-vsctl br-set-external-id '{}' bridge-uplink '{}'", + interface.bridge, interface.patch_port + ) + .ok(); + } +} + +/// Renders generic-BF4 exact `phys_port_name` discovery before any OVS mutation. +fn render_bf4_pf_preflight(topology: &DpfInterceptBridging) -> String { + // VFs share discovery with their parent PF, so resolve every PF exactly once. + let pf_identities: BTreeSet<_> = topology + .interfaces() + .iter() + .map(|interface| { + let mut identity = interface.identity; + identity.vf_id = None; + identity + }) + .collect(); + if pf_identities.is_empty() { + return String::new(); + } + + // The helper resolves exact semantic names and reports cardinality before returning a netdev. + let mut script = String::from(concat!( + "sys_class_net=${NICO_SYS_CLASS_NET:-/sys/class/net}\n", + "resolve_dpf_pf() {\n", + " local semantic_name=$1 result_variable=$2 phys_port_name netdev\n", + " local -a matches=()\n", + " for phys_port_name in \"$sys_class_net\"/*/phys_port_name; do\n", + " [[ -r \"$phys_port_name\" ]] || continue\n", + " if [[ \"$(<\"$phys_port_name\")\" == \"$semantic_name\" ]]; then\n", + " netdev=$(basename \"${phys_port_name%/phys_port_name}\")\n", + " matches+=(\"$netdev\")\n", + " fi\n", + " done\n", + " if (( ${#matches[@]} != 1 )); then\n", + " echo \"expected exactly one BF4 PF netdev with phys_port_name ${semantic_name}; found ${#matches[@]}\" >&2\n", + " return 1\n", + " fi\n", + " printf -v \"$result_variable\" '%s' \"${matches[0]}\"\n", + "}\n", + )); + + for identity in &pf_identities { + writeln!( + script, + "resolve_dpf_pf '{}' '{}' || exit 1", + identity.bf4_phys_port_name(), + bf4_pf_variable(identity.controller_id, identity.pf_id) + ) + .ok(); + } + + let identities: Vec<_> = pf_identities.into_iter().collect(); + for (index, left_identity) in identities.iter().enumerate() { + for right_identity in &identities[index + 1..] { + let left = bf4_pf_variable(left_identity.controller_id, left_identity.pf_id); + let right = bf4_pf_variable(right_identity.controller_id, right_identity.pf_id); + writeln!( + script, + "if [[ \"${{{left}}}\" == \"${{{right}}}\" ]]; then echo 'BF4 PF identities {} and {} resolved to the same netdev' >&2; exit 1; fi", + left_identity.bf4_phys_port_name(), + right_identity.bf4_phys_port_name() + ) + .ok(); + } + } + script +} + +/// Returns the shell variable holding one discovered generic-BF4 PF netdev. +fn bf4_pf_variable(controller_id: u8, pf_id: u8) -> String { + format!("dpf_c{controller_id}p{pf_id}_netdev") +} + /// OVS raw config script for the BF4 flavor. fn get_bf4_astra_ovs_defaults() -> String { concat!( @@ -178,10 +338,46 @@ pub fn default_flavor_for( // Selects the DPUFlavor variant to build for the given deployment type. deployment_type: DpuDeploymentType, ) -> Result { + default_flavor_for_with_topology( + namespace, + proxy, + deployment_type, + DEFAULT_DPU_NUM_OF_VFS, + DEFAULT_PF_TOTAL_SF_RESERVED, + None, + None, + ) +} + +/// Builds a platform flavor from validated VF, SF, topology, and effective-inventory inputs. +pub(crate) fn default_flavor_for_with_topology( + namespace: &str, + proxy: &Option, + deployment_type: DpuDeploymentType, + num_of_vfs: u32, + pf_total_sf: u32, + intercept_bridging: Option<&DpfInterceptBridging>, + dhcp_acl_interfaces: Option<&[DpuServiceInterfaceTemplateDefinition]>, +) -> Result { + // Astra deliberately ignores both site-wide inputs. match deployment_type { - DpuDeploymentType::Bf4Generic => flavor_bf4(namespace, proxy), + DpuDeploymentType::Bf4Generic => flavor_bf4_with_topology( + namespace, + proxy, + num_of_vfs, + pf_total_sf, + intercept_bridging, + dhcp_acl_interfaces, + ), DpuDeploymentType::Bf4Astra => flavor_bf4_astra(namespace, proxy), - DpuDeploymentType::Bf3 => default_flavor(namespace, proxy), + DpuDeploymentType::Bf3 => default_flavor_with_topology( + namespace, + proxy, + num_of_vfs, + pf_total_sf, + intercept_bridging, + dhcp_acl_interfaces, + ), } } @@ -197,6 +393,25 @@ pub fn default_flavor_for( pub fn flavor_bf4( namespace: &str, proxy: &Option, +) -> Result { + flavor_bf4_with_topology( + namespace, + proxy, + DEFAULT_DPU_NUM_OF_VFS, + DEFAULT_PF_TOTAL_SF_RESERVED, + None, + None, + ) +} + +/// Builds generic BF4 flavor state from the validated site VF count and intercept-bridging topology. +fn flavor_bf4_with_topology( + namespace: &str, + proxy: &Option, + num_of_vfs: u32, + pf_total_sf: u32, + intercept_bridging: Option<&DpfInterceptBridging>, + dhcp_acl_interfaces: Option<&[DpuServiceInterfaceTemplateDefinition]>, ) -> Result { let bfcfg_parameters = vec![ "UPDATE_ATF_UEFI=yes".to_string(), @@ -213,19 +428,21 @@ pub fn flavor_bf4( dpu_mode: Some(DpuFlavorDpuMode::ZeroTrust), dpu_resources: None, bfcfg_parameters: Some(bfcfg_parameters), - config_files: Some(get_config_files(proxy)?), + config_files: Some(get_config_files(proxy, dhcp_acl_interfaces)?), containerd_config: None, grub: Some(bf4_grub_params()), host_network_interface_configs: None, - nvconfig: Some(vec![get_bf4_default_nvconfig()]), + nvconfig: Some(vec![get_bf4_nvconfig(num_of_vfs, pf_total_sf)]), ovs: Some(crate::crds::dpuflavors_generated::DpuFlavorOvs { - raw_config_script: Some(get_bf4_ovs_defaults()), + raw_config_script: Some(get_bf4_ovs_defaults_with_topology(intercept_bridging)), }), sysctl: None, system_reserved_resources: None, ew_nic_configurations: None, packages: None, - systemd_services: None, + // rawConfigScript sets the value during provisioning. Retain this ordered oneshot so + // DPF versions with systemdServices support also enforce it after network readiness. + systemd_services: Some(vec![ovn_encap_systemd_service()]), host_os_init: None, scalable_functions: None, }, @@ -432,6 +649,25 @@ fn bf4_astra_ew_nic_configurations() -> Vec { pub fn default_flavor( namespace: &str, proxy: &Option, +) -> Result { + default_flavor_with_topology( + namespace, + proxy, + DEFAULT_DPU_NUM_OF_VFS, + DEFAULT_PF_TOTAL_SF_RESERVED, + None, + None, + ) +} + +/// Builds BF3 flavor state from the validated site VF count and intercept-bridging topology. +fn default_flavor_with_topology( + namespace: &str, + proxy: &Option, + num_of_vfs: u32, + pf_total_sf: u32, + intercept_bridging: Option<&DpfInterceptBridging>, + dhcp_acl_interfaces: Option<&[DpuServiceInterfaceTemplateDefinition]>, ) -> Result { let bfcfg_parameters = vec![ "UPDATE_ATF_UEFI=yes".to_string(), @@ -448,19 +684,21 @@ pub fn default_flavor( dpu_mode: Some(DpuFlavorDpuMode::ZeroTrust), dpu_resources: None, bfcfg_parameters: Some(bfcfg_parameters), - config_files: Some(get_config_files(proxy)?), + config_files: Some(get_config_files(proxy, dhcp_acl_interfaces)?), containerd_config: None, grub: Some(get_default_grub()), host_network_interface_configs: None, - nvconfig: Some(vec![get_default_nvconfig()]), + nvconfig: Some(vec![get_nvconfig(num_of_vfs, pf_total_sf)]), ovs: Some(crate::crds::dpuflavors_generated::DpuFlavorOvs { - raw_config_script: Some(get_default_ovs_defaults()), + raw_config_script: Some(get_default_ovs_defaults_with_topology(intercept_bridging)), }), sysctl: None, system_reserved_resources: None, ew_nic_configurations: None, packages: None, - systemd_services: None, + // rawConfigScript sets the value during provisioning. Retain this ordered oneshot so + // DPF versions with systemdServices support also enforce it after network readiness. + systemd_services: Some(vec![ovn_encap_systemd_service()]), host_os_init: None, scalable_functions: None, }, @@ -492,6 +730,7 @@ fn get_default_grub() -> DpuFlavorGrub { /// Returns the base set of config files, plus an optional containerd proxy drop-in if `proxy` is set. fn get_config_files( proxy: &Option, + dhcp_acl_interfaces: Option<&[DpuServiceInterfaceTemplateDefinition]>, ) -> Result, crate::error::DpfError> { let mut config_files = vec![ DpuFlavorConfigFiles { @@ -516,7 +755,7 @@ fn get_config_files( path: "/var/lib/hbn/etc/cumulus/acl/policy.d/10-dhcp.rules".to_string(), operation: Some(DpuFlavorConfigFilesOperation::Override), permissions: Some("0644".to_string()), - raw: Some(dhcp_acl_rules()), + raw: Some(dhcp_acl_rules(dhcp_acl_interfaces)), content_from: None, r#type: None, }, @@ -568,6 +807,7 @@ fn get_config_files( r#type: None, }, ]; + config_files.extend(ovn_encap_config_files()); if let Some(proxy) = proxy { validate_proxy_string(&proxy.https_proxy, "https_proxy")?; @@ -606,11 +846,78 @@ fn get_config_files( Ok(config_files) } -fn get_bf4_default_nvconfig() -> DpuFlavorNvconfig { + +/// Returns the provisioning files that set `ovn-encap-ip` after networking and OVS. +fn ovn_encap_config_files() -> Vec { + vec![ + DpuFlavorConfigFiles { + path: OVN_ENCAP_SCRIPT_PATH.to_string(), + operation: Some(DpuFlavorConfigFilesOperation::Override), + permissions: Some("0755".to_string()), + // Keep the oneshot body identical to the provisioning-time rawConfigScript action. + raw: Some(format!( + "#!/bin/bash\nset -euo pipefail\n{}", + ovn_encap_ip_commands() + )), + content_from: None, + r#type: None, + }, + DpuFlavorConfigFiles { + path: format!("/etc/systemd/system/{OVN_ENCAP_SERVICE_NAME}"), + operation: Some(DpuFlavorConfigFilesOperation::Override), + permissions: Some("0644".to_string()), + raw: Some(format!( + concat!( + "[Unit]\n", + "Description=Configure OVN tunnel encapsulation address\n", + "Wants=network-online.target\n", + "After=network-online.target openvswitch-switch.service openvswitch.service\n", + "\n", + "[Service]\n", + "Type=oneshot\n", + "ExecStart={}\n", + "RemainAfterExit=yes\n", + "\n", + "[Install]\n", + "WantedBy=multi-user.target\n", + ), + OVN_ENCAP_SCRIPT_PATH + )), + content_from: None, + r#type: None, + }, + ] +} + +/// Shell commands shared by provisioning-time OVS bootstrap and the retained systemd oneshot. +fn ovn_encap_ip_commands() -> &'static str { + concat!( + "mapfile -t oob_ipv4_addresses < <(\n", + " ip -4 -o address show dev oob_net0 scope global |\n", + " awk '{sub(/\\/.*/, \"\", $4); if (NF) print $4}'\n", + ")\n", + "if (( ${#oob_ipv4_addresses[@]} != 1 )); then\n", + " echo \"expected exactly one global IPv4 address on oob_net0; found ${#oob_ipv4_addresses[@]}\" >&2\n", + " exit 1\n", + "fi\n", + "ovs-vsctl --timeout 15 set Open_vSwitch . \"external_ids:ovn-encap-ip=${oob_ipv4_addresses[0]}\"\n", + ) +} + +/// Returns the DPUFlavor request that enables and starts the OVN address oneshot. +fn ovn_encap_systemd_service() -> DpuFlavorSystemdServices { + DpuFlavorSystemdServices { + name: OVN_ENCAP_SERVICE_NAME.to_string(), + operation: DpuFlavorSystemdServicesOperation::EnableAndStart, + } +} + +/// Builds generic-BF4 nvconfig with the validated site VF population. +fn get_bf4_nvconfig(num_of_vfs: u32, pf_total_sf: u32) -> DpuFlavorNvconfig { let parameters = vec![ "PF_BAR2_ENABLE=0".to_string(), "PER_PF_NUM_SF=1".to_string(), - "PF_TOTAL_SF=30".to_string(), + format!("PF_TOTAL_SF={pf_total_sf}"), "PF_SF_BAR_SIZE=14".to_string(), "NUM_PF_MSIX_VALID=0".to_string(), "PF_NUM_PF_MSIX_VALID=1".to_string(), @@ -619,7 +926,7 @@ fn get_bf4_default_nvconfig() -> DpuFlavorNvconfig { "INTERNAL_CPU_OFFLOAD_ENGINE=0".to_string(), "SRIOV_EN=1".to_string(), "LAG_RESOURCE_ALLOCATION=1".to_string(), - "NUM_OF_VFS=16".to_string(), + format!("NUM_OF_VFS={num_of_vfs}"), "LINK_TYPE_P1=ETH".to_string(), "LINK_TYPE_P2=ETH".to_string(), ]; @@ -1188,11 +1495,12 @@ fn get_bf4_astra_config_files( Ok(config_files) } -fn get_default_nvconfig() -> DpuFlavorNvconfig { +/// Builds BF3 nvconfig with the validated site VF population. +fn get_nvconfig(num_of_vfs: u32, pf_total_sf: u32) -> DpuFlavorNvconfig { let parameters = vec![ "PF_BAR2_ENABLE=0".to_string(), "PER_PF_NUM_SF=1".to_string(), - "PF_TOTAL_SF=30".to_string(), + format!("PF_TOTAL_SF={pf_total_sf}"), "PF_SF_BAR_SIZE=10".to_string(), "NUM_PF_MSIX_VALID=0".to_string(), "PF_NUM_PF_MSIX_VALID=1".to_string(), @@ -1201,7 +1509,7 @@ fn get_default_nvconfig() -> DpuFlavorNvconfig { "INTERNAL_CPU_OFFLOAD_ENGINE=0".to_string(), "SRIOV_EN=1".to_string(), "LAG_RESOURCE_ALLOCATION=1".to_string(), - "NUM_OF_VFS=16".to_string(), + format!("NUM_OF_VFS={num_of_vfs}"), "HIDE_PORT2_PF=True".to_string(), "NUM_OF_PF=1".to_string(), "LINK_TYPE_P1=ETH".to_string(), @@ -1240,28 +1548,59 @@ fn get_bf4_astra_nvconfig() -> DpuFlavorNvconfig { } } -/// DHCP ACL rules: drop DHCP broadcasts from host-facing interfaces. -fn dhcp_acl_rules() -> String { +/// Renders DHCP broadcast drops for the host-facing interfaces in the effective inventory. +fn dhcp_acl_rules(interfaces: Option<&[DpuServiceInterfaceTemplateDefinition]>) -> String { let mut rules = String::from("[iptables]\n"); - for iface in - std::iter::once("pf0hpf_if".to_string()).chain((0..=15).map(|i| format!("pf0vf{i}_if"))) - { + let mut append_rule = |interface_name: &str| { rules.push_str(&format!( "-t filter -A FORWARD -p udp -d 255.255.255.255 \ - --dport 67 -m physdev --physdev-in {iface} \ + --dport 67 -m physdev --physdev-in {interface_name} \ -m comment --comment 'offload:0' -j DROP\n" )); + }; + + if let Some(interfaces) = interfaces { + // Configured PF/VF identities are represented as Patch interfaces. Read their HBN endpoint + // names directly so sparse inventories receive rules for exactly their selected endpoints. + for interface_name in interfaces + .iter() + .filter(|interface| { + matches!( + &interface.iface_type, + DpuServiceInterfaceTemplateType::Patch(_) + ) + }) + .flat_map(|interface| interface.chained_svc_if.iter().flatten()) + .filter_map(|(service_name, interface_name)| { + (service_name == DOCA_HBN_SERVICE_NAME).then_some(interface_name) + }) + { + append_rule(interface_name); + } + } else { + // ROLLOUT COMPATIBILITY (DPU REPROVISIONING): retain the byte-for-byte legacy ACL for + // inventory-free BF3/BF4 flavors. Deriving it from the smaller static ServiceInterface + // inventory would alter every existing flavor hash even though their policy is unchanged. + append_rule("pf0hpf_if"); + for vf_id in 0..=15 { + append_rule(&format!("pf0vf{vf_id}_if")); + } } rules } #[cfg(test)] mod tests { + use std::fs; + use std::process::{Command, Output}; + use carbide_test_support::Outcome::*; use carbide_test_support::{Case, check_cases, scenarios, value_scenarios}; use super::*; - use crate::types::DpfProxyDetails; + use crate::types::{ + DpfInterceptBridge, DpfInterceptBridging, DpfInterfaceIdentity, DpfProxyDetails, + }; fn proxy(https_proxy: &str, no_proxy: &[&str]) -> Option { Some(DpfProxyDetails { @@ -1285,6 +1624,328 @@ mod tests { .unwrap() } + /// Provides deterministic PF/VF entries for flavor topology tests. + fn intercept_bridging() -> DpfInterceptBridging { + DpfInterceptBridging::new( + vec![ + DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: Some(4), + }, + "br-vf4", + "p-vf4", + ), + DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: None, + }, + "br-pf3", + "p-pf3", + ), + ], + 16, + ) + .expect("flavor topology fixture must be valid") + } + + /// Executes generic-BF4 preflight against a synthetic `sys/class/net` tree. + fn run_bf4_preflight( + topology: &DpfInterceptBridging, + phys_port_names: &[(&str, &str)], + prefix: &str, + suffix: &str, + ) -> Output { + let fixture = + std::env::temp_dir().join(format!("carbide-dpf-bf4-sysfs-{}", uuid::Uuid::new_v4())); + + // Materialize only the semantic sysfs surface consumed by provisioning. + for (netdev, phys_port_name) in phys_port_names { + let netdev_dir = fixture.join(netdev); + fs::create_dir_all(&netdev_dir).expect("synthetic netdev directory must be created"); + fs::write(netdev_dir.join("phys_port_name"), phys_port_name) + .expect("synthetic phys_port_name must be written"); + } + let script = format!("{prefix}\n{}\n{suffix}", render_bf4_pf_preflight(topology)); + let output = Command::new("bash") + .arg("-c") + .arg(script) + .env("NICO_SYS_CLASS_NET", &fixture) + .output() + .expect("bash must execute synthetic BF4 preflight"); + fs::remove_dir_all(&fixture).expect("synthetic BF4 sysfs fixture must be removed"); + output + } + + /// Executes the flavor-provided OVN address script with synthetic `ip` output. + fn run_ovn_encap_script(addresses: &[&str]) -> Output { + let ip_output = addresses + .iter() + .enumerate() + .map(|(index, address)| { + format!( + "{}: oob_net0 inet {address}/24 brd 10.0.0.255 scope global oob_net0", + index + 1 + ) + }) + .collect::>() + .join("\\n"); + let raw = ovn_encap_config_files()[0] + .raw + .clone() + .expect("OVN script file must have inline content"); + let script = format!( + "ip() {{ printf '%b' '{ip_output}'; }}\novs-vsctl() {{ printf '%s\\n' \"$*\"; }}\n{raw}" + ); + Command::new("bash") + .arg("-c") + .arg(script) + .output() + .expect("bash must execute synthetic OVN address script") + } + + /// Verifies BF3 peer bootstrap uses typed PF/VF names and never owns patch pairs. + #[test] + fn bf3_intercept_bridging_bootstrap_renders_expected_raw_representors() { + // Render BF3 bootstrap for one configured PF and VF. + let topology = intercept_bridging(); + let script = get_default_ovs_defaults_with_topology(Some(&topology)); + + // BF3 drops controller only from its platform raw-netdev convention. + assert!(script.contains("host_representor='pf3hpf'")); + assert!(script.contains("host_representor='pf3vf4'")); + assert!(script.contains("_ovs-vsctl --may-exist add-br 'br-pf3'")); + assert!(script.contains( + "_ovs-vsctl --if-exists del-port \"$host_representor\" -- --may-exist add-port" + )); + assert!(script.contains("bridge-uplink 'p-vf4'")); + assert!(script.contains("external_ids='{}' || true")); + + // DPF owns both patch ports; flavor bootstrap neither creates them nor edits legacy SFC. + assert!(!script.contains("sfc.conf")); + assert!(!script.contains("type=patch")); + assert!(!script.contains("add-port 'br-pf3' 'p-pf3'")); + } + + /// Verifies generic-BF4 exact semantic discovery and VF-name derivation. + #[test] + fn bf4_intercept_bridging_preflight_resolves_exact_pf_and_derives_vf_suffix() { + // Render and execute preflight for the selected semantic PF identity. + let topology = intercept_bridging(); + + // Exact semantic names resolve independently of unrelated sysfs entries. + let output = run_bf4_preflight( + &topology, + &[("en8f2", "c2pf3"), ("noise", "c2pf30")], + "", + "printf '%s' \"$dpf_c2p3_netdev\"", + ); + assert!(output.status.success(), "preflight failed: {output:?}"); + assert_eq!(String::from_utf8_lossy(&output.stdout), "en8f2"); + + // VF discovery is intentionally absent; the expected VF is the PF netdev plus its suffix. + let script = get_bf4_ovs_defaults_with_topology(Some(&topology)); + assert!(script.contains("host_representor=\"${dpf_c2p3_netdev}vf4\"")); + assert!(script.contains("external_ids='{}' || true")); + assert!(!script.contains("phys_port_name 'c2pf3vf4'")); + } + + /// Verifies generic-BF4 preflight fails before OVS mutation for missing or ambiguous PFs. + #[test] + fn bf4_intercept_bridging_preflight_rejects_missing_and_ambiguous_pf_matches() { + // Include the required PF and one VF whose parent is the only discovery target. + let topology = DpfInterceptBridging::new( + vec![ + DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: None, + }, + "br-pf3", + "p-pf3", + ), + DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: Some(4), + }, + "br-vf4", + "p-vf4", + ), + ], + 16, + ) + .unwrap(); + + value_scenarios!( + run = |matches: Vec<(&str, &str)>| { + let output = run_bf4_preflight(&topology, &matches, "", ""); + ( + output.status.success(), + String::from_utf8_lossy(&output.stderr).into_owned(), + ) + }; + "missing exact PF" { + // A similar semantic name must not satisfy exact matching. + vec![("en8f2", "c2pf30")] => ( + false, + "expected exactly one BF4 PF netdev with phys_port_name c2pf3; found 0\n".to_string(), + ), + } + + "ambiguous exact PF" { + // Multiple exact matches are unsafe and must fail before bridge creation. + vec![("en8f2", "c2pf3"), ("en9f2", "c2pf3")] => ( + false, + "expected exactly one BF4 PF netdev with phys_port_name c2pf3; found 2\n".to_string(), + ), + } + ); + } + + /// Verifies every generic-BF4 PF resolves before the first OVS mutation. + #[test] + fn bf4_intercept_bridging_preflight_precedes_all_ovs_mutation() { + // Locate the final preflight call and the first inherited OVS cleanup operation. + let topology = intercept_bridging(); + let script = get_bf4_ovs_defaults_with_topology(Some(&topology)); + + let final_resolution = script + .find("resolve_dpf_pf 'c2pf3'") + .expect("last configured PF must be resolved"); + let first_mutation = script + .find("ovs-vsctl --if-exists del-br") + .expect("base OVS cleanup must remain present"); + assert!(final_resolution < first_mutation); + } + + /// Verifies configured VF and SF counts affect BF3/generic BF4 but never Astra. + #[test] + fn flavor_nvconfig_uses_platform_appropriate_vf_and_sf_counts() { + // Extract nvconfig parameters uniformly across all flavor variants. + let parameters = + |flavor: DPUFlavor| flavor.spec.nvconfig.unwrap()[0].parameters.clone().unwrap(); + + // BF3 and generic BF4 consume the validated site value. + let bf3 = parameters(default_flavor_with_topology("ns", &None, 3, 61, None, None).unwrap()); + assert!(bf3.contains(&"NUM_OF_VFS=3".to_string())); + assert!(bf3.contains(&"PF_TOTAL_SF=61".to_string())); + let generic_bf4 = + parameters(flavor_bf4_with_topology("ns", &None, 5, 63, None, None).unwrap()); + assert!(generic_bf4.contains(&"NUM_OF_VFS=5".to_string())); + assert!(generic_bf4.contains(&"PF_TOTAL_SF=63".to_string())); + + // Astra retains its established fixed hardware configuration. + let astra = parameters(flavor_bf4_astra("ns", &None).unwrap()); + assert!(astra.contains(&"NUM_OF_VFS=46".to_string())); + assert!(astra.contains(&"PF_TOTAL_SF=30".to_string())); + } + + /// Verifies normalized input order cannot change rendered flavor identity. + #[test] + fn intercept_bridging_input_order_does_not_change_flavor_hash() { + // Rebuild the same logical topology from reverse input order. + let topology = intercept_bridging(); + let reversed = + DpfInterceptBridging::new(topology.interfaces().iter().cloned().rev().collect(), 16) + .expect("reordered topology fixture must remain valid"); + + // Flavor hashing must depend on typed topology content, not legacy map iteration order. + let flavor_name = |topology: &DpfInterceptBridging| { + let interfaces = crate::sdk::build_effective_dpu_interfaces(16, Some(topology)); + default_flavor_with_topology( + "ns", + &None, + 16, + DEFAULT_PF_TOTAL_SF_RESERVED + 7, + Some(topology), + Some(&interfaces), + ) + .unwrap() + .unique_name(DEFAULT_FLAVOR_NAME) + .unwrap() + }; + assert_eq!(flavor_name(&topology), flavor_name(&reversed)); + } + + /// Verifies the shared OVN address action enforces exact address cardinality. + #[test] + fn ovn_encap_script_requires_one_global_ipv4_address() { + // Exercise the installed script with controlled management-address output. + value_scenarios!( + run = |addresses: &[&str]| { + let output = run_ovn_encap_script(addresses); + ( + output.status.success(), + String::from_utf8_lossy(&output.stdout).into_owned(), + ) + }; + "no global address" { + // Provisioning must fail instead of leaving an empty OVN tunnel address. + &[] as &[&str] => (false, String::new()), + } + + "one global address" { + // The sole address is persisted directly in Open_vSwitch external IDs. + &["10.0.0.4"] as &[&str] => ( + true, + "--timeout 15 set Open_vSwitch . external_ids:ovn-encap-ip=10.0.0.4\n".to_string(), + ), + } + + "multiple global addresses" { + // Ambiguous management addressing must fail rather than select arbitrarily. + &["10.0.0.4", "10.0.0.5"] as &[&str] => (false, String::new()), + } + ); + } + + /// Verifies BF3 and generic BF4 set the per-DPU OVN address during OVS provisioning. + #[test] + fn ovs_bootstrap_ends_with_ovn_encap_ip_configuration() { + // Both rawConfigScript variants execute the strict oneshot body in a best-effort subshell. + let expected = format!("(\n{}) || true\n", ovn_encap_ip_commands()); + value_scenarios!( + run = |script: String| script.ends_with(&expected); + "BF3 provisioning" { + get_default_ovs_defaults_with_topology(None) => true, + } + + "generic BF4 provisioning" { + get_bf4_ovs_defaults_with_topology(None) => true, + } + ); + } + + /// Verifies the retained OVN oneshot is installed after network readiness and either OVS unit. + #[test] + fn ovn_encap_oneshot_has_required_systemd_ordering() { + // Read the rendered unit and its DPUFlavor enablement request. + let files = ovn_encap_config_files(); + let unit = files + .iter() + .find(|file| file.path.ends_with(OVN_ENCAP_SERVICE_NAME)) + .and_then(|file| file.raw.as_ref()) + .expect("OVN systemd unit must be rendered"); + let service = ovn_encap_systemd_service(); + + // Both supported OVS unit names are ordered before the exact address action. + assert!(unit.contains( + "After=network-online.target openvswitch-switch.service openvswitch.service" + )); + assert!(unit.contains(&format!("ExecStart={OVN_ENCAP_SCRIPT_PATH}"))); + assert_eq!(service.name, OVN_ENCAP_SERVICE_NAME); + assert!(matches!( + service.operation, + DpuFlavorSystemdServicesOperation::EnableAndStart + )); + } + // ── validate_proxy_string ────────────────────────────────────────────── // // The pure validator at the heart of the proxy path. `DpfError` is not @@ -1738,16 +2399,16 @@ mod tests { .unwrap() .len() }; - "no proxy yields seven base files" { - None => 7, + "no proxy yields nine base files" { + None => 9, } - "proxy with empty no_proxy appends an eighth" { - proxy("http://proxy:3128", &[]) => 8, + "proxy with empty no_proxy appends a tenth" { + proxy("http://proxy:3128", &[]) => 10, } "proxy with no_proxy list still appends exactly one" { - proxy("http://proxy:3128", &["10.0.0.0/8", "localhost"]) => 8, + proxy("http://proxy:3128", &["10.0.0.0/8", "localhost"]) => 10, } ); } @@ -1776,7 +2437,7 @@ mod tests { #[test] fn base_config_file_paths_are_present() { - // The seven base files always exist regardless of proxy, with these paths. + // The base files always exist regardless of proxy, with these paths. let files = default_flavor("ns", &None) .unwrap() .spec @@ -2026,7 +2687,7 @@ mod tests { #[test] fn dhcp_acl_rules_shape() { - let rules = dhcp_acl_rules(); + let rules = dhcp_acl_rules(None); value_scenarios!( run = |v| v; "starts with the iptables header" { @@ -2059,6 +2720,67 @@ mod tests { ); } + /// Verifies configured ACLs follow the exact sparse PF/VF inventory. + #[test] + fn configured_dhcp_acl_rules_follow_effective_inventory() { + // Build one PF and the highest VMaaS-addressable VF as a sparse inventory. + let topology = DpfInterceptBridging::new( + vec![ + DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: None, + }, + "br-pf3", + "p-pf3", + ), + DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: Some(15), + }, + "br-vf15", + "p-vf15", + ), + ], + 16, + ) + .expect("sparse VF15 topology must be valid"); + let interfaces = crate::sdk::build_effective_dpu_interfaces(16, Some(&topology)); + let rules = dhcp_acl_rules(Some(&interfaces)); + + // Only the configured host-facing HBN endpoints should receive ACL rules. + value_scenarios!( + run = |value| value; + "selected PF is covered" { + // The selected hardware PF is exposed to HBN through canonical logical PF0. + rules.contains("--physdev-in pf0hpf_if ") => true, + } + + "sparse VF15 is covered" { + // The highest VMaaS-addressable VF retains the broadcast policy. + rules.contains("--physdev-in pf0vf15_if ") => true, + } + + "unconfigured VF14 is absent" { + // Exact inventory generation must not restore the old unconditional range. + rules.contains("pf0vf14_if") => false, + } + + "physical uplinks are absent" { + // Fixed physical endpoints are not host-facing DHCP ingress interfaces. + rules.contains("--physdev-in p0_if ") => false, + } + + "header plus selected PF and VF rules" { + // The sparse inventory contains exactly two host-facing endpoints. + rules.lines().count() == 3 => true, + } + ); + } + // ── get_default_ovs_defaults (pure formatter) ────────────────────────── #[test] @@ -2067,7 +2789,7 @@ mod tests { [Case { scenario: "doca/offload/br-sfc setup lines present", input: ( - get_default_ovs_defaults(), + get_default_ovs_defaults_with_topology(None), &[ "other_config:doca-init=true", "other_config:hw-offload=true", @@ -2087,7 +2809,7 @@ mod tests { #[test] fn default_nvconfig_shape() { - let nv = get_default_nvconfig(); + let nv = get_nvconfig(16, DEFAULT_PF_TOTAL_SF_RESERVED); value_scenarios!( run = |v| v; "device is the only allowed wildcard variant" { @@ -2113,6 +2835,14 @@ mod tests { .map(|p| p.iter().any(|s| s == "NUM_OF_VFS=16")) == Some(true) => true, } + + "carries the legacy default PF_TOTAL_SF" { + nv + .parameters + .as_ref() + .map(|p| p.iter().any(|s| s == "PF_TOTAL_SF=30")) + == Some(true) => true, + } ); } } diff --git a/crates/dpf/src/lib.rs b/crates/dpf/src/lib.rs index 11fc76aab9..121edcc3cd 100644 --- a/crates/dpf/src/lib.rs +++ b/crates/dpf/src/lib.rs @@ -25,7 +25,8 @@ //! The DPF SDK abstracts away the complexity of managing DPF CRDs, providing //! a clean interface for: //! -//! - Initializing DPF resources (BFB, DPUFlavor, DPUDeployment with services) +//! - Initializing DPF resources (provisioning source, DPUFlavor, and +//! DPUDeployment with services) //! - Registering and managing DPU devices //! - Registering and managing DPU nodes (hosts with DPUs) //! - Watching for DPF events via callbacks @@ -74,17 +75,19 @@ pub use k8s_openapi::apimachinery::pkg::util::intstr::IntOrString; pub use repository::{DpfRepository, KubeRepository}; pub use sdk::{ DpfSdk, DpfSdkBuilder, DpuProvisioningSource, NoLabels, ResourceLabeler, build_deployment, - build_service_configuration, build_service_interface, build_service_nad, - build_service_template, dpu_cr_name, dpu_device_cr_name, dpu_node_cr_name, - node_id_from_dpu_node_cr_name, + build_effective_dpu_interfaces, build_service_configuration, build_service_interface, + build_service_nad, build_service_template, calculate_pf_total_sf, dpu_cr_name, + dpu_device_cr_name, dpu_node_cr_name, node_id_from_dpu_node_cr_name, }; pub use services::{DEFAULT_DOCA_HELM_REGISTRY, ServiceRegistryConfig}; pub use types::{ - BlueFieldSoftwareParams, BmcPasswordProvider, ConfigPortsServiceType, DpuDeploymentType, - DpuDeviceInfo, DpuErrorEvent, DpuEvent, DpuMismatch, DpuNodeInfo, DpuPhase, DpuReadyEvent, - DpuServiceVersion, InitDpfResourcesConfig, MaintenanceEvent, RebootRequiredEvent, - ServiceChainSwitch, ServiceConfigPort, ServiceConfigPortProtocol, ServiceDefinition, - ServiceInterface, ServiceNAD, ServiceNADResourceType, + BlueFieldSoftwareParams, BmcPasswordProvider, ConfigPortsServiceType, DEFAULT_DPU_NUM_OF_VFS, + DEFAULT_PF_TOTAL_SF_RESERVED, DPU_ENABLED_NODE_LABEL, DpfInterceptBridge, DpfInterceptBridging, + DpfInterfaceIdentity, DpuDeploymentType, DpuDeviceInfo, DpuErrorEvent, DpuEvent, DpuMismatch, + DpuNodeInfo, DpuPhase, DpuReadyEvent, DpuServiceVersion, InitDpfResourcesConfig, + MaintenanceEvent, RebootRequiredEvent, ServiceChainSwitch, ServiceConfigPort, + ServiceConfigPortProtocol, ServiceDefinition, ServiceInterface, ServiceNAD, + ServiceNADResourceType, }; pub use watcher::{DpuWatcher, DpuWatcherBuilder}; diff --git a/crates/dpf/src/sdk.rs b/crates/dpf/src/sdk.rs index d270929040..6463282efe 100644 --- a/crates/dpf/src/sdk.rs +++ b/crates/dpf/src/sdk.rs @@ -17,6 +17,7 @@ //! DPF SDK - High-level interface for DPF operations. +use std::borrow::Cow; use std::collections::{BTreeMap, HashMap}; use std::sync::Arc; use std::time::Duration; @@ -58,12 +59,18 @@ use crate::crds::dpuserviceconfigurations_generated::{ }; use crate::crds::dpuserviceinterfaces_generated::{ DPUServiceInterface, DpuServiceInterfaceSpec, DpuServiceInterfaceTemplate, - DpuServiceInterfaceTemplateSpec, DpuServiceInterfaceTemplateSpecTemplate, - DpuServiceInterfaceTemplateSpecTemplateMetadata, DpuServiceInterfaceTemplateSpecTemplateSpec, + DpuServiceInterfaceTemplateSpec, DpuServiceInterfaceTemplateSpecNodeSelector, + DpuServiceInterfaceTemplateSpecTemplate, DpuServiceInterfaceTemplateSpecTemplateMetadata, + DpuServiceInterfaceTemplateSpecTemplateSpec, DpuServiceInterfaceTemplateSpecTemplateSpecInterfaceType, + DpuServiceInterfaceTemplateSpecTemplateSpecPatch, DpuServiceInterfaceTemplateSpecTemplateSpecPf, + DpuServiceInterfaceTemplateSpecTemplateSpecPfNicSelector, + DpuServiceInterfaceTemplateSpecTemplateSpecPfNicSelectorType, DpuServiceInterfaceTemplateSpecTemplateSpecPhysical, DpuServiceInterfaceTemplateSpecTemplateSpecVf, + DpuServiceInterfaceTemplateSpecTemplateSpecVfNicSelector, + DpuServiceInterfaceTemplateSpecTemplateSpecVfNicSelectorType, }; use crate::crds::dpuservicenads_generated::{ DPUServiceNAD, DpuServiceNadResourceType, DpuServiceNadSpec, @@ -81,23 +88,38 @@ use crate::repository::{ K8sConfigRepository, }; use crate::types::{ - BlueFieldSoftwareParams, BmcPasswordProvider, ConfigPortsServiceType, DHCP_SERVER_SERVICE_NAME, - DOCA_HBN_SERVICE_NAME, DPU_AGENT_SERVICE_NAME, DTS_SERVICE_NAME, DpfProxyDetails, + BlueFieldSoftwareParams, BmcPasswordProvider, ConfigPortsServiceType, + DEFAULT_PF_TOTAL_SF_RESERVED, DHCP_SERVER_SERVICE_NAME, DOCA_HBN_SERVICE_NAME, + DPU_AGENT_SERVICE_NAME, DPU_ENABLED_NODE_LABEL, DTS_SERVICE_NAME, DpfInterceptBridging, DpuDeploymentType, DpuDeviceInfo, DpuDeviceSummary, DpuMismatch, DpuNodeInfo, DpuNodeSummary, - DpuPhase, DpuServiceInterfaceTemplateDefinition, DpuServiceInterfaceTemplateType, - DpuServiceVersion, DpuSummary, FMDS_SERVICE_NAME, HostDpfSnapshot, InitDpfResourcesConfig, - OTEL_COLLECTOR_SERVICE_NAME, ServiceConfigPortProtocol, ServiceDefinition, - ServiceNADResourceType, ServiceTemplateVersion, + DpuPhase, DpuServiceInterfacePatch, DpuServiceInterfaceTemplateDefinition, + DpuServiceInterfaceTemplateType, DpuServiceVersion, DpuSummary, FMDS_SERVICE_NAME, + HostDpfSnapshot, InitDpfResourcesConfig, MAX_BLUEFIELD_VFS_PER_PF, OTEL_COLLECTOR_SERVICE_NAME, + ServiceConfigPortProtocol, ServiceDefinition, ServiceNADResourceType, ServiceTemplateVersion, }; use crate::watcher::DpuWatcherBuilder; const SECRET_NAME: &str = "bmc-shared-password"; const BFB_NAME_PREFIX: &str = "bf-bundle"; const BLUEFIELD_SOFTWARE_NAME_PREFIX: &str = "bf-software"; -/// Label set by the DPF operator on each DPU CR pointing back to its owning -/// DPUDeployment. Value format: `_`. +const MAX_HBN_SERVICE_INTERFACES: usize = 32; +/// Label set by DPF on deployment-owned resources and propagated to the corresponding +/// DPU-cluster Node. Value format: `_`. const DPU_OWNED_BY_DEPLOYMENT_LABEL: &str = "svc.dpu.nvidia.com/owned-by-dpudeployment"; +/// Returns DPF's canonical ownership-label value for one DPUDeployment. +fn dpu_deployment_owner_label_value(namespace: &str, deployment_name: &str) -> String { + format!("{namespace}_{deployment_name}") +} + +/// Selects the DPU-cluster Node owned by one DPUDeployment. +fn dpu_cluster_node_selector(namespace: &str, deployment_name: &str) -> BTreeMap { + BTreeMap::from([( + DPU_OWNED_BY_DEPLOYMENT_LABEL.to_string(), + dpu_deployment_owner_label_value(namespace, deployment_name), + )]) +} + pub(crate) const RESTART_ANNOTATION: &str = "provisioning.dpu.nvidia.com/dpunode-external-reboot-required"; pub(crate) const HOLD_ANNOTATION: &str = "provisioning.dpu.nvidia.com/wait-for-external-nodeeffect"; @@ -318,8 +340,11 @@ where self, config: &InitDpfResourcesConfig, ) -> Result, DpfError> { + // Validate before `init_secret_and_task` writes the shared BMC Secret. + let resolved = resolve_initialization_inventory(config)?; let sdk = self.init_secret_and_task().await?; - sdk.create_initialization_objects(config).await?; + sdk.create_initialization_objects_resolved(config, resolved) + .await?; Ok(sdk) } } @@ -570,12 +595,22 @@ async fn create_bluefield_software( async fn create_dpu_flavor( repo: &R, namespace: &str, - default_flavor_name: &str, - proxy: &Option, - deployment_type: DpuDeploymentType, + config: &InitDpfResourcesConfig, + resolved: &ResolvedInitialization<'_>, ) -> Result { - let mut flavor = crate::flavor::default_flavor_for(namespace, proxy, deployment_type)?; - let name = flavor.unique_name(default_flavor_name)?; + let mut flavor = crate::flavor::default_flavor_for_with_topology( + namespace, + &config.proxy, + config.deployment_type, + config.num_of_vfs, + resolved.pf_total_sf, + config.intercept_bridging.as_ref(), + config + .intercept_bridging + .as_ref() + .map(|_| resolved.interfaces.as_ref()), + )?; + let name = flavor.unique_name(&config.flavor_name)?; flavor.metadata.name = Some(name.clone()); match DpuFlavorRepository::create(repo, &flavor).await { @@ -625,6 +660,19 @@ pub fn deployment_cr_suffix(deployment_type: DpuDeploymentType) -> &'static str } } +/// Suffix appended to deployment-scoped DPUServiceInterface CR names. +/// +/// Unlike the existing service CR compatibility scheme, every deployment type +/// is suffixed. The migration is opt-in; operators must remove the old +/// generation manually because NICo neither detects nor deletes it. +fn service_interface_cr_suffix(deployment_type: DpuDeploymentType) -> &'static str { + match deployment_type { + DpuDeploymentType::Bf3 => "bf3", + DpuDeploymentType::Bf4Generic => "bf4", + DpuDeploymentType::Bf4Astra => "astra", + } +} + /// Per-deployment CR name for a service or NAD: its logical name with the /// deployment suffix appended (or the logical name unchanged when the suffix is /// empty, as for BF3). The logical name (used as the DPUDeployment `services` @@ -908,10 +956,8 @@ pub fn build_deployment( }) }; - let mut node_labels = BTreeMap::from([( - "feature.node.kubernetes.io/dpu-enabled".to_string(), - "true".to_string(), - )]); + let mut node_labels = + BTreeMap::from([(DPU_ENABLED_NODE_LABEL.to_string(), "true".to_string())]); node_labels.extend(deployment_node_labels); DPUDeployment { @@ -1133,12 +1179,234 @@ pub fn build_dpu_interfaces_vec() -> Vec interfaces } -/// Build a single `DPUServiceInterface` CR from a template definition. -pub fn build_service_interface( +/// Builds the effective BF3/generic-BF4 interface inventory for one site configuration. +pub fn build_effective_dpu_interfaces( + num_of_vfs: u32, + intercept_bridging: Option<&DpfInterceptBridging>, +) -> Vec { + // Absence preserves the supported static inventory while projecting the hardware VF count. + let Some(topology) = intercept_bridging else { + return build_dpu_interfaces_vec() + .into_iter() + .filter(|interface| { + !matches!(&interface.iface_type, DpuServiceInterfaceTemplateType::Vf) + || u32::try_from(interface.vf_id).is_ok_and(|vf_id| vf_id < num_of_vfs) + }) + .collect(); + }; + + // Configured intercept bridging replaces every ordinary PF/VF while retaining platform physical ports. + let mut interfaces: Vec<_> = build_dpu_interfaces_vec() + .into_iter() + .filter(|interface| { + matches!( + &interface.iface_type, + DpuServiceInterfaceTemplateType::Physical + ) + }) + .collect(); + interfaces.extend(topology.interfaces().iter().map(|interface| { + let identity = interface.identity; + let name = identity.resource_name(); + let service_name = identity.service_interface_stem(); + let mut chained_svc_if = vec![ + ( + DOCA_HBN_SERVICE_NAME.to_string(), + format!("{service_name}_if"), + ), + ( + DHCP_SERVER_SERVICE_NAME.to_string(), + format!("d_{service_name}_if"), + ), + ]; + + // PFs additionally expose FMDS; VFs must never receive that endpoint. + if identity.vf_id.is_none() { + chained_svc_if.push(( + FMDS_SERVICE_NAME.to_string(), + format!("f_{service_name}_if"), + )); + } + + DpuServiceInterfaceTemplateDefinition { + name, + iface_type: DpuServiceInterfaceTemplateType::Patch(DpuServiceInterfacePatch { + peer_bridge: interface.bridge.clone(), + peer_patch_name: interface.patch_port.clone(), + }), + pf_id: i64::from(identity.pf_id), + vf_id: i64::from(identity.vf_id.unwrap_or_default()), + chained_svc_if: Some(chained_svc_if), + } + })); + interfaces +} + +/// Calculates BF3 or generic-BF4 SF capacity, preserving the legacy total without topology. +pub fn calculate_pf_total_sf( + interfaces: &[DpuServiceInterfaceTemplateDefinition], + intercept_bridging: Option<&DpfInterceptBridging>, + reserved: u32, +) -> Result { + // ROLLOUT COMPATIBILITY (DPU REPROVISIONING): inventory-free deployments must retain the + // historical behavior where the configured reserved value is the complete PF_TOTAL_SF pool. + // Adding the static endpoints here would change every existing BF3/BF4 flavor hash and force + // those DPUs through an unrequested re-ingestion. + if intercept_bridging.is_none() { + return Ok(reserved); + } + + // HBN's chart supports at most 32 attached interfaces. Validate the rendered topology rather + // than relying only on the configured one-PF/VF15 limit so custom public-SDK inventories + // cannot bypass the service boundary. + let hbn_interfaces = interfaces + .iter() + .flat_map(|interface| interface.chained_svc_if.iter().flatten()) + .filter(|(service, _)| service == DOCA_HBN_SERVICE_NAME) + .count(); + if hbn_interfaces > MAX_HBN_SERVICE_INTERFACES { + return Err(DpfError::ConfigError(format!( + "configured DPF topology requires {hbn_interfaces} HBN interfaces, exceeding the supported maximum of {MAX_HBN_SERVICE_INTERFACES}" + ))); + } + + // Configured inventory expands the SF pool by exactly the endpoints NICo asks DPF services to + // consume. The reserved population remains available to DPF, firmware, and non-NICo users. + let managed_endpoints = interfaces.iter().try_fold(0u32, |total, interface| { + let interface_endpoints = + u32::try_from(interface.chained_svc_if.as_ref().map_or(0, Vec::len)).map_err(|_| { + DpfError::ConfigError("DPF service endpoint count exceeds u32".to_string()) + })?; + total.checked_add(interface_endpoints).ok_or_else(|| { + DpfError::ConfigError("DPF service endpoint count exceeds u32".to_string()) + }) + })?; + managed_endpoints.checked_add(reserved).ok_or_else(|| { + DpfError::ConfigError(format!( + "configured DPF service endpoints ({managed_endpoints}) plus \ + dpf.pf_total_sf_reserved ({reserved}) exceed u32" + )) + }) +} + +/// Validated initialization state that borrows caller-provided interfaces and owns SDK defaults. +struct ResolvedInitialization<'a> { + interfaces: Cow<'a, [DpuServiceInterfaceTemplateDefinition]>, + pf_total_sf: u32, +} + +/// Resolves initialization interfaces and validates their SF capacity without writing resources. +fn resolve_initialization_inventory<'a>( + config: &'a InitDpfResourcesConfig, +) -> Result, DpfError> { + if config.num_of_vfs > MAX_BLUEFIELD_VFS_PER_PF { + return Err(DpfError::ConfigError(format!( + "DPF num_of_vfs must be <= {MAX_BLUEFIELD_VFS_PER_PF}" + ))); + } + + // Astra's static interface inventory is safe only when deployment selectors isolate it from + // BF3 and generic-BF4 nodes. + if matches!(config.deployment_type, DpuDeploymentType::Bf4Astra) + && !config.deployment_scoped_service_interfaces + { + return Err(DpfError::ConfigError( + "BF4 Astra requires deployment_scoped_service_interfaces=true".to_string(), + )); + } + + // A normalized topology is valid only for the VF population supplied to its constructor. + // Direct SDK callers can construct both inputs independently, so reject mismatches before + // building a flavor or writing any initialization resource. + if !matches!(config.deployment_type, DpuDeploymentType::Bf4Astra) + && let Some(topology) = &config.intercept_bridging + && topology.num_of_vfs() != config.num_of_vfs + { + return Err(DpfError::ConfigError(format!( + "DPF intercept bridging was validated for num_of_vfs={}, but initialization requested num_of_vfs={}", + topology.num_of_vfs(), + config.num_of_vfs + ))); + } + + let interfaces = if !matches!(config.deployment_type, DpuDeploymentType::Bf4Astra) + && let Some(topology) = config.intercept_bridging.as_ref() + { + let projected = build_effective_dpu_interfaces(config.num_of_vfs, Some(topology)); + + // Topology is the authoritative PF/VF inventory. Accepting a different caller projection + // would make flavor OVS state, DHCP ACLs, ServiceInterfaces, and service chains disagree. + if !config.interfaces.is_empty() && config.interfaces != projected { + let first_mismatch = config + .interfaces + .iter() + .zip(&projected) + .find(|(received, expected)| received != expected) + .map(|(received, expected)| { + (Some(received.name.as_str()), Some(expected.name.as_str())) + }) + .or_else(|| { + config + .interfaces + .get(projected.len()) + .map(|received| (Some(received.name.as_str()), None)) + }) + .or_else(|| { + projected + .get(config.interfaces.len()) + .map(|expected| (None, Some(expected.name.as_str()))) + }) + .unwrap_or((None, None)); + return Err(DpfError::ConfigError(format!( + "custom DPF interface inventory must match the effective intercept-bridging topology projection (first differing interface: received {}, expected {}; received {} interfaces, expected {})", + first_mismatch.0.unwrap_or(""), + first_mismatch.1.unwrap_or(""), + config.interfaces.len(), + projected.len(), + ))); + } + + if config.interfaces.is_empty() { + Cow::Owned(projected) + } else { + Cow::Borrowed(config.interfaces.as_slice()) + } + } else if config.interfaces.is_empty() { + // Astra retains its established static inventory and ignores site topology policy. + Cow::Owned(match config.deployment_type { + DpuDeploymentType::Bf4Astra => build_dpu_interfaces_vec(), + DpuDeploymentType::Bf3 | DpuDeploymentType::Bf4Generic => { + build_effective_dpu_interfaces(config.num_of_vfs, None) + } + }) + } else { + Cow::Borrowed(config.interfaces.as_slice()) + }; + + let pf_total_sf = match config.deployment_type { + // Astra owns a fixed BF4+CX9 flavor outside the site inventory contract. + DpuDeploymentType::Bf4Astra => DEFAULT_PF_TOTAL_SF_RESERVED, + DpuDeploymentType::Bf3 | DpuDeploymentType::Bf4Generic => calculate_pf_total_sf( + interfaces.as_ref(), + config.intercept_bridging.as_ref(), + config.pf_total_sf_reserved, + )?, + }; + + Ok(ResolvedInitialization { + interfaces, + pf_total_sf, + }) +} + +/// Builds one DPUServiceInterface with an optional deployment suffix and node selector. +fn build_service_interface_with_scope( iface: &DpuServiceInterfaceTemplateDefinition, namespace: &str, + suffix: &str, + dpu_cluster_node_labels: Option<&BTreeMap>, ) -> DPUServiceInterface { - let (interface_type, physical, pf, vf) = match iface.iface_type { + let (interface_type, physical, pf, vf, patch) = match &iface.iface_type { DpuServiceInterfaceTemplateType::Physical => ( DpuServiceInterfaceTemplateSpecTemplateSpecInterfaceType::Physical, Some(DpuServiceInterfaceTemplateSpecTemplateSpecPhysical { @@ -1146,6 +1414,7 @@ pub fn build_service_interface( }), None, None, + None, ), DpuServiceInterfaceTemplateType::Pf => ( DpuServiceInterfaceTemplateSpecTemplateSpecInterfaceType::Pf, @@ -1153,9 +1422,18 @@ pub fn build_service_interface( Some(DpuServiceInterfaceTemplateSpecTemplateSpecPf { pf_id: iface.pf_id, virtual_network: None, - nic_selector: None, + // Preserve the legacy unscoped spec; controller selection belongs to the explicit + // deployment-scoping migration and would otherwise reconcile existing resources. + nic_selector: dpu_cluster_node_labels.is_some().then_some( + DpuServiceInterfaceTemplateSpecTemplateSpecPfNicSelector { + controller_number: Some(1), + pci: None, + r#type: DpuServiceInterfaceTemplateSpecTemplateSpecPfNicSelectorType::Dpu, + }, + ), }), None, + None, ), DpuServiceInterfaceTemplateType::Vf => ( DpuServiceInterfaceTemplateSpecTemplateSpecInterfaceType::Vf, @@ -1170,20 +1448,44 @@ pub fn build_service_interface( pf_id: iface.pf_id, vf_id: iface.vf_id, virtual_network: None, - nic_selector: None, + // Keep the VF selector symmetric with its parent PF and absent in legacy mode. + nic_selector: dpu_cluster_node_labels.is_some().then_some( + DpuServiceInterfaceTemplateSpecTemplateSpecVfNicSelector { + controller_number: Some(1), + pci: None, + r#type: DpuServiceInterfaceTemplateSpecTemplateSpecVfNicSelectorType::Dpu, + }, + ), + }), + None, + ), + DpuServiceInterfaceTemplateType::Patch(patch) => ( + DpuServiceInterfaceTemplateSpecTemplateSpecInterfaceType::Patch, + None, + None, + None, + Some(DpuServiceInterfaceTemplateSpecTemplateSpecPatch { + peer_bridge: patch.peer_bridge.clone(), + peer_external_i_ds: None, + peer_patch_name: Some(patch.peer_patch_name.clone()), }), ), - _ => unimplemented!("interface type not supported"), }; + let resource_name = service_cr_name(&iface.name, suffix); let mut cr = DPUServiceInterface::new( - &iface.name, + &resource_name, DpuServiceInterfaceSpec { cluster_selector: None, template: DpuServiceInterfaceTemplate { metadata: None, spec: DpuServiceInterfaceTemplateSpec { - node_selector: None, + node_selector: dpu_cluster_node_labels.map(|labels| { + DpuServiceInterfaceTemplateSpecNodeSelector { + match_expressions: None, + match_labels: Some(labels.clone()), + } + }), template: DpuServiceInterfaceTemplateSpecTemplate { metadata: Some(DpuServiceInterfaceTemplateSpecTemplateMetadata { annotations: None, @@ -1201,7 +1503,7 @@ pub fn build_service_interface( service: None, vf, vlan: None, - patch: None, + patch, }, }, }, @@ -1217,22 +1519,43 @@ pub fn build_service_interface( cr } -/// Build each standard DPU service interface template and apply it to the repository in one pass. -pub async fn apply_service_interface_templates< +/// Builds the legacy unscoped DPUServiceInterface retained for compatibility. +pub fn build_service_interface( + iface: &DpuServiceInterfaceTemplateDefinition, + namespace: &str, +) -> DPUServiceInterface { + build_service_interface_with_scope(iface, namespace, "", None) +} + +/// Builds and applies deployment-scoped DPUServiceInterfaces in one pass. +async fn apply_service_interface_templates_with_scope< R: crate::repository::DpuServiceInterfaceRepository, >( repo: &R, namespace: &str, interfaces: &[DpuServiceInterfaceTemplateDefinition], + suffix: &str, + dpu_cluster_node_labels: Option<&BTreeMap>, ) -> Result<(), crate::error::DpfError> { for iface in interfaces { - let cr = build_service_interface(iface, namespace); + let cr = + build_service_interface_with_scope(iface, namespace, suffix, dpu_cluster_node_labels); crate::repository::DpuServiceInterfaceRepository::apply(repo, &cr).await?; } Ok(()) } -#[allow(clippy::too_many_arguments)] +/// Builds and applies the legacy unscoped DPUServiceInterfaces in one pass. +pub async fn apply_service_interface_templates< + R: crate::repository::DpuServiceInterfaceRepository, +>( + repo: &R, + namespace: &str, + interfaces: &[DpuServiceInterfaceTemplateDefinition], +) -> Result<(), crate::error::DpfError> { + apply_service_interface_templates_with_scope(repo, namespace, interfaces, "", None).await +} + async fn create_flavor_services_and_deployment< R: DpuServiceTemplateRepository + DpuServiceConfigurationRepository @@ -1246,18 +1569,55 @@ async fn create_flavor_services_and_deployment< namespace: &str, labeler: &L, services: &[ServiceDefinition], - deployment_name: &str, source: &DpuProvisioningSource, - default_flavor_name: &str, - proxy: &Option, - deployment_type: DpuDeploymentType, + config: &InitDpfResourcesConfig, + resolved: &ResolvedInitialization<'_>, ) -> Result<(), DpfError> { - let flavor_name = - create_dpu_flavor(repo, namespace, default_flavor_name, proxy, deployment_type).await?; + let deployment_type = config.deployment_type; + let interfaces = resolved.interfaces.as_ref(); + let deployment_node_labels = labeler.node_labels_for_deployment_type(deployment_type)?; + if config.deployment_scoped_service_interfaces && deployment_node_labels.is_empty() { + return Err(DpfError::ConfigError(format!( + "deployment-scoped initialization requires management-plane DPUNode labels for {deployment_type:?}" + ))); + } - let interfaces = build_dpu_interfaces_vec(); + let flavor_name = create_dpu_flavor(repo, namespace, config, resolved).await?; - apply_service_interface_templates(repo, namespace, &interfaces).await?; + let interface_suffix = if config.deployment_scoped_service_interfaces { + service_interface_cr_suffix(deployment_type) + } else { + "" + }; + let dpu_cluster_node_labels = config + .deployment_scoped_service_interfaces + .then(|| dpu_cluster_node_selector(namespace, &config.deployment_name)); + + // ROLLOUT SAFETY (DPF DATA PLANE): + // + // The disabled path must retain the legacy resource names and empty selectors so installing a + // new NICo release does not trigger a scoping migration for existing BF3/BF4 interfaces. The + // enabled path intentionally creates `-bf3`, `-bf4`, and `-astra` resources. + // + // LABEL-PLANE SAFETY: A DPUServiceInterface node selector is evaluated against Nodes in the + // remote DPU cluster, not management-cluster DPUNode CRs. DPF propagates its canonical + // `owned-by-dpudeployment=_` label to those remote Nodes, so scoped + // interfaces must select that label. The deployment-class labels above remain exclusively for + // management-plane DPUNode and DPUDeployment selection. Reusing them here matches zero remote + // Nodes and prevents DPF from instantiating concrete ServiceInterfaces. + // + // Changing either names or selectors triggers DPF reconciliation and must remain an explicit + // operator-controlled migration. + // + // Patch CRs require their peer bridge, so preserve flavor creation before interface templates. + apply_service_interface_templates_with_scope( + repo, + namespace, + interfaces, + interface_suffix, + dpu_cluster_node_labels.as_ref(), + ) + .await?; // Each deployment gets its own service/NAD CRs (suffixed by deployment type) // so BF3 and BF4 do not overwrite each other's Helm values/versions in the @@ -1283,14 +1643,13 @@ async fn create_flavor_services_and_deployment< } } - let deployment_node_labels = labeler.node_labels_for_deployment_type(deployment_type)?; let deployment = build_deployment( services, - deployment_name, + &config.deployment_name, source, &flavor_name, namespace, - &interfaces, + interfaces, deployment_node_labels, deployment_type, ); @@ -1323,6 +1682,18 @@ impl< pub async fn create_initialization_objects( &self, config: &InitDpfResourcesConfig, + ) -> Result<(), DpfError> { + // Keep validation here for split-phase callers that construct the SDK separately. + let resolved = resolve_initialization_inventory(config)?; + self.create_initialization_objects_resolved(config, resolved) + .await + } + + /// Applies initialization resources from state that has already passed pure preflight. + async fn create_initialization_objects_resolved( + &self, + config: &InitDpfResourcesConfig, + resolved: ResolvedInitialization<'_>, ) -> Result<(), DpfError> { let source = match &config.bluefield_software { Some(params) => DpuProvisioningSource::BlueFieldSoftware( @@ -1342,11 +1713,9 @@ impl< &self.namespace, &self.labeler, &services, - &config.deployment_name, &source, - &config.flavor_name, - &config.proxy, - config.deployment_type, + config, + &resolved, ) .await?; @@ -1674,7 +2043,7 @@ impl DpfSdk { .filter(|d| dpu_deployment_is_ready(d)) .filter_map(|d| { let name = d.metadata.name.as_deref()?; - Some((format!("{}_{}", self.namespace, name), d)) + Some((dpu_deployment_owner_label_value(&self.namespace, name), d)) }) .collect(); @@ -2206,7 +2575,419 @@ mod tests { use crate::repository::{ DpuDeviceRepository, DpuFlavorRepository, DpuNodeRepository, DpuRepository, }; - use crate::types::{DpuDeviceInfo, DpuNodeInfo}; + use crate::types::{ + DpfInterceptBridge, DpfInterceptBridging, DpfInterfaceIdentity, DpfProxyDetails, + DpuDeviceInfo, DpuNodeInfo, + }; + + /// Verifies scoped ServiceInterface names distinguish every deployment class. + #[test] + fn service_interface_suffixes_cover_all_deployment_types() { + value_scenarios!( + run = service_interface_cr_suffix; + "BF3" { + // BF3 is suffixed too because scoped mode is an explicit namespace-wide migration. + DpuDeploymentType::Bf3 => "bf3", + } + + "generic BF4" { + // Generic BF4 must not share interface resources with BF3 or Astra. + DpuDeploymentType::Bf4Generic => "bf4", + } + + "BF4 Astra" { + // Astra's BF4+CX9 inventory remains isolated from generic BF4. + DpuDeploymentType::Bf4Astra => "astra", + } + ); + } + + /// Verifies controller selectors are confined to the explicit deployment-scoping migration. + #[test] + fn pf_and_vf_nic_selectors_follow_interface_scope() { + let interfaces = build_dpu_interfaces_vec(); + let dpu_cluster_node_labels = + BTreeMap::from([("deployment".to_string(), "bf3".to_string())]); + let controller_number = |interface: &DPUServiceInterface| { + let spec = &interface.spec.template.spec.template.spec; + spec.pf + .as_ref() + .and_then(|pf| pf.nic_selector.as_ref()) + .and_then(|selector| selector.controller_number) + .or_else(|| { + spec.vf + .as_ref() + .and_then(|vf| vf.nic_selector.as_ref()) + .and_then(|selector| selector.controller_number) + }) + }; + + for interface_name in ["pf0hpf", "pf0vf0"] { + let definition = interfaces + .iter() + .find(|interface| interface.name == interface_name) + .expect("static PF/VF definition must exist"); + + // The public legacy builder must retain its byte-compatible absent selector. + assert_eq!( + controller_number(&build_service_interface(definition, TEST_NAMESPACE)), + None, + ); + + // Scoped resources explicitly select DPU controller 1. + assert_eq!( + controller_number(&build_service_interface_with_scope( + definition, + TEST_NAMESPACE, + "bf3", + Some(&dpu_cluster_node_labels), + )), + Some(1), + ); + } + } + + /// Counts effective VFs and per-service endpoints in one interface inventory. + fn interface_counts( + interfaces: &[DpuServiceInterfaceTemplateDefinition], + ) -> (usize, usize, usize, usize, usize) { + let endpoint_count = |service_name: &str| { + interfaces + .iter() + .filter(|interface| { + interface.chained_svc_if.as_ref().is_some_and(|chains| { + chains.iter().any(|(service, _)| service == service_name) + }) + }) + .count() + }; + ( + interfaces + .iter() + .filter(|interface| { + matches!(&interface.iface_type, DpuServiceInterfaceTemplateType::Vf) + }) + .count(), + interfaces.len(), + endpoint_count(DOCA_HBN_SERVICE_NAME), + endpoint_count(DHCP_SERVER_SERVICE_NAME), + endpoint_count(FMDS_SERVICE_NAME), + ) + } + + /// Provides a validated configured topology for effective-inventory tests. + fn configured_topology() -> DpfInterceptBridging { + DpfInterceptBridging::new( + vec![ + DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: Some(4), + }, + "br-vf4", + "p-vf4", + ), + DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: None, + }, + "br-pf3", + "p-pf3", + ), + ], + 16, + ) + .expect("configured inventory fixture must be valid") + } + + /// Provides BF3 initialization inputs for flavor persistence and conflict tests. + fn flavor_test_config(proxy: Option) -> InitDpfResourcesConfig { + InitDpfResourcesConfig { + proxy, + ..Default::default() + } + } + + /// Verifies static inventory filtering pins minimum, default, and maximum counts. + #[test] + fn effective_static_inventory_follows_provisioned_vf_count() { + value_scenarios!( + run = |num_of_vfs| interface_counts(&build_effective_dpu_interfaces(num_of_vfs, None)); + "no provisioned VFs" { + // Fixed physical and PF entries remain when hardware exposes no VFs. + 0 => (0, 4, 4, 1, 1), + } + + "default VF population" { + // The full historical pf0vf0 through pf0vf13 inventory is retained. + 16 => (14, 18, 18, 9, 1), + } + + "maximum VF population" { + // Hardware VFs above pf0vf13 remain intentionally unclaimed in static mode. + 126 => (14, 18, 18, 9, 1), + } + ); + } + + /// Verifies configured intercept bridging is the complete PF/VF inventory and service policy. + #[test] + fn effective_configured_inventory_replaces_static_pf_and_vf_entries() { + // Build one configured PF and one configured VF outside the historical static identities. + let topology = configured_topology(); + let interfaces = build_effective_dpu_interfaces(16, Some(&topology)); + + // Only p0, p1, and the two configured Patch interfaces remain. + assert_eq!(interface_counts(&interfaces), (0, 4, 4, 2, 1)); + assert_eq!( + interfaces + .iter() + .map(|interface| interface.name.as_str()) + .collect::>(), + ["p0", "p1", "c2pf3", "c2pf3vf4"] + ); + assert!(interfaces[2..].iter().all(|interface| matches!( + &interface.iface_type, + DpuServiceInterfaceTemplateType::Patch(_) + ))); + + // Deployment switches select the exact same interface labels. + let services = [ + DOCA_HBN_SERVICE_NAME, + DHCP_SERVER_SERVICE_NAME, + FMDS_SERVICE_NAME, + ] + .into_iter() + .map(|name| ServiceDefinition::new(name, "repo", "chart", "1")) + .collect::>(); + let deployment = build_deployment( + &services, + "deployment", + &DpuProvisioningSource::Bfb("bfb".to_string()), + "flavor", + TEST_NAMESPACE, + &interfaces, + BTreeMap::new(), + DpuDeploymentType::Bf3, + ); + let switches = deployment.spec.service_chains.unwrap().switches; + assert_eq!(switches.len(), interfaces.len()); + assert_eq!( + switches + .iter() + .map(|switch| { + switch.ports[0] + .service_interface + .as_ref() + .unwrap() + .match_labels["interface"] + .as_str() + }) + .collect::>(), + ["p0", "p1", "c2pf3", "c2pf3vf4"] + ); + } + + /// Verifies configured inventories add their exact service endpoint population to the reserved + /// SF capacity while inventory-free deployments retain their legacy total. + #[test] + fn pf_total_sf_follows_effective_inventory_and_compatibility_mode() { + // Build both meaningful inventory modes from the same production projection path. + let static_interfaces = build_effective_dpu_interfaces(16, None); + let configured_topology = DpfInterceptBridging::new( + std::iter::once(DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: None, + }, + "br-pf3", + "p-pf3", + )) + .chain((0..=15).map(|vf_id| { + DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: Some(vf_id), + }, + format!("br-vf{vf_id}"), + format!("p-vf{vf_id}"), + ) + })) + .collect(), + 16, + ) + .expect("one PF and sixteen VFs must be a valid configured topology"); + let configured_interfaces = build_effective_dpu_interfaces(16, Some(&configured_topology)); + + // The configured cases count every HBN, DHCP, and FMDS endpoint, including fixed uplinks. + value_scenarios!( + run = |(interfaces, intercept_bridging)| { + calculate_pf_total_sf( + interfaces, + intercept_bridging, + DEFAULT_PF_TOTAL_SF_RESERVED, + ) + .unwrap() + }; + "legacy static inventory" { + // Static endpoints are intentionally not added because doing so would reprovision existing DPUs. + (&static_interfaces, None) => 30, + } + + "configured PF and sixteen VF inventory" { + // Two fixed HBN, three PF, and two endpoints per VF consume 37 managed SFs. + (&configured_interfaces, Some(&configured_topology)) => 67, + } + ); + + // The maximum supported topology remains below HBN's 32-interface boundary. + assert_eq!(interface_counts(&configured_interfaces), (0, 19, 19, 17, 1)); + } + + /// Verifies invalid SF arithmetic is rejected before it can become a wrapped NVConfig value. + #[test] + fn pf_total_sf_rejects_overflow() { + // Any configured endpoint added to the maximum reserve must overflow. + let topology = configured_topology(); + let interfaces = build_effective_dpu_interfaces(16, Some(&topology)); + + // Configuration failure is preferable to emitting an unusable DPUFlavor. + assert!(matches!( + calculate_pf_total_sf(&interfaces, Some(&topology), u32::MAX), + Err(DpfError::ConfigError(_)) + )); + } + + /// Verifies custom SDK inventories cannot exceed HBN's interface capacity. + #[test] + fn topology_rejects_more_than_thirty_two_hbn_interfaces() { + // A valid topology selects endpoint-derived sizing; custom callers may supply their own + // rendered interface vector, so the guard must validate that vector directly. + let topology = configured_topology(); + let interfaces = vec![DpuServiceInterfaceTemplateDefinition { + name: "oversized-hbn".to_string(), + iface_type: DpuServiceInterfaceTemplateType::Physical, + pf_id: 0, + vf_id: 0, + chained_svc_if: Some( + (0..=MAX_HBN_SERVICE_INTERFACES) + .map(|index| (DOCA_HBN_SERVICE_NAME.to_string(), format!("hbn{index}_if"))) + .collect(), + ), + }]; + + // Rejecting during pure capacity resolution keeps the invalid inventory out of Kubernetes. + assert!(matches!( + calculate_pf_total_sf( + &interfaces, + Some(&topology), + DEFAULT_PF_TOTAL_SF_RESERVED, + ), + Err(DpfError::ConfigError(message)) if message.contains("exceeding the supported maximum of 32") + )); + } + + /// Verifies a topology cannot be initialized with a different VF population than the one + /// against which its identities and provisioned representor names were validated. + #[test] + fn initialization_rejects_mismatched_topology_vf_count() { + // The shared configured topology is validated for the default population of 16 VFs. + let config = InitDpfResourcesConfig { + num_of_vfs: 8, + intercept_bridging: Some(configured_topology()), + ..Default::default() + }; + + // Pure preflight must reject the mismatch before any repository write is possible. + assert!(matches!( + resolve_initialization_inventory(&config), + Err(DpfError::ConfigError(message)) + if message.contains("validated for num_of_vfs=16") + && message.contains("requested num_of_vfs=8") + )); + } + + /// Verifies a caller-provided inventory cannot diverge from its authoritative topology. + #[test] + fn initialization_rejects_mismatched_topology_interface_projection() { + let topology = configured_topology(); + let mut interfaces = + build_effective_dpu_interfaces(crate::DEFAULT_DPU_NUM_OF_VFS, Some(&topology)); + interfaces[1].name = "unexpected".to_string(); + let config = InitDpfResourcesConfig { + intercept_bridging: Some(topology), + // A non-empty custom inventory previously bypassed projection and could diverge from + // the Patch-backed HBN endpoints used to generate the DHCP ACL. + interfaces, + ..Default::default() + }; + + assert!(matches!( + resolve_initialization_inventory(&config), + Err(DpfError::ConfigError(message)) + if message.contains("first differing interface: received unexpected, expected p1") + && message.contains("received 4 interfaces, expected 4") + )); + } + + /// Verifies callers may still provide the canonical topology projection explicitly. + #[test] + fn initialization_accepts_matching_topology_interface_projection() { + let topology = configured_topology(); + let interfaces = + build_effective_dpu_interfaces(crate::DEFAULT_DPU_NUM_OF_VFS, Some(&topology)); + let config = InitDpfResourcesConfig { + intercept_bridging: Some(topology), + interfaces: interfaces.clone(), + ..Default::default() + }; + + let resolved = resolve_initialization_inventory(&config) + .expect("canonical custom topology projection must be accepted"); + assert_eq!(resolved.interfaces.as_ref(), interfaces.as_slice()); + } + + /// Verifies the public initialization boundary rejects unsupported hardware VF populations. + #[test] + fn initialization_rejects_hardware_vf_count_above_platform_limit() { + // Direct SDK callers do not pass through api-core configuration deserialization. + let config = InitDpfResourcesConfig { + num_of_vfs: MAX_BLUEFIELD_VFS_PER_PF + 1, + ..Default::default() + }; + + // Pure preflight runs before the SDK writes its shared BMC Secret. + assert!(matches!( + resolve_initialization_inventory(&config), + Err(DpfError::ConfigError(message)) if message.contains("num_of_vfs must be <= 126") + )); + } + + /// Verifies Patch CR serialization follows the installed controller contract. + #[test] + fn configured_interface_serializes_one_dpf_owned_patch() { + // Select the configured PF from the shared effective inventory. + let topology = configured_topology(); + let interfaces = build_effective_dpu_interfaces(16, Some(&topology)); + let configured_pf = interfaces + .iter() + .find(|interface| interface.name == "c2pf3") + .expect("configured PF interface must exist"); + + // Patch serialization owns only the peer pair and omits caller-provided metadata. + let cr = build_service_interface(configured_pf, TEST_NAMESPACE); + let spec = &cr.spec.template.spec.template.spec; + let patch = spec.patch.as_ref().expect("Patch definition must be set"); + assert_eq!(patch.peer_bridge, "br-pf3"); + assert_eq!(patch.peer_patch_name.as_deref(), Some("p-pf3")); + assert!(patch.peer_external_i_ds.is_none()); + assert!(spec.pf.is_none() && spec.vf.is_none() && spec.physical.is_none()); + } fn already_exists_error(name: &str) -> DpfError { DpfError::KubeError(kube::Error::Api(Box::new( @@ -3503,15 +4284,11 @@ mod tests { #[tokio::test] async fn test_create_dpu_flavor_fresh() { let mock = SdkMock::new(); - let name = create_dpu_flavor( - &mock, - TEST_NAMESPACE, - crate::flavor::DEFAULT_FLAVOR_NAME, - &None, - DpuDeploymentType::Bf3, - ) - .await - .unwrap(); + let config = flavor_test_config(None); + let resolved = resolve_initialization_inventory(&config).unwrap(); + let name = create_dpu_flavor(&mock, TEST_NAMESPACE, &config, &resolved) + .await + .unwrap(); // Returned name should have the expected "-" shape. assert!( @@ -3533,22 +4310,16 @@ mod tests { #[tokio::test] async fn test_create_dpu_flavor_fresh_with_proxy() { - use crate::types::DpfProxyDetails; let mock = SdkMock::new(); let proxy = Some(DpfProxyDetails { https_proxy: "http://proxy.corp:3128".to_string(), no_proxy: vec!["10.0.0.0/8".to_string()], }); - - let name_with_proxy = create_dpu_flavor( - &mock, - TEST_NAMESPACE, - crate::flavor::DEFAULT_FLAVOR_NAME, - &proxy, - DpuDeploymentType::Bf3, - ) - .await - .unwrap(); + let config = flavor_test_config(proxy); + let resolved = resolve_initialization_inventory(&config).unwrap(); + let name_with_proxy = create_dpu_flavor(&mock, TEST_NAMESPACE, &config, &resolved) + .await + .unwrap(); // Proxy flavor must get a different hash than the no-proxy flavor. let name_no_proxy = { @@ -3592,15 +4363,11 @@ mod tests { } let mock = AlwaysConflictsMock::default(); - let err = create_dpu_flavor( - &mock, - TEST_NAMESPACE, - crate::flavor::DEFAULT_FLAVOR_NAME, - &None, - DpuDeploymentType::Bf3, - ) - .await - .unwrap_err(); + let config = flavor_test_config(None); + let resolved = resolve_initialization_inventory(&config).unwrap(); + let err = create_dpu_flavor(&mock, TEST_NAMESPACE, &config, &resolved) + .await + .unwrap_err(); assert!( matches!(err, DpfError::InvalidState(_)), @@ -3624,15 +4391,11 @@ mod tests { .unwrap() .insert(SdkMock::key(&terminating_flavor), terminating_flavor); - let err = create_dpu_flavor( - &mock, - TEST_NAMESPACE, - crate::flavor::DEFAULT_FLAVOR_NAME, - &None, - DpuDeploymentType::Bf3, - ) - .await - .unwrap_err(); + let config = flavor_test_config(None); + let resolved = resolve_initialization_inventory(&config).unwrap(); + let err = create_dpu_flavor(&mock, TEST_NAMESPACE, &config, &resolved) + .await + .unwrap_err(); assert!( matches!(err, DpfError::InvalidState(_)), "expected InvalidState, got: {err:?}" @@ -3656,15 +4419,11 @@ mod tests { .insert(SdkMock::key(&flavor), flavor); let expected_name = flavor_name.clone(); - let returned = create_dpu_flavor( - &mock, - TEST_NAMESPACE, - crate::flavor::DEFAULT_FLAVOR_NAME, - &None, - DpuDeploymentType::Bf3, - ) - .await - .unwrap(); + let config = flavor_test_config(None); + let resolved = resolve_initialization_inventory(&config).unwrap(); + let returned = create_dpu_flavor(&mock, TEST_NAMESPACE, &config, &resolved) + .await + .unwrap(); // The whole contract of this branch is "reuse what's already there". `.unwrap()` // only proved it didn't error -- so check it hands back the existing flavor's name diff --git a/crates/dpf/src/test/sdk_initialization.rs b/crates/dpf/src/test/sdk_initialization.rs index 834e4ab513..cbbdc0538a 100644 --- a/crates/dpf/src/test/sdk_initialization.rs +++ b/crates/dpf/src/test/sdk_initialization.rs @@ -17,7 +17,7 @@ //! Tests for DPF SDK initialization resources and lookup behavior. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::future::Future; use std::sync::Arc; @@ -41,6 +41,7 @@ use crate::repository::{ DpuServiceInterfaceRepository, DpuServiceNADRepository, DpuServiceTemplateRepository, K8sConfigRepository, }; +use crate::sdk::ResourceLabeler; use crate::types::*; const TEST_NS: &str = "sdk-init-ns"; @@ -72,6 +73,85 @@ struct InitializationMock { secrets: Arc>>>, } +/// Supplies deterministic deployment labels so scoped-interface tests verify isolation without +/// depending on production label construction. +#[derive(Clone, Copy)] +struct InitializationLabeler; + +impl ResourceLabeler for InitializationLabeler { + fn node_labels_for_deployment_type( + &self, + deployment_type: DpuDeploymentType, + ) -> Result, DpfError> { + let deployment_label = match deployment_type { + DpuDeploymentType::Bf3 => "test.nvidia.com/bf3", + DpuDeploymentType::Bf4Generic => "test.nvidia.com/bf4", + DpuDeploymentType::Bf4Astra => "test.nvidia.com/astra", + }; + Ok(BTreeMap::from([ + ( + "feature.node.kubernetes.io/dpu-enabled".to_string(), + "true".to_string(), + ), + (deployment_label.to_string(), "true".to_string()), + ])) + } +} + +/// Provides one selected PF and VF so initialization tests can verify that every generated +/// resource consumes the same normalized intercept topology. +fn configured_intercept_bridging() -> DpfInterceptBridging { + DpfInterceptBridging::new( + vec![ + DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: None, + }, + "br-pf3", + "p-pf3", + ), + DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id: 2, + pf_id: 3, + vf_id: Some(4), + }, + "br-vf4", + "p-vf4", + ), + ], + 16, + ) + .expect("configured intercept topology must be valid") +} + +/// Provides an otherwise valid Astra configuration that isolates the scoping invariant. +fn unscoped_astra_config() -> InitDpfResourcesConfig { + InitDpfResourcesConfig { + bluefield_software: Some(BlueFieldSoftwareParams { + os_iso: "http://example.com/astra.iso".to_string(), + pldm_fw_bundle: Some("http://example.com/astra.pldm".to_string()), + }), + deployment_name: "astra-deployment".to_string(), + deployment_type: DpuDeploymentType::Bf4Astra, + ..Default::default() + } +} + +/// Asserts every DPF initialization CR store is empty so both public paths prove no-write safety. +fn assert_no_initialization_crs(mock: &InitializationMock) { + assert!(mock.bfbs.is_empty()); + assert!(mock.bluefield_softwares.is_empty()); + assert!(mock.flavors.is_empty()); + assert!(mock.deployments.is_empty()); + assert!(mock.service_templates.is_empty()); + assert!(mock.service_configs.is_empty()); + assert!(mock.nads.is_empty()); + assert!(mock.service_interfaces.is_empty()); +} + #[async_trait] impl BfbRepository for InitializationMock { async fn get(&self, name: &str, ns: &str) -> Result, DpfError> { @@ -389,6 +469,19 @@ async fn test_create_initialization_objects() { .unwrap(); assert!(deployment.is_some()); + // The default migration mode preserves legacy names and the absent node selector. + let p0 = DpuServiceInterfaceRepository::get(&mock, "p0", TEST_NS) + .await + .unwrap() + .expect("legacy p0 ServiceInterface must exist"); + assert!(p0.spec.template.spec.node_selector.is_none()); + assert!( + DpuServiceInterfaceRepository::get(&mock, "p0-bf3", TEST_NS) + .await + .unwrap() + .is_none() + ); + let secret = K8sConfigRepository::get_secret(&mock, "bmc-shared-password", TEST_NS) .await .unwrap(); @@ -397,6 +490,84 @@ async fn test_create_initialization_objects() { drop(sdk); } +/// Verifies SF overflow fails before the builder writes its BMC Secret or any DPF CR because +/// invalid capacity must not leave a partially initialized DPF namespace. +#[tokio::test] +async fn sf_overflow_fails_before_initialization_writes() { + // Use a valid configured inventory so the SF sum fails only at the arithmetic boundary. + let mock = InitializationMock::default(); + let config = InitDpfResourcesConfig { + intercept_bridging: Some(configured_intercept_bridging()), + pf_total_sf_reserved: u32::MAX, + ..Default::default() + }; + + // Preflight must reject the configuration before the first initialization write. + let result = crate::sdk::DpfSdkBuilder::new(mock.clone(), TEST_NS, "test-password".to_string()) + .initialize(&config) + .await; + assert!(matches!(result, Err(DpfError::ConfigError(_)))); + assert!(mock.secrets.is_empty()); + assert!(mock.bfbs.is_empty()); + assert!(mock.flavors.is_empty()); + assert!(mock.deployments.is_empty()); +} + +/// Verifies one-shot Astra initialization rejects global interfaces before its first write. +#[tokio::test] +async fn unscoped_astra_builder_initialization_writes_nothing() { + // Attempt an otherwise valid Astra initialization through the public builder. + let mock = InitializationMock::default(); + let result = crate::sdk::DpfSdkBuilder::new(mock.clone(), TEST_NS, "test-password".to_string()) + .with_labeler(InitializationLabeler) + .initialize(&unscoped_astra_config()) + .await; + + // Pure preflight must reject before the builder writes even its shared Secret. + let Err(error) = result else { + panic!("unscoped Astra initialization must fail"); + }; + assert!(matches!(&error, DpfError::ConfigError(_))); + assert!( + error + .to_string() + .contains("BF4 Astra requires deployment_scoped_service_interfaces=true") + ); + assert!(mock.secrets.is_empty()); + assert_no_initialization_crs(&mock); +} + +/// Verifies split-phase Astra initialization preserves its existing Secret and writes no CRs. +#[tokio::test] +async fn unscoped_astra_split_initialization_writes_no_resources() { + // Build the SDK first, establishing the split-phase path's expected Secret baseline. + let mock = InitializationMock::default(); + let sdk = crate::sdk::DpfSdkBuilder::new(mock.clone(), TEST_NS, "test-password".to_string()) + .with_labeler(InitializationLabeler) + .build_without_resources() + .await + .unwrap(); + let secret_before = K8sConfigRepository::get_secret(&mock, "bmc-shared-password", TEST_NS) + .await + .unwrap() + .expect("split-phase SDK construction must write its shared Secret"); + + // Supplying unsafe Astra configuration must perform no subsequent initialization write. + let error = sdk + .create_initialization_objects(&unscoped_astra_config()) + .await + .expect_err("unscoped Astra initialization must fail"); + assert!(matches!(error, DpfError::ConfigError(_))); + assert_eq!( + K8sConfigRepository::get_secret(&mock, "bmc-shared-password", TEST_NS) + .await + .unwrap() + .expect("scoping rejection must preserve the shared Secret"), + secret_before + ); + assert_no_initialization_crs(&mock); +} + #[tokio::test] async fn test_create_initialization_objects_bluefield_software() { let mock = InitializationMock::default(); @@ -446,6 +617,426 @@ async fn test_create_initialization_objects_bluefield_software() { drop(sdk); } +/// Verifies configured BF3 and generic BF4 coexist with scoped Astra while flavors, Patch +/// interfaces, service chains, and selectors retain one deployment-specific inventory view. +#[tokio::test] +async fn scoped_bf3_bf4_and_astra_initialization_coexists() { + // Build one SDK so all three deployment classes share the production namespace. + let mock = InitializationMock::default(); + let sdk = crate::sdk::DpfSdkBuilder::new(mock.clone(), TEST_NS, "test-password".to_string()) + .with_labeler(InitializationLabeler) + .build_without_resources() + .await + .unwrap(); + + // Supply every mandatory logical service so deployment references mirror production setup. + let services = [ + DTS_SERVICE_NAME, + DOCA_HBN_SERVICE_NAME, + DPU_AGENT_SERVICE_NAME, + DHCP_SERVER_SERVICE_NAME, + FMDS_SERVICE_NAME, + OTEL_COLLECTOR_SERVICE_NAME, + ] + .into_iter() + .map(|name| ServiceDefinition::new(name, "repo", "chart", "1.0.0")) + .collect::>(); + let topology = configured_intercept_bridging(); + let configs = [ + // BF3 proves the configured topology through the BFB flavor path. + InitDpfResourcesConfig { + bfb_url: "http://example.com/bf3.bfb".to_string(), + deployment_name: "bf3-deployment".to_string(), + flavor_name: "bf3-flavor".to_string(), + services: services.clone(), + deployment_scoped_service_interfaces: true, + intercept_bridging: Some(topology.clone()), + deployment_type: DpuDeploymentType::Bf3, + ..Default::default() + }, + // Generic BF4 proves the same topology through its BlueFieldSoftware path. + InitDpfResourcesConfig { + bluefield_software: Some(BlueFieldSoftwareParams { + os_iso: "http://example.com/bf4.iso".to_string(), + pldm_fw_bundle: Some("http://example.com/bf4.pldm".to_string()), + }), + deployment_name: "bf4-deployment".to_string(), + flavor_name: "bf4-flavor".to_string(), + services: services.clone(), + deployment_scoped_service_interfaces: true, + intercept_bridging: Some(topology), + deployment_type: DpuDeploymentType::Bf4Generic, + ..Default::default() + }, + // Astra proves its fixed BF4+CX9 inventory remains isolated from both configured classes. + InitDpfResourcesConfig { + bluefield_software: Some(BlueFieldSoftwareParams { + os_iso: "http://example.com/astra.iso".to_string(), + pldm_fw_bundle: Some("http://example.com/astra.pldm".to_string()), + }), + deployment_name: "astra-deployment".to_string(), + flavor_name: "astra-flavor".to_string(), + services, + deployment_scoped_service_interfaces: true, + deployment_type: DpuDeploymentType::Bf4Astra, + ..Default::default() + }, + ]; + + // Apply every class through the public split-initialization path used by multi-deployment setup. + for config in &configs { + sdk.create_initialization_objects(config).await.unwrap(); + } + + // All immutable flavors and deployments must coexist without overwrite. + assert_eq!( + DpuDeploymentRepository::list(&mock, TEST_NS) + .await + .unwrap() + .len(), + 3 + ); + assert_eq!(mock.flavors.len(), 3); + + // The effective inventories must produce exact, non-overlapping scoped resource names. + let interfaces = DpuServiceInterfaceRepository::list(&mock, TEST_NS) + .await + .unwrap(); + let interface_names = interfaces + .iter() + .map(|interface| interface.metadata.name.clone().unwrap()) + .collect::>(); + let mut expected_interface_names = BTreeSet::new(); + for suffix in ["bf3", "bf4"] { + for logical_name in ["p0", "p1", "c2pf3", "c2pf3vf4"] { + expected_interface_names.insert(format!("{logical_name}-{suffix}")); + } + } + let mut astra_logical_names = ["p0", "p1", "pf0hpf", "pf1hpf"] + .into_iter() + .map(|name| name.to_string()) + .collect::>(); + astra_logical_names.extend((0..14).map(|vf_id| format!("pf0vf{vf_id}"))); + expected_interface_names.extend( + astra_logical_names + .iter() + .map(|logical_name| format!("{logical_name}-astra")), + ); + assert_eq!(interface_names, expected_interface_names); + + // Each interface group must select the remote DPU Node by DPF deployment ownership. The + // management-plane class labels used by DPUNode selectors do not exist in the DPU cluster. + for (suffix, deployment_name) in [ + ("bf3", "bf3-deployment"), + ("bf4", "bf4-deployment"), + ("astra", "astra-deployment"), + ] { + let expected_labels = BTreeMap::from([( + "svc.dpu.nvidia.com/owned-by-dpudeployment".to_string(), + format!("{TEST_NS}_{deployment_name}"), + )]); + let resource_suffix = format!("-{suffix}"); + let scoped_interfaces = interfaces.iter().filter(|interface| { + interface + .metadata + .name + .as_deref() + .is_some_and(|name| name.ends_with(&resource_suffix)) + }); + for interface in scoped_interfaces { + assert_eq!( + interface + .spec + .template + .spec + .node_selector + .as_ref() + .and_then(|selector| selector.match_labels.as_ref()), + Some(&expected_labels) + ); + } + } + + // Both configured classes must serialize the same exact DPF-owned Patch pairs. + for suffix in ["bf3", "bf4"] { + for (logical_name, peer_bridge, peer_patch_name) in [ + ("c2pf3", "br-pf3", "p-pf3"), + ("c2pf3vf4", "br-vf4", "p-vf4"), + ] { + let resource_name = format!("{logical_name}-{suffix}"); + let interface = interfaces + .iter() + .find(|interface| { + interface.metadata.name.as_deref() == Some(resource_name.as_str()) + }) + .expect("configured scoped Patch interface must exist"); + let logical_label = interface + .spec + .template + .spec + .template + .metadata + .as_ref() + .and_then(|metadata| metadata.labels.as_ref()) + .and_then(|labels| labels.get("interface")); + assert_eq!(logical_label.map(String::as_str), Some(logical_name)); + let patch = interface + .spec + .template + .spec + .template + .spec + .patch + .as_ref() + .expect("configured interface must be Patch-backed"); + assert_eq!(patch.peer_bridge, peer_bridge); + assert_eq!(patch.peer_patch_name.as_deref(), Some(peer_patch_name)); + } + } + + let configured_deployments = [ + // BF3 must render the selected raw PF while consuming the shared topology inventory. + ( + "bf3-deployment", + DpuDeploymentType::Bf3, + "host_representor='pf3hpf'", + ), + // Generic BF4 must resolve the selected PF by its exact semantic identity. + ( + "bf4-deployment", + DpuDeploymentType::Bf4Generic, + "resolve_dpf_pf 'c2pf3'", + ), + ]; + for (deployment_name, deployment_type, platform_marker) in configured_deployments { + // The deployment-referenced flavor must contain the configured topology and SF total. + let deployment = DpuDeploymentRepository::get(&mock, deployment_name, TEST_NS) + .await + .unwrap() + .expect("configured deployment must exist"); + let flavor_name = deployment.spec.dpus.flavor.as_deref().unwrap(); + let flavor = DpuFlavorRepository::get(&mock, flavor_name, TEST_NS) + .await + .unwrap() + .expect("deployment-referenced flavor must exist"); + let nvconfig = flavor.spec.nvconfig.as_ref().unwrap()[0] + .parameters + .as_ref() + .unwrap(); + assert!( + nvconfig + .iter() + .any(|parameter| parameter == "PF_TOTAL_SF=37") + ); + let ovs_script = flavor + .spec + .ovs + .as_ref() + .and_then(|ovs| ovs.raw_config_script.as_ref()) + .unwrap(); + assert!(ovs_script.contains("add-br 'br-pf3'")); + assert!(ovs_script.contains("add-br 'br-vf4'")); + assert!(ovs_script.contains(platform_marker)); + + let expected_labels = InitializationLabeler + .node_labels_for_deployment_type(deployment_type) + .unwrap(); + assert_eq!( + deployment.spec.dpus.dpu_sets.as_ref().unwrap()[0] + .dpu_node_selector + .as_ref() + .and_then(|selector| selector.match_labels.as_ref()), + Some(&expected_labels) + ); + + // Service-chain ports must expose the exact runtime endpoints for the same inventory. + let switches = &deployment.spec.service_chains.as_ref().unwrap().switches; + let chain_interfaces = switches + .iter() + .map(|switch| { + switch.ports[0] + .service_interface + .as_ref() + .unwrap() + .match_labels["interface"] + .as_str() + }) + .collect::>(); + assert_eq!(chain_interfaces, ["p0", "p1", "c2pf3", "c2pf3vf4"]); + let chain_endpoints = switches + .iter() + .flat_map(|switch| { + let interface_name = switch.ports[0] + .service_interface + .as_ref() + .unwrap() + .match_labels["interface"] + .as_str(); + switch + .ports + .iter() + .filter_map(|port| port.service.as_ref()) + .map(move |service| { + ( + interface_name, + service.name.as_str(), + service.interface.as_str(), + ) + }) + }) + .collect::>(); + assert_eq!( + chain_endpoints, + [ + ("p0", DOCA_HBN_SERVICE_NAME, "p0_if"), + ("p1", DOCA_HBN_SERVICE_NAME, "p1_if"), + ("c2pf3", DOCA_HBN_SERVICE_NAME, "pf0hpf_if"), + ("c2pf3", DHCP_SERVER_SERVICE_NAME, "d_pf0hpf_if"), + ("c2pf3", FMDS_SERVICE_NAME, "f_pf0hpf_if"), + ("c2pf3vf4", DOCA_HBN_SERVICE_NAME, "pf0vf4_if"), + ("c2pf3vf4", DHCP_SERVER_SERVICE_NAME, "d_pf0vf4_if"), + ] + ); + } + + // Astra must retain its fixed inventory and flavor without leaking configured Patch state. + let astra = DpuDeploymentRepository::get(&mock, "astra-deployment", TEST_NS) + .await + .unwrap() + .expect("Astra deployment must exist"); + assert_eq!(astra.spec.dpus.astra_enabled, Some(true)); + let expected_astra_labels = InitializationLabeler + .node_labels_for_deployment_type(DpuDeploymentType::Bf4Astra) + .unwrap(); + assert_eq!( + astra.spec.dpus.dpu_sets.as_ref().unwrap()[0] + .dpu_node_selector + .as_ref() + .and_then(|selector| selector.match_labels.as_ref()), + Some(&expected_astra_labels) + ); + let astra_chain_interfaces = astra + .spec + .service_chains + .as_ref() + .unwrap() + .switches + .iter() + .map(|switch| { + switch.ports[0] + .service_interface + .as_ref() + .unwrap() + .match_labels["interface"] + .clone() + }) + .collect::>(); + assert_eq!(astra_chain_interfaces, astra_logical_names); + let astra_flavor = + DpuFlavorRepository::get(&mock, astra.spec.dpus.flavor.as_deref().unwrap(), TEST_NS) + .await + .unwrap() + .expect("Astra deployment-referenced flavor must exist"); + assert!( + astra_flavor.spec.nvconfig.as_ref().unwrap()[0] + .parameters + .as_ref() + .unwrap() + .iter() + .any(|parameter| parameter == "PF_TOTAL_SF=30") + ); + assert!( + !astra_flavor + .spec + .ovs + .as_ref() + .and_then(|ovs| ovs.raw_config_script.as_ref()) + .unwrap() + .contains("br-pf3") + ); + assert!( + interfaces + .iter() + .filter(|interface| interface + .metadata + .name + .as_deref() + .is_some_and(|name| name.ends_with("-astra"))) + .all(|interface| interface.spec.template.spec.template.spec.patch.is_none()) + ); + + drop(sdk); +} + +/// Verifies existing ServiceInterfaces cannot block either initialization mode and remain +/// untouched because names and logical labels are not NICo ownership evidence. +#[tokio::test] +async fn existing_service_interfaces_do_not_block_or_get_deleted() { + let cases = [ + // A matching name and logical label must not be treated as NICo-owned legacy state. + ("vendor-uplink", "vendor-uplink", true), + // A deployment-like suffix must not be treated as NICo-owned scoped state. + ("vendor-uplink-bf3", "vendor-uplink", false), + ]; + + for (existing_name, logical_name, desired_scoped) in cases { + // Seed a foreign-managed resource whose name and inner label resemble one NICo mode. + let mock = InitializationMock::default(); + let mut existing = crate::sdk::build_service_interface( + &crate::sdk::build_dpu_interfaces_vec()[0], + TEST_NS, + ); + existing.metadata.name = Some(existing_name.to_string()); + existing.metadata.labels = Some(BTreeMap::from([( + "app.kubernetes.io/managed-by".to_string(), + "vendor-dpf-operator".to_string(), + )])); + existing + .spec + .template + .spec + .template + .metadata + .as_mut() + .unwrap() + .labels + .as_mut() + .unwrap() + .insert("interface".to_string(), logical_name.to_string()); + let existing_snapshot = serde_json::to_value(&existing).unwrap(); + mock.service_interfaces + .insert(resource_key(&existing), existing); + + // Initialize the opposite shape in both directions through the complete SDK path. + let config = InitDpfResourcesConfig { + bfb_url: "http://example.com/test.bfb".to_string(), + deployment_scoped_service_interfaces: desired_scoped, + ..Default::default() + }; + let sdk = + crate::sdk::DpfSdkBuilder::new(mock.clone(), TEST_NS, "test-password".to_string()) + .with_labeler(InitializationLabeler) + .initialize(&config) + .await + .unwrap(); + + // Initialization must apply its own resources without mutating or pruning the existing one. + assert!(!mock.bfbs.is_empty()); + assert!(!mock.flavors.is_empty()); + assert!(!mock.deployments.is_empty()); + let existing_after = DpuServiceInterfaceRepository::get(&mock, existing_name, TEST_NS) + .await + .unwrap() + .expect("pre-existing ServiceInterface must remain"); + assert_eq!( + serde_json::to_value(existing_after).unwrap(), + existing_snapshot + ); + + drop(sdk); + } +} + /// Verifies a missing referenced template fails the complete inventory lookup /// so callers cannot mistake an incomplete operator view for current state. #[tokio::test] diff --git a/crates/dpf/src/types.rs b/crates/dpf/src/types.rs index 4b218072ad..71e3a138ed 100644 --- a/crates/dpf/src/types.rs +++ b/crates/dpf/src/types.rs @@ -43,6 +43,8 @@ impl BmcPasswordProvider for String { /// Service name constants for use across crates pub const DOCA_HBN_SERVICE_NAME: &str = "doca-hbn"; pub const DHCP_SERVER_SERVICE_NAME: &str = "carbide-dhcp-server"; +/// Shared marker applied to every DPF-managed DPUNode. +pub const DPU_ENABLED_NODE_LABEL: &str = "feature.node.kubernetes.io/dpu-enabled"; pub const FMDS_SERVICE_NAME: &str = "carbide-fmds"; pub const DPU_AGENT_SERVICE_NAME: &str = "carbide-dpu-agent"; pub const OTEL_COLLECTOR_SERVICE_NAME: &str = "carbide-otelcol"; @@ -50,9 +52,19 @@ pub const DTS_SERVICE_NAME: &str = "dts"; pub const DOCA_WEAVE_DHCP_AGENT_SERVICE_NAME: &str = "doca-weave-dhcp-agent"; pub const DOCA_WEAVE_FLOW_CONTROLLER_SERVICE_NAME: &str = "doca-weave-flow-controller"; pub const DOCA_XPLANE_SERVICE_NAME: &str = "doca-xplane"; - -/// Configuration for creating DPF operator resources (BFB, DPUFlavor, -/// DPUDeployment, service templates, etc.) during initialization. +/// Hash-stable legacy VF population used by default DPF flavors and SDK initialization. +pub const DEFAULT_DPU_NUM_OF_VFS: u32 = 16; +/// Default SF capacity reserved beyond configured NICo-managed service endpoints. +pub const DEFAULT_PF_TOTAL_SF_RESERVED: u32 = 30; +// Keep direct SDK validation aligned with api-core's general BlueField provisioning bound without +// coupling this lightweight crate to the complete API configuration model. +pub(crate) const MAX_BLUEFIELD_VFS_PER_PF: u32 = 126; +// Keep topology validation aligned with `model::instance::config::network::INTERFACE_VFID_MAX` +// without adding the complete API model as a DPF SDK dependency. +const MAX_INSTANCE_VF_ID: u8 = 15; + +/// Configuration for creating DPF operator resources (BFB or +/// BlueFieldSoftware, DPUFlavor, DPUDeployment, services, etc.). #[derive(Debug, Clone)] pub struct InitDpfResourcesConfig { /// URL for the BFB (BlueField Bundle) image. Used for BF3-class DPUs. @@ -71,6 +83,25 @@ pub struct InitDpfResourcesConfig { /// When empty, `default_services()` is used automatically. pub services: Vec, + /// Number of hardware VFs provisioned per DPU PF for BF3 and generic BF4. + pub num_of_vfs: u32, + /// SF capacity reserved beyond configured NICo-managed service endpoints. + /// Without intercept bridging, this remains the complete legacy `PF_TOTAL_SF` value. + pub pf_total_sf_reserved: u32, + /// Enables deployment-scoped DPUServiceInterface names and node selectors. + /// False preserves the legacy global resource naming and selector mode for + /// BF3 and generic BF4; BF4 Astra requires this to be true. + /// Mode transitions require manual old-resource cleanup and DPU re-ingestion; + /// the SDK neither detects nor deletes the previous generation. + pub deployment_scoped_service_interfaces: bool, + /// Optional intercept-bridging topology for BF3 and generic BF4. `Some` replaces the + /// ordinary static PF/VF inventory and contains exactly one configured PF. + pub intercept_bridging: Option, + /// Effective interface inventory shared by ServiceInterfaces, service chains, + /// and caller-built service definitions. Empty asks the SDK to build it. With + /// intercept bridging, a non-empty inventory must exactly match the SDK projection. + pub interfaces: Vec, + pub proxy: Option, /// Deployment type — determines which DPUFlavor spec to build. pub deployment_type: DpuDeploymentType, @@ -96,6 +127,11 @@ impl Default for InitDpfResourcesConfig { deployment_name: "dpu-deployment".to_string(), flavor_name: crate::flavor::DEFAULT_FLAVOR_NAME.to_string(), services: Vec::new(), + num_of_vfs: DEFAULT_DPU_NUM_OF_VFS, + pf_total_sf_reserved: DEFAULT_PF_TOTAL_SF_RESERVED, + deployment_scoped_service_interfaces: false, + intercept_bridging: None, + interfaces: Vec::new(), proxy: None, deployment_type: DpuDeploymentType::Bf3, } @@ -138,7 +174,7 @@ pub struct ServiceConfigPort { pub node_port: Option, } -/// Service Network Attachment Definition (NAD) +/// Resource kind allocated by a service Network Attachment Definition (NAD). #[derive(Debug, Clone)] pub enum ServiceNADResourceType { Vf, @@ -193,19 +229,313 @@ pub struct ServiceDefinition { pub service_nad: Option, } -/// Service Network Attachment Definition (NAD) -#[derive(Debug, Clone)] +/// Interface kind rendered into a DPUServiceInterface template. +#[derive(Debug, Clone, PartialEq, Eq)] pub enum DpuServiceInterfaceTemplateType { - Vlan, Physical, Pf, Vf, - Ovn, - Service, + Patch(DpuServiceInterfacePatch), +} + +/// Typed identity of a PF or VF selected for DPF intercept bridging topology. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub struct DpfInterfaceIdentity { + pub controller_id: u8, + pub pf_id: u8, + pub vf_id: Option, +} + +impl DpfInterfaceIdentity { + /// Returns the deterministic DPF resource name for this identity. + pub fn resource_name(self) -> String { + match self.vf_id { + Some(vf_id) => format!("c{}pf{}vf{vf_id}", self.controller_id, self.pf_id), + None => format!("c{}pf{}", self.controller_id, self.pf_id), + } + } + + /// Returns the canonical service-local endpoint stem. + /// + /// Configured topology may select any hardware controller/PF, but NICo's + /// HBN, DHCP, and FMDS runtime contract supports one selected PF. Hardware + /// identity remains in DPF resource names and flavor topology; service + /// containers consistently expose that selected parent as logical PF0. + pub fn service_interface_stem(self) -> String { + match self.vf_id { + Some(vf_id) => format!("pf0vf{vf_id}"), + None => "pf0hpf".to_string(), + } + } + + /// Returns the semantic generic-BF4 PF identity exposed by `phys_port_name`. + pub fn bf4_phys_port_name(self) -> String { + format!("c{}pf{}", self.controller_id, self.pf_id) + } + + /// Returns the BF3 raw representor name selected by this identity. + pub fn bf3_raw_netdev_name(self) -> String { + match self.vf_id { + Some(vf_id) => format!("pf{}vf{vf_id}", self.pf_id), + None => format!("pf{}hpf", self.pf_id), + } + } +} + +/// One normalized intercept-bridging interface consumed by DPF resource generation. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DpfInterceptBridge { + pub(crate) identity: DpfInterfaceIdentity, + pub(crate) bridge: String, + pub(crate) patch_port: String, +} + +impl DpfInterceptBridge { + /// Builds one normalized entry without retaining its legacy configuration key. + pub fn new( + identity: DpfInterfaceIdentity, + bridge: impl Into, + patch_port: impl Into, + ) -> Self { + Self { + identity, + bridge: bridge.into(), + patch_port: patch_port.into(), + } + } +} + +/// Deterministic, validated intercept-bridging topology shared by BF3 and generic BF4. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DpfInterceptBridging { + interfaces: Vec, + num_of_vfs: u32, +} + +impl DpfInterceptBridging { + /// Requires one PF, validates configured identities and rendered names, then sorts by identity. + pub fn new( + mut interfaces: Vec, + num_of_vfs: u32, + ) -> Result { + if num_of_vfs > MAX_BLUEFIELD_VFS_PER_PF { + return Err(crate::DpfError::ConfigError(format!( + "DPF num_of_vfs must be <= {MAX_BLUEFIELD_VFS_PER_PF}" + ))); + } + + // Canonical order makes every downstream resource and flavor hash deterministic. + interfaces.sort_by_key(|interface| interface.identity); + + // FMDS serves one logical PF endpoint, so every configured replacement inventory must + // include the selected PF. Existing validation then guarantees that PF is unique. + if !interfaces + .iter() + .any(|interface| interface.identity.vf_id.is_none()) + { + return Err(crate::DpfError::ConfigError( + "DPF intercept bridging requires exactly one configured PF interface".to_string(), + )); + } + + // Track generated names in each namespace while validating every configured entry. + let mut identities = std::collections::BTreeSet::new(); + let mut resource_names = std::collections::BTreeSet::new(); + let mut hbn_interface_names = std::collections::BTreeSet::new(); + let mut dhcp_interface_names = std::collections::BTreeSet::new(); + let mut fmds_interface_names = std::collections::BTreeSet::new(); + let mut ovs_names = BTreeMap::from([ + ("br-sfc".to_string(), "DPF service-chain bridge".to_string()), + ("br-hbn".to_string(), "DPF HBN bridge".to_string()), + ("p0".to_string(), "fixed physical interface".to_string()), + ("p1".to_string(), "fixed physical interface".to_string()), + ]); + + let selected_parents = interfaces + .iter() + .map(|interface| (interface.identity.controller_id, interface.identity.pf_id)) + .collect::>(); + + // Runtime service configuration addresses one logical PF. The typed identity still + // selects which hardware controller/PF becomes that logical PF0. + if selected_parents.len() > 1 { + return Err(crate::DpfError::ConfigError( + "DPF intercept bridging supports one selected controller/PF parent".to_string(), + )); + } + let &(_, selected_pf_id) = selected_parents + .first() + .expect("the required PF interface supplies one selected parent"); + + for interface in &interfaces { + let identity = interface.identity; + let resource_name = identity.resource_name(); + + // A duplicate identity would otherwise render the same Kubernetes and service names. + if !identities.insert(identity) { + return Err(crate::DpfError::ConfigError(format!( + "duplicate DPF intercept-bridging interface identity {resource_name}" + ))); + } + + // PF selections are independent of the provisioned VF population. Topology VFs are + // exclusively VM instance endpoints, so their identity is bounded by both hardware + // provisioning and the instance network model. + if let Some(vf_id) = identity.vf_id { + if vf_id > MAX_INSTANCE_VF_ID { + return Err(crate::DpfError::ConfigError(format!( + "DPF intercept-bridging interface {resource_name} selects VF {vf_id}, but instance networking supports at most VF{MAX_INSTANCE_VF_ID}" + ))); + } + if u32::from(vf_id) >= num_of_vfs { + return Err(crate::DpfError::ConfigError(format!( + "DPF intercept-bridging interface {resource_name} selects a VF outside num_of_vfs={num_of_vfs}" + ))); + } + } + + validate_linux_netdev_name(&interface.bridge, "bridge", &resource_name)?; + validate_ovs_patch_name(&interface.patch_port, &resource_name)?; + + // Resource names are derived from typed numeric identity, but retain an explicit + // collision gate so future naming changes cannot silently alias resources. + if !resource_names.insert(resource_name.clone()) { + return Err(crate::DpfError::ConfigError(format!( + "generated DPF intercept-bridging resource name {resource_name} is not unique" + ))); + } + + let service_name = identity.service_interface_stem(); + for (name, names) in [ + (format!("{service_name}_if"), &mut hbn_interface_names), + (format!("d_{service_name}_if"), &mut dhcp_interface_names), + ] { + if name.len() > 15 || !names.insert(name.clone()) { + return Err(crate::DpfError::ConfigError(format!( + "generated DPF service interface name {name} is invalid or not unique" + ))); + } + } + if identity.vf_id.is_none() { + let fmds_name = format!("f_{service_name}_if"); + if fmds_name.len() > 15 || !fmds_interface_names.insert(fmds_name.clone()) { + return Err(crate::DpfError::ConfigError(format!( + "generated DPF service interface name {fmds_name} is invalid or not unique" + ))); + } + } + + // DPF creates the local br-sfc side of each Patch pair with this deterministic name. + // It is an OVS object even though operators configure only the peer-side name, so it + // must participate in the same collision gate as every explicit bridge and port. + let local_patch_port = format!("p_brsfc_to_{}", interface.patch_port); + for (name, purpose) in [ + (&interface.bridge, "intermediate bridge"), + (&interface.patch_port, "peer patch port"), + (&local_patch_port, "DPF-generated local patch port"), + ] { + if let Some(existing) = ovs_names.insert( + name.clone(), + format!("{purpose} for DPF intercept-bridging interface {resource_name}"), + ) { + return Err(crate::DpfError::ConfigError(format!( + "OVS name {name} for {purpose} on DPF intercept-bridging interface {resource_name} conflicts with {existing}" + ))); + } + } + } + + // BF3 provisions the complete NUM_OF_VFS population even when the replacement inventory + // selects only a sparse subset. Reserve every materialized raw representor, not just the + // configured identities, because all of them share OVS's global object namespace. + if let Some((raw_netdev, existing)) = ovs_names + .iter() + .find(|(name, _)| is_provisioned_bf3_raw_netdev(name, selected_pf_id, num_of_vfs)) + { + return Err(crate::DpfError::ConfigError(format!( + "expected provisioned BF3 raw netdev {raw_netdev} conflicts with {existing}" + ))); + } + + Ok(Self { + interfaces, + num_of_vfs, + }) + } + + /// Returns the normalized entries in deterministic typed-identity order. + pub(crate) fn interfaces(&self) -> &[DpfInterceptBridge] { + &self.interfaces + } + + /// Returns the VF population against which this topology was validated. + pub(crate) fn num_of_vfs(&self) -> u32 { + self.num_of_vfs + } +} + +/// Returns whether an OVS object name aliases a raw BF3 representor provisioned for the PF. +fn is_provisioned_bf3_raw_netdev(name: &str, pf_id: u8, num_of_vfs: u32) -> bool { + let pf_stem = format!("pf{pf_id}"); + if name == format!("{pf_stem}hpf") { + return true; + } + + let vf_stem = format!("{pf_stem}vf"); + name.strip_prefix(&vf_stem) + .and_then(|suffix| suffix.parse::().ok()) + .is_some_and(|vf_id| vf_id < num_of_vfs && name == format!("{vf_stem}{vf_id}")) +} + +/// Rejects Linux netdev names outside NICo's shell-safe DPF contract. +fn validate_linux_netdev_name( + name: &str, + purpose: &str, + resource_name: &str, +) -> Result<(), crate::DpfError> { + // Intermediate bridges are Linux netdevs and must fit IFNAMSIZ. + let valid = (1..=15).contains(&name.len()) + && name.as_bytes().first().is_some_and(u8::is_ascii_lowercase) + && name.bytes().all(|character| { + character.is_ascii_lowercase() || character.is_ascii_digit() || character == b'-' + }); + if !valid { + return Err(crate::DpfError::ConfigError(format!( + "DPF intercept bridging {purpose} name {name:?} for interface {resource_name} must be 1 to 15 lowercase ASCII letters, digits, or hyphens and start with a letter" + ))); + } + Ok(()) +} + +/// Rejects OVS patch-port names outside NICo's shell-safe DPF contract. +fn validate_ovs_patch_name(name: &str, resource_name: &str) -> Result<(), crate::DpfError> { + // Patch ports are OVSDB objects, not Linux netdev names. The checked-in DPF CRD has no + // maxLength and demonstrates an autogenerated name longer than 15 characters containing + // underscores, so do not invent a Linux-netdev length restriction here. + let valid = !name.is_empty() + && name.as_bytes().first().is_some_and(u8::is_ascii_lowercase) + && name.bytes().all(|character| { + character.is_ascii_lowercase() + || character.is_ascii_digit() + || matches!(character, b'-' | b'_') + }); + if !valid { + return Err(crate::DpfError::ConfigError(format!( + "DPF intercept bridging patch port name {name:?} for interface {resource_name} must be non-empty, start with a lowercase ASCII letter, and contain only lowercase ASCII letters, digits, hyphens, or underscores" + ))); + } + Ok(()) +} + +/// Patch-pair data owned by a DPUServiceInterface. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DpuServiceInterfacePatch { + pub(crate) peer_bridge: String, + pub(crate) peer_patch_name: String, } /// Network interface for a DPU service. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct DpuServiceInterfaceTemplateDefinition { /// Interface name. pub name: String, @@ -508,6 +838,261 @@ mod tests { use super::*; + /// Provides one normalized intercept-bridging interface for topology validation cases. + fn intercept_bridge( + controller_id: u8, + pf_id: u8, + vf_id: Option, + bridge: &str, + patch_port: &str, + ) -> DpfInterceptBridge { + DpfInterceptBridge::new( + DpfInterfaceIdentity { + controller_id, + pf_id, + vf_id, + }, + bridge, + patch_port, + ) + } + + /// Verifies all DPF intercept bridging identity, boundary, and rendered-name rejection rules. + #[test] + fn dpf_intercept_bridging_validates_identity_and_ovs_contracts() { + value_scenarios!( + run = |(interfaces, num_of_vfs)| DpfInterceptBridging::new(interfaces, num_of_vfs).is_ok(); + "empty replacement topology" { + // FMDS cannot start when a configured replacement inventory omits its PF. + (vec![], 0) => false, + } + + "VF-only replacement topology" { + // A VF cannot supply the single PF endpoint required by FMDS. + (vec![intercept_bridge(2, 3, Some(4), "br-vf", "p-vf")], 16) => false, + } + + "PF independent of VF count" { + // PF selections remain valid even when hardware provisioning creates no VFs. + (vec![intercept_bridge(2, 3, None, "br-pf", "p-pf")], 0) => true, + } + + "VF below configured population" { + // The highest provisioned VF index is accepted. + ( + vec![ + intercept_bridge(2, 3, None, "br-pf", "p-pf"), + intercept_bridge(2, 3, Some(15), "br-vf", "p-vf"), + ], + 16, + ) => true, + } + + "maximum typed VF endpoint" { + // Controller/PF identity remains fully typed while VM addressing stops at VF15. + ( + vec![ + intercept_bridge(255, 255, None, "br-pf", "p-pf"), + intercept_bridge(255, 255, Some(15), "br-vf", "p-vf"), + ], + 126, + ) => true, + } + + "VF at configured population" { + // VF indices are zero-based, so equality with num_of_vfs is out of range. + ( + vec![ + intercept_bridge(2, 3, None, "br-pf", "p-pf"), + intercept_bridge(2, 3, Some(16), "br-vf", "p-vf"), + ], + 16, + ) => false, + } + + "VF outside configured population below instance limit" { + // VF8 is valid for VM addressing but absent from a four-VF hardware population. + ( + vec![ + intercept_bridge(2, 3, None, "br-pf", "p-pf"), + intercept_bridge(2, 3, Some(8), "br-vf", "p-vf"), + ], + 4, + ) => false, + } + + "VF above instance addressing limit" { + // Additional hardware VFs are not valid VMaaS topology endpoints. + ( + vec![ + intercept_bridge(2, 3, None, "br-pf", "p-pf"), + intercept_bridge(2, 3, Some(16), "br-vf", "p-vf"), + ], + 126, + ) => false, + } + + "hardware VF population above platform limit" { + // BlueField provisioning supports at most 126 hardware VFs per PF. + (vec![intercept_bridge(2, 3, None, "br-pf", "p-pf")], 127) => false, + } + + "duplicate typed identity" { + // Different legacy entries cannot select the same DPF interface twice. + ( + vec![ + intercept_bridge(2, 3, None, "br-pf", "p-pf"), + intercept_bridge(2, 3, Some(4), "br-a", "p-a"), + intercept_bridge(2, 3, Some(4), "br-b", "p-b"), + ], + 16, + ) => false, + } + + "multiple selected parents" { + // Service runtimes expose one logical PF0, so entries cannot span hardware parents. + ( + vec![ + intercept_bridge(1, 3, None, "br-a", "p-a"), + intercept_bridge(2, 3, Some(4), "br-b", "p-b"), + ], + 16, + ) => false, + } + + "long OVS patch port" { + // Patch ports are OVSDB objects and do not inherit Linux's 15-byte netdev limit. + ( + vec![intercept_bridge( + 1, + 2, + None, + "br-host", + "patch-br-host-to-hbn", + )], + 16, + ) => true, + } + + "DPF generated patch-port grammar" { + // The checked-in CRD documents underscore-containing autogenerated patch names. + ( + vec![intercept_bridge( + 1, + 2, + None, + "br-host", + "p_brovn_to_brsfc_7aea60f7", + )], + 16, + ) => true, + } + + "duplicate bridge" { + // One intermediate bridge cannot own two independently managed Patch peers. + ( + vec![ + intercept_bridge(1, 2, None, "br-pf", "p-pf"), + intercept_bridge(1, 2, Some(3), "br-shared", "p-a"), + intercept_bridge(1, 2, Some(4), "br-shared", "p-b"), + ], + 16, + ) => false, + } + + "bridge and patch collision" { + // Bridge and port names share OVS's global object namespace in this topology. + ( + vec![ + intercept_bridge(1, 2, None, "br-pf", "p-pf"), + intercept_bridge(1, 2, Some(3), "br-a", "p-shared"), + intercept_bridge(1, 2, Some(4), "p-shared", "p-b"), + ], + 16, + ) => false, + } + + "DPF-generated local patch collision" { + // DPF derives the br-sfc-side port from the configured peer patch name. + ( + vec![ + intercept_bridge(1, 2, None, "br-pf", "x"), + intercept_bridge(1, 2, Some(3), "br-vf", "p_brsfc_to_x"), + ], + 16, + ) => false, + } + + "omitted provisioned VF representor collision" { + // NUM_OF_VFS materializes VF4 even when the sparse inventory does not select it. + (vec![intercept_bridge(1, 2, None, "pf2vf4", "p-pf")], 16) => false, + } + + "reserved DPF bridge" { + // An intermediate bridge must not alias DPF's service-chain bridge. + (vec![intercept_bridge(1, 2, None, "br-sfc", "p-a")], 16) => false, + } + + "unsafe shell character" { + // Restrictive validation prevents shell interpolation and OVS ambiguity. + (vec![intercept_bridge(1, 2, None, "br_bad", "p-a")], 16) => false, + } + + "uppercase name" { + // The installed cross-surface contract uses lowercase names only. + (vec![intercept_bridge(1, 2, None, "Br-bad", "p-a")], 16) => false, + } + + "unsafe patch-port character" { + // Patch names are rendered into shell-safe flavor data and cannot contain quotes. + (vec![intercept_bridge(1, 2, None, "br-host", "p'bad")], 16) => false, + } + + "name exceeds Linux netdev limit" { + // Linux-backed OVS ports and bridges must fit the 15-character interface name. + (vec![intercept_bridge(1, 2, None, "abcdefghijklmnop", "p-a")], 16) => false, + } + ); + + // A valid-range VM VF must reach the hardware-population gate rather than the VF15 gate. + assert!(matches!( + DpfInterceptBridging::new( + vec![ + intercept_bridge(2, 3, None, "br-pf", "p-pf"), + intercept_bridge(2, 3, Some(8), "br-vf", "p-vf"), + ], + 4, + ), + Err(crate::DpfError::ConfigError(message)) + if message + == "DPF intercept-bridging interface c2pf3vf8 selects a VF outside num_of_vfs=4" + )); + } + + /// Verifies normalization order and generated names depend only on typed identity. + #[test] + fn dpf_intercept_bridging_sorts_by_typed_identity() { + // Supply entries in reverse identity order. + let topology = DpfInterceptBridging::new( + vec![ + intercept_bridge(2, 1, Some(3), "br-b", "p-b"), + intercept_bridge(2, 1, None, "br-a", "p-a"), + ], + 16, + ) + .expect("valid topology must normalize"); + + // Stable names and sorting prevent needless immutable flavor hash churn. + assert_eq!( + topology + .interfaces() + .iter() + .map(|interface| interface.identity.resource_name()) + .collect::>(), + ["c2pf1", "c2pf1vf3"] + ); + } + /// `DpuPhase::from(DpuStatusPhase)` is a total conversion; every operator /// status phase maps to exactly one simplified `DpuPhase`. This folds the /// old `test_dpu_phase_from_status` and enumerates all 17 source variants, @@ -760,8 +1345,9 @@ mod tests { } /// `InitDpfResourcesConfig::default()` seeds the documented defaults: an - /// empty BFB URL and services list, the `dpu-deployment` name, the crate - /// default flavor, and no proxy. Probe each field independently. + /// empty BFB URL and inventories, the `dpu-deployment` name, the crate + /// default flavor, 16 VFs, 30 reserved SFs, no intercept-bridging topology, + /// and no proxy. Probe each field independently. #[test] fn init_dpf_resources_config_default_fields() { value_scenarios!( @@ -788,6 +1374,28 @@ mod tests { () => 0usize, } ); + value_scenarios!( + run = |()| { + let config = InitDpfResourcesConfig::default(); + ( + config.num_of_vfs, + config.pf_total_sf_reserved, + config.deployment_scoped_service_interfaces, + config.intercept_bridging.is_none(), + config.interfaces.is_empty(), + ) + }; + "interface projection defaults" { + // Existing SDK callers retain the historical static 16-VF behavior. + () => ( + DEFAULT_DPU_NUM_OF_VFS, + DEFAULT_PF_TOTAL_SF_RESERVED, + false, + true, + true, + ), + } + ); value_scenarios!( run = |()| InitDpfResourcesConfig::default().proxy.is_none(); "proxy is none" { diff --git a/crates/machine-controller/src/dpf.rs b/crates/machine-controller/src/dpf.rs index 35761107d1..37c74d5127 100644 --- a/crates/machine-controller/src/dpf.rs +++ b/crates/machine-controller/src/dpf.rs @@ -23,8 +23,9 @@ use std::sync::Arc; use async_trait::async_trait; use carbide_dpf::types::{DpuServiceVersion, HostDpfSnapshot, ServiceTemplateVersion}; use carbide_dpf::{ - BmcPasswordProvider, DpfError, DpfSdk, DpuDeploymentType, DpuDeviceInfo, DpuNodeInfo, DpuPhase, - DpuWatcher, KubeRepository, ResourceLabeler, node_id_from_dpu_node_cr_name, + BmcPasswordProvider, DPU_ENABLED_NODE_LABEL, DpfError, DpfSdk, DpuDeploymentType, + DpuDeviceInfo, DpuNodeInfo, DpuPhase, DpuWatcher, KubeRepository, ResourceLabeler, + node_id_from_dpu_node_cr_name, }; use carbide_uuid::machine::MachineId; use model::dpu_machine_update::OutdatedDpfDpu; @@ -44,6 +45,9 @@ const DPU_MACHINE_ID_LABEL: &str = "carbide.nvidia.com/dpu-machine-id"; /// carbide-controlled. Propagates to the DPU CR. const CONTROLLED_DEVICE_LABEL: &str = "carbide.nvidia.com/controlled.device"; +/// Label populated with the host BMC address on both DPUDevice and DPUNode resources. +pub const HOST_BMC_IP_LABEL: &str = "carbide.nvidia.com/host-bmc-ip"; + /// Trait for DPF SDK operations used by Carbide. /// /// The DPF operator owns provisioning; Carbide declares setup (deployment, devices, node), @@ -212,10 +216,7 @@ impl ResourceLabeler for CarbideDPFLabeler { fn device_labels(&self, info: &DpuDeviceInfo) -> BTreeMap { BTreeMap::from([ (CONTROLLED_DEVICE_LABEL.to_string(), "true".to_string()), - ( - "carbide.nvidia.com/host-bmc-ip".to_string(), - info.host_bmc_ip.to_string(), - ), + (HOST_BMC_IP_LABEL.to_string(), info.host_bmc_ip.to_string()), ( "carbide.nvidia.com/is-primary-dpu".to_string(), info.is_primary.to_string(), @@ -230,10 +231,7 @@ impl ResourceLabeler for CarbideDPFLabeler { fn node_labels(&self) -> BTreeMap { BTreeMap::from([ (self.node_label_key.clone(), "true".to_string()), - ( - "feature.node.kubernetes.io/dpu-enabled".to_string(), - "true".to_string(), - ), + (DPU_ENABLED_NODE_LABEL.to_string(), "true".to_string()), ]) } @@ -252,10 +250,7 @@ impl ResourceLabeler for CarbideDPFLabeler { } fn node_context_labels(&self, info: &DpuNodeInfo) -> BTreeMap { - BTreeMap::from([( - "carbide.nvidia.com/host-bmc-ip".to_string(), - info.host_bmc_ip.to_string(), - )]) + BTreeMap::from([(HOST_BMC_IP_LABEL.to_string(), info.host_bmc_ip.to_string())]) } fn dpu_label_selector(&self) -> Option { diff --git a/crates/rpc/src/errors.rs b/crates/rpc/src/errors.rs index 73adc079df..fde95e9163 100644 --- a/crates/rpc/src/errors.rs +++ b/crates/rpc/src/errors.rs @@ -24,7 +24,7 @@ use tonic::Status; pub enum RpcDataConversionError { #[error("field {0} is not valid base64")] InvalidBase64Data(&'static str), - #[error("virtual function ID of value {0} is not in the expected range 1-16")] + #[error("virtual function ID of value {0} is not in the expected range 0-15")] InvalidVirtualFunctionId(usize), #[error("IP address {0} is not valid")] InvalidIpAddress(String), diff --git a/crates/rpc/src/model/instance/config/network.rs b/crates/rpc/src/model/instance/config/network.rs index 722f53ce9d..4ca3541f74 100644 --- a/crates/rpc/src/model/instance/config/network.rs +++ b/crates/rpc/src/model/instance/config/network.rs @@ -125,6 +125,15 @@ enum VFAllocationType { type DeviceVFIdsMap = HashMap<(Option, u32), Vec<(rpc::InterfaceFunctionType, Option)>>; +/// Converts one explicit wire VF identity without allowing a wider protobuf value to alias a VF. +fn convert_wire_virtual_function_id(wire_id: u32) -> Result { + let model_id = u8::try_from(wire_id) + .map_err(|_| RpcDataConversionError::InvalidVirtualFunctionId(wire_id as usize))?; + InterfaceFunctionId::try_virtual_from(model_id) + .map(|_| model_id) + .map_err(|_| RpcDataConversionError::InvalidVirtualFunctionId(wire_id as usize)) +} + fn validate_virtual_function_ids_and_get_allocation_method( interfaces: &[rpc::InstanceInterfaceConfig], ) -> Result { @@ -162,6 +171,10 @@ fn validate_virtual_function_ids_and_get_allocation_method( )); } + for wire_id in all_vf_ids.iter().filter_map(|vf_info| vf_info.1) { + convert_wire_virtual_function_id(wire_id)?; + } + for vf_info in device_vf_ids.values() { let vf_ids = vf_info .iter() @@ -228,10 +241,6 @@ impl TryFrom for InstanceNetworkConfig { let function_id = match iface_type { InterfaceFunctionType::Physical => InterfaceFunctionId::Physical {}, InterfaceFunctionType::Virtual => { - // Note that this might overflow if the RPC call delivers more than - // 256 VFs. However that's ok - the `InstanceNetworkConfig.validate()` - // call will declare those configs as invalid later on anyway. - // We mainly don't want to crash here. InterfaceFunctionId::Virtual { id: if allocation_type == VFAllocationType::Carbide { let assigned_vfs = assigned_vfs_map @@ -241,8 +250,11 @@ impl TryFrom for InstanceNetworkConfig { *assigned_vfs = assigned_vfs.saturating_add(1); id } else { - // Already validated. - iface.virtual_function_id.unwrap_or_default() as u8 + // Keep conversion defensive even though the complete wire inventory + // was range-checked before per-interface conversion. + convert_wire_virtual_function_id( + iface.virtual_function_id.unwrap_or_default(), + )? }, } }