Skip to content

Commit

Permalink
Remove VNICs from XDE devices
Browse files Browse the repository at this point in the history
  • Loading branch information
FelixMcFelix committed Jul 2, 2024
1 parent 680757d commit 0d6b385
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 97 deletions.
32 changes: 2 additions & 30 deletions illumos-utils/src/opte/port.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,6 @@ pub struct PortData {
pub(crate) subnet: IpNet,
/// Information about the virtual gateway, aka OPTE
pub(crate) gateway: Gateway,
/// Name of the VNIC the OPTE port is bound to.
// TODO-remove(#2932): Remove this once we can put Viona directly on top of an
// OPTE port device.
//
// NOTE: This is intentionally not an actual `Vnic` object. We'd like to
// delete the VNIC manually in `PortInner::drop`, because we _can't_ delete
// the xde device if we fail to delete the VNIC. See
// https://github.com/oxidecomputer/opte/issues/178 for more details. This
// can be changed back to a real VNIC when that is resolved, and the Drop
// impl below can simplify to just call `drop(self.vnic)`.
pub(crate) vnic: String,
}

#[derive(Debug)]
Expand All @@ -57,18 +46,6 @@ impl core::ops::Deref for PortInner {
#[cfg(target_os = "illumos")]
impl Drop for PortInner {
fn drop(&mut self) {
if let Err(e) = crate::dladm::Dladm::delete_vnic(&self.vnic) {
eprintln!(
"WARNING: Failed to delete OPTE port overlay VNIC \
while dropping port. The VNIC will not be cleaned up \
properly, and the xde device itself will not be deleted. \
Both the VNIC and the xde device must be deleted out \
of band, and it will not be possible to recreate the xde \
device until then. Error: {:?}",
e
);
return;
}
let err = match opte_ioctl::OpteHdl::open(opte_ioctl::OpteHdl::XDE_CTL)
{
Ok(hdl) => {
Expand All @@ -81,9 +58,8 @@ impl Drop for PortInner {
Err(e) => e,
};
eprintln!(
"WARNING: OPTE port overlay VNIC deleted, but failed \
to delete the xde device. It must be deleted out \
of band, and it will not be possible to recreate the xde \
"WARNING: Failed to delete the xde device. It must be deleted
out of band, and it will not be possible to recreate the xde \
device until then. Error: {:?}",
err,
);
Expand Down Expand Up @@ -130,10 +106,6 @@ impl Port {
&self.inner.subnet
}

pub fn vnic_name(&self) -> &str {
&self.inner.vnic
}

pub fn slot(&self) -> u8 {
self.inner.slot
}
Expand Down
52 changes: 0 additions & 52 deletions illumos-utils/src/opte/port_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,57 +304,6 @@ impl PortManager {
rules,
})?;

// TODO-remove(#2932): Create a VNIC on top of this device, to hook Viona into.
//
// Viona is the illumos MAC provider that implements the VIRTIO
// specification. It sits on top of a MAC provider, which is responsible
// for delivering frames to the underlying data link. The guest includes
// a driver that handles the virtio-net specification on their side,
// which talks to Viona.
//
// In theory, Viona works with any MAC provider. However, there are
// implicit assumptions, in both Viona _and_ MAC, that require Viona to
// be built on top of a VNIC specifically. There is probably a good deal
// of work required to relax that assumption, so in the meantime, we
// create a superfluous VNIC on the OPTE device, solely so Viona can use
// it.
let vnic = {
let vnic_name = format!("v{}", port_name);
#[cfg(target_os = "illumos")]
if let Err(e) = crate::dladm::Dladm::create_vnic(
&crate::dladm::PhysicalLink(port_name.clone()),
&vnic_name,
Some(nic.mac),
None,
1500,
) {
slog::warn!(
self.inner.log,
"Failed to create overlay VNIC for xde device";
"port_name" => &port_name,
"err" => ?e
);
if let Err(e) = hdl.delete_xde(&port_name) {
slog::warn!(
self.inner.log,
"Failed to clean up xde device after failure to create overlay VNIC";
"err" => ?e
);
}
return Err(e.into());
}
debug!(
self.inner.log,
"Created overlay VNIC for xde device";
"port_name" => &port_name,
"vnic_name" => &vnic_name,
);

// NOTE: We intentionally use a string rather than the Vnic type
// here. See the notes on the `opte::PortInner::vnic` field.
vnic_name
};

let (port, ticket) = {
let mut ports = self.inner.ports.lock().unwrap();
let ticket = PortTicket::new(nic.id, nic.kind, self.inner.clone());
Expand All @@ -366,7 +315,6 @@ impl PortManager {
vni,
subnet: nic.subnet,
gateway,
vnic,
});
let old = ports.insert((nic.id, nic.kind), port.clone());
assert!(
Expand Down
20 changes: 9 additions & 11 deletions illumos-utils/src/running_zone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,15 +657,13 @@ impl RunningZone {
port_idx,
}
})?;
// TODO-remove(#2932): Switch to using port directly once vnic is no longer needed.
let addrobj =
AddrObject::new(port.vnic_name(), name).map_err(|err| {
EnsureAddressError::AddrObject {
request: AddressRequest::Dhcp,
zone: self.inner.name.clone(),
err,
}
})?;
let addrobj = AddrObject::new(port.name(), name).map_err(|err| {
EnsureAddressError::AddrObject {
request: AddressRequest::Dhcp,
zone: self.inner.name.clone(),
err,
}
})?;
let zone = Some(self.inner.name.as_ref());
if let IpAddr::V4(gateway) = port.gateway().ip() {
let addr =
Expand All @@ -684,7 +682,7 @@ impl RunningZone {
&private_ip.to_string(),
"-interface",
"-ifp",
port.vnic_name(),
port.name(),
])?;
self.run_cmd(&[
"/usr/sbin/route",
Expand Down Expand Up @@ -1427,7 +1425,7 @@ impl<'a> ZoneBuilder<'a> {

let mut net_device_names: Vec<String> = opte_ports
.iter()
.map(|(port, _)| port.vnic_name().to_string())
.map(|(port, _)| port.name().to_string())
.chain(std::iter::once(control_vnic.name().to_string()))
.chain(bootstrap_vnic.as_ref().map(|vnic| vnic.name().to_string()))
.chain(links.iter().map(|nic| nic.name().to_string()))
Expand Down
4 changes: 1 addition & 3 deletions sled-agent/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,9 +651,7 @@ impl InstanceRunner {
let nics = running_zone
.opte_ports()
.map(|port| propolis_client::types::NetworkInterfaceRequest {
// TODO-correctness: Remove `.vnic()` call when we use the port
// directly.
name: port.vnic_name().to_string(),
name: port.name().to_string(),
slot: propolis_client::types::Slot(port.slot()),
})
.collect();
Expand Down
2 changes: 1 addition & 1 deletion sled-agent/src/services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,7 @@ impl ServiceManager {
})
})?;

let opte_interface = port.vnic_name();
let opte_interface = port.name();
let opte_gateway = port.gateway().ip().to_string();
let opte_ip = port.ip().to_string();

Expand Down

0 comments on commit 0d6b385

Please sign in to comment.