Skip to content

Commit 05ed654

Browse files
zeeshanlakhanizeeshanlakhani
committed
[multicast] Drop (m)vlan from multicast groups
Egress multicast (instances sending to external receivers) is not in MVP scope, so the MVLAN field for VLAN-tagged upstream traffic is unnecessary, as it probably won't be attached specifically to a group, and is worth revisiting. Changes: - Drop mvlan column from multicast_group table (schema v213) - Remove mvlan from Rust structs, SQL queries, and API views - Add TODO scope documentation When egress support lands, VLAN tagging will be reintroduced with proper uplink port configuration.
1 parent 1c9d172 commit 05ed654

File tree

18 files changed

+44
-155
lines changed

18 files changed

+44
-155
lines changed

‎nexus/db-model/src/multicast_group.rs‎

Lines changed: 4 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ use nexus_db_schema::schema::{
9191
use nexus_types::external_api::views;
9292
use nexus_types::identity::Resource as IdentityResource;
9393
use omicron_common::api::external::{self, IdentityMetadata};
94-
use omicron_common::vlan::VlanID;
9594
use omicron_uuid_kinds::SledKind;
9695

9796
use crate::typed_uuid::DbTypedUuid;
@@ -180,28 +179,6 @@ pub struct ExternalMulticastGroup {
180179
/// Source IP addresses for Source-Specific Multicast (SSM).
181180
/// Empty array means any source is allowed.
182181
pub source_ips: Vec<IpNetwork>,
183-
/// Multicast VLAN (MVLAN) for egress multicast traffic to upstream networks.
184-
///
185-
/// When specified, this VLAN ID is passed to switches (via DPD) as part of
186-
/// the `ExternalForwarding` configuration to tag multicast packets leaving
187-
/// the rack. This enables multicast traffic to traverse VLAN-segmented
188-
/// upstream networks (e.g., peering with external multicast sources/receivers
189-
/// on specific VLANs).
190-
///
191-
/// The MVLAN value is sent to switches during group creation/updates and
192-
/// controls VLAN tagging for egress traffic only; it does not affect ingress
193-
/// multicast traffic received by the rack. Switch port selection for egress
194-
/// traffic remains pending (see TODOs in `nexus/src/app/multicast/dataplane.rs`).
195-
///
196-
/// Valid range when specified: 2-4094 (IEEE 802.1Q; Dendrite requires >= 2).
197-
///
198-
/// Database Type: i16 (INT2) - this field uses `i16` (INT2) for storage
199-
/// efficiency, unlike other VLAN columns in the schema which use `SqlU16`
200-
/// (forcing INT4). Direct `i16` is appropriate here since VLANs fit in
201-
/// INT2's range.
202-
///
203-
/// TODO(multicast): Remove mvlan field - being deprecated from multicast groups
204-
pub mvlan: Option<i16>,
205182
/// Associated underlay group for NAT.
206183
/// Initially None in ["Creating"](MulticastGroupState::Creating) state,
207184
/// populated by reconciler when group becomes ["Active"](MulticastGroupState::Active).
@@ -300,32 +277,19 @@ pub struct MulticastGroupMember {
300277

301278
// Conversions to external API views
302279

303-
impl TryFrom<ExternalMulticastGroup> for views::MulticastGroup {
304-
type Error = external::Error;
305-
306-
fn try_from(group: ExternalMulticastGroup) -> Result<Self, Self::Error> {
307-
let mvlan = group
308-
.mvlan
309-
.map(|vlan| VlanID::new(vlan as u16))
310-
.transpose()
311-
.map_err(|e| {
312-
external::Error::internal_error(&format!(
313-
"invalid VLAN ID: {e:#}"
314-
))
315-
})?;
316-
317-
Ok(views::MulticastGroup {
280+
impl From<ExternalMulticastGroup> for views::MulticastGroup {
281+
fn from(group: ExternalMulticastGroup) -> Self {
282+
views::MulticastGroup {
318283
identity: group.identity(),
319284
multicast_ip: group.multicast_ip.ip(),
320285
source_ips: group
321286
.source_ips
322287
.into_iter()
323288
.map(|ip| ip.ip())
324289
.collect(),
325-
mvlan,
326290
ip_pool_id: group.ip_pool_id,
327291
state: group.state.to_string(),
328-
})
292+
}
329293
}
330294
}
331295

@@ -367,7 +331,6 @@ pub struct IncompleteExternalMulticastGroup {
367331
// Optional address requesting that a specific multicast IP address be
368332
// allocated or provided
369333
pub explicit_address: Option<IpNetwork>,
370-
pub mvlan: Option<i16>,
371334
pub vni: Vni,
372335
pub tag: Option<String>,
373336
}
@@ -381,7 +344,6 @@ pub struct IncompleteExternalMulticastGroupParams {
381344
pub ip_pool_id: Uuid,
382345
pub explicit_address: Option<IpAddr>,
383346
pub source_ips: Vec<IpNetwork>,
384-
pub mvlan: Option<i16>,
385347
pub vni: Vni,
386348
pub tag: Option<String>,
387349
}
@@ -397,7 +359,6 @@ impl IncompleteExternalMulticastGroup {
397359
ip_pool_id: params.ip_pool_id,
398360
source_ips: params.source_ips,
399361
explicit_address: params.explicit_address.map(|ip| ip.into()),
400-
mvlan: params.mvlan,
401362
vni: params.vni,
402363
tag: params.tag,
403364
}

‎nexus/db-model/src/schema_versions.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
1616
///
1717
/// This must be updated when you change the database schema. Refer to
1818
/// schema/crdb/README.adoc in the root of this repository for details.
19-
pub const SCHEMA_VERSION: Version = Version::new(212, 0, 0);
19+
pub const SCHEMA_VERSION: Version = Version::new(213, 0, 0);
2020

2121
/// List of all past database schema versions, in *reverse* order
2222
///
@@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::new(|| {
2828
// | leaving the first copy as an example for the next person.
2929
// v
3030
// KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
31+
KnownVersion::new(213, "multicast-drop-mvlan"),
3132
KnownVersion::new(212, "multicast-member-ip-and-indexes"),
3233
KnownVersion::new(211, "blueprint-sled-config-subnet"),
3334
KnownVersion::new(210, "one-big-ereport-table"),

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

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ use omicron_common::api::external::{
3434
IdentityMetadataCreateParams, ListResultVec, LookupResult, LookupType,
3535
ResourceType, UpdateResult,
3636
};
37-
use omicron_common::vlan::VlanID;
3837
use omicron_uuid_kinds::{GenericUuid, MulticastGroupUuid};
3938

4039
use crate::authz;
@@ -56,7 +55,6 @@ pub(crate) struct MulticastGroupAllocationParams {
5655
pub ip: Option<IpAddr>,
5756
pub pool: Option<authz::IpPool>,
5857
pub source_ips: Option<Vec<IpAddr>>,
59-
pub mvlan: Option<VlanID>,
6058
}
6159

6260
impl DataStore {
@@ -165,7 +163,6 @@ impl DataStore {
165163
ip: params.multicast_ip,
166164
pool: authz_pool,
167165
source_ips: params.source_ips.clone(),
168-
mvlan: params.mvlan,
169166
},
170167
)
171168
.await
@@ -405,7 +402,6 @@ impl DataStore {
405402
ip_pool_id: authz_pool.id(),
406403
explicit_address: params.ip,
407404
source_ips: source_ip_networks,
408-
mvlan: params.mvlan.map(|vlan_id| u16::from(vlan_id) as i16),
409405
vni,
410406
// Set DPD tag to the group UUID to ensure uniqueness across lifecycle.
411407
// This prevents tag collision when group names are reused.
@@ -767,7 +763,6 @@ mod tests {
767763
},
768764
multicast_ip: None,
769765
source_ips: None,
770-
mvlan: None,
771766
};
772767
datastore
773768
.multicast_group_create(&opctx, &params1, Some(authz_pool.clone()))
@@ -782,7 +777,6 @@ mod tests {
782777
},
783778
multicast_ip: None,
784779
source_ips: None,
785-
mvlan: None,
786780
};
787781
datastore
788782
.multicast_group_create(&opctx, &params2, Some(authz_pool.clone()))
@@ -797,7 +791,6 @@ mod tests {
797791
},
798792
multicast_ip: None,
799793
source_ips: None,
800-
mvlan: None,
801794
};
802795
let result3 = datastore
803796
.multicast_group_create(&opctx, &params3, Some(authz_pool.clone()))
@@ -870,7 +863,6 @@ mod tests {
870863
},
871864
multicast_ip: None,
872865
source_ips: None,
873-
mvlan: None,
874866
};
875867

876868
let group_default = datastore
@@ -895,7 +887,6 @@ mod tests {
895887
},
896888
multicast_ip: None,
897889
source_ips: None,
898-
mvlan: None,
899890
};
900891
let group_explicit = datastore
901892
.multicast_group_create(&opctx, &params_explicit, None)
@@ -1022,7 +1013,6 @@ mod tests {
10221013
},
10231014
multicast_ip: Some("224.1.3.3".parse().unwrap()),
10241015
source_ips: None,
1025-
mvlan: None,
10261016
};
10271017

10281018
let external_group = datastore
@@ -1119,7 +1109,6 @@ mod tests {
11191109
},
11201110
multicast_ip: Some("224.3.1.5".parse().unwrap()),
11211111
source_ips: None,
1122-
mvlan: None,
11231112
};
11241113

11251114
let group = datastore
@@ -1584,7 +1573,6 @@ mod tests {
15841573
},
15851574
multicast_ip: Some("224.3.1.5".parse().unwrap()),
15861575
source_ips: None,
1587-
mvlan: None,
15881576
};
15891577

