Skip to content

[multicast] Drop (m)vlan from multicast groups #9451

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zeeshanlakhani merged 3 commits into from
Mar 18, 2026
Merged

[multicast] Drop (m)vlan from multicast groups #9451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
70f8b39
drop mvlan from multicast group API
zeeshanlakhani Mar 7, 2026
3a0bea8
merge main into drop-mvlan
zeeshanlakhani Mar 16, 2026
6f12edd
[review] add expectorate tests for multicast group allocation CTE que…
zeeshanlakhani Mar 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1043,8 +1043,6 @@ opt-level = 3
# drift = { path = "../drift" }
# dropshot = { path = "../dropshot/dropshot" }
# dropshot_endpoint = { path = "../dropshot/dropshot_endpoint" }
# dropshot-api-manager = { path = "../dropshot-api-manager/crates/dropshot-api-manager" }
# dropshot-api-manager-types = { path = "../dropshot-api-manager/crates/dropshot-api-manager-types" }
# progenitor = { path = "../progenitor/progenitor" }
# progenitor-client = { path = "../progenitor/progenitor-client" }
# steno = { path = "../steno" }
Expand Down
33 changes: 3 additions & 30 deletions nexus/db-model/src/multicast_group.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,28 +178,6 @@ pub struct ExternalMulticastGroup {
pub ip_pool_range_id: Uuid,
/// Primary multicast IP address (overlay/external).
pub multicast_ip: IpNetwork,
/// Multicast VLAN (MVLAN) for egress multicast traffic to upstream networks.
///
/// When specified, this VLAN ID is passed to switches (via DPD) as part of
/// the `ExternalForwarding` configuration to tag multicast packets leaving
/// the rack. This enables multicast traffic to traverse VLAN-segmented
/// upstream networks (e.g., peering with external multicast sources/receivers
/// on specific VLANs).
///
/// The MVLAN value is sent to switches during group creation/updates and
/// controls VLAN tagging for egress traffic only; it does not affect ingress
/// multicast traffic received by the rack. Switch port selection for egress
/// traffic remains pending (see TODOs in `nexus/src/app/multicast/dataplane.rs`).
///
/// Valid range when specified: 2-4094 (IEEE 802.1Q; Dendrite requires >= 2).
///
/// Database Type: i16 (INT2) - this field uses `i16` (INT2) for storage
/// efficiency, unlike other VLAN columns in the schema which use `SqlU16`
/// (forcing INT4). Direct `i16` is appropriate here since VLANs fit in
/// INT2's range.
///
/// TODO(multicast): Remove mvlan field - being deprecated from multicast groups
pub mvlan: Option<i16>,
/// Associated underlay group for NAT.
/// Initially None in ["Creating"](MulticastGroupState::Creating) state,
/// populated by reconciler when group becomes ["Active"](MulticastGroupState::Active).
Expand Down Expand Up @@ -248,11 +226,11 @@ pub struct MulticastGroupMemberValues {
pub parent_id: Uuid,
pub sled_id: Option<DbTypedUuid<SledKind>>,
pub state: MulticastGroupMemberState,
// version_added and version_removed are omitted - database assigns these
// via DEFAULT nextval()
pub multicast_ip: IpNetwork,
/// Source IPs for source-filtered multicast (optional for ASM, required for SSM).
pub source_ips: Vec<IpNetwork>,
// version_added and version_removed are omitted - database assigns these
// via DEFAULT nextval()
}

/// A member of a multicast group (instance that receives multicast traffic).
Expand Down Expand Up @@ -329,9 +307,7 @@ impl TryFrom<MulticastGroupMember> for multicast_types::MulticastGroupMember {

/// An incomplete external multicast group, used to store state required for
/// issuing the database query that selects an available multicast IP and stores
/// the resulting record.
///
/// Note: tag is computed in SQL as `{uuid}:{multicast_ip}`.
/// the resulting record. Tag is computed in SQL as `{uuid}:{multicast_ip}`.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct IncompleteExternalMulticastGroup {
pub id: Uuid,
Expand All @@ -341,7 +317,6 @@ pub struct IncompleteExternalMulticastGroup {
pub ip_pool_id: Uuid,
/// Optional address requesting a specific multicast IP be allocated.
pub explicit_address: Option<IpNetwork>,
pub mvlan: Option<i16>,
pub vni: Vni,
}

Expand All @@ -353,7 +328,6 @@ pub struct IncompleteExternalMulticastGroupParams {
pub description: String,
pub ip_pool_id: Uuid,
pub explicit_address: Option<IpAddr>,
pub mvlan: Option<i16>,
pub vni: Vni,
}

Expand All @@ -367,7 +341,6 @@ impl IncompleteExternalMulticastGroup {
time_created: Utc::now(),
ip_pool_id: params.ip_pool_id,
explicit_address: params.explicit_address.map(|ip| ip.into()),
mvlan: params.mvlan,
vni: params.vni,
}
}
Expand Down
3 changes: 2 additions & 1 deletion nexus/db-model/src/schema_versions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
///
/// This must be updated when you change the database schema. Refer to
/// schema/crdb/README.adoc in the root of this repository for details.
pub const SCHEMA_VERSION: Version = Version::new(239, 0, 0);
pub const SCHEMA_VERSION: Version = Version::new(240, 0, 0);

/// List of all past database schema versions, in *reverse* order
///
Expand All @@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::new(|| {
// | leaving the first copy as an example for the next person.
// v
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
KnownVersion::new(240, "multicast-drop-mvlan"),
KnownVersion::new(239, "fm-alert-request"),
KnownVersion::new(238, "fewer-nullable-columns"),
KnownVersion::new(237, "switch-slot-enum"),
Expand Down
75 changes: 19 additions & 56 deletions nexus/db-queries/src/db/datastore/multicast/groups.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ use omicron_common::api::external::{
IdentityMetadataCreateParams, IpVersion, ListResultVec, LookupResult,
LookupType, ResourceType, UpdateResult,
};
use omicron_common::vlan::VlanID;
use omicron_uuid_kinds::{GenericUuid, MulticastGroupUuid};

use super::EnsureUnderlayResult;
Expand All @@ -54,44 +53,24 @@ use crate::db::pagination::paginated;
use crate::db::queries::external_multicast_group::NextExternalMulticastGroup;
use crate::db::update_and_check::{UpdateAndCheck, UpdateStatus};

/// External multicast group with computed source IPs from members.
///
/// The `source_ips` field contains the union of all member source IPs,
/// computed via a separate query. This struct enables a clean `TryFrom`
/// conversion to the API view.
///
// TODO(multicast): Remove mvlan field, being deprecated from multicast groups
/// External multicast group with computed source filter state from members.
#[derive(Clone, Debug)]
pub struct ExternalMulticastGroupWithSources {
pub group: ExternalMulticastGroup,
pub source_ips: Vec<IpAddr>,
pub has_any_source_member: bool,
}

impl TryFrom<ExternalMulticastGroupWithSources> for views::MulticastGroup {
type Error = external::Error;

fn try_from(
value: ExternalMulticastGroupWithSources,
) -> Result<Self, Self::Error> {
let mvlan = value
.group
.mvlan
.map(|vlan| VlanID::new(vlan as u16))
.transpose()
.map_err(|e| {
external::Error::internal_error(&format!(
"invalid VLAN ID: {e:#}"
))
})?;

Ok(views::MulticastGroup {
impl From<ExternalMulticastGroupWithSources> for views::MulticastGroup {
fn from(value: ExternalMulticastGroupWithSources) -> Self {
views::MulticastGroup {
identity: value.group.identity(),
multicast_ip: value.group.multicast_ip.ip(),
source_ips: value.source_ips,
mvlan,
has_any_source_member: value.has_any_source_member,
ip_pool_id: value.group.ip_pool_id,
state: value.group.state.to_string(),
})
}
}
}

Expand All @@ -104,7 +83,6 @@ pub(crate) struct MulticastGroupAllocationParams {
pub identity: IdentityMetadataCreateParams,
/// How to allocate the multicast IP address.
pub ip_allocation: MulticastIpAllocation,
pub mvlan: Option<VlanID>,
/// Derived for whether the joining member has source IPs.
/// Used for default pool selection -> if true, prefer SSM pool first.
pub has_sources: bool,
Expand Down Expand Up @@ -283,7 +261,6 @@ impl DataStore {
MulticastGroupAllocationParams {
identity: params.identity.clone(),
ip_allocation,
mvlan: params.mvlan,
has_sources: params.has_sources,
},
)
Expand Down Expand Up @@ -344,12 +321,21 @@ impl DataStore {
let filter_state_map = self
.multicast_groups_source_filter_state(opctx, &[group_id])
.await?;
let source_ips = filter_state_map
let (source_ips, has_any_source_member) = filter_state_map
.get(&group_id.into_untyped_uuid())
.map(|state| state.specific_sources.iter().copied().collect())
.map(|state| {
(
state.specific_sources.iter().copied().collect(),
state.has_any_source_member,
)
})
.unwrap_or_default();

Ok(ExternalMulticastGroupWithSources { group, source_ips })
Ok(ExternalMulticastGroupWithSources {
group,
source_ips,
has_any_source_member,
})
}

/// Lookup an external multicast group by IP address.
Expand Down Expand Up @@ -643,7 +629,6 @@ impl DataStore {
description: params.identity.description.clone(),
ip_pool_id: authz_pool.id(),
explicit_address: explicit_ip,
mvlan: params.mvlan.map(|vlan_id| u16::from(vlan_id) as i16),
vni,
},
);
Expand Down Expand Up @@ -1013,7 +998,6 @@ mod tests {
description: "First group".to_string(),
},
multicast_ip: None,
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand All @@ -1031,7 +1015,6 @@ mod tests {
description: "Second group".to_string(),
},
multicast_ip: None,
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand All @@ -1049,7 +1032,6 @@ mod tests {
description: "Should fail".to_string(),
},
multicast_ip: None,
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down Expand Up @@ -1078,7 +1060,6 @@ mod tests {
description: "Should reuse freed IP".to_string(),
},
multicast_ip: None,
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down Expand Up @@ -1161,7 +1142,6 @@ mod tests {
description: "Group using default pool".to_string(),
},
multicast_ip: None,
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand All @@ -1187,7 +1167,6 @@ mod tests {
description: "Second group from default pool".to_string(),
},
multicast_ip: None,
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down Expand Up @@ -1315,7 +1294,6 @@ mod tests {
description: "Comprehensive test group".to_string(),
},
multicast_ip: Some("224.1.3.3".parse().unwrap()),
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down Expand Up @@ -1416,7 +1394,6 @@ mod tests {
description: "Group for IP reuse test".to_string(),
},
multicast_ip: Some(target_ip),
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down Expand Up @@ -1444,7 +1421,6 @@ mod tests {
description: "Second group reusing same IP".to_string(),
},
multicast_ip: Some(target_ip),
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down Expand Up @@ -1525,7 +1501,6 @@ mod tests {
description: "Group for deallocation testing".to_string(),
},
multicast_ip: None,
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down Expand Up @@ -1650,7 +1625,6 @@ mod tests {
description: "Test group for fetch operations".to_string(),
},
multicast_ip: Some("224.100.10.5".parse().unwrap()),
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down Expand Up @@ -1757,7 +1731,6 @@ mod tests {
description: "Fleet-wide group 1".to_string(),
},
multicast_ip: Some("224.100.20.10".parse().unwrap()),
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand All @@ -1768,7 +1741,6 @@ mod tests {
description: "Fleet-wide group 2".to_string(),
},
multicast_ip: Some("224.100.20.11".parse().unwrap()),
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand All @@ -1779,7 +1751,6 @@ mod tests {
description: "Fleet-wide group 3".to_string(),
},
multicast_ip: Some("224.100.20.12".parse().unwrap()),
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down Expand Up @@ -1888,7 +1859,6 @@ mod tests {
description: "Test group for state transitions".to_string(),
},
multicast_ip: Some("224.100.30.5".parse().unwrap()),
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down Expand Up @@ -2225,7 +2195,6 @@ mod tests {
description: "Group using ASM pool".to_string(),
},
multicast_ip: None, // No explicit IP -> triggers pool auto-selection
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down Expand Up @@ -2316,7 +2285,6 @@ mod tests {
description: "Should fall back to ASM when no SSM".to_string(),
},
multicast_ip: None,
mvlan: None,
has_sources: true,
ip_version: None,
};
Expand Down Expand Up @@ -2389,7 +2357,6 @@ mod tests {
description: "Should prefer SSM over ASM".to_string(),
},
multicast_ip: None,
mvlan: None,
has_sources: true,
ip_version: None,
};
Expand All @@ -2413,7 +2380,6 @@ mod tests {
description: "has_sources=false should use ASM".to_string(),
},
multicast_ip: None,
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down Expand Up @@ -2493,7 +2459,6 @@ mod tests {
description: "First group for collision test".to_string(),
},
multicast_ip: Some("224.10.1.1".parse().unwrap()),
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand All @@ -2509,7 +2474,6 @@ mod tests {
description: "Second group for collision test".to_string(),
},
multicast_ip: Some("224.10.1.2".parse().unwrap()),
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down Expand Up @@ -2633,7 +2597,6 @@ mod tests {
description: "Group for salt testing".to_string(),
},
multicast_ip: Some("224.20.1.1".parse().unwrap()),
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down
1 change: 0 additions & 1 deletion nexus/db-queries/src/db/datastore/multicast/members.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,6 @@ mod tests {
description: "Creating test group".to_string(),
},
multicast_ip: Some("224.10.1.6".parse().unwrap()),
mvlan: None,
has_sources: false,
ip_version: None,
};
Expand Down
Loading
Loading