diff --git a/Cargo.lock b/Cargo.lock index 29bfa908..c74840d0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -188,7 +188,7 @@ dependencies = [ "slog", "slog-async", "slog-term", - "softnpu 0.2.0 (git+https://github.com/oxidecomputer/softnpu?branch=main)", + "softnpu 0.2.0 (git+https://github.com/oxidecomputer/softnpu?branch=zl%2Ftable-clear)", "strum 0.28.0", "thiserror 2.0.18", "tofino 0.1.0 (git+https://github.com/oxidecomputer/tofino?branch=main)", @@ -5061,7 +5061,18 @@ dependencies = [ [[package]] name = "p4rs" version = "0.1.0" -source = "git+https://github.com/oxidecomputer/p4?branch=main#c13435444832c28e0fb19bc65eaa8b431583a1cf" +source = "git+https://github.com/oxidecomputer/p4?branch=main#c8b190f46901d4c1b8eef1ca35be243b18653471" +dependencies = [ + "bitvec", + "num", + "serde", + "usdt 0.5.0", +] + +[[package]] +name = "p4rs" +version = "0.1.0" +source = "git+https://github.com/oxidecomputer/p4?branch=zl%2Fupsert-table-entries#b9baa79b4dc277501f860325da34403b89fb4ee9" dependencies = [ "bitvec", "num", @@ -7291,9 +7302,9 @@ dependencies = [ [[package]] name = "softnpu" version = "0.2.0" -source = "git+https://github.com/oxidecomputer/softnpu?branch=main#7e015d167a914777bc21434d1c61f205f22993b1" +source = "git+https://github.com/oxidecomputer/softnpu?branch=zl%2Ftable-clear#63687602f0d26add71d4dae4ac62d46563ba23f5" dependencies = [ - "p4rs", + "p4rs 0.1.0 (git+https://github.com/oxidecomputer/p4?branch=zl%2Fupsert-table-entries)", "serde", "serde_json", "tokio", @@ -7304,7 +7315,7 @@ name = "softnpu" version = "0.2.0" source = "git+https://github.com/oxidecomputer/softnpu#7e015d167a914777bc21434d1c61f205f22993b1" dependencies = [ - "p4rs", + "p4rs 0.1.0 (git+https://github.com/oxidecomputer/p4?branch=main)", "serde", "serde_json", "tokio", diff --git a/Cargo.toml b/Cargo.toml index 1dbe3ba6..28cc77c8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -57,7 +57,7 @@ oxnet = { version = "0.1.6", default-features = false, features = ["schemars", " propolis = { git = "https://github.com/oxidecomputer/propolis" } smf = { git = "https://github.com/illumos/smf-rs" } slog-error-chain = { git = "https://github.com/oxidecomputer/slog-error-chain", branch = "main" } -softnpu-lib = { git = "https://github.com/oxidecomputer/softnpu" , package = "softnpu" , branch = "main"} +softnpu-lib = { git = "https://github.com/oxidecomputer/softnpu" , package = "softnpu" , branch = "zl/table-clear"} tofino = { git = "https://github.com/oxidecomputer/tofino", branch = "main" } transceiver-controller = { git = "https://github.com/oxidecomputer/transceiver-control", branch = "main" } diff --git a/asic/src/chaos/fault.rs b/asic/src/chaos/fault.rs index 99a15b8d..b568a252 100644 --- a/asic/src/chaos/fault.rs +++ b/asic/src/chaos/fault.rs @@ -46,31 +46,32 @@ pub(super) struct Fault { calls: u64, /// The call numbers that fail. range: CallRange, + /// The error returned by a failing execution. + err: fn(String) -> AsicError, } impl Fault { pub(super) fn new(range: CallRange) -> Self { - Self { calls: 0, range } + Self { calls: 0, range, err: AsicError::Synthetic } } } impl Handle { pub(super) fn check_mc_fault(&self, op: McOp) -> AsicResult<()> { - if self.config.mc_config.unsupported.contains(&op) { - return Err(AsicError::OperationUnsupported); - } - let failed_call = { let mut faults = self.mc_faults.lock().unwrap(); faults.get_mut(&op).and_then(|fault| { fault.calls += 1; - fault.range.contains(fault.calls).then_some(fault.calls) + fault + .range + .contains(fault.calls) + .then_some((fault.calls, fault.err)) }) }; - if let Some(call) = failed_call { - slog::error!(self.log, "chaos error: {} (call {})", op, call); - return Err(AsicError::Synthetic(op.to_string())); + if let Some((call, err)) = failed_call { + slog::error!(self.log, "chaos error: {op} (call {call})"); + return Err(err(op.to_string())); } self.config.mc_config.chaos(op).unfurled(&self.log, &op.to_string()) @@ -93,20 +94,20 @@ impl Handle { let mut faults = self.table_faults.lock().unwrap(); faults.get_mut(&(table, op)).and_then(|fault| { fault.calls += 1; - fault.range.contains(fault.calls).then_some(fault.calls) + fault + .range + .contains(fault.calls) + .then_some((fault.calls, fault.err)) }) }; match failed_call { - Some(call) => { + Some((call, err)) => { slog::error!( self.log, - "chaos table error: {} on {} (call {})", - op, - table, - call + "chaos table error: {op} on {table} (call {call})" ); - Err(AsicError::Synthetic(op.to_string())) + Err(err(op.to_string())) } None => Ok(()), } @@ -117,11 +118,10 @@ impl Handle { table: TableType, op: TableOp, range: CallRange, + error: fn(String) -> AsicError, ) { - self.table_faults - .lock() - .unwrap() - .insert((table, op), Fault::new(range)); + let fault = Fault { err: error, ..Fault::new(range) }; + self.table_faults.lock().unwrap().insert((table, op), fault); } pub fn clear_table_faults(&self, table: TableType, op: TableOp) { diff --git a/asic/src/chaos/mod.rs b/asic/src/chaos/mod.rs index 3df78875..2590bd12 100644 --- a/asic/src/chaos/mod.rs +++ b/asic/src/chaos/mod.rs @@ -63,7 +63,7 @@ impl Chaos { /// Return a chaos error according to the underlying probability value. pub fn unfurled(&self, log: &Logger, message: &str) -> AsicResult<()> { if self.value > random() { - slog::error!(log, "chaos error: {}", message); + slog::error!(log, "chaos error: {message}"); return Err(AsicError::Synthetic(message.into())); } Ok(()) @@ -150,7 +150,7 @@ impl TableChaos { if let Some(value) = self.values.get(&id) && *value > random() { - slog::error!(log, "chaos table error: {}", message); + slog::error!(log, "chaos table error: {message}"); return Err(AsicError::Synthetic(message.into())); } Ok(()) @@ -201,6 +201,7 @@ pub enum TableOp { EntryAdd, EntryUpdate, EntryDel, + EntryList, } impl fmt::Display for TableOp { @@ -209,6 +210,7 @@ impl fmt::Display for TableOp { Self::EntryAdd => "table_entry_add", Self::EntryUpdate => "table_entry_update", Self::EntryDel => "table_entry_del", + Self::EntryList => "table_entry_list", }) } } @@ -226,8 +228,6 @@ pub struct AsicMulticastConfig { pub mc_port_list: Chaos, #[serde(default)] pub faults: BTreeMap, - #[serde(default)] - pub unsupported: BTreeSet, } impl AsicMulticastConfig { @@ -316,7 +316,6 @@ impl AsicConfig { mc_set_max_nodes: Chaos::new(v), mc_port_list: Chaos::new(v), faults: BTreeMap::new(), - unsupported: BTreeSet::new(), }, } } diff --git a/asic/src/chaos/table.rs b/asic/src/chaos/table.rs index 6083dc1e..510a89b6 100644 --- a/asic/src/chaos/table.rs +++ b/asic/src/chaos/table.rs @@ -112,9 +112,10 @@ impl TableOps for Table { fn get_entries( &self, - _hdl: &Handle, + hdl: &Handle, _from_hardware: bool, ) -> AsicResult> { + hdl.check_table_fault(self.type_, TableOp::EntryList)?; self.keys .lock() .unwrap() diff --git a/asic/src/softnpu/mcast.rs b/asic/src/softnpu/mcast.rs new file mode 100644 index 00000000..ddc2747d --- /dev/null +++ b/asic/src/softnpu/mcast.rs @@ -0,0 +1,305 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/ +// +// Copyright 2026 Oxide Computer Company + +//! [sidecar-lite] handles packet replication via port bitmaps in the P4 +//! pipeline. This module tracks group membership for the `AsicMulticastOps` +//! contract. +//! +//! [sidecar-lite]: https://github.com/oxidecomputer/sidecar-lite + +use std::collections::{HashMap, hash_map::Entry}; + +use aal::{AsicError, AsicResult}; + +/// The sidecar-lite replication table that holds underlay/external +/// port bitmaps for an underlay multicast group. +pub const MCAST_REPLICATION_V6_TABLE: &str = + "ingress.mcast.mcast_replication_v6"; + +/// The sidecar-lite action that writes the external and underlay port bitmaps +/// (and replication id) for a [`MCAST_REPLICATION_V6_TABLE`] entry. +pub const SET_PORT_BITMAP_ACTION: &str = "set_port_bitmap"; + +/// The number of ports a group's bitmap can contain, matching sidecar-lite's +/// `bit<128>` port bitmaps. +const PORT_BITMAP_WIDTH: u16 = u128::BITS as u16; + +/// Multicast group membership, defined as a port bitmap per group. +/// +/// SoftNPU has no replication engine to inherently support group membership, +/// so we track it here instead. The bitmaps get resolved when dpd writes a +/// replication entry, and dpd rewrites that entry after each membership change. +#[derive(Default)] +pub struct McGroupData { + groups: HashMap, +} + +fn no_group(group_id: u16) -> AsicError { + AsicError::Missing(format!("no such multicast group: {group_id}")) +} + +impl McGroupData { + /// Get the list of multicast domains. + pub fn domains(&self) -> Vec { + self.groups.keys().copied().collect() + } + + /// Build a 128-bit port bitmap for a group. Bit N is set if port N + /// is a member. + /// + /// Returns zero for unknown groups. + pub fn port_bitmap(&self, group_id: u16) -> u128 { + self.groups.get(&group_id).copied().unwrap_or(0) + } + + /// Get the number of ports in a multicast domain. + pub fn domain_port_count(&self, group_id: u16) -> AsicResult { + self.groups + .get(&group_id) + .map(|ports| ports.count_ones() as usize) + .ok_or_else(|| no_group(group_id)) + } + + /// List the ports in a multicast domain (in ascending order). + pub fn domain_port_list(&self, group_id: u16) -> AsicResult> { + let ports = + self.groups.get(&group_id).ok_or_else(|| no_group(group_id))?; + Ok((0..PORT_BITMAP_WIDTH) + .filter(|&port| *ports & (1u128 << port) != 0) + .collect()) + } + + /// Add a port to a multicast domain. Port must be < 128 to fit + /// within sidecar-lite's 128-bit replication bitmap. + pub fn domain_port_add( + &mut self, + group_id: u16, + port: u16, + _rid: u16, + _level1_excl_id: u16, + ) -> AsicResult<()> { + if port >= PORT_BITMAP_WIDTH { + return Err(AsicError::InvalidArg(format!( + "port {port} exceeds softnpu 128-port bitmap limit" + ))); + } + + let ports = + self.groups.get_mut(&group_id).ok_or_else(|| no_group(group_id))?; + let bit = 1u128 << port; + if *ports & bit != 0 { + return Err(AsicError::Exists(format!( + "multicast group {group_id} already contains port {port}" + ))); + } + *ports |= bit; + Ok(()) + } + + /// Remove a port from a multicast domain. + pub fn domain_port_remove( + &mut self, + group_id: u16, + port: u16, + ) -> AsicResult<()> { + let ports = + self.groups.get_mut(&group_id).ok_or_else(|| no_group(group_id))?; + let bit = 1u128.checked_shl(u32::from(port)).unwrap_or(0); + if *ports & bit == 0 { + return Err(AsicError::Missing(format!( + "multicast group {group_id} doesn't contain port {port}" + ))); + } + *ports &= !bit; + Ok(()) + } + + /// Create a multicast domain. + pub fn domain_create(&mut self, group_id: u16) -> AsicResult<()> { + match self.groups.entry(group_id) { + Entry::Occupied(_) => Err(AsicError::Exists(format!( + "multicast group {group_id} already exists" + ))), + Entry::Vacant(vacant) => { + vacant.insert(0); + Ok(()) + } + } + } + + /// Destroy a multicast domain. + pub fn domain_destroy(&mut self, group_id: u16) -> AsicResult<()> { + self.groups.remove(&group_id).ok_or_else(|| no_group(group_id))?; + Ok(()) + } + + /// Get the total number of multicast domains. + pub fn domains_count(&self) -> usize { + self.groups.len() + } + + /// Validate that the current group count does not exceed the limit. + pub fn set_max_nodes(&self, max_nodes: u32) -> AsicResult<()> { + let total = self.domains_count(); + if total > usize::try_from(max_nodes).unwrap_or(usize::MAX) { + return Err(AsicError::InvalidArg(format!( + "number of multicast groups {total} exceeds max nodes {max_nodes}" + ))); + } + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn group_lifecycle() { + let mut group_data = McGroupData::default(); + + // Create group, add ports. + group_data.domain_create(100).unwrap(); + group_data.domain_port_add(100, 1, 0, 0).unwrap(); + group_data.domain_port_add(100, 5, 0, 0).unwrap(); + + assert_eq!(group_data.domain_port_count(100).unwrap(), 2); + assert_eq!(group_data.domain_port_list(100).unwrap(), vec![1, 5]); + assert_eq!(group_data.domains_count(), 1); + + // Remove a port. + group_data.domain_port_remove(100, 1).unwrap(); + assert_eq!(group_data.domain_port_count(100).unwrap(), 1); + assert_eq!(group_data.domain_port_list(100).unwrap(), vec![5]); + + // Destroy group. + group_data.domain_destroy(100).unwrap(); + assert_eq!(group_data.domains_count(), 0); + } + + #[test] + fn duplicate_group_rejected() { + let mut group_data = McGroupData::default(); + group_data.domain_create(1).unwrap(); + assert!(matches!( + group_data.domain_create(1), + Err(AsicError::Exists(_)) + )); + } + + #[test] + fn duplicate_port_rejected() { + let mut group_data = McGroupData::default(); + group_data.domain_create(1).unwrap(); + group_data.domain_port_add(1, 5, 0, 0).unwrap(); + assert!(matches!( + group_data.domain_port_add(1, 5, 0, 0), + Err(AsicError::Exists(_)) + )); + } + + #[test] + fn remove_nonexistent_port_rejected() { + let mut group_data = McGroupData::default(); + group_data.domain_create(1).unwrap(); + assert!(matches!( + group_data.domain_port_remove(1, 99), + Err(AsicError::Missing(_)) + )); + } + + #[test] + fn operations_on_missing_group_rejected() { + let mut group_data = McGroupData::default(); + assert!(matches!( + group_data.domain_port_add(404, 1, 0, 0), + Err(AsicError::Missing(_)) + )); + assert!(matches!( + group_data.domain_port_remove(404, 1), + Err(AsicError::Missing(_)) + )); + assert!(matches!( + group_data.domain_port_count(404), + Err(AsicError::Missing(_)) + )); + assert!(matches!( + group_data.domain_port_list(404), + Err(AsicError::Missing(_)) + )); + assert!(matches!( + group_data.domain_destroy(404), + Err(AsicError::Missing(_)) + )); + } + + #[test] + fn port_bitmap_zero_for_empty_or_unknown_group() { + let mut group_data = McGroupData::default(); + group_data.domain_create(1).unwrap(); + assert_eq!(group_data.port_bitmap(1), 0); + assert_eq!(group_data.port_bitmap(999), 0); + } + + #[test] + fn port_bitmap_tracks_membership() { + let mut group_data = McGroupData::default(); + group_data.domain_create(1).unwrap(); + group_data.domain_port_add(1, 0, 0, 0).unwrap(); + group_data.domain_port_add(1, 3, 0, 0).unwrap(); + group_data.domain_port_add(1, 7, 0, 0).unwrap(); + assert_eq!(group_data.port_bitmap(1), (1 << 0) | (1 << 3) | (1 << 7)); + + group_data.domain_port_remove(1, 0).unwrap(); + assert_eq!(group_data.port_bitmap(1), (1 << 3) | (1 << 7)); + } + + #[test] + fn port_bitmap_high_port() { + let mut group_data = McGroupData::default(); + group_data.domain_create(1).unwrap(); + group_data.domain_port_add(1, 127, 0, 0).unwrap(); + + let bm = group_data.port_bitmap(1); + assert_eq!(bm & (1u128 << 127), 1u128 << 127); + } + + #[test] + fn port_list_covers_bitmap_range() { + let mut group_data = McGroupData::default(); + group_data.domain_create(1).unwrap(); + assert!(group_data.domain_port_list(1).unwrap().is_empty()); + group_data.domain_port_add(1, 127, 0, 0).unwrap(); + group_data.domain_port_add(1, 0, 0, 0).unwrap(); + assert_eq!(group_data.domain_port_list(1).unwrap(), vec![0, 127]); + } + + #[test] + fn port_add_rejects_ports_above_127() { + let mut group_data = McGroupData::default(); + group_data.domain_create(1).unwrap(); + // Port 128 is out of range for a 128-bit bitmap. + assert!(matches!( + group_data.domain_port_add(1, 128, 0, 0), + Err(AsicError::InvalidArg(_)) + )); + } + + #[test] + fn set_max_nodes_validates() { + let mut group_data = McGroupData::default(); + group_data.domain_create(1).unwrap(); + group_data.domain_create(2).unwrap(); + + assert!(matches!( + group_data.set_max_nodes(1), + Err(AsicError::InvalidArg(_)) + )); + assert!(group_data.set_max_nodes(2).is_ok()); + assert!(group_data.set_max_nodes(100).is_ok()); + } +} diff --git a/asic/src/softnpu/mgmt.rs b/asic/src/softnpu/mgmt.rs index 65f24338..a803ea93 100644 --- a/asic/src/softnpu/mgmt.rs +++ b/asic/src/softnpu/mgmt.rs @@ -5,9 +5,11 @@ // Copyright 2026 Oxide Computer Company use std::fs::OpenOptions; +use std::io; use std::io::Read; use std::io::Write; +use aal::{AsicError, AsicResult}; use propolis::hw::virtio::softnpu::{MANAGEMENT_MESSAGE_PREAMBLE, SOFTNPU_TTY}; use softnpu_lib::ManagementRequest; use std::os::unix::net::UnixDatagram; @@ -49,11 +51,18 @@ impl std::str::FromStr for SoftnpuManagement { } /// Send a management request to a SoftNPU asic. -pub fn write(msg: ManagementRequest, config: &ManagementConfig) { +pub fn write( + msg: ManagementRequest, + config: &ManagementConfig, +) -> AsicResult<()> { match config { ManagementConfig::UART => write_uart(msg), ManagementConfig::UDS { socket_path } => write_uds(msg, socket_path), } + .map_err(|err| AsicError::Io { + ctx: "SoftNPU management write".to_string(), + err, + }) } /// Read a management request response from a SoftNPU asic. @@ -64,27 +73,28 @@ pub fn read(msg: ManagementRequest, config: &ManagementConfig) -> String { } } -fn write_uart(msg: ManagementRequest) { +fn write_uart(msg: ManagementRequest) -> io::Result<()> { let mut buf = Vec::new(); buf.push(MANAGEMENT_MESSAGE_PREAMBLE); - let mut js = serde_json::to_vec(&msg).unwrap(); + let mut js = serde_json::to_vec(&msg)?; js.retain(|x| *x != b'\n'); buf.extend_from_slice(&js); buf.push(b'\n'); - let mut f = OpenOptions::new().write(true).open(SOFTNPU_TTY).unwrap(); + let mut f = OpenOptions::new().write(true).open(SOFTNPU_TTY)?; - f.write_all(&buf).unwrap(); - f.sync_all().unwrap(); + f.write_all(&buf)?; + f.sync_all() } -fn write_uds(msg: ManagementRequest, socket_path: &str) { - let uds = UnixDatagram::unbound().unwrap(); +fn write_uds(msg: ManagementRequest, socket_path: &str) -> io::Result<()> { + let uds = UnixDatagram::unbound()?; - let buf = serde_json::to_vec(&msg).unwrap(); + let buf = serde_json::to_vec(&msg)?; let mut server_handle = socket_path.to_owned(); server_handle.push_str(SERVER_PATH); - uds.send_to(&buf, server_handle).unwrap(); + uds.send_to(&buf, server_handle)?; + Ok(()) } fn read_uart(msg: ManagementRequest) -> String { @@ -109,7 +119,7 @@ fn read_uart(msg: ManagementRequest) -> String { fn read_uds(msg: ManagementRequest, socket_path: &str) -> String { let client_uds = bind_uds(socket_path); - write_uds(msg, socket_path); + write_uds(msg, socket_path).unwrap(); let mut buf = vec![0u8; 10240]; let n = client_uds.recv(&mut buf).unwrap(); diff --git a/asic/src/softnpu/mod.rs b/asic/src/softnpu/mod.rs index c6cf6977..38310561 100644 --- a/asic/src/softnpu/mod.rs +++ b/asic/src/softnpu/mod.rs @@ -7,7 +7,7 @@ use std::collections::HashMap; use std::sync::Mutex; -use slog::{Logger, o}; +use slog::{Logger, debug, o}; use tokio::sync::mpsc; use uuid::Uuid; @@ -25,8 +25,9 @@ use common::ports::{ PortFec, PortId, PortMedia, PortPrbsMode, PortSpeed, TxEq, }; -use softnpu_lib::ManagementRequest; +use softnpu_lib::{ManagementRequest, ManagementResponse}; +pub mod mcast; pub mod mgmt; pub mod table; @@ -102,8 +103,8 @@ pub struct Handle { log: Logger, ports: Mutex>, pub mgmt_config: mgmt::ManagementConfig, - update_tx: Mutex>>, + mc_data: Mutex, } impl Handle { @@ -129,6 +130,7 @@ impl Handle { ports: Mutex::new(HashMap::new()), mgmt_config, update_tx: Mutex::new(None), + mc_data: Mutex::new(mcast::McGroupData::default()), }) } @@ -146,51 +148,74 @@ impl Handle { impl AsicMulticastOps for Handle { fn mc_domains(&self) -> Vec { - let len = self.ports.lock().unwrap().len() as u16; - (0..len).collect() + let mc_data = self.mc_data.lock().unwrap(); + mc_data.domains() } - fn mc_port_count(&self, _group_id: u16) -> AsicResult { - Ok(self.ports.lock().unwrap().len()) + fn mc_port_count(&self, group_id: u16) -> AsicResult { + let mc_data = self.mc_data.lock().unwrap(); + mc_data.domain_port_count(group_id) } - fn mc_port_list(&self, _group_id: u16) -> AsicResult> { - // TODO: Read members from replication table bitmaps via DumpRequest. - Err(AsicError::OperationUnsupported) + fn mc_port_list(&self, group_id: u16) -> AsicResult> { + self.mc_data.lock().unwrap().domain_port_list(group_id) } fn mc_port_add( &self, - _group_id: u16, - _port: u16, - _rid: u16, - _level1_excl_id: u16, + group_id: u16, + port: u16, + rid: u16, + level1_excl_id: u16, ) -> AsicResult<()> { - Err(AsicError::OperationUnsupported) - } - - fn mc_port_remove(&self, _group_id: u16, _port: u16) -> AsicResult<()> { - Ok(()) + slog::info!( + self.log, + "adding port {port} to multicast group {group_id}" + ); + self.mc_data.lock().unwrap().domain_port_add( + group_id, + port, + rid, + level1_excl_id, + ) + } + + fn mc_port_remove(&self, group_id: u16, port: u16) -> AsicResult<()> { + slog::info!( + self.log, + "removing port {port} from multicast group {group_id}" + ); + self.mc_data.lock().unwrap().domain_port_remove(group_id, port) } - fn mc_group_create(&self, _group_id: u16) -> AsicResult<()> { - Err(AsicError::OperationUnsupported) + fn mc_group_create(&self, group_id: u16) -> AsicResult<()> { + slog::info!(self.log, "creating multicast group {group_id}"); + let mut mc_data = self.mc_data.lock().unwrap(); + mc_data.domain_create(group_id) } - fn mc_group_destroy(&self, _group_id: u16) -> AsicResult<()> { - Ok(()) + fn mc_group_destroy(&self, group_id: u16) -> AsicResult<()> { + slog::info!(self.log, "destroying multicast group {group_id}"); + let mut mc_data = self.mc_data.lock().unwrap(); + mc_data.domain_destroy(group_id) } fn mc_groups_count(&self) -> AsicResult { - Ok(self.ports.lock().unwrap().len()) + let mc_data = self.mc_data.lock().unwrap(); + Ok(mc_data.domains_count()) } fn mc_set_max_nodes( &self, - _max_nodes: u32, - _max_link_aggregated_nodes: u32, + max_nodes: u32, + max_link_aggregated_nodes: u32, ) -> AsicResult<()> { - Ok(()) + slog::info!( + self.log, + "setting max nodes to {max_nodes}, \ + max link aggregated nodes to {max_link_aggregated_nodes}" + ); + self.mc_data.lock().unwrap().set_max_nodes(max_nodes) } } @@ -246,7 +271,7 @@ impl AsicOps for Handle { let asic_port_id = self.port_to_asic_id(port_hdl)?; let tx = self.update_tx.lock().unwrap(); if let Some(tx) = tx.as_ref() { - // When a port is enabled in softnpu, it automatically comes online. + // When a port is enabled in SoftNPU, it automatically comes online. // When switching between enabled and disabled, we send the main body // of dpd the PortUpdate events we would expect to see on real hardware. let present = @@ -272,7 +297,7 @@ impl AsicOps for Handle { } } } else { - slog::debug!(self.log, "no PortUpdate handler registered"); + debug!(self.log, "no PortUpdate handler registered"); } Ok(()) } @@ -361,13 +386,28 @@ impl AsicOps for Handle { } fn get_connectors(&self) -> Vec { - let msg = ManagementRequest::RadixRequest; - crate::softnpu::mgmt::write(msg, &self.mgmt_config); let response = crate::softnpu::mgmt::read( ManagementRequest::RadixRequest, &self.mgmt_config, ); - let radix: u16 = response.trim().parse().unwrap(); + // NOTE: propolis responds over the UART in ASCII, due to a guest tty + // without a raw mode. + // + // + // + // NOTE: standalone SoftNPU responds over UDS with a JSON + // `ManagementResponse`. + // + // + let radix: u16 = match &self.mgmt_config { + mgmt::ManagementConfig::UART => response.trim().parse().unwrap(), + mgmt::ManagementConfig::UDS { .. } => { + match serde_json::from_str(response.trim()).unwrap() { + ManagementResponse::RadixResponse(radix) => radix, + other => panic!("unexpected radix response: {other:?}"), + } + } + }; let mut result = vec![Connector::CPU]; for i in 1..radix + 1 { diff --git a/asic/src/softnpu/table.rs b/asic/src/softnpu/table.rs index 56d9c1fe..c8e5fb43 100644 --- a/asic/src/softnpu/table.rs +++ b/asic/src/softnpu/table.rs @@ -4,8 +4,10 @@ // // Copyright 2026 Oxide Computer Company +use std::hash::Hash; + use slog::{error, trace}; -use softnpu_lib::{ManagementRequest, TableAdd, TableRemove}; +use softnpu_lib::{ManagementRequest, TableAdd, TableClear, TableRemove}; use crate::softnpu::Handle; use aal::{ @@ -14,9 +16,10 @@ use aal::{ }; use common::table::TableType; -/// Represents a handle to a SoftNPU ASIC table. The `id` member corresponds to -/// the table path in the P4 program. Well known sidecar-lite.p4 paths follow -/// below. +/// Represents a handle to a SoftNPU ASIC table. +/// +/// The `type_` member identifies the table and `softnpu_table_name()` +/// maps it to the corresponding path in sidecar-lite.p4. pub struct Table { type_: TableType, implemented: bool, @@ -24,6 +27,8 @@ pub struct Table { } impl Table { + /// Return the sidecar-lite table name for this table, or `None` if the + /// table is not implemented in the SoftNPU backend. pub fn softnpu_table_name(&self) -> Option<&'static str> { if self.implemented { match self.type_ { @@ -44,6 +49,28 @@ impl Table { TableType::ArpIpv4 => Some("ingress.resolver.resolver_v4"), TableType::NeighborIpv6 => Some("ingress.resolver.resolver_v6"), TableType::PortMacAddress => Some("ingress.mac.mac_rewrite"), + TableType::McastIpv6 => { + Some(crate::softnpu::mcast::MCAST_REPLICATION_V6_TABLE) + } + TableType::McastIpv4SrcFilter => { + Some("ingress.mcast.mcast_source_filter_v4") + } + TableType::McastIpv6SrcFilter => { + Some("ingress.mcast.mcast_source_filter_v6") + } + TableType::NatIngressIpv4Mcast => { + Some("ingress.nat.nat_v4_mcast") + } + TableType::NatIngressIpv6Mcast => { + Some("ingress.nat.nat_v6_mcast") + } + TableType::McastEgressDecapPorts => { + Some("egress.tbl_decap_ports") + } + TableType::PortMacAddressMcast => Some("egress.mcast_src_mac"), + TableType::McastEgressPortMapping => { + Some("egress.asic_id_to_port") + } _ => panic!( "implemented table {} has no softnpu table", self.type_ @@ -53,15 +80,89 @@ impl Table { None } } + + /// Translate a `configure_mcastv6` request into a sidecar-lite + /// `set_port_bitmap` addition. + /// + /// dpd sends `(mcast_grp_a, mcast_grp_b, rid, level1_excl_id, + /// level2_excl_id)`, while sidecar-lite expects + /// `(external, underlay, rid)`, from `McGroupData` membership. + /// + /// SoftNPU's bitmap is a write-time snapshot (the Sidecar's replication + /// engine tracks membership live). dpd rewrites the entry after membership + /// changes to refresh it. + fn mcast_replication_add( + &self, + hdl: &Handle, + table: String, + keyset_data: Vec, + action_data: &aal::ActionData, + ) -> AsicResult<()> { + let action = &action_data.action; + let external_grp = arg_u16(hdl, action_data, action, "mcast_grp_a")?; + let underlay_grp = arg_u16(hdl, action_data, action, "mcast_grp_b")?; + let rid = arg_u16(hdl, action_data, action, "rid")?; + + let (external_bitmap, underlay_bitmap) = { + let mc_data = hdl.mc_data.lock().unwrap(); + ( + mc_data.port_bitmap(external_grp), + mc_data.port_bitmap(underlay_grp), + ) + }; + + let parameter_data = [ + external_bitmap.to_le_bytes().as_slice(), + underlay_bitmap.to_le_bytes().as_slice(), + rid.to_le_bytes().as_slice(), + ] + .concat(); + + slog::debug!( + hdl.log, + "mcast replication add: external_grp={external_grp} \ + underlay_grp={underlay_grp} external_bitmap={external_bitmap:#x} \ + underlay_bitmap={underlay_bitmap:#x} rid={rid}" + ); + trace!(hdl.log, "table: {table}"); + trace!(hdl.log, "keyset_data:\n{keyset_data:#?}"); + trace!(hdl.log, "parameter_data:\n{parameter_data:#?}"); + + add_entry( + hdl, + table, + crate::softnpu::mcast::SET_PORT_BITMAP_ACTION.to_string(), + keyset_data, + parameter_data, + ) + } +} + +/// Send a `TableAdd` request for `table`, which replaces any entry already +/// under the key (oxidecomputer/p4#282), serving as both an add and an update. +fn add_entry( + hdl: &Handle, + table: String, + action: String, + keyset_data: Vec, + parameter_data: Vec, +) -> AsicResult<()> { + let add = ManagementRequest::TableAdd(TableAdd { + table, + action, + keyset_data, + parameter_data, + }); + crate::softnpu::mgmt::write(add, &hdl.mgmt_config) } -// All tables are defined to be 1024 entries deep +// All tables are defined to be 4096 entries deep. const TABLE_SIZE: usize = 4096; impl TableOps for Table { fn new(hdl: &Handle, type_: TableType) -> AsicResult { - // TODO just mapping sidecar.p4 things onto simplified sidecar-lite.p4 - // things to get started. + // Mapping sidecar.p4 table types onto simplified sidecar-lite.p4 + // equivalents. let implemented = match type_ { TableType::RouteIdxIpv4 | TableType::RouteFwdIpv4 @@ -75,23 +176,40 @@ impl TableOps for Table { | TableType::NatIngressIpv4 | TableType::NatIngressIpv6 | TableType::AttachedSubnetIpv4 - | TableType::AttachedSubnetIpv6 => true, + | TableType::AttachedSubnetIpv6 + | TableType::McastIpv6 + | TableType::McastIpv4SrcFilter + | TableType::McastIpv6SrcFilter + | TableType::McastEgressDecapPorts + | TableType::McastEgressPortMapping + | TableType::PortMacAddressMcast + | TableType::NatIngressIpv4Mcast + | TableType::NatIngressIpv6Mcast => true, x => { - error!(hdl.log, "TABLE NOT HANDLED {x}"); + error!(hdl.log, "table not handled: {x}"); false } }; - Ok(Table { type_, implemented, size: TABLE_SIZE }) + let tbl = Table { type_, implemented, size: TABLE_SIZE }; + // The SoftNPU pipeline outlives the dpd process. Clear each table + // before the control plane reapplies its configuration. + tbl.clear(hdl)?; + Ok(tbl) } fn size(&self) -> usize { self.size } - fn clear(&self, _hdl: &Handle) -> AsicResult<()> { - //TODO implement in softnpu - Ok(()) + fn clear(&self, hdl: &Handle) -> AsicResult<()> { + let Some(table) = self.softnpu_table_name() else { + return Ok(()); + }; + let msg = ManagementRequest::TableClear(TableClear { + table: table.to_string(), + }); + crate::softnpu::mgmt::write(msg, &hdl.mgmt_config) } fn entry_add( @@ -103,16 +221,32 @@ impl TableOps for Table { let Some(table) = self.softnpu_table_name() else { return Ok(()); }; + let table = table.to_string(); let name = self.type_.to_string(); let match_data = key.key_to_ir().unwrap(); let action_data = data.action_to_ir().unwrap(); + let fields = match_data.fields; trace!(hdl.log, "entry_add called"); trace!(hdl.log, "table: {name}"); - trace!(hdl.log, "match_data:\n{:#?}", match_data); - trace!(hdl.log, "action_data:\n{:#?}", action_data); + trace!(hdl.log, "match_data: {fields:#?}"); + trace!(hdl.log, "action_data:\n{action_data:#?}"); - let keyset_data = keyset_data(match_data.fields, self.type_); + let keyset_data = keyset_data(fields, self.type_); + + // Multicast replication entries get handled separately because + // sidecar-lite's `set_port_bitmap` op requires the port bitmaps + // for the group IDs that dpd sends in `configure_mcastv6`. + if let (TableType::McastIpv6, "configure_mcastv6") = + (self.type_, action_data.action.as_str()) + { + return self.mcast_replication_add( + hdl, + table, + keyset_data, + &action_data, + ); + } let (action, parameter_data) = match ( self.type_, @@ -432,130 +566,77 @@ impl TableOps for Table { | (TableType::NatIngressIpv6, "forward_ipv6_to") | (TableType::AttachedSubnetIpv4, "forward_to_v4") | (TableType::AttachedSubnetIpv6, "forward_to_v6") => { - let mut target = Vec::new(); - let mut vni = Vec::new(); - let mut mac = Vec::new(); + forward_to_sled_params(hdl, &name, action_data)? + } + (TableType::NatIngressIpv4Mcast, "mcast_forward_ipv4_to") + | (TableType::NatIngressIpv6Mcast, "mcast_forward_ipv6_to") => { + forward_to_sled_params(hdl, &name, action_data)? + } + // Multicast source filters: no action parameters. + (TableType::McastIpv4SrcFilter, "allow_source_mcastv4") + | (TableType::McastIpv6SrcFilter, "allow_source_mcastv6") => { + ("allow_source", Vec::new()) + } + // Multicast egress decap: see `pack_decap_bitmap`. + (TableType::McastEgressDecapPorts, "set_decap_ports") => { + ("set_decap_ports", pack_decap_bitmap(&action_data)) + } + (TableType::McastEgressDecapPorts, "set_decap_ports_and_vlan") => { + let mut params = pack_decap_bitmap(&action_data); + let vlan_id = arg_u16(hdl, &action_data, &name, "vlan_id")?; + params.extend_from_slice(&vlan_id.to_le_bytes()); + ("set_decap_ports_and_vlan", params) + } + // Multicast egress MAC rewrite. + (TableType::PortMacAddressMcast, "rewrite") => { + let mut params = Vec::new(); for arg in action_data.args { - match arg.name.as_str() { - "target" => { - // "target" is 128 bits - let mut data: Vec = Vec::new(); - match &arg.value { - ValueTypes::U64(_) => { - // Currently the ValueType is always Ptr - error!( - hdl.log, - "expected ValueType::Ptr, \ - received ValueType::U64" - ); - return Ok(()); - } - ValueTypes::Ptr(v) => { - data.extend_from_slice(v.as_slice()); - } - } - let len = data.len(); - let buf = &mut data[len - 16..]; - buf.reverse(); - target.extend_from_slice(buf); - } - "vni" => { - // "vni" is 24 bits - let mut data: Vec = Vec::new(); - match &arg.value { - ValueTypes::U64(v) => { - data.extend_from_slice(&v.to_le_bytes()); - } - ValueTypes::Ptr(_) => { - // Currently the ValueType is always U64 - error!( - hdl.log, - "expected ValueType::U64, \ - received ValueType::Ptr" - ); - return Ok(()); - } - } - vni.extend_from_slice(&data[0..3]); - } - "inner_mac" => { - // "mac" is 48 bits - let mut data: Vec = Vec::new(); - match &arg.value { - ValueTypes::U64(v) => { - data.extend_from_slice(&v.to_le_bytes()); - } - ValueTypes::Ptr(_) => { - // Currently the ValueType is always U64 - error!( - hdl.log, - "expected ValueType::U64, \ - received ValueType::Ptr" - ); - return Ok(()); - } - } - mac.extend_from_slice(&data[0..6]) + match arg.value { + ValueTypes::U64(v) => { + let mac = v.to_le_bytes(); + params.extend_from_slice(&mac[0..6]); } - _ => { - error!(hdl.log, "unknown argument: {}", arg.name); - return Ok(()); + ValueTypes::Ptr(v) => { + params.extend_from_slice(v.as_slice()); } } } - let mut params = Vec::new(); - // arguments currently don't arrive in the correct order, - // so we'll order them manually - params.extend_from_slice(target.as_slice()); - params.extend_from_slice(vni.as_slice()); - params.extend_from_slice(mac.as_slice()); - ("forward_to_sled", params) + ("rewrite_src_mac", params) + } + (TableType::McastEgressPortMapping, "set_port_number") => { + ("set_port_number", vec![port_number_param(&action_data)?]) } + // sidecar-lite's resolvers support drop (by default). + (TableType::ArpIpv4, "drop") + | (TableType::NeighborIpv6, "drop") => ("drop", Vec::new()), + // The local_v4/local_v6 tables have no drop action and dpd programs + // dropv4/dropv6 during port IP setup. + (TableType::PortAddrIpv4, "dropv4") + | (TableType::PortAddrIpv6, "dropv6") => return Ok(()), (_, x) => { - error!(hdl.log, "ACTION NOT HANDLED {name} {x}"); - return Ok(()); + error!(hdl.log, "action not handled: {name} {x}"); + return Err(AsicError::InvalidArg(format!( + "{name}: action {x} not handled by the softnpu backend" + ))); } }; let action = action.to_string(); trace!(hdl.log, "sending request to softnpu"); - trace!(hdl.log, "table: {name}"); - trace!(hdl.log, "action: {:#?}", action); - trace!(hdl.log, "keyset_data:\n{:#?}", keyset_data); - trace!(hdl.log, "parameter_data:\n{:#?}", parameter_data); + trace!(hdl.log, "table: {table}"); + trace!(hdl.log, "action: {action:#?}"); + trace!(hdl.log, "keyset_data:\n{keyset_data:#?}"); + trace!(hdl.log, "parameter_data:\n{parameter_data:#?}"); - let msg = ManagementRequest::TableAdd(TableAdd { - table: table.to_string(), - action, - keyset_data, - parameter_data, - }); - - crate::softnpu::mgmt::write(msg, &hdl.mgmt_config); - - Ok(()) + add_entry(hdl, table, action, keyset_data, parameter_data) } - fn entry_update( + fn entry_update( &self, hdl: &Handle, key: &M, data: &A, ) -> AsicResult<()> { - let Some(_table) = self.softnpu_table_name() else { - return Ok(()); - }; - let name = self.type_.to_string(); - - let match_data = key.key_to_ir().unwrap(); - let action_data = data.action_to_ir().unwrap(); - - trace!(hdl.log, "entry_update called"); - trace!(hdl.log, "table: {name}"); - trace!(hdl.log, "match_data:\n{:#?}", match_data); - trace!(hdl.log, "action_data:\n{:#?}", action_data); - - //TODO implement in softnpu - Ok(()) + self.entry_add(hdl, key, data) } fn entry_del( @@ -566,27 +647,25 @@ impl TableOps for Table { let Some(table) = self.softnpu_table_name() else { return Ok(()); }; + let table = table.to_string(); let name = self.type_.to_string(); let match_data = key.key_to_ir().unwrap(); + let fields = match_data.fields; trace!(hdl.log, "entry_del called"); trace!(hdl.log, "table: {name}"); - trace!(hdl.log, "match_data:\n{:#?}", match_data); + trace!(hdl.log, "match_data: {fields:#?}"); - let keyset_data = keyset_data(match_data.fields, self.type_); + let keyset_data = keyset_data(fields, self.type_); trace!(hdl.log, "sending request to softnpu"); - trace!(hdl.log, "table: {name}"); - trace!(hdl.log, "keyset_data:\n{:#?}", keyset_data); + trace!(hdl.log, "table: {table}"); + trace!(hdl.log, "keyset_data:\n{keyset_data:#?}"); - let msg = ManagementRequest::TableRemove(TableRemove { - keyset_data, - table: table.to_string(), - }); + let msg = + ManagementRequest::TableRemove(TableRemove { keyset_data, table }); - crate::softnpu::mgmt::write(msg, &hdl.mgmt_config); - - Ok(()) + crate::softnpu::mgmt::write(msg, &hdl.mgmt_config) } fn get_entries( @@ -594,7 +673,7 @@ impl TableOps for Table { _hdl: &Handle, _from_hardware: bool, ) -> AsicResult> { - Err(aal::AsicError::OperationUnsupported) + Err(AsicError::OperationUnsupported) } fn get_counters( @@ -606,8 +685,147 @@ impl TableOps for Table { } } +/// Extract the `forward_to_sled` action parameters shared by unicast and +/// multicast NAT/attached-subnet actions. +/// +/// Returns the sidecar-lite action name and serialized parameter bytes. +fn forward_to_sled_params( + hdl: &Handle, + name: &str, + action_data: aal::ActionData, +) -> AsicResult<(&'static str, Vec)> { + for arg in &action_data.args { + if !matches!(arg.name.as_str(), "target" | "vni" | "inner_mac") { + error!(hdl.log, "unknown argument: {} in {name}", arg.name); + return Err(AsicError::InvalidArg(format!( + "{name}: unknown argument {}", + arg.name + ))); + } + } + + let target = arg_ptr(hdl, &action_data, name, "target")?; + let vni = arg_u64(hdl, &action_data, name, "vni")?; + let mac = arg_u64(hdl, &action_data, name, "inner_mac")?; + + let Some(start) = target.len().checked_sub(16) else { + return Err(AsicError::InvalidArg(format!( + "{name}: target is {} bytes, expected at least 16", + target.len() + ))); + }; + + let mut params = Vec::new(); + let mut target_bytes = target[start..].to_vec(); + target_bytes.reverse(); + params.extend_from_slice(&target_bytes); + params.extend_from_slice(&vni.to_le_bytes()[0..3]); + params.extend_from_slice(&mac.to_le_bytes()[0..6]); + Ok(("forward_to_sled", params)) +} + +fn find_arg<'a>( + action_data: &'a aal::ActionData, + action: &str, + arg_name: &str, +) -> AsicResult<&'a ValueTypes> { + action_data + .args + .iter() + .find(|arg| arg.name == arg_name) + .map(|arg| &arg.value) + .ok_or_else(|| { + AsicError::InvalidArg(format!( + "{action}: missing argument {arg_name}" + )) + }) +} + +fn arg_u64( + hdl: &Handle, + action_data: &aal::ActionData, + action: &str, + arg_name: &str, +) -> AsicResult { + match find_arg(action_data, action, arg_name)? { + ValueTypes::U64(v) => Ok(*v), + ValueTypes::Ptr(_) => { + error!(hdl.log, "expected ValueType::U64, received ValueType::Ptr"); + Err(AsicError::InvalidArg(format!( + "{action}: {arg_name} expects ValueType::U64" + ))) + } + } +} + +fn arg_u16( + hdl: &Handle, + action_data: &aal::ActionData, + action: &str, + arg_name: &str, +) -> AsicResult { + let v = arg_u64(hdl, action_data, action, arg_name)?; + u16::try_from(v).map_err(|_| { + AsicError::InvalidArg(format!("{action}: {arg_name} {v} exceeds u16")) + }) +} + +fn arg_ptr<'a>( + hdl: &Handle, + action_data: &'a aal::ActionData, + action: &str, + arg_name: &str, +) -> AsicResult<&'a [u8]> { + match find_arg(action_data, action, arg_name)? { + ValueTypes::Ptr(v) => Ok(v.as_slice()), + ValueTypes::U64(_) => { + error!(hdl.log, "expected ValueType::Ptr, received ValueType::U64"); + Err(AsicError::InvalidArg(format!( + "{action}: {arg_name} expects ValueType::Ptr" + ))) + } + } +} + +/// Pack the low 4 chunks (ports 0-127) of dpd's 8x32-bit decap port +/// bitmap into the 16 bytes of sidecar-lite's `bit<128>` `decap_bitmap` +/// field (in little-endian). +/// +/// NOTE: The high chunks are always zero on this backend and ports >= 128 +/// are rejected when adding members. Emitting exactly 16 bytes keeps +/// any following action parameter (the tagged variant's vlan_id) at +/// the offset sidecar-lite reads it from. +fn pack_decap_bitmap(args: &aal::ActionData) -> Vec { + let mut chunks = [0u32; 4]; + for arg in &args.args { + if let ValueTypes::U64(v) = &arg.value { + match arg.name.as_str() { + "ports_0" => chunks[0] = *v as u32, + "ports_1" => chunks[1] = *v as u32, + "ports_2" => chunks[2] = *v as u32, + "ports_3" => chunks[3] = *v as u32, + _ => {} + } + } + } + chunks.iter().flat_map(|c| c.to_le_bytes()).collect() +} + +fn port_number_param(args: &aal::ActionData) -> AsicResult { + args.args + .iter() + .find(|arg| arg.name == "port_number") + .and_then(|arg| u8::try_from(&arg.value).ok()) + .ok_or_else(|| { + AsicError::InvalidArg(format!( + "{}: missing or invalid port_number", + args.action + )) + }) +} + /// Extract keys from `match_data` and ensure that they are -/// in a data structure with the correct length +/// in a data structure with the correct length. fn keyset_data(match_data: Vec, table: TableType) -> Vec { let mut keyset_data: Vec = Vec::new(); for m in match_data { @@ -622,7 +840,7 @@ fn keyset_data(match_data: Vec, table: TableType) -> Vec { keyset_data.extend_from_slice(&data[..4]); } TableType::NeighborIpv6 => { - // "nexthop_ipv4" => bit<128> + // "nexthop_ipv6" => bit<128> let mut buf = Vec::new(); serialize_value_type(&x, &mut buf); buf.reverse(); @@ -655,6 +873,44 @@ fn keyset_data(match_data: Vec, table: TableType) -> Vec { buf.reverse(); keyset_data.extend_from_slice(&buf); } + // Multicast replication: hdr.ipv6.dst => bit<128> + TableType::McastIpv6 => { + let mut buf = Vec::new(); + serialize_value_type(&x, &mut buf); + buf.reverse(); + keyset_data.extend_from_slice(&buf); + } + // Multicast source filter exact keys: inner dst => + // bit<32> or bit<128>. The LPM src key is handled + // in the LPM branch. + TableType::McastIpv4SrcFilter => { + serialize_value_type(&x, &mut data); + keyset_data.extend_from_slice(&data[..4]); + } + TableType::McastIpv6SrcFilter => { + let mut buf = Vec::new(); + serialize_value_type(&x, &mut buf); + buf.reverse(); + keyset_data.extend_from_slice(&buf); + } + // Multicast NAT: dst => bit<32> or bit<128> + TableType::NatIngressIpv4Mcast => { + serialize_value_type(&x, &mut data); + keyset_data.extend_from_slice(&data[..4]); + } + TableType::NatIngressIpv6Mcast => { + let mut buf = Vec::new(); + serialize_value_type(&x, &mut buf); + buf.reverse(); + keyset_data.extend_from_slice(&buf); + } + // Multicast egress tables: port => bit<16> + TableType::McastEgressDecapPorts + | TableType::McastEgressPortMapping + | TableType::PortMacAddressMcast => { + serialize_value_type(&x, &mut data); + keyset_data.extend_from_slice(&data[..2]); + } _ => { serialize_value_type(&x, &mut keyset_data); } @@ -670,7 +926,13 @@ fn keyset_data(match_data: Vec, table: TableType) -> Vec { serialize_value_type_be(&x.prefix, &mut data); keyset_data.extend_from_slice(&data[data.len() - 4..]); // prefix length for longest prefix match operation - keyset_data.push(x.len as u8) + keyset_data.push(x.len as u8); + } + TableType::McastIpv4SrcFilter => { + // "src_addr" => bit<32> lpm + serialize_value_type_be(&x.prefix, &mut data); + keyset_data.extend_from_slice(&data[data.len() - 4..]); + keyset_data.push(x.len as u8); } _ => { serialize_value_type_be(&x.prefix, &mut keyset_data); @@ -682,7 +944,7 @@ fn keyset_data(match_data: Vec, table: TableType) -> Vec { MatchEntryValue::Range(x) => { match table { TableType::NatIngressIpv4 | TableType::NatIngressIpv6 => { - // "l4_dst_port" => ingress.nat_id: range => bit<16> + // "l4_dst_port" => ingress.nat_id: range => bit<16> let low = &x.low.to_le_bytes(); let high = &x.high.to_le_bytes(); keyset_data.extend_from_slice(&low[..2]); @@ -725,3 +987,61 @@ fn serialize_value_type_be(x: &ValueTypes, data: &mut Vec) { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn port_mapping_serializes_for_sidecar_lite() { + let fields = vec![MatchEntryField { + name: "egress_port".to_string(), + value: MatchEntryValue::Value(ValueTypes::U64(0x0102)), + }]; + assert_eq!( + keyset_data(fields, TableType::McastEgressPortMapping), + vec![0x02, 0x01] + ); + + let action_data = aal::ActionData { + action: "set_port_number".to_string(), + args: vec![aal::ActionArg { + name: "port_number".to_string(), + value: ValueTypes::U64(17), + }], + }; + assert_eq!(port_number_param(&action_data).unwrap(), 17); + + let missing = aal::ActionData { + action: "set_port_number".to_string(), + args: Vec::new(), + }; + assert!(matches!( + port_number_param(&missing), + Err(AsicError::InvalidArg(_)) + )); + } + + #[test] + fn mcast_nat_keys_serialize_for_sidecar_lite() { + let v4 = vec![MatchEntryField { + name: "dst_addr".to_string(), + value: MatchEntryValue::Value(ValueTypes::U64(0xe000_0101)), + }]; + assert_eq!( + keyset_data(v4, TableType::NatIngressIpv4Mcast), + vec![0x01, 0x01, 0x00, 0xe0] + ); + + let addr: std::net::Ipv6Addr = "ff0e::1:1010".parse().unwrap(); + let v6 = vec![MatchEntryField { + name: "dst_addr".to_string(), + value: MatchEntryValue::Value(ValueTypes::Ptr( + addr.octets().to_vec(), + )), + }]; + let mut expected = addr.octets().to_vec(); + expected.reverse(); + assert_eq!(keyset_data(v6, TableType::NatIngressIpv6Mcast), expected); + } +} diff --git a/dpd-client/tests/integration_tests/mcast.rs b/dpd-client/tests/integration_tests/mcast.rs index e076d499..15190c81 100644 --- a/dpd-client/tests/integration_tests/mcast.rs +++ b/dpd-client/tests/integration_tests/mcast.rs @@ -7543,6 +7543,98 @@ async fn test_source_filter_update_to_any() -> TestResult { "After update with Any, should have only 1 entry (the /0)" ); + // Narrow to specific sources for (*,G) -> (S,G) transitions, which is + // exercised below with `sources: None`. + let narrow_entry = types::MulticastGroupUpdateExternalEntry { + internal_forwarding: types::ExternalInternalForwarding { + nat_target: nat_target.clone(), + }, + external_forwarding: types::ExternalForwarding { vlan_id: Some(10) }, + sources: Some(vec![ + types::IpSrc::Exact("192.168.1.1".parse().unwrap()), + types::IpSrc::Exact("192.168.1.2".parse().unwrap()), + ]), + }; + + let narrowed = switch + .client + .multicast_group_update_external( + &external_group_ip.try_into().unwrap(), + &TEST_TAG, + &narrow_entry, + ) + .await + .expect("Should narrow external group sources") + .into_inner(); + + assert_eq!( + narrowed.sources.as_deref().map(<[_]>::len), + Some(2), + "Narrowing should restore 2 specific sources" + ); + + let after_narrowed_table = switch + .client + .table_dump(SOURCE_FILTER_IPV4_TABLE, false) + .await + .expect("Should dump table after narrowing"); + + assert_eq!( + after_narrowed_table.entries.len(), + baseline_count + 2, + "Narrowing should restore 2 specific entries" + ); + + let keep_entry = types::MulticastGroupUpdateExternalEntry { + internal_forwarding: types::ExternalInternalForwarding { + nat_target: nat_target.clone(), + }, + external_forwarding: types::ExternalForwarding { vlan_id: Some(10) }, + sources: None, + }; + + let kept = switch + .client + .multicast_group_update_external( + &external_group_ip.try_into().unwrap(), + &TEST_TAG, + &keep_entry, + ) + .await + .expect("Should update external group with sources: None") + .into_inner(); + + assert_eq!( + kept.sources, narrowed.sources, + "sources: None must preserve the current filter" + ); + + // Round-trip via GET to confirm the stored representation matches. + let fetched = switch + .client + .multicast_group_get(&external_group_ip) + .await + .expect("Should get external group") + .into_inner(); + + assert_eq!( + get_sources(&fetched), + kept.sources, + "GET after sources: None must agree with the update response" + ); + + let after_keep_table = switch + .client + .table_dump(SOURCE_FILTER_IPV4_TABLE, false) + .await + .expect("Should dump table after sources: None"); + + assert_eq!( + after_keep_table.entries.len(), + baseline_count + 2, + "sources: None must keep the 2 specific entries" + ); + // Cleanup in correct order: external first, then internal cleanup_test_group(switch, external_group_ip, &TEST_TAG).await.unwrap(); cleanup_test_group(switch, internal_group_ip, &TEST_TAG).await diff --git a/dpd/src/mcast/mod.rs b/dpd/src/mcast/mod.rs index e1b32e0b..9409b51e 100644 --- a/dpd/src/mcast/mod.rs +++ b/dpd/src/mcast/mod.rs @@ -414,12 +414,77 @@ impl From<&MulticastGroup> for MulticastGroupResponse { } } +/// Multicast groups awaiting repair, with source-filter keys whose removal +/// can't be confirmed. +/// +/// A failed rollback of an external update can leave source filters in the +/// ASIC that are absent from the stored group. Repair and deletion clear these +/// unobserved sources first. +/// +/// NOTE: On backends with per-table dumps (e.g., Tofino), the dump in +/// `del_entries_where` already handles this. We track them here for +/// backends without per-table dumps (e.g., SoftNPU), where a leftover +/// `/0` could admit every source. +#[derive(Debug, Default)] +struct PendingRepairs(BTreeMap>); + +impl PendingRepairs { + fn contains(&self, key: &Q) -> bool + where + GroupKey: Borrow, + Q: Ord + ?Sized, + { + self.0.contains_key(key) + } + + fn insert(&mut self, key: GroupKey) { + self.0.entry(key).or_default(); + } + + fn extend(&mut self, keys: impl IntoIterator) { + keys.into_iter().for_each(|key| self.insert(key)); + } + + fn add_sources( + &mut self, + key: GroupKey, + srcs: impl IntoIterator, + ) { + self.0.entry(key).or_default().extend(srcs); + } + + fn sources(&self, key: &Q) -> BTreeSet + where + GroupKey: Borrow, + Q: Ord + ?Sized, + { + self.0.get(key).cloned().unwrap_or_default() + } + + fn remove(&mut self, key: &Q) -> bool + where + GroupKey: Borrow, + Q: Ord + ?Sized, + { + self.0.remove(key).is_some() + } + + #[cfg(all(test, feature = "chaos"))] + fn is_empty(&self) -> bool { + self.0.is_empty() + } + + fn clear(&mut self) { + self.0.clear(); + } +} + /// Stores multicast group configurations. #[derive(Debug)] pub struct MulticastGroupData { /// Multicast group configurations keyed by group IP. groups: BTreeMap, - pending_repairs: BTreeSet, + pending_repairs: PendingRepairs, /// The stack of available group IDs for O(1) allocation. /// Pre-populated with all IDs from GENERATOR_START to u16::MAX-1. free_group_ids: Arc>>, @@ -444,7 +509,7 @@ impl MulticastGroupData { Self { groups: BTreeMap::new(), - pending_repairs: BTreeSet::new(), + pending_repairs: PendingRepairs::default(), free_group_ids, nat_target_refs: BTreeMap::new(), } @@ -805,6 +870,22 @@ fn del_group_locked( debug!(s.log, "deleting multicast group for IP {group_ip}"); + let stored_srcs = match &group.kind { + GroupKind::External { sources: srcs, .. } => Some(srcs), + GroupKind::Underlay { .. } => None, + }; + clear_unobserved_sources( + s, + group_ip, + &mcast.pending_repairs.sources(&group_ip), + stored_srcs, + ) + .map_err(|_| { + DpdError::McastRepairPending(format!( + "multicast group {group_ip} state is uncertain; retry the request" + )) + })?; + let rollback_ctx = GroupDeleteRollbackContext::new(s, &group); let mut deleted = Vec::new(); @@ -969,7 +1050,9 @@ fn repair_pending_external_group( external_forwarding: ExternalForwarding { vlan_id: *vlan_id }, sources: None, }; + let unobserved = mcast.pending_repairs.sources(&group_ip); let repaired = (|| { + clear_unobserved_sources(s, group_ip, &unobserved, Some(sources))?; repair_nat_target(s, mcast, *nat_target)?; update_external_tables(s, &group, &stored, sources, &[])?; let underlay = mcast.get_nat_target_group(group_ip, *nat_target)?; @@ -992,6 +1075,37 @@ fn repair_pending_external_group( Ok(()) } +/// Clear unobserved source filters for `group_ip` that aren't in `stored`. +fn clear_unobserved_sources( + s: &Switch, + group_ip: IpAddr, + unobserved: &BTreeSet, + stored: Option<&SourceFilter>, +) -> DpdResult<()> { + apply_each( + unobserved + .iter() + .filter(|source| { + !stored + .is_some_and(|srcs| srcs.iter().any(|src| src == **source)) + }) + .cloned(), + |source| clear_source_filter(s, group_ip, source), + ) + .map_err(|failures| { + for (source, e) in failures.iter() { + error!( + s.log, + "failed to clear unobserved source filter"; + "group_ip" => %group_ip, + "source" => ?source, + "error" => ?e, + ); + } + failures.head.1 + }) +} + fn repair_nat_target( s: &Switch, mcast: &mut MulticastGroupData, @@ -1095,8 +1209,13 @@ pub(crate) fn modify_group_external( ) { let err = rollback_ctx.rollback_external(e, &new_sources); if matches!(err, DpdError::Unwind { .. }) { - mcast.pending_repairs.extend([ + mcast.pending_repairs.add_sources( group_key, + new_sources + .iter() + .filter(|new| !old_sources.iter().any(|old| old == *new)), + ); + mcast.pending_repairs.extend([ GroupKey::underlay(old_underlay_ip), GroupKey::underlay(new_underlay_ip), ]); @@ -1167,7 +1286,8 @@ pub(crate) fn modify_group_underlay( } // Repair the route before changing memberships or replication. - if mcast.pending_repairs.contains(&group_key) { + let repair = mcast.pending_repairs.contains(&group_key); + if repair { table::mcast::mcast_route::set_ipv6_entry(s, group_ip.into(), None)?; mc_group_set(s, existing_group.external_group_id())?; mc_group_set(s, existing_group.underlay_group_id())?; @@ -1179,9 +1299,22 @@ pub(crate) fn modify_group_underlay( MulticastReplicationInfo::new(original.external_group_id()) }); + // The decap bitmap must cover external ports before they are added to + // ASIC groups. + // + // Normal updates write the bitmap before port changes only when widening + // it. A group pending repair may have lost its bitmap during rollback; + // restore coverage for its stored external members as well. let widened = (!members.is_empty()) - .then(|| original.widened_external_bitmap(&members)) + .then(|| { + original.widened_external_bitmap(&members).or_else(|| { + repair.then(|| { + create_port_bitmap(original.members(), Direction::External) + }) + }) + }) .flatten(); + let mut added_members = Vec::new(); let mut removed_members = Vec::new(); let programmed = (|| { @@ -1215,27 +1348,33 @@ pub(crate) fn modify_group_underlay( .map_err(|e| (UpdateStep::Tables, e)); } - // Ensure the bitmap exists before the replication entry. The VLAN - // comes from the referencing external group. The bitmap covers - // overlay members only because decapsulation applies only to overlay - // traffic. - table::mcast::mcast_egress::set_bitmap_entry( - s, - original.external_group_id(), - &create_port_bitmap(&members, Direction::External), - external_group_vlan_id, - ) - .map_err(|e| (UpdateStep::Tables, e))?; - - // Ensure the replication entry on every update with members. - // - // An entry lost to a partial delete can then come back. It's inserted - // after the bitmap, where a rollback deletes a newly added bitmap on - // failure, and if that delete also fails, then the leftover is a - // bitmap that nothing can replicate toward (instead of a replication - // entry with no decap entry). + let union_bitmap = widened.clone().unwrap_or_else(|| { + create_port_bitmap(original.members(), Direction::External) + }); + + // Write the replication entry on membership updates. Backends + // whose replication entry persists a write-time port bitmap (e.g., + // SoftNPU) only see membership changes when the bitmap is rewritten. set_replication_entry(s, &original, &info) - .map_err(|e| (UpdateStep::Tables, e)) + .map_err(|e| (UpdateStep::Tables, e))?; + + // Only narrow the decap port bitmap after the replication entry is + // rewritten, keeping decap at a superset of the ports replication can + // still send to. Backends whose replication entry holds a write-time + // port bitmap (e.g., SoftNPU) keep sending to removed ports until the + // replication entry is rewritten, and those packet copies must still + // have their Geneve header stripped. + let new_bitmap = create_port_bitmap(&members, Direction::External); + if new_bitmap != union_bitmap { + table::mcast::mcast_egress::set_bitmap_entry( + s, + original.external_group_id(), + &new_bitmap, + external_group_vlan_id, + ) + .map_err(|e| (UpdateStep::Tables, e))?; + } + Ok(()) })(); if let Err((step, e)) = programmed { @@ -1373,7 +1512,12 @@ fn resync_after_failed_unwind( } } - match reprogram_underlay_entries(s, &resynced, vlan_id) { + match reprogram_underlay_entries( + s, + &resynced, + vlan_id, + group_entry.members(), + ) { Ok(()) => Some(resynced), Err(e) => { error!( @@ -1400,10 +1544,11 @@ fn clear_group_after_failed_resync( ) } -fn reprogram_underlay_entries( +fn reprogram_underlay_entries<'a>( s: &Switch, - group: &MulticastGroup, + group: &'a MulticastGroup, vlan_id: Option, + previous_members: impl IntoIterator, ) -> DpdResult<()> { let GroupKind::Underlay { group_ip, replication: repl } = &group.kind else { @@ -1412,8 +1557,22 @@ fn reprogram_underlay_entries( match repl { Replication::Active { info, .. } => { - set_group_bitmap_entries(s, group, vlan_id)?; + let union_bitmap = create_port_bitmap( + group.members().chain(previous_members), + Direction::External, + ); + table::mcast::mcast_egress::set_bitmap_entry( + s, + group.external_group_id(), + &union_bitmap, + vlan_id, + )?; set_replication_entry(s, group, info)?; + let bitmap = + create_port_bitmap(group.members(), Direction::External); + if bitmap != union_bitmap { + set_group_bitmap_entries(s, group, vlan_id)?; + } } Replication::Empty => cleanup_empty_group_replication(s, group)?, } @@ -1440,9 +1599,6 @@ fn resync_members_from_asic( let asic_ids = match s.asic_hdl.mc_port_list(group_id) { Ok(asic_ids) => asic_ids, Err(AsicError::Missing(_)) => Vec::new(), - Err(AsicError::OperationUnsupported) => { - return Some(group_entry.clone()); - } Err(e) => { error!( s.log, @@ -1664,7 +1820,7 @@ fn ensure_bitmap_vlan( res => return res, } - reprogram_underlay_entries(s, group, vlan_id) + reprogram_underlay_entries(s, group, vlan_id, group.members()) } /// Remove source filters for a multicast group. @@ -1817,13 +1973,29 @@ fn clear_source_filter( SourceFilterKey::new(group_ip, source).and_then(|key| key.clear(s)) } -/// Remove ASIC source-filter entries for `group_ip` that are not in -/// `sources`. +/// Remove ASIC source-filter entries for `group` that are not in `sources`. +/// +/// Stored sources not in `sources` are cleared first, and then any other +/// listed entry for the group address is removed. +/// +/// Backends that don't list entries (e.g., SoftNPU) only clear the stored +/// sources. fn remove_unwanted_source_filters( s: &Switch, - group_ip: IpAddr, + group: &MulticastGroup, sources: &SourceFilter, ) -> DpdResult<()> { + let group_ip = group.ip(); + if let GroupKind::External { sources: old_sources, .. } = &group.kind { + apply_each( + old_sources + .iter() + .filter(|old| !sources.iter().any(|new| new == *old)), + |source| clear_source_filter(s, group_ip, source), + ) + .map_err(|failures| failures.head.1)?; + } + let mut ipv4_sources = BTreeSet::new(); let mut ipv6_sources = BTreeSet::new(); for source in sources.iter() { @@ -1838,17 +2010,17 @@ fn remove_unwanted_source_filters( } match group_ip { - IpAddr::V4(group) => { + IpAddr::V4(ipv4) => { table::mcast::mcast_src_filter::retain_ipv4_sources( s, - group, + ipv4, &ipv4_sources, ) } - IpAddr::V6(group) => { + IpAddr::V6(ipv6) => { table::mcast::mcast_src_filter::retain_ipv6_sources( s, - group, + ipv6, &ipv6_sources, ) } @@ -2213,15 +2385,6 @@ fn remove_unwanted_group_members( ); continue; } - Err(AsicError::OperationUnsupported) => { - debug!( - s.log, - "multicast group readback unsupported, skipping unwanted port removal"; - "group_ip" => %group_ip, - "group_id" => group_id, - ); - continue; - } Err(e) => return Err(e.into()), }; @@ -2342,7 +2505,7 @@ fn update_external_tables( set_source_filter(s, group_ip, source) }) .map_err(|failures| failures.head.1)?; - remove_unwanted_source_filters(s, group_ip, new_sources)?; + remove_unwanted_source_filters(s, group_entry, new_sources)?; let new_vlan_id = new_group_info.external_forwarding.vlan_id; @@ -2464,20 +2627,15 @@ fn delete_group_tables( match &group.kind { GroupKind::External { sources, .. } => { deleted.push(DeleteStep::SourceFilters); - let removed = match group_ip { + remove_source_filters(s, group_ip, sources)?; + match group_ip { IpAddr::V4(ipv4) => { table::mcast::mcast_src_filter::del_ipv4_entries(s, ipv4) } IpAddr::V6(ipv6) => { table::mcast::mcast_src_filter::del_ipv6_entries(s, ipv6) } - }; - match removed { - Err(DpdError::Switch(AsicError::OperationUnsupported)) => { - remove_source_filters(s, group_ip, sources)? - } - res => res?, - } + }?; match group_ip { IpAddr::V4(ipv4) => { table::mcast::mcast_nat::clear_ipv4_entry(s, ipv4) diff --git a/dpd/src/mcast/rollback.rs b/dpd/src/mcast/rollback.rs index 3ed91ff7..28b06783 100644 --- a/dpd/src/mcast/rollback.rs +++ b/dpd/src/mcast/rollback.rs @@ -20,9 +20,9 @@ use super::{ Direction, ExternalMulticastIp, GroupKind, IpSrc, MulticastGroup, MulticastGroupId, MulticastGroupMember, MulticastReplicationInfo, Replication, SourceFilter, UnderlayMulticastIpv6, clear_source_filter, - mc_group_clear, mc_group_set, mc_port_clear, mc_port_set, - restore_source_filters, set_group_bitmap_entries, set_replication_entry, - set_source_filter, update_fwding_tables, + create_port_bitmap, mc_group_clear, mc_group_set, mc_port_clear, + mc_port_set, restore_source_filters, set_group_bitmap_entries, + set_replication_entry, set_source_filter, update_fwding_tables, }; use crate::{ Switch, table, @@ -407,7 +407,23 @@ impl<'a> GroupCreateRollbackContext<'a> { return Ok(()); }; - unwind_outcome([ + self.log_rollback_error( + "delete IPv6 replication entry", + &format!("for group {ipv6}"), + table::mcast::mcast_replication::clear_ipv6_entry( + self.switch, + ipv6, + ), + ) + .inspect_err(|_| { + warn!( + self.switch.log, + "replication entry delete failed, skipping decap bitmap delete"; + "group" => %ipv6, + "external_group_id" => %self.external_id, + ); + }) + .and_then(|_| { self.log_rollback_error( "delete IPv6 egress bitmap entry", &format!("for external group {}", self.external_id), @@ -415,16 +431,8 @@ impl<'a> GroupCreateRollbackContext<'a> { self.switch, self.external_id, ), - ), - self.log_rollback_error( - "delete IPv6 replication entry", - &format!("for group {ipv6}"), - table::mcast::mcast_replication::clear_ipv6_entry( - self.switch, - ipv6, - ), - ), - ]) + ) + }) } fn remove_external_entries(&self) -> DpdResult<()> { @@ -528,15 +536,58 @@ impl<'a> GroupUpdateRollbackContext<'a> { Self { switch, original_group, table_vlan_id } } - /// Restore table entries to original state. - fn restore_tables(&self) -> DpdResult<()> { + fn restore_union_bitmap( + &self, + added_ports: &[MulticastGroupMember], + ) -> DpdResult<()> { + if !matches!( + self.original_group.kind, + GroupKind::Underlay { replication: Replication::Active { .. }, .. } + ) { + return Ok(()); + } + + // Port rollback can immediately enable replicas on Tofino for example. + // + // This restores decapsulation for the original and added members before + // modifying any ports. + let union_bitmap = create_port_bitmap( + self.original_group.members().chain(added_ports), + Direction::External, + ); + self.log_rollback_error( + "restore decap bitmap", + &format!("for group {}", self.group_ip()), + table::mcast::mcast_egress::set_bitmap_entry( + self.switch, + self.external_group_id(), + &union_bitmap, + self.table_vlan_id, + ), + ) + } + + fn restore_tables( + &self, + added_ports: &[MulticastGroupMember], + ports_restored: bool, + ) -> DpdResult<()> { match &self.original_group.kind { GroupKind::Underlay { replication: Replication::Active { .. }, .. - } => self - .restore_fwding_tables() - .and_then(|()| self.restore_replication()), + } => { + self.restore_replication()?; + if ports_restored + && added_ports + .iter() + .any(|member| member.direction == Direction::External) + { + self.restore_fwding_tables() + } else { + Ok(()) + } + } GroupKind::Underlay { replication: Replication::Empty, .. } => { unwind_outcome([ self.remove_added_bitmap(), @@ -636,6 +687,10 @@ impl<'a> GroupUpdateRollbackContext<'a> { /// Undo an underlay group's port changes, restore its tables, and report /// the result with the failure that originally triggered the rollback. + /// + /// If the decap bitmap can't be restored to cover both original + /// and added members, rollback removes the added ports and avoids re-adding + /// removed ports. pub(crate) fn rollback_underlay_and_restore( &self, err: DpdError, @@ -643,11 +698,23 @@ impl<'a> GroupUpdateRollbackContext<'a> { added_ports: &[MulticastGroupMember], removed_ports: &[MulticastGroupMember], ) -> DpdError { - after_unwind( - err, - self.rollback_underlay_update(added_ports, removed_ports, repl) - .and(self.restore_tables()), - ) + if let Err(unwind) = self.restore_union_bitmap(added_ports) { + if !removed_ports.is_empty() { + warn!( + self.switch.log, + "decap bitmap restore failed, leaving removed ports out \ + of the group"; + "group" => %self.group_ip(), + "skipped_ports" => removed_ports.len(), + ); + } + let ports = self.rollback_underlay_update(added_ports, &[], repl); + return after_unwind(err, unwind_outcome([Err(unwind), ports])); + } + let ports = + self.rollback_underlay_update(added_ports, removed_ports, repl); + let tables = self.restore_tables(added_ports, ports.is_ok()); + after_unwind(err, ports.and(tables)) } } @@ -1738,6 +1805,7 @@ mod chaos_tests { source_table, TableOp::EntryDel, CallRange::starting_at(1), + AsicError::Synthetic, ); let update = || MulticastGroupUpdateExternalEntry { internal_forwarding: ExternalInternalForwarding { @@ -1793,6 +1861,132 @@ mod chaos_tests { } } + #[test] + fn test_external_cleanup_without_listing() { + for (group_ip, source_ip, del) in [ + ("224.1.2.3", "192.0.2.1", false), + ("ff0e::1", "2001:db8::1", false), + ("224.1.2.3", "192.0.2.1", true), + ("ff0e::1", "2001:db8::1", true), + ] { + let external_ip: IpAddr = group_ip.parse().unwrap(); + let (_, source_table, _) = external_tables(external_ip); + let (switch, _logs) = chaos_switch(Config::default()); + let (underlay_ip, _) = install_underlay_group(&switch); + let nat_target = external_nat_target(external_ip, underlay_ip); + let tag = "external-unlisted-cleanup-test"; + add_group_external( + &switch, + external_entry( + external_ip, + tag, + nat_target, + None, + serde_json::json!([{ "Exact": source_ip }]), + ), + ) + .unwrap(); + let stored = + serde_json::to_value(get_group(&switch, external_ip).unwrap()) + .unwrap(); + let stored_sources = source_filter_prefixes(&switch, external_ip); + + switch.asic_hdl.inject_table_faults( + source_table, + TableOp::EntryList, + CallRange::starting_at(1), + |_| AsicError::OperationUnsupported, + ); + switch.asic_hdl.inject_table_faults( + source_table, + TableOp::EntryDel, + CallRange::starting_at(1), + AsicError::Synthetic, + ); + let err = modify_group_external( + &switch, + ExternalMulticastIp::new(external_ip).unwrap(), + tag, + MulticastGroupUpdateExternalEntry { + internal_forwarding: ExternalInternalForwarding { + nat_target: nat_target.try_into().unwrap(), + }, + external_forwarding: ExternalForwarding { vlan_id: None }, + sources: Some(vec![SourceEntry::Any]), + }, + ) + .unwrap_err(); + assert!(matches!(err, DpdError::Unwind { .. }), "{err:?}"); + assert_eq!(table_usage(&switch, source_table).0, 2); + + let res = if del { + del_group(&switch, external_ip, tag) + } else { + get_group(&switch, external_ip).map(|_| ()) + }; + assert!( + matches!(res, Err(DpdError::McastRepairPending(_))), + "{res:?}" + ); + assert_eq!(table_usage(&switch, source_table).0, 2); + { + let mcast = switch.mcast.lock().unwrap(); + assert!(mcast.pending_repairs.contains(&external_ip)); + assert_eq!( + mcast.pending_repairs.sources(&external_ip), + BTreeSet::from([IpSrc::Any]) + ); + assert_eq!( + serde_json::to_value(MulticastGroupResponse::from( + &mcast.groups[&external_ip] + )) + .unwrap(), + stored, + ); + assert_eq!( + mcast.nat_target_refs.get(&underlay_ip).map(GroupKey::ip), + Some(external_ip) + ); + } + + switch.asic_hdl.clear_table_faults(source_table, TableOp::EntryDel); + if del { + del_group(&switch, external_ip, tag).unwrap(); + assert_eq!(table_usage(&switch, source_table).0, 0); + assert!(route_action(&switch, external_ip).is_none()); + assert!(nat_target_addr(&switch, external_ip).is_none()); + let mcast = switch.mcast.lock().unwrap(); + assert!(!mcast.groups.contains_key(&external_ip)); + assert!(!mcast.nat_target_refs.contains_key(&underlay_ip)); + } else { + assert_eq!( + serde_json::to_value( + get_group(&switch, external_ip).unwrap() + ) + .unwrap(), + stored, + ); + assert_eq!(table_usage(&switch, source_table).0, 1); + } + assert!( + !switch + .mcast + .lock() + .unwrap() + .pending_repairs + .contains(&external_ip) + ); + + switch + .asic_hdl + .clear_table_faults(source_table, TableOp::EntryList); + assert_eq!( + source_filter_prefixes(&switch, external_ip), + if del { BTreeSet::new() } else { stored_sources } + ); + } + } + #[test] fn test_external_delete_removes_untracked_source_filters() { for (group_ip, source_ip, stray_ip) in [ @@ -1827,6 +2021,60 @@ mod chaos_tests { } } + // Without per-table dumps (e.g., SoftNPU), only the group's stored + // sources get removed; the untracked entries remain in place. + #[test] + fn test_delete_unlisted_source_filters() { + for (group_ip, first_source, second_source, stray_ip) in [ + ("224.1.2.3", "192.0.2.1", "192.0.2.2", "192.0.2.3"), + ("ff0e::1", "2001:db8::1", "2001:db8::2", "2001:db8::3"), + ] { + let external_ip: IpAddr = group_ip.parse().unwrap(); + let (_, source_table, _) = external_tables(external_ip); + let (switch, _logs) = chaos_switch(Config::default()); + let (underlay_ip, _) = install_underlay_group(&switch); + let tag = "external-delete-unlisted-test"; + add_group_external( + &switch, + external_entry( + external_ip, + tag, + external_nat_target(external_ip, underlay_ip), + None, + serde_json::json!([ + { "Exact": first_source }, + { "Exact": second_source }, + ]), + ), + ) + .unwrap(); + set_source_filter( + &switch, + external_ip, + IpSrc::Exact(stray_ip.parse().unwrap()), + ) + .unwrap(); + assert_eq!(source_filter_prefixes(&switch, external_ip).len(), 3); + + switch.asic_hdl.inject_table_faults( + source_table, + TableOp::EntryList, + CallRange::starting_at(1), + |_| AsicError::OperationUnsupported, + ); + del_group(&switch, external_ip, tag).unwrap(); + switch + .asic_hdl + .clear_table_faults(source_table, TableOp::EntryList); + + let prefix_len = if external_ip.is_ipv4() { 32 } else { 128 }; + assert_eq!( + source_filter_prefixes(&switch, external_ip), + BTreeSet::from([format!("{stray_ip}/{prefix_len}")]) + ); + } + } + #[test] fn test_external_delete_vlan_cleanup_failure_marks_group() { for group_ip in ["224.1.2.3", "ff0e::1"] { @@ -1853,6 +2101,7 @@ mod chaos_tests { TableType::McastEgressDecapPorts, TableOp::EntryUpdate, CallRange::starting_at(1), + AsicError::Synthetic, ); let err = del_group(&switch, external_ip, tag).unwrap_err(); assert!(matches!(err, DpdError::Unwind { .. }), "{err:?}"); @@ -1894,11 +2143,13 @@ mod chaos_tests { TableType::RouteIpv6Mcast, TableOp::EntryAdd, CallRange::nth(1), + AsicError::Synthetic, ); switch.asic_hdl.inject_table_faults( TableType::McastIpv6, TableOp::EntryDel, CallRange::nth(1), + AsicError::Synthetic, ); let err = add_group_underlay(&switch, create()).unwrap_err(); @@ -1932,6 +2183,7 @@ mod chaos_tests { TableType::RouteIpv6Mcast, TableOp::EntryAdd, CallRange::starting_at(1), + AsicError::Synthetic, ); let err = get_group_underlay(&switch, membership_group_ip()).unwrap_err(); @@ -2087,11 +2339,13 @@ mod chaos_tests { route_table, TableOp::EntryAdd, CallRange::nth(1), + AsicError::Synthetic, ); switch.asic_hdl.inject_table_faults( nat_table, TableOp::EntryDel, CallRange::nth(1), + AsicError::Synthetic, ); let err = add_group_external(&switch, create()).unwrap_err(); @@ -2144,6 +2398,7 @@ mod chaos_tests { route_table, TableOp::EntryAdd, CallRange::starting_at(1), + AsicError::Synthetic, ); let err = get_group(&switch, external_ip).unwrap_err(); assert!(matches!(err, DpdError::McastRepairPending(_)), "{err:?}"); @@ -2204,11 +2459,13 @@ mod chaos_tests { TableType::McastIpv6, TableOp::EntryUpdate, CallRange::nth(1), + AsicError::Synthetic, ); switch.asic_hdl.inject_table_faults( TableType::McastEgressDecapPorts, TableOp::EntryUpdate, CallRange::nth(1), + AsicError::Synthetic, ); let tag = "external-create-vlan-test"; @@ -2787,6 +3044,12 @@ mod chaos_tests { u32::from(!bitmap_missing), "{context}" ); + switch + .mcast + .lock() + .unwrap() + .pending_repairs + .insert(underlay_key); let requested_members = if membership_changed { let kept: Vec<_> = members @@ -2824,6 +3087,10 @@ mod chaos_tests { let groups = switch.mcast.lock().unwrap(); assert!(groups.groups.contains_key(&underlay_key)); + assert!( + !groups.pending_repairs.contains(&underlay_key), + "{context}" + ); assert_eq!( groups.free_group_ids.lock().unwrap().len(), free_ids - 2, @@ -3006,6 +3273,7 @@ mod chaos_tests { TableType::McastEgressDecapPorts, TableOp::EntryDel, CallRange::nth(1), + AsicError::Synthetic, ); } } @@ -3785,9 +4053,10 @@ mod chaos_tests { let (tag, members) = underlay_tag_and_members(&switch, underlay_key); switch.asic_hdl.inject_table_faults( - TableType::McastEgressDecapPorts, - TableOp::EntryUpdate, - CallRange::between(2, 3), + TableType::McastIpv6, + TableOp::EntryAdd, + CallRange::between(1, 2), + AsicError::Synthetic, ); table::mcast::mcast_replication::clear_ipv6_entry( @@ -3818,7 +4087,7 @@ mod chaos_tests { let rollback_op = if port_remove_fails { "mc_port_remove" } else { - "table_entry_update" + "table_entry_add" }; assert!( matches!( @@ -3828,7 +4097,7 @@ mod chaos_tests { init.as_ref(), DpdError::Switch( aal::AsicError::Synthetic(op) - ) if op == "table_entry_update" + ) if op == "table_entry_add" ) && unwind.len() == 1 && matches!( &unwind.head, @@ -3849,10 +4118,10 @@ mod chaos_tests { let expected: &[(&str, &str)] = if port_remove_fails { &[ ("port changes", "mc_port_remove"), - ("restore VLAN settings", "table_entry_update"), + ("restore replication settings", "table_entry_add"), ] } else { - &[("restore VLAN settings", "table_entry_update")] + &[("restore replication settings", "table_entry_add")] }; { let errs = logs.entries.lock().unwrap(); @@ -4573,51 +4842,38 @@ mod chaos_tests { let mut with_dup = unsorted.to_vec(); with_dup.push(unsorted[1].clone()); - for readback in [true, false] { - let mut config = Config::default(); - if !readback { - config - .asic_config - .mc_config - .unsupported - .insert(McOp::McPortList); - } - let (switch, _logs) = chaos_switch(config); + let (switch, _logs) = chaos_switch(Config::default()); - let created = add_group_underlay( - &switch, - MulticastGroupCreateUnderlayEntry { - group_ip: membership_group_ip(), - tag: Some(MEMBERSHIP_TEST_TAG.to_string()), - members: unsorted.to_vec(), - }, - ) - .unwrap(); - assert_eq!(created.members, sorted, "readback={readback}"); - assert_consistent_membership(&switch, &sorted, readback); + let created = add_group_underlay( + &switch, + MulticastGroupCreateUnderlayEntry { + group_ip: membership_group_ip(), + tag: Some(MEMBERSHIP_TEST_TAG.to_string()), + members: unsorted.to_vec(), + }, + ) + .unwrap(); + assert_eq!(created.members, sorted); + assert_consistent_membership(&switch, &sorted); - let updated = put_membership(&switch, &with_dup).unwrap(); - assert_eq!(updated.members, sorted, "readback={readback}"); - assert_consistent_membership(&switch, &sorted, readback); - } + let updated = put_membership(&switch, &with_dup).unwrap(); + assert_eq!(updated.members, sorted); + assert_consistent_membership(&switch, &sorted); } fn assert_consistent_membership( switch: &Switch, expected: &[MulticastGroupMember], - readback: bool, ) { - assert_eq!(stored_membership(switch), expected, "readback={readback}"); + assert_eq!(stored_membership(switch), expected); for (reader, read) in READERS.into_iter().zip(read_memberships(switch)) { - assert_eq!(read, expected, "{reader:?} readback={readback}"); - } - if readback { - assert_eq!( - asic_membership(switch), - expected_membership(switch, expected) - ); + assert_eq!(read, expected, "{reader:?}"); } + assert_eq!( + asic_membership(switch), + expected_membership(switch, expected) + ); } #[test] @@ -4655,6 +4911,7 @@ mod chaos_tests { TableType::McastIpv6, TableOp::EntryUpdate, CallRange::starting_at(1), + AsicError::Synthetic, ); assert_uncertain_reads(&switch); @@ -4689,6 +4946,7 @@ mod chaos_tests { TableType::McastIpv6, TableOp::EntryUpdate, CallRange::starting_at(1), + AsicError::Synthetic, ); for read in read_memberships(&switch) { assert_eq!(read, members[..2]); @@ -4744,24 +5002,13 @@ mod chaos_tests { } fn underlay_read_restores_tables(reader: Reader) { - for (failed_table, readback) in [ - (TableType::McastIpv6, true), - (TableType::McastEgressDecapPorts, true), - (TableType::RouteIpv6Mcast, true), - (TableType::McastIpv6, false), - (TableType::McastEgressDecapPorts, false), - (TableType::RouteIpv6Mcast, false), + for failed_table in [ + TableType::McastIpv6, + TableType::McastEgressDecapPorts, + TableType::RouteIpv6Mcast, ] { let members = membership_members(); - let mut config = Config::default(); - if !readback { - config - .asic_config - .mc_config - .unsupported - .insert(McOp::McPortList); - } - let (switch, _logs) = chaos_switch(config); + let (switch, _logs) = chaos_switch(Config::default()); create_membership_group(&switch, &members); let external_ip: IpAddr = "239.1.2.3".parse().unwrap(); add_group_external( @@ -4810,6 +5057,7 @@ mod chaos_tests { failed_table, TableOp::EntryAdd, CallRange::starting_at(1), + AsicError::Synthetic, ); assert_uncertain_reads(&switch); @@ -4821,15 +5069,13 @@ mod chaos_tests { .pending_repairs .contains(&GroupKey::underlay(membership_group_ip())) ); - if readback { - assert_eq!( - asic_membership(&switch), - expected_membership(&switch, &members), - ); - } + assert_eq!( + asic_membership(&switch), + expected_membership(&switch, &members), + ); switch.asic_hdl.clear_table_faults(failed_table, TableOp::EntryAdd); - let context = format!("{reader:?}, {failed_table}, {readback}"); + let context = format!("{reader:?}, {failed_table}"); let expected = members.iter().cloned().collect::>(); assert_eq!( read_membership(&switch, reader) @@ -5040,6 +5286,272 @@ mod chaos_tests { } } + #[test] + fn test_underlay_narrow_failure_restores_group() { + let members = membership_members(); + for replacement in [None, Some(rear_member(3, Direction::External))] { + let context = format!("replacement={replacement:?}"); + let narrow_call = 1 + u64::from(replacement.is_some()); + let mut new_members = members[..2].to_vec(); + new_members.extend(replacement); + let (switch, logs) = chaos_switch(Config::default()); + create_membership_group(&switch, &members); + + let external_id = membership_group_id(&switch, Direction::External); + let bitmap_before = bitmap_entry(&switch, external_id); + let repl_before = + forwarding_table_snapshot(&switch, TableType::McastIpv6); + let repl_updates = table_usage(&switch, TableType::McastIpv6).3; + let group_before = { + let groups = switch.mcast.lock().unwrap(); + format!( + "{:?}", + groups.groups[&GroupKey::underlay(membership_group_ip())] + ) + }; + + switch.asic_hdl.inject_table_faults( + TableType::McastEgressDecapPorts, + TableOp::EntryUpdate, + CallRange::nth(narrow_call), + AsicError::Synthetic, + ); + let err = put_membership(&switch, &new_members).unwrap_err(); + assert!( + matches!( + &err, + DpdError::Switch(AsicError::Synthetic(op)) + if op == "table_entry_update" + ), + "{context}: {err:?}" + ); + switch.asic_hdl.clear_table_faults( + TableType::McastEgressDecapPorts, + TableOp::EntryUpdate, + ); + + assert_eq!( + bitmap_entry(&switch, external_id), + bitmap_before, + "{context}" + ); + assert_eq!( + forwarding_table_snapshot(&switch, TableType::McastIpv6), + repl_before, + "{context}" + ); + assert_eq!( + table_usage(&switch, TableType::McastIpv6).3, + repl_updates + 2, + "{context}" + ); + assert_eq!( + asic_membership(&switch), + expected_membership(&switch, &members), + "{context}" + ); + let groups = switch.mcast.lock().unwrap(); + let key = GroupKey::underlay(membership_group_ip()); + assert_eq!( + format!("{:?}", groups.groups[&key]), + group_before, + "{context}" + ); + assert!(!groups.pending_repairs.contains(&key), "{context}"); + assert!(logs.entries.lock().unwrap().is_empty(), "{context}"); + } + } + + #[test] + fn test_underlay_rollback_keeps_decap() { + #[derive(Clone, Copy, Debug)] + enum RestoreFailure { + Bitmap, + Ports, + Replication, + } + + for (narrow_applied, failure) in [ + (false, RestoreFailure::Replication), + (true, RestoreFailure::Replication), + (true, RestoreFailure::Ports), + (true, RestoreFailure::Bitmap), + ] { + let context = + format!("narrow_applied={narrow_applied}, failure={failure:?}"); + let [removed, underlay, added] = membership_members(); + let members = [removed, underlay.clone()]; + let new_members = [underlay, added]; + let (switch, logs) = chaos_switch(Config::default()); + create_membership_group(&switch, &members); + let original = switch.mcast.lock().unwrap().groups + [&GroupKey::underlay(membership_group_ip())] + .clone(); + let repl = original.replication_info().unwrap(); + let external_id = original.external_group_id(); + let union_bitmap = create_port_bitmap( + members.iter().chain(&new_members), + Direction::External, + ); + let new_bitmap = + create_port_bitmap(&new_members, Direction::External); + + table::mcast::mcast_egress::set_bitmap_entry( + &switch, + external_id, + &union_bitmap, + None, + ) + .unwrap(); + let mut added_ports = Vec::new(); + let mut removed_ports = Vec::new(); + process_membership_changes( + &switch, + &new_members, + &original, + repl, + &mut added_ports, + &mut removed_ports, + ) + .unwrap(); + + set_replication_entry(&switch, &original, repl).unwrap(); + if narrow_applied { + table::mcast::mcast_egress::set_bitmap_entry( + &switch, + external_id, + &new_bitmap, + None, + ) + .unwrap(); + } + + let repl_before = table_usage(&switch, TableType::McastIpv6); + let (op, failed_call) = match failure { + RestoreFailure::Bitmap => { + switch.asic_hdl.inject_table_faults( + TableType::McastEgressDecapPorts, + TableOp::EntryUpdate, + CallRange::nth(1), + AsicError::Synthetic, + ); + ("restore decap bitmap", "table_entry_update") + } + RestoreFailure::Ports => { + switch.asic_hdl.inject_mc_faults( + McOp::McPortRemove, + CallRange::nth(1), + ); + ("port changes", "mc_port_remove") + } + RestoreFailure::Replication => { + switch.asic_hdl.inject_table_faults( + TableType::McastIpv6, + TableOp::EntryUpdate, + CallRange::nth(1), + AsicError::Synthetic, + ); + ("restore replication settings", "table_entry_update") + } + }; + + let err = GroupUpdateRollbackContext::new(&switch, &original, None) + .rollback_underlay_and_restore( + DpdError::Switch(AsicError::Synthetic( + "table_entry_update".to_string(), + )), + repl, + &added_ports, + &removed_ports, + ); + let DpdError::Unwind { initial, unwind } = &err else { + panic!("{context}: expected a failed unwind: {err:?}"); + }; + assert!( + matches!( + initial.as_ref(), + DpdError::Switch(AsicError::Synthetic(op)) + if op == "table_entry_update" + ), + "{context}: {err:?}" + ); + assert_eq!(unwind.len(), 1, "{context}: {err:?}"); + assert!( + matches!( + &unwind.head, + DpdError::Switch(AsicError::Synthetic(op)) + if op == failed_call + ), + "{context}: {err:?}" + ); + + let expected_bitmap = match failure { + RestoreFailure::Bitmap => new_bitmap, + _ => union_bitmap.clone(), + }; + assert_eq!( + bitmap_entry(&switch, external_id), + Some((expected_bitmap, None)), + "{context}" + ); + let expected_members: Vec<_> = match failure { + RestoreFailure::Bitmap => new_members[..1].to_vec(), + RestoreFailure::Ports => { + members.iter().chain(&new_members[1..]).cloned().collect() + } + RestoreFailure::Replication => members.to_vec(), + }; + assert_eq!( + asic_membership(&switch), + expected_membership(&switch, &expected_members), + "{context}" + ); + let repl_updates = + u64::from(matches!(failure, RestoreFailure::Ports)); + assert_eq!( + table_usage(&switch, TableType::McastIpv6), + ( + repl_before.0, + repl_before.1, + repl_before.2, + repl_before.3 + repl_updates, + ), + "{context}" + ); + { + let errors = logs.entries.lock().unwrap(); + assert_eq!(errors.len(), 1, "{context}: {errors:?}"); + assert_eq!(errors[0]["operation"], op, "{context}"); + assert!( + errors[0]["error"].contains(failed_call), + "{context}: {errors:?}" + ); + } + + let mut pending = original.clone(); + let GroupKind::Underlay { replication, .. } = &mut pending.kind + else { + panic!("expected an underlay group"); + }; + replication.add_members(repl.clone(), &new_members); + switch.asic_hdl.inject_table_faults( + TableType::McastIpv6, + TableOp::EntryUpdate, + CallRange::nth(1), + AsicError::Synthetic, + ); + assert!( + resync_after_failed_unwind(&switch, &pending, None).is_none(), + "{context}" + ); + assert_eq!( + bitmap_entry(&switch, external_id), + Some((union_bitmap, None)), + "{context}" + ); + } + } + #[test] fn test_underlay_write_removes_member_left_by_failed_unwind() { let [kept_member, added_member, failing_member] = membership_members(); @@ -5306,6 +5818,7 @@ mod chaos_tests { failed_table, TableOp::EntryDel, CallRange::starting_at(1), + AsicError::Synthetic, ); fail_unwind_to_empty_group(&switch); @@ -5346,16 +5859,17 @@ mod chaos_tests { fn external_member_put_with_failed_unwind( init_members: &[MulticastGroupMember], tgt_members: &[MulticastGroupMember], - decap_update_failures: CallRange, + update_failures: CallRange, before_put: impl FnOnce(&Switch), ) -> (Switch, RollbackLog) { let (switch, logs) = chaos_switch(Config::default()); create_membership_group(&switch, init_members); before_put(&switch); switch.asic_hdl.inject_table_faults( - TableType::McastEgressDecapPorts, + TableType::McastIpv6, TableOp::EntryUpdate, - decap_update_failures, + update_failures, + AsicError::Synthetic, ); let removals = init_members .iter() @@ -5400,7 +5914,7 @@ mod chaos_tests { let (switch, logs) = external_member_put_with_failed_unwind( &init_members, &tgt_members, - CallRange::nth(2), + CallRange::nth(1), |_| {}, ); let external_id = membership_group_id(&switch, Direction::External); @@ -5424,10 +5938,9 @@ mod chaos_tests { } fn clear_external_member_put_faults(switch: &Switch) { - switch.asic_hdl.clear_table_faults( - TableType::McastEgressDecapPorts, - TableOp::EntryUpdate, - ); + switch + .asic_hdl + .clear_table_faults(TableType::McastIpv6, TableOp::EntryUpdate); clear_membership_faults(switch); } @@ -5437,7 +5950,7 @@ mod chaos_tests { let (switch, logs) = external_member_put_with_failed_unwind( &init_members, &tgt_members, - CallRange::starting_at(2), + CallRange::starting_at(1), |_| {}, ); @@ -5456,7 +5969,7 @@ mod chaos_tests { let (switch, logs) = external_member_put_with_failed_unwind( &init_members, &tgt_members, - CallRange::starting_at(2), + CallRange::starting_at(1), |switch| { add_group_external( switch, @@ -5502,7 +6015,7 @@ mod chaos_tests { let (switch, logs) = external_member_put_with_failed_unwind( &init_members, &tgt_members, - CallRange::starting_at(2), + CallRange::starting_at(1), |switch| { if group_destroy_fails { switch.asic_hdl.inject_mc_faults( @@ -5514,6 +6027,7 @@ mod chaos_tests { TableType::RouteIpv6Mcast, TableOp::EntryDel, CallRange::nth(1), + AsicError::Synthetic, ); } }, @@ -5558,73 +6072,45 @@ mod chaos_tests { let [kept_member, _, _] = membership_members(); let tgt_members = [kept_member]; - for readback in [true, false] { - let mut config = Config::default(); - if !readback { - config - .asic_config - .mc_config - .unsupported - .insert(McOp::McPortList); - } - let (switch, logs) = chaos_switch(config); - create_membership_group(&switch, &[]); - let external_id = membership_group_id(&switch, Direction::External); - assert_eq!(table_usage(&switch, TableType::McastIpv6).0, 0); + let (switch, logs) = chaos_switch(Config::default()); + create_membership_group(&switch, &[]); + let external_id = membership_group_id(&switch, Direction::External); + assert_eq!(table_usage(&switch, TableType::McastIpv6).0, 0); - switch.asic_hdl.inject_table_faults( - TableType::McastIpv6, - TableOp::EntryAdd, - CallRange::nth(1), - ); - switch - .asic_hdl - .inject_mc_faults(McOp::McPortRemove, CallRange::nth(1)); - let err = put_membership(&switch, &tgt_members).unwrap_err(); - let DpdError::Unwind { initial, unwind } = &err else { - panic!("expected a failed unwind: {err:?}"); - }; - assert!( - matches!( - initial.as_ref(), - DpdError::Switch(AsicError::Synthetic(op)) - if op == "table_entry_add" - ), - "{err:?}" - ); - assert!(is_synthetic(&unwind.head, McOp::McPortRemove), "{err:?}"); + switch.asic_hdl.inject_table_faults( + TableType::McastIpv6, + TableOp::EntryAdd, + CallRange::nth(1), + AsicError::Synthetic, + ); + switch.asic_hdl.inject_mc_faults(McOp::McPortRemove, CallRange::nth(1)); + let err = put_membership(&switch, &tgt_members).unwrap_err(); - if readback { - assert_eq!( - asic_membership(&switch), - expected_membership(&switch, &tgt_members), - ); - } else { - assert_eq!( - [Direction::External, Direction::Underlay].map( - |direction| switch - .asic_hdl - .mc_port_count(membership_group_id( - &switch, direction, - )) - .unwrap(), - ), - expected_membership(&switch, &tgt_members) - .map(|members| members.len()), - ); - } - assert_eq!(stored_membership(&switch), tgt_members.to_vec()); - assert!(switch.mcast.lock().unwrap().pending_repairs.is_empty()); - assert_eq!(table_usage(&switch, TableType::McastIpv6).0, 1); - assert_eq!( - bitmap_entry(&switch, external_id), - Some(( - create_port_bitmap(&tgt_members, Direction::External), - None, - )), - ); - assert_eq!(logs.entries.lock().unwrap().len(), 1); - } + assert_eq!( + asic_membership(&switch), + expected_membership(&switch, &tgt_members), + ); + assert_eq!(stored_membership(&switch), tgt_members.to_vec()); + assert!(switch.mcast.lock().unwrap().pending_repairs.is_empty()); + assert_eq!(table_usage(&switch, TableType::McastIpv6).0, 1); + assert_eq!( + bitmap_entry(&switch, external_id), + Some((create_port_bitmap(&tgt_members, Direction::External), None)), + ); + assert_eq!(logs.entries.lock().unwrap().len(), 1); + + let DpdError::Unwind { initial, unwind } = &err else { + panic!("expected a failed unwind: {err:?}"); + }; + assert!( + matches!( + initial.as_ref(), + DpdError::Switch(AsicError::Synthetic(op)) + if op == "table_entry_add" + ), + "{err:?}" + ); + assert!(is_synthetic(&unwind.head, McOp::McPortRemove), "{err:?}"); } #[test] diff --git a/dpd/src/table/mcast/mod.rs b/dpd/src/table/mcast/mod.rs index ebc8afc2..9e9013e2 100644 --- a/dpd/src/table/mcast/mod.rs +++ b/dpd/src/table/mcast/mod.rs @@ -13,12 +13,15 @@ use std::{ net::{Ipv4Addr, Ipv6Addr}, }; -use aal::{ActionParse, MatchParse}; +use aal::{ActionParse, AsicError, MatchParse}; use aal_macros::*; use common::table::TableType; use slog::debug; -use crate::{Switch, types::DpdResult}; +use crate::{ + Switch, + types::{DpdError, DpdResult}, +}; pub(crate) mod mcast_egress; pub(crate) mod mcast_nat; @@ -39,9 +42,26 @@ where M: MatchParse + Hash + fmt::Display, A: ActionParse, { - let keys: Vec = s - .table_get(table)? - .get_entries::(&s.asic_hdl, false)? + let entries = + match s.table_get(table)?.get_entries::(&s.asic_hdl, false) { + Ok(entries) => entries, + // TODO: SoftNPU currently doesn't support per-table dumps. + // + // Its dump request returns all tables, and the UART reader can't + // reliably handle large responses. Add per-table dumps and read complete + // UART responses before removing the `OperationUnsupported` fallback. + Err(DpdError::Switch(AsicError::OperationUnsupported)) => { + debug!( + s.log, + "per-table dump unsupported, skip clearing"; + "table" => %table, + ); + return Ok(()); + } + Err(e) => return Err(e), + }; + + let keys: Vec = entries .into_iter() .filter_map(|(key, _)| matches(&key).then_some(key)) .collect();