15901578
let group = datastore
@@ -1716,7 +1704,6 @@ mod tests {
17161704
},
17171705
multicast_ip: None, // Let it allocate from pool
17181706
source_ips: None,
1719-
mvlan: None,
17201707
};
17211708
let group = datastore
17221709
.multicast_group_create(
@@ -1928,7 +1915,6 @@ mod tests {
19281915
},
19291916
multicast_ip: Some(target_ip),
19301917
source_ips: None,
1931-
mvlan: None,
19321918
};
19331919

19341920
let group1 = datastore
@@ -1955,7 +1941,6 @@ mod tests {
19551941
},
19561942
multicast_ip: Some(target_ip),
19571943
source_ips: None,
1958-
mvlan: None,
19591944
};
19601945

19611946
let group2 = datastore
@@ -2040,7 +2025,6 @@ mod tests {
20402025
},
20412026
multicast_ip: None,
20422027
source_ips: None,
2043-
mvlan: None,
20442028
};
20452029

20462030
let group1 = datastore
@@ -2057,7 +2041,6 @@ mod tests {
20572041
},
20582042
multicast_ip: None,
20592043
source_ips: None,
2060-
mvlan: None,
20612044
};
20622045

20632046
let result2 = datastore
@@ -2087,7 +2070,6 @@ mod tests {
20872070
},
20882071
multicast_ip: None,
20892072
source_ips: None,
2090-
mvlan: None,
20912073
};
20922074

