Skip to content

Commit

Permalink
Allocates guest MAC addresses via a next item query
Browse files Browse the repository at this point in the history
- Provides random base address within the guest-visible range of
  A8:40:25:F0:00:00-A8:40:25:FE:FF:FF.
- Switch the database representation of MAC addresses in the
  `network_interface` table from a string to INT8. The string
  representation is human-readable, but doesn't allow any other
  operations on it (such as addition). There's also no native
  MACADDR support in CRDB, so we don't lose much by switching to a new
  repr. Any client code needs to parse/interpret the value anyway, and a
  byte-representation in some ways is more natural or obvious.
- Adds a NextItem query for generating a random guest MAC address. This
  is the upshot of the change to an integer, that we can add to it and
  thus use the NextItem query machinery.
- Adds a variant of the `NetworkInterfaceError` for detecting MAC
  address exhaustion.
  • Loading branch information
bnaecker committed May 31, 2022
1 parent 4dfb749 commit 80092af
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 60 deletions.
10 changes: 9 additions & 1 deletion common/src/sql/dbinit.sql
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,15 @@ CREATE TABLE omicron.public.network_interface (
vpc_id UUID NOT NULL,
/* FK into VPCSubnet table. */
subnet_id UUID NOT NULL,
mac STRING(17) NOT NULL, -- e.g., "ff:ff:ff:ff:ff:ff"

/*
* The EUI-48 MAC address of the guest interface.
*
* Note that we use the bytes of a 64-bit integer, in big-endian byte order
* to represent the MAC.
*/
mac INT8 NOT NULL,

ip INET NOT NULL,
/*
* Limited to 8 NICs per instance. This value must be kept in sync with
Expand Down
2 changes: 0 additions & 2 deletions nexus/src/app/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -662,14 +662,12 @@ impl super::Nexus {
.vpc_subnet_name(&subnet_name)
.fetch()
.await?;
let mac = db::model::MacAddr::new()?;
let interface_id = Uuid::new_v4();
let interface = db::model::IncompleteNetworkInterface::new(
interface_id,
authz_instance.id(),
authz_vpc.id(),
db_subnet,
mac,
params.identity.clone(),
params.ip,
)?;
Expand Down
5 changes: 0 additions & 5 deletions nexus/src/app/sagas/instance_create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,14 +311,11 @@ async fn sic_create_custom_network_interfaces(
.fetch()
.await
.map_err(ActionError::action_failed)?;
let mac =
db::model::MacAddr::new().map_err(ActionError::action_failed)?;
let interface = db::model::IncompleteNetworkInterface::new(
interface_id,
instance_id,
authz_vpc.id(),
db_subnet.clone(),
mac,
params.identity.clone(),
params.ip,
)
Expand Down Expand Up @@ -412,14 +409,12 @@ async fn sic_create_default_network_interface(
.await
.map_err(ActionError::action_failed)?;

let mac = db::model::MacAddr::new().map_err(ActionError::action_failed)?;
let interface_id = Uuid::new_v4();
let interface = db::model::IncompleteNetworkInterface::new(
interface_id,
instance_id,
authz_vpc.id(),
db_subnet.clone(),
mac,
interface_params.identity.clone(),
interface_params.ip,
)
Expand Down
84 changes: 58 additions & 26 deletions nexus/src/db/model/macaddr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,55 +8,87 @@ use diesel::pg::Pg;
use diesel::serialize::{self, ToSql};
use diesel::sql_types;
use omicron_common::api::external;
use rand::{rngs::StdRng, SeedableRng};
use std::convert::TryFrom;
use rand::thread_rng;
use rand::Rng;

#[derive(Clone, Copy, Debug, PartialEq, AsExpression, FromSqlRow)]
#[diesel(sql_type = sql_types::Text)]
#[diesel(sql_type = sql_types::BigInt)]
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;
// 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.
Ok(Self(external::MacAddr(macaddr::MacAddr6::from(addr))))
// Guest MAC addresses begin with the Oxide OUI A8:40:25. Further, guest
// address are constrained to be in the virtual address range
// A8:40:24:F_:__:__. Even further, the range F0:00:00 - FE:FF:FF is
// reserved for customer-visible addresses (FF:00:00-FF:FF:FF is for
// system MAC addresses). See RFD 174 for the discussion of the virtual
// range, and
// https://github.com/oxidecomputer/omicron/pull/955#discussion_r856432498
// for an initial discussion of the customer/system address range split.
pub(crate) const MIN_GUEST_ADDR: i64 = 0xA8_40_25_F0_00_00;
pub(crate) const MAX_GUEST_ADDR: i64 = 0xA8_40_25_FE_FF_FF;

/// Generate a random MAC address for a guest network interface
pub fn random_guest() -> Self {
let value =
thread_rng().gen_range(Self::MIN_GUEST_ADDR..=Self::MAX_GUEST_ADDR);
Self::from_i64(value)
}

/// Construct a MAC address from its i64 big-endian byte representation.
// NOTE: This is the representation used in the database.
pub(crate) fn from_i64(value: i64) -> Self {
let bytes = value.to_be_bytes();
Self(external::MacAddr(macaddr::MacAddr6::new(
bytes[2], bytes[3], bytes[4], bytes[5], bytes[6], bytes[7],
)))
}

/// Convert a MAC address to its i64 big-endian byte representation
// NOTE: This is the representation used in the database.
pub(crate) fn to_i64(self) -> i64 {
let bytes = self.0.as_bytes();
i64::from_be_bytes([
0, 0, bytes[0], bytes[1], bytes[2], bytes[3], bytes[4], bytes[5],
])
}
}

NewtypeFrom! { () pub struct MacAddr(external::MacAddr); }
NewtypeDeref! { () pub struct MacAddr(external::MacAddr); }

impl ToSql<sql_types::Text, Pg> for MacAddr {
impl ToSql<sql_types::BigInt, Pg> for MacAddr {
fn to_sql<'a>(
&'a self,
out: &mut serialize::Output<'a, '_, Pg>,
) -> serialize::Result {
<String as ToSql<sql_types::Text, Pg>>::to_sql(
&self.0.to_string(),
<i64 as ToSql<sql_types::BigInt, Pg>>::to_sql(
&self.to_i64(),
&mut out.reborrow(),
)
}
}

impl<DB> FromSql<sql_types::Text, DB> for MacAddr
impl<DB> FromSql<sql_types::BigInt, DB> for MacAddr
where
DB: Backend,
String: FromSql<sql_types::Text, DB>,
i64: FromSql<sql_types::BigInt, DB>,
{
fn from_sql(bytes: RawValue<DB>) -> deserialize::Result<Self> {
external::MacAddr::try_from(String::from_sql(bytes)?)
.map(MacAddr)
.map_err(|e| e.into())
let value = i64::from_sql(bytes)?;
Ok(MacAddr::from_i64(value))
}
}

#[cfg(test)]
mod tests {
use super::MacAddr;

#[test]
fn test_mac_to_int_conversions() {
let original: i64 = 0xa8_40_25_ff_00_01;
let mac = MacAddr::from_i64(original);
assert_eq!(mac.0.as_bytes(), &[0xa8, 0x40, 0x25, 0xff, 0x00, 0x01]);
let conv = mac.to_i64();
assert_eq!(original, conv);
}
}
6 changes: 1 addition & 5 deletions nexus/src/db/model/network_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,9 @@ impl From<NetworkInterface> for external::NetworkInterface {
#[derive(Clone, Debug)]
pub struct IncompleteNetworkInterface {
pub identity: NetworkInterfaceIdentity,

pub instance_id: Uuid,
pub vpc_id: Uuid,
pub subnet: VpcSubnet,
pub mac: MacAddr,
pub ip: Option<std::net::IpAddr>,
}

Expand All @@ -60,15 +58,13 @@ impl IncompleteNetworkInterface {
instance_id: Uuid,
vpc_id: Uuid,
subnet: VpcSubnet,
mac: MacAddr,
identity: external::IdentityMetadataCreateParams,
ip: Option<std::net::IpAddr>,
) -> Result<Self, external::Error> {
if let Some(ip) = ip {
subnet.check_requestable_addr(ip)?;
};
let identity = NetworkInterfaceIdentity::new(interface_id, identity);

Ok(Self { identity, instance_id, subnet, vpc_id, mac, ip })
Ok(Self { identity, instance_id, subnet, vpc_id, ip })
}
}
Loading

0 comments on commit 80092af

Please sign in to comment.