[vlan] bound VLAN IDs to 1..=4094 per IEEE 802.1Q

common/src/network.rs

@@ -41,14 +41,31 @@ pub fn generate_ipv6_link_local(mac: MacAddr) -> Ipv6Addr {
 #[derive(Error, Debug, Clone)]
 pub enum VlanError {
     /// Not a valid VLAN ID
-    #[error("Invalid VLAN tag: {}", .0)]
+    #[error("Invalid VLAN ID {}, must be in the range 1..=4094", .0)]
     InvalidVlan(u16),
 }
 
+/// Validate a VLAN ID against the configurable range `1..=4094`.
+///
+/// Per [IEEE 802.1Q] §9.6, VID 0 is the null VID used for priority tagging
+/// and carries no VLAN membership, and VID 4095 is reserved for
+/// implementation use and must not be configured. VID 1 is the default
+/// port VID.
+///
+/// # Errors
+///
+/// Returns [`VlanError::InvalidVlan`] if the ID falls outside `1..=4094`.
+///
+/// [IEEE 802.1Q]: https://ieeexplore.ieee.org/document/10004498
+// TODO: replace with a validated `VlanId` newtype in oxnet, shared with
+// omicron's `common/src/vlan.rs` bounds check.
 pub fn validate_vlan(id: impl Into<u16>) -> Result<(), VlanError> {
     let id: u16 = id.into();
-    #[allow(clippy::manual_range_contains)]
-    if id < 2 || id > 4095 { Err(VlanError::InvalidVlan(id)) } else { Ok(()) }
+    if !(1..=4094).contains(&id) {
+        Err(VlanError::InvalidVlan(id))
+    } else {
+        Ok(())
+    }
 }
 
 #[cfg(test)]

dpd-client/tests/integration_tests/mcast.rs

@@ -552,7 +552,7 @@ async fn test_group_creation_with_validation() -> TestResult {
             nat_target: Some(nat_target.clone()),
         },
         external_forwarding: types::ExternalForwarding {
-            vlan_id: Some(4096), // Invalid: VLAN ID must be 1-4095
+            vlan_id: Some(4096), // Invalid: VLAN ID must be 1-4094
         },
         sources: None,
     };
@@ -574,15 +574,15 @@ async fn test_group_creation_with_validation() -> TestResult {
         _ => panic!("Expected ErrorResponse for invalid group ID"),
     }
 
-    // Test with reserved VLAN ID 0 (also invalid)
+    // Test with the null VLAN ID 0 (also invalid)
     let external_vlan0 = types::MulticastGroupCreateExternalEntry {
         group_ip: IpAddr::V4(MULTICAST_TEST_IPV4),
         tag: Some(TEST_TAG.to_string()),
         internal_forwarding: types::InternalForwarding {
             nat_target: Some(nat_target.clone()),
         },
         external_forwarding: types::ExternalForwarding {
-            vlan_id: Some(0), // Invalid: VLAN 0 is reserved
+            vlan_id: Some(0), // Invalid: VLAN 0 is the null VID
         },
         sources: None,
     };
@@ -591,7 +591,7 @@ async fn test_group_creation_with_validation() -> TestResult {
         .client
         .multicast_group_create_external(&external_vlan0)
         .await
-        .expect_err("Should fail with reserved VLAN ID 0");
+        .expect_err("Should fail with null VLAN ID 0");
 
     match res {
         Error::ErrorResponse(inner) => {
@@ -601,37 +601,37 @@ async fn test_group_creation_with_validation() -> TestResult {
                 "Expected 400 Bad Request status code for VLAN 0"
             );
         }
-        _ => panic!("Expected ErrorResponse for reserved VLAN ID 0"),
+        _ => panic!("Expected ErrorResponse for null VLAN ID 0"),
     }
 
-    // Test with reserved VLAN ID 1 (also invalid)
-    let external_vlan1 = types::MulticastGroupCreateExternalEntry {
+    // Test with reserved VLAN ID 4095 (also invalid)
+    let external_vlan4095 = types::MulticastGroupCreateExternalEntry {
         group_ip: IpAddr::V4(MULTICAST_TEST_IPV4),
         tag: Some(TEST_TAG.to_string()),
         internal_forwarding: types::InternalForwarding {
             nat_target: Some(nat_target.clone()),
         },
         external_forwarding: types::ExternalForwarding {
-            vlan_id: Some(1), // Invalid: VLAN 1 is reserved
+            vlan_id: Some(4095), // Invalid: VLAN 4095 is reserved
         },
         sources: None,
     };
 
     let res = switch
         .client
-        .multicast_group_create_external(&external_vlan1)
+        .multicast_group_create_external(&external_vlan4095)
         .await
-        .expect_err("Should fail with reserved VLAN ID 1");
+        .expect_err("Should fail with reserved VLAN ID 4095");
 
     match res {
         Error::ErrorResponse(inner) => {
             assert_eq!(
                 inner.status(),
                 400,
-                "Expected 400 Bad Request status code for VLAN 1"
+                "Expected 400 Bad Request status code for VLAN 4095"
             );
         }
-        _ => panic!("Expected ErrorResponse for reserved VLAN ID 1"),
+        _ => panic!("Expected ErrorResponse for reserved VLAN ID 4095"),
     }
 
     // Test with valid parameters

uplinkd/src/main.rs

@@ -110,11 +110,10 @@ struct Opt {
 }
 
 fn parse_vlan_id(vlan_id: &str) -> Result<Option<u16>, anyhow::Error> {
-    match vlan_id.parse() {
-        Err(_) => Err(anyhow!("invalid vlan id: {vlan_id}")),
-        Ok(vlan_id) if vlan_id > 1 && vlan_id < 4096 => Ok(Some(vlan_id)),
-        Ok(vlan_id) => Err(anyhow!("vlan id out of range: {vlan_id}")),
-    }
+    let id: u16 =
+        vlan_id.parse().map_err(|_| anyhow!("invalid vlan id: {vlan_id}"))?;
+    common::network::validate_vlan(id)?;
+    Ok(Some(id))
 }
 
 // Given an interface, return a tuple with the underlying link name and any

uplinkd/src/sys.rs

@@ -64,17 +64,13 @@ impl FromStr for UplinkAddress {
         let address = address
             .parse()
             .map_err(|_| anyhow!("not a valid ip address: {address}"))?;
-        let vlan_id = match vlan_id {
-            None => Ok(None),
-            Some(v) => match v.parse() {
-                Err(_) => Err(anyhow!("invalid vlan id: {v}")),
-                Ok(vlan_id) if vlan_id > 1 && vlan_id < 4096 => {
-                    Ok(Some(vlan_id))
-                }
-                Ok(vlan_id) => Err(anyhow!("vlan id out of range: {vlan_id}")),
-            },
-        }?;
-        Ok(UplinkAddress { address, vlan_id })
+        let vlan_id = vlan_id
+            .map(|v| {
+                v.parse::<u16>()
+                    .map_err(|_| anyhow!("invalid vlan id: {v}"))
+            })
+            .transpose()?;
+        UplinkAddress::new(address, vlan_id)
     }
 }