20932075
let group3 = datastore
@@ -2173,7 +2155,6 @@ mod tests {
21732155
},
21742156
multicast_ip: None,
21752157
source_ips: None,
2176-
mvlan: None,
21772158
};
21782159

21792160
let group = datastore
@@ -2300,7 +2281,6 @@ mod tests {
23002281
"10.0.0.1".parse().unwrap(),
23012282
"10.0.0.2".parse().unwrap(),
23022283
]),
2303-
mvlan: None,
23042284
};
23052285

23062286
let group = datastore
@@ -2407,7 +2387,6 @@ mod tests {
24072387
},
24082388
multicast_ip: Some("224.100.20.10".parse().unwrap()),
24092389
source_ips: None,
2410-
mvlan: None,
24112390
};
24122391

24132392
let params_2 = MulticastGroupCreate {
@@ -2417,7 +2396,6 @@ mod tests {
24172396
},
24182397
multicast_ip: Some("224.100.20.11".parse().unwrap()),
24192398
source_ips: None,
2420-
mvlan: None,
24212399
};
24222400

24232401
let params_3 = MulticastGroupCreate {
@@ -2427,7 +2405,6 @@ mod tests {
24272405
},
24282406
multicast_ip: Some("224.100.20.12".parse().unwrap()),
24292407
source_ips: None,
2430-
mvlan: None,
24312408
};
24322409

24332410
// Create groups (all are fleet-scoped)
@@ -2535,7 +2512,6 @@ mod tests {
25352512
},
25362513
multicast_ip: Some("224.100.30.5".parse().unwrap()),
25372514
source_ips: None,
2538-
mvlan: None,
25392515
};
25402516

25412517
// Create group - starts in "Creating" state

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -926,7 +926,6 @@ mod tests {
926926
multicast_ip: Some("224.10.1.6".parse().unwrap()),
927927
source_ips: None,
928928
// Pool resolved via authz_pool argument to datastore call
929-
mvlan: None,
930929
};
931930

932931
let creating_group = datastore

