Add a unique VNI to each VPC

Author: bnaecker

Cargo.lock

@@ -1677,6 +1677,62 @@ impl JsonSchema for MacAddr {
     }
 }
 
+/// A Geneve Virtual Network Identifier
+#[derive(
+    Debug,
+    Clone,
+    Copy,
+    PartialEq,
+    Eq,
+    Hash,
+    PartialOrd,
+    Ord,
+    Deserialize,
+    Serialize,
+    JsonSchema,
+)]
+pub struct Vni(u32);
+
+impl Vni {
+    const MAX_VNI: u32 = 1 << 24;
+
+    /// Create a new random VNI.
+    pub fn random() -> Self {
+        use rand::Rng;
+        Self(rand::thread_rng().gen_range(0..=Self::MAX_VNI))
+    }
+}
+
+impl From<Vni> for u32 {
+    fn from(vni: Vni) -> u32 {
+        vni.0
+    }
+}
+
+impl TryFrom<u32> for Vni {
+    type Error = Error;
+
+    fn try_from(x: u32) -> Result<Self, Error> {
+        if x <= Self::MAX_VNI {
+            Ok(Self(x))
+        } else {
+            Err(Error::internal_error(
+                format!("Invalid Geneve VNI: {}", x).as_str(),
+            ))
+        }
+    }
+}
+
+impl TryFrom<i32> for Vni {
+    type Error = Error;
+
+    fn try_from(x: i32) -> Result<Self, Error> {
+        Self::try_from(u32::try_from(x).map_err(|_| {
+            Error::internal_error(format!("Invalid Geneve VNI: {}", x).as_str())
+        })?)
+    }
+}
+
 /// A `NetworkInterface` represents a virtual network interface device.
 #[derive(ObjectIdentity, Clone, Debug, Deserialize, JsonSchema, Serialize)]
 pub struct NetworkInterface {

common/src/api/external/mod.rs

@@ -592,6 +592,13 @@ CREATE TABLE omicron.public.vpc (
     system_router_id UUID NOT NULL,
     dns_name STRING(63) NOT NULL,
 
+    /*
+     * The Geneve Virtual Network Identifier for this VPC. Note that this is a
+     * 24-bit unsigned value, properties which are checked in the application,
+     * not the database.
+     */
+    vni INT4 NOT NULL,
+
     /* The IPv6 prefix allocated to subnets. */
     ipv6_prefix INET NOT NULL,
 
@@ -606,6 +613,11 @@ CREATE UNIQUE INDEX ON omicron.public.vpc (
 ) WHERE
     time_deleted IS NULL;
 
+CREATE UNIQUE INDEX ON omicron.public.vpc (
+    vni
+) WHERE
+    time_deleted IS NULL;
+
 CREATE TABLE omicron.public.vpc_subnet (
     /* Identity metadata (resource) */
     id UUID PRIMARY KEY,

common/src/sql/dbinit.sql

@@ -1368,7 +1368,7 @@ impl DataStore {
     }
 
     /// Return the information about an instance's network interfaces required
-    /// for the sled agent to instantiate it via OPTE.
+    /// for the sled agent to instantiate them via OPTE.
     ///
     /// OPTE requires information that's currently split across the network
     /// interface and VPC subnet tables. This query just joins those for each
@@ -1381,6 +1381,7 @@ impl DataStore {
         opctx.authorize(authz::Action::ListChildren, authz_instance).await?;
 
         use db::schema::network_interface;
+        use db::schema::vpc;
         use db::schema::vpc_subnet;
 
         // The record type for the results of the below JOIN query
@@ -1391,7 +1392,7 @@ impl DataStore {
             mac: db::model::MacAddr,
             ipv4_block: db::model::Ipv4Net,
             ipv6_block: db::model::Ipv6Net,
-            vpc_id: Uuid,
+            vni: db::model::Vni,
             slot: i16,
         }
 
@@ -1407,7 +1408,7 @@ impl DataStore {
                     ip: nic.ip.ip().to_string(),
                     mac: sled_client_types::MacAddr::from(nic.mac.0),
                     subnet: sled_client_types::IpNet::from(ip_subnet),
-                    vpc_id: nic.vpc_id,
+                    vni: sled_client_types::Vni::from(nic.vni.0),
                     slot: u8::try_from(nic.slot).unwrap(),
                 }
             }
@@ -1420,17 +1421,18 @@ impl DataStore {
                 vpc_subnet::table
                     .on(network_interface::subnet_id.eq(vpc_subnet::id)),
             )
+            .inner_join(vpc::table.on(vpc_subnet::vpc_id.eq(vpc::id)))
             .order_by(network_interface::slot)
             // TODO-cleanup: Having to specify each column again is less than
-            // ideal. However, this type doesn't appear to have the `Row`
-            // associated type in the documentation. DRY this out.
+            // ideal, but we can't derive `Selectable` since this is the result
+            // of a JOIN and not from a single table. DRY this out if possible.
             .select((
                 network_interface::name,
                 network_interface::ip,
                 network_interface::mac,
                 vpc_subnet::ipv4_block,
                 vpc_subnet::ipv6_block,
-                network_interface::vpc_id,
+                vpc::vni,
                 network_interface::slot,
             ))
             .get_results_async::<NicInfo>(self.pool_authorized(opctx).await?)

nexus/src/db/datastore.rs

@@ -552,15 +552,25 @@ pub struct MacAddr(pub external::MacAddr);
 impl MacAddr {
     /// Generate a unique MAC address for an interface
     pub fn new() -> Result<Self, external::Error> {
-        use rand::Fill;
-        // Use the Oxide OUI A8 40 25
-        let mut addr = [0xA8, 0x40, 0x25, 0x00, 0x00, 0x00];
-        addr[3..].try_fill(&mut StdRng::from_entropy()).map_err(|_| {
-            external::Error::internal_error("failed to generate MAC")
-        })?;
-        // From RFD 174, Oxide virtual MACs are constrained to have these bits
-        // set.
-        addr[3] |= 0xF0;
+        // From RFD 174, the last three octets of Oxide virtual MACs are
+        // constrained to be in the range F0:00:00 - FF:FF:FF. However, we're
+        // further splitting this into a range for customer instances, F0:00:00
+        // - EF:FF:FF, and VPC / system MAC addresses, FF:00:00 - FF:FF:FF.
+        // See
+        // https://github.com/oxidecomputer/omicron/pull/955#discussion_r856432498
+        // for the initial discussion surrounding this split.
+        //
+        // TODO-completeness: Update this comment point to the relevant RFD(s)
+        // once they're created.
+        use rand::Rng;
+        const MIN: u32 = 0x00_00_00;
+        const MAX: u32 = 0x0E_FF_FF;
+        let bytes = rand::thread_rng().gen_range(MIN..=MAX).to_be_bytes();
+
+        // Use the Oxide OUI A8 40 25, and set the first nibble of the third
+        // byte, to ensure we're in the virtual address range.
+        let addr = [0xA8, 0x40, 0x25, 0xF0 | bytes[1], bytes[2], bytes[3]];
+
         // TODO-correctness: We should use an explicit allocator for the MACs
         // given the small address space. Right now creation requests may fail
         // due to MAC collision, especially given the 20-bit space.
@@ -1848,6 +1858,35 @@ impl OximeterInfo {
     }
 }
 
+#[derive(Clone, Debug, Copy, AsExpression, FromSqlRow)]
+#[diesel(sql_type = sql_types::Int4)]
+pub struct Vni(pub external::Vni);
+
+impl<DB> ToSql<sql_types::Int4, DB> for Vni
+where
+    DB: Backend<BindCollector = diesel::query_builder::bind_collector::RawBytesBindCollector<DB>>,
+    i32: ToSql<sql_types::Int4, DB>,
+{
+    fn to_sql<'b>(
+        &'b self,
+        out: &mut serialize::Output<'b, '_, DB>,
+    ) -> serialize::Result {
+        // Reborrowing is necessary to ensure that the lifetime of the temporary
+        // i32 created here and `out` is the same, i.e., that `'b = '_`.
+        i32::try_from(u32::from(self.0)).unwrap().to_sql(&mut out.reborrow())
+    }
+}
+
+impl<DB> FromSql<sql_types::Int4, DB> for Vni
+where
+    DB: Backend,
+    i32: FromSql<sql_types::Int4, DB>,
+{
+    fn from_sql(bytes: RawValue<DB>) -> deserialize::Result<Self> {
+        Ok(Vni(external::Vni::try_from(i32::from_sql(bytes)?)?))
+    }
+}
+
 #[derive(Queryable, Insertable, Clone, Debug, Selectable, Resource)]
 #[diesel(table_name = vpc)]
 pub struct Vpc {
@@ -1856,6 +1895,7 @@ pub struct Vpc {
 
     pub project_id: Uuid,
     pub system_router_id: Uuid,
+    pub vni: Vni,
     pub ipv6_prefix: Ipv6Net,
     pub dns_name: Name,
 
@@ -1890,6 +1930,7 @@ impl Vpc {
             identity,
             project_id,
             system_router_id,
+            vni: Vni(external::Vni::random()),
             ipv6_prefix,
             dns_name: params.dns_name.into(),
             firewall_gen: Generation::new(),
@@ -2709,6 +2750,7 @@ impl UpdateAvailableArtifact {
 
 #[cfg(test)]
 mod tests {
+    use super::MacAddr;
     use super::Uuid;
     use super::VpcSubnet;
     use ipnetwork::Ipv4Network;
@@ -2830,4 +2872,19 @@ mod tests {
         assert!(!subnet.check_requestable_addr("fd00::1".parse().unwrap()));
         assert!(subnet.check_requestable_addr("fd00::1:1".parse().unwrap()));
     }
+
+    #[test]
+    fn test_random_mac_address() {
+        let mac = MacAddr::new().expect("Failed to create random MAC");
+        let bytes = mac.0.as_bytes();
+        assert_eq!(&bytes[..3], &[0xA8, 0x40, 0x25], "Invalid Oxide OUI");
+        assert_eq!(
+            bytes[3] & 0xF0,
+            0xF0,
+            concat!(
+                "Customer MAC addresses should be in the virtual range ",
+                "a8:40:25:f0:00:00 - a8:40:25:fe:ff:ff"
+            )
+        );
+    }
 }

nexus/src/db/model.rs

@@ -333,6 +333,7 @@ table! {
         time_deleted -> Nullable<Timestamptz>,
         project_id -> Uuid,
         system_router_id -> Uuid,
+        vni -> Int4,
         ipv6_prefix -> Inet,
         dns_name -> Text,
         firewall_gen -> Int8,

nexus/src/db/schema.rs

@@ -1850,23 +1850,6 @@ impl Nexus {
             .derive_guest_network_interface_info(&opctx, &authz_instance)
             .await?;
 
-        /*
-            .instance_list_network_interfaces(
-                &opctx,
-                &authz_instance,
-                &DataPageParams {
-                    marker: None,
-                    direction: dropshot::PaginationOrder::Ascending,
-                    limit: std::num::NonZeroU32::new(MAX_NICS_PER_INSTANCE)
-                        .unwrap(),
-                },
-            )
-            .await?
-            .iter()
-            .map(|x| x.clone().into())
-            .collect();
-        */
-
         // Ask the sled agent to begin the state change.  Then update the
         // database to reflect the new intermediate state.  If this update is
         // not the newest one, that's fine.  That might just mean the sled agent

nexus/src/nexus.rs

@@ -915,7 +915,7 @@
         "maxLength": 63
       },
       "NetworkInterface": {
-        "description": "Information required to construct virtual network interfaces for a guest",
+        "description": "Information required to construct a virtual network interface for a guest",
         "type": "object",
         "properties": {
           "ip": {
@@ -936,9 +936,8 @@
           "subnet": {
             "$ref": "#/components/schemas/IpNet"
           },
-          "vpc_id": {
-            "type": "string",
-            "format": "uuid"
+          "vni": {
+            "$ref": "#/components/schemas/Vni"
           }
         },
         "required": [
@@ -947,7 +946,7 @@
           "name",
           "slot",
           "subnet",
-          "vpc_id"
+          "vni"
         ]
       },
       "ServiceEnsureBody": {
@@ -1017,6 +1016,12 @@
           "zone"
         ]
       },
+      "Vni": {
+        "description": "A Geneve Virtual Network Identifier",
+        "type": "integer",
+        "format": "uint32",
+        "minimum": 0
+      },
       "VolumeConstructionRequest": {
         "oneOf": [
           {