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

Egress multicast (instances sending to external receivers) is not in MVP scope. The mvlan field for VLAN-tagged upstream traffic is unnecessary since it probably will not be attached specifically to a group. Worth revisiting when egress support lands.

This PR also replaces mvlan with has_any_source_member on the multicast group view, which tracks whether any member joined without source filtering, using for source IP logic. The source_ips field now contains only IPs from members that explicitly specified sources, making the semantics clearer. The group-to-view conversion simplifies from TryFrom to From.

Author: zeeshanlakhani

Cargo.toml

@@ -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" }

nexus/db-model/src/multicast_group.rs

@@ -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).
@@ -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).
@@ -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,
@@ -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,
 }
 
@@ -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,
 }
 
@@ -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,
         }
     }

nexus/db-model/src/schema_versions.rs

@@ -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
 ///
@@ -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"),

nexus/db-queries/src/db/datastore/multicast/groups.rs

@@ -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;
@@ -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(),
-        })
+        }
     }
 }
 
@@ -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,
@@ -283,7 +261,6 @@ impl DataStore {
             MulticastGroupAllocationParams {
                 identity: params.identity.clone(),
                 ip_allocation,
-                mvlan: params.mvlan,
                 has_sources: params.has_sources,
             },
         )
@@ -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.
@@ -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,
             },
         );
@@ -1013,7 +998,6 @@ mod tests {
                     description: "First group".to_string(),
                 },
                 multicast_ip: None,
-                mvlan: None,
                 has_sources: false,
                 ip_version: None,
             };
@@ -1031,7 +1015,6 @@ mod tests {
                     description: "Second group".to_string(),
                 },
                 multicast_ip: None,
-                mvlan: None,
                 has_sources: false,
                 ip_version: None,
             };
@@ -1049,7 +1032,6 @@ mod tests {
                     description: "Should fail".to_string(),
                 },
                 multicast_ip: None,
-                mvlan: None,
                 has_sources: false,
                 ip_version: None,
             };
@@ -1078,7 +1060,6 @@ mod tests {
                     description: "Should reuse freed IP".to_string(),
                 },
                 multicast_ip: None,
-                mvlan: None,
                 has_sources: false,
                 ip_version: None,
             };
@@ -1161,7 +1142,6 @@ mod tests {
                 description: "Group using default pool".to_string(),
             },
             multicast_ip: None,
-            mvlan: None,
             has_sources: false,
             ip_version: None,
         };
@@ -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,
         };
@@ -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,
         };
@@ -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,
         };
@@ -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,
         };
@@ -1525,7 +1501,6 @@ mod tests {
                 description: "Group for deallocation testing".to_string(),
             },
             multicast_ip: None,
-            mvlan: None,
             has_sources: false,
             ip_version: None,
         };
@@ -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,
         };
@@ -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,
         };
@@ -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,
         };
@@ -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,
         };
@@ -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,
         };
@@ -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,
         };
@@ -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,
         };
@@ -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,
         };
@@ -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,
         };
@@ -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,
         };
@@ -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,
         };
@@ -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,
         };

nexus/db-queries/src/db/datastore/multicast/members.rs

@@ -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,
         };