‎nexus/db-queries/src/db/pub_test_utils/multicast.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,6 @@ pub async fn create_test_group_with_state(
194194
},
195195
multicast_ip: Some(multicast_ip.parse().unwrap()),
196196
source_ips: None,
197-
mvlan: None,
198197
};
199198

200199
let group = datastore

‎nexus/db-queries/src/db/queries/external_multicast_group.rs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,6 @@ impl NextExternalMulticastGroup {
118118
}
119119
out.push_sql("]::inet[] AS source_ips, ");
120120

121-
// MVLAN for external uplink forwarding
122-
out.push_bind_param::<sql_types::Nullable<sql_types::Int2>, Option<i16>>(&self.group.mvlan)?;
123-
out.push_sql(" AS mvlan, ");
124-
125121
out.push_bind_param::<sql_types::Nullable<sql_types::Uuid>, Option<Uuid>>(&None)?;
126122
out.push_sql(" AS underlay_group_id, ");
127123

@@ -274,10 +270,10 @@ impl QueryFragment<Pg> for NextExternalMulticastGroup {
274270
out.push_sql("INSERT INTO ");
275271
schema::multicast_group::table.walk_ast(out.reborrow())?;
276272
out.push_sql(
277-
" (id, name, description, time_created, time_modified, time_deleted, ip_pool_id, ip_pool_range_id, vni, multicast_ip, source_ips, mvlan, underlay_group_id, tag, state, version_added, version_removed)
278-
SELECT id, name, description, time_created, time_modified, time_deleted, ip_pool_id, ip_pool_range_id, vni, multicast_ip, source_ips, mvlan, underlay_group_id, tag, state, version_added, version_removed FROM next_external_multicast_group
273+
" (id, name, description, time_created, time_modified, time_deleted, ip_pool_id, ip_pool_range_id, vni, multicast_ip, source_ips, underlay_group_id, tag, state, version_added, version_removed)
274+
SELECT id, name, description, time_created, time_modified, time_deleted, ip_pool_id, ip_pool_range_id, vni, multicast_ip, source_ips, underlay_group_id, tag, state, version_added, version_removed FROM next_external_multicast_group
279275
WHERE NOT EXISTS (SELECT 1 FROM previously_allocated_group)
280-
RETURNING id, name, description, time_created, time_modified, time_deleted, ip_pool_id, ip_pool_range_id, vni, multicast_ip, source_ips, mvlan, underlay_group_id, tag, state, version_added, version_removed",
276+
RETURNING id, name, description, time_created, time_modified, time_deleted, ip_pool_id, ip_pool_range_id, vni, multicast_ip, source_ips, underlay_group_id, tag, state, version_added, version_removed",
281277
);
282278
out.push_sql("), ");
283279

@@ -288,9 +284,9 @@ impl QueryFragment<Pg> for NextExternalMulticastGroup {
288284

289285
// Return either the newly inserted or previously allocated group
290286
out.push_sql(
291-
"SELECT id, name, description, time_created, time_modified, time_deleted, ip_pool_id, ip_pool_range_id, vni, multicast_ip, source_ips, mvlan, underlay_group_id, tag, state, version_added, version_removed FROM previously_allocated_group
287+
"SELECT id, name, description, time_created, time_modified, time_deleted, ip_pool_id, ip_pool_range_id, vni, multicast_ip, source_ips, underlay_group_id, tag, state, version_added, version_removed FROM previously_allocated_group
292288
UNION ALL
293-
SELECT id, name, description, time_created, time_modified, time_deleted, ip_pool_id, ip_pool_range_id, vni, multicast_ip, source_ips, mvlan, underlay_group_id, tag, state, version_added, version_removed FROM multicast_group",
289+
SELECT id, name, description, time_created, time_modified, time_deleted, ip_pool_id, ip_pool_range_id, vni, multicast_ip, source_ips, underlay_group_id, tag, state, version_added, version_removed FROM multicast_group",
294290
);
295291

296292
Ok(())

‎nexus/db-schema/src/schema.rs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2773,7 +2773,6 @@ table! {
27732773
vni -> Int4,
27742774
multicast_ip -> Inet,
27752775
source_ips -> Array<Inet>,
2776-
mvlan -> Nullable<Int2>,
27772776
underlay_group_id -> Nullable<Uuid>,
27782777
tag -> Nullable<Text>,
27792778
state -> crate::enums::MulticastGroupStateEnum,

0 commit comments

Comments
 (0)