From ef883d89fb95c50b78050c1633e54f8eac2f3821 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Tue, 9 Apr 2024 12:41:37 +0100 Subject: [PATCH 01/11] Setup router rule delete commands in OPTEADM This also adds in the `RouterClass` specifier, which does not yet do anything. --- bin/opteadm/src/bin/opteadm.rs | 47 ++++++++++++++++++++++++++-------- bin/opteadm/src/lib.rs | 11 +++++++- crates/opte-api/src/cmd.rs | 4 ++- crates/opte-api/src/lib.rs | 4 +-- lib/opte-ioctl/src/lib.rs | 11 +++++++- lib/oxide-vpc/src/api.rs | 38 ++++++++++++++++++++++++++- xde/src/xde.rs | 19 ++++++++++++++ 7 files changed, 118 insertions(+), 16 deletions(-) diff --git a/bin/opteadm/src/bin/opteadm.rs b/bin/opteadm/src/bin/opteadm.rs index 8cc45206..1d353d59 100644 --- a/bin/opteadm/src/bin/opteadm.rs +++ b/bin/opteadm/src/bin/opteadm.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -// Copyright 2023 Oxide Computer Company +// Copyright 2024 Oxide Computer Company use clap::Args; use clap::Parser; @@ -25,6 +25,7 @@ use opteadm::COMMIT_COUNT; use oxide_vpc::api::AddRouterEntryReq; use oxide_vpc::api::Address; use oxide_vpc::api::ClearVirt2BoundaryReq; +use oxide_vpc::api::DelRouterEntryReq; use oxide_vpc::api::DhcpCfg; use oxide_vpc::api::ExternalIpCfg; use oxide_vpc::api::Filters as FirewallFilters; @@ -38,6 +39,7 @@ use oxide_vpc::api::PortInfo; use oxide_vpc::api::Ports; use oxide_vpc::api::ProtoFilter; use oxide_vpc::api::RemFwRuleReq; +use oxide_vpc::api::RouterClass; use oxide_vpc::api::RouterTarget; use oxide_vpc::api::SNat4Cfg; use oxide_vpc::api::SNat6Cfg; @@ -208,13 +210,14 @@ enum Command { /// Add a new router entry, either IPv4 or IPv6. AddRouterEntry { - /// The OPTE port to which the route is added - #[arg(short)] - port: String, - /// The network destination to which the route applies. - dest: IpCidr, - /// The location to which traffic matching the destination is sent. - target: RouterTarget, + #[command(flatten)] + route: RouterRule, + }, + + /// Remove an existing router entry, either IPv4 or IPv6. + DelRouterEntry { + #[command(flatten)] + route: RouterRule, }, /// Configure external network addresses used by a port for VPC-external traffic. @@ -253,6 +256,19 @@ impl From for FirewallFilters { } } +#[derive(Debug, Parser)] +struct RouterRule { + /// The OPTE port to which the route change is applied. + #[arg(short)] + port: String, + /// The network destination to which the route applies. + dest: IpCidr, + /// The location to which traffic matching the destination is sent. + target: RouterTarget, + /// The class of router a rule belongs to ('system' or 'custom') + class: RouterClass, +} + // TODO: expand this to allow for v4 and v6 simultaneously? /// Per-port configuration for rack-external networking. #[derive(Args, Clone, Debug)] @@ -682,11 +698,22 @@ fn main() -> anyhow::Result<()> { hdl.clear_v2b(&req)?; } - Command::AddRouterEntry { port, dest, target } => { - let req = AddRouterEntryReq { port_name: port, dest, target }; + Command::AddRouterEntry { + route: RouterRule { port, dest, target, class }, + } => { + let req = + AddRouterEntryReq { port_name: port, dest, target, class }; hdl.add_router_entry(&req)?; } + Command::DelRouterEntry { + route: RouterRule { port, dest, target, class }, + } => { + let req = + DelRouterEntryReq { port_name: port, dest, target, class }; + hdl.del_router_entry(&req)?; + } + Command::SetExternalIps { port, external_net } => { if let Ok(cfg) = ExternalIpCfg::::try_from(external_net.clone()) diff --git a/bin/opteadm/src/lib.rs b/bin/opteadm/src/lib.rs index 1112fcf8..44608b61 100644 --- a/bin/opteadm/src/lib.rs +++ b/bin/opteadm/src/lib.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -// Copyright 2023 Oxide Computer Company +// Copyright 2024 Oxide Computer Company //! OPTE driver administration library @@ -16,6 +16,7 @@ use oxide_vpc::api::AddFwRuleReq; use oxide_vpc::api::AddRouterEntryReq; use oxide_vpc::api::ClearVirt2BoundaryReq; use oxide_vpc::api::CreateXdeReq; +use oxide_vpc::api::DelRouterEntryReq; use oxide_vpc::api::DeleteXdeReq; use oxide_vpc::api::DhcpCfg; use oxide_vpc::api::FirewallRule; @@ -259,6 +260,14 @@ impl OpteAdm { run_cmd_ioctl(self.device.as_raw_fd(), cmd, Some(&req)) } + pub fn del_router_entry( + &self, + req: &DelRouterEntryReq, + ) -> Result { + let cmd = OpteCmd::DelRouterEntry; + run_cmd_ioctl(self.device.as_raw_fd(), cmd, Some(&req)) + } + pub fn set_external_ips( &self, req: &SetExternalIpsReq, diff --git a/crates/opte-api/src/cmd.rs b/crates/opte-api/src/cmd.rs index 7d828b13..48ddb663 100644 --- a/crates/opte-api/src/cmd.rs +++ b/crates/opte-api/src/cmd.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -// Copyright 2023 Oxide Computer Company +// Copyright 2024 Oxide Computer Company use super::encap::Vni; use super::ip::IpCidr; @@ -36,6 +36,7 @@ pub enum OpteCmd { ClearVirt2Boundary = 53, // clear a v2b mapping DumpVirt2Boundary = 54, // dump the v2b mappings AddRouterEntry = 60, // add a router entry for IP dest + DelRouterEntry = 61, // remove a router entry for IP dest CreateXde = 70, // create a new xde device DeleteXde = 71, // delete an xde device SetXdeUnderlay = 72, // set xde underlay devices @@ -63,6 +64,7 @@ impl TryFrom for OpteCmd { 53 => Ok(Self::ClearVirt2Boundary), 54 => Ok(Self::DumpVirt2Boundary), 60 => Ok(Self::AddRouterEntry), + 61 => Ok(Self::DelRouterEntry), 70 => Ok(Self::CreateXde), 71 => Ok(Self::DeleteXde), 72 => Ok(Self::SetXdeUnderlay), diff --git a/crates/opte-api/src/lib.rs b/crates/opte-api/src/lib.rs index c82d1cb2..12008bf5 100644 --- a/crates/opte-api/src/lib.rs +++ b/crates/opte-api/src/lib.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -// Copyright 2023 Oxide Computer Company +// Copyright 2024 Oxide Computer Company #![no_std] #![deny(unreachable_patterns)] @@ -47,7 +47,7 @@ pub use ulp::*; /// /// We rely on CI and the check-api-version.sh script to verify that /// this number is incremented anytime the oxide-api code changes. -pub const API_VERSION: u64 = 28; +pub const API_VERSION: u64 = 29; /// Major version of the OPTE package. pub const MAJOR_VERSION: u64 = 0; diff --git a/lib/opte-ioctl/src/lib.rs b/lib/opte-ioctl/src/lib.rs index 7906ecf2..5c6abf91 100644 --- a/lib/opte-ioctl/src/lib.rs +++ b/lib/opte-ioctl/src/lib.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -// Copyright 2022 Oxide Computer Company +// Copyright 2024 Oxide Computer Company use opte::api::CmdOk; use opte::api::NoResp; @@ -15,6 +15,7 @@ use opte::api::XDE_IOC_OPTE_CMD; use oxide_vpc::api::AddRouterEntryReq; use oxide_vpc::api::ClearVirt2BoundaryReq; use oxide_vpc::api::CreateXdeReq; +use oxide_vpc::api::DelRouterEntryReq; use oxide_vpc::api::DeleteXdeReq; use oxide_vpc::api::DhcpCfg; use oxide_vpc::api::ListPortsResp; @@ -179,6 +180,14 @@ impl OpteHdl { run_cmd_ioctl(self.device.as_raw_fd(), cmd, Some(&req)) } + pub fn del_router_entry( + &self, + req: &DelRouterEntryReq, + ) -> Result { + let cmd = OpteCmd::DelRouterEntry; + run_cmd_ioctl(self.device.as_raw_fd(), cmd, Some(&req)) + } + pub fn set_fw_rules(&self, req: &SetFwRulesReq) -> Result { let cmd = OpteCmd::SetFwRules; run_cmd_ioctl(self.device.as_raw_fd(), cmd, Some(&req)) diff --git a/lib/oxide-vpc/src/api.rs b/lib/oxide-vpc/src/api.rs index 8f146f83..30942267 100644 --- a/lib/oxide-vpc/src/api.rs +++ b/lib/oxide-vpc/src/api.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -// Copyright 2023 Oxide Computer Company +// Copyright 2024 Oxide Computer Company use alloc::string::String; use alloc::string::ToString; @@ -414,6 +414,40 @@ impl Display for RouterTarget { } } +/// The class of router which a rule belongs to. +#[derive(Clone, Debug, Copy, Deserialize, Serialize)] +pub enum RouterClass { + /// The rule belongs to the shared VPC-wide router. + System, + /// The rule belongs to the subnet-specific router, and has precendence + /// over a `System` rule of equal priority. + Custom, +} + +#[cfg(any(feature = "std", test))] +impl FromStr for RouterClass { + type Err = String; + + fn from_str(s: &str) -> Result { + match s.to_ascii_lowercase().as_str() { + "system" => Ok(Self::System), + "custom" => Ok(Self::Custom), + lower => Err(format!( + "unexpected router class {lower} -- expected 'system' or 'custom'" + )), + } + } +} + +impl Display for RouterClass { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::System => write!(f, "System"), + Self::Custom => write!(f, "Custom"), + } + } +} + /// Xde create ioctl parameter data. /// /// The bulk of the information is provided via [`VpcCfg`]. @@ -505,6 +539,7 @@ pub struct AddRouterEntryReq { pub port_name: String, pub dest: IpCidr, pub target: RouterTarget, + pub class: RouterClass, } #[derive(Clone, Debug, Deserialize, Serialize)] @@ -512,6 +547,7 @@ pub struct DelRouterEntryReq { pub port_name: String, pub dest: IpCidr, pub target: RouterTarget, + pub class: RouterClass, } #[derive(Clone, Debug, Deserialize, Serialize)] diff --git a/xde/src/xde.rs b/xde/src/xde.rs index e588b4cd..572c8fe5 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -73,6 +73,7 @@ use oxide_vpc::api::AddFwRuleReq; use oxide_vpc::api::AddRouterEntryReq; use oxide_vpc::api::ClearVirt2BoundaryReq; use oxide_vpc::api::CreateXdeReq; +use oxide_vpc::api::DelRouterEntryReq; use oxide_vpc::api::DeleteXdeReq; use oxide_vpc::api::DhcpCfg; use oxide_vpc::api::ListPortsResp; @@ -582,6 +583,11 @@ unsafe extern "C" fn xde_ioc_opte_cmd(karg: *mut c_void, mode: c_int) -> c_int { hdlr_resp(&mut env, resp) } + OpteCmd::DelRouterEntry => { + let resp = del_router_entry_hdlr(&mut env); + hdlr_resp(&mut env, resp) + } + OpteCmd::DumpTcpFlows => { let resp = dump_tcp_flows_hdlr(&mut env); hdlr_resp(&mut env, resp) @@ -2147,6 +2153,19 @@ fn add_router_entry_hdlr(env: &mut IoctlEnvelope) -> Result { router::add_entry(&dev.port, req.dest, req.target) } +#[no_mangle] +fn del_router_entry_hdlr(env: &mut IoctlEnvelope) -> Result { + let req: DelRouterEntryReq = env.copy_in_req()?; + let devs = unsafe { xde_devs.read() }; + let mut iter = devs.iter(); + let dev = match iter.find(|x| x.devname == req.port_name) { + Some(dev) => dev, + None => return Err(OpteError::PortNotFound(req.port_name)), + }; + + router::del_entry(&dev.port, req.dest, req.target) +} + #[no_mangle] fn add_fw_rule_hdlr(env: &mut IoctlEnvelope) -> Result { let req: AddFwRuleReq = env.copy_in_req()?; From 1ebbbb6d4a8a675aa4c1988a556818343819fbc1 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Tue, 9 Apr 2024 13:06:19 +0100 Subject: [PATCH 02/11] Correctly plumb through delete result from xde --- bin/opteadm/src/bin/opteadm.rs | 7 ++++++- bin/opteadm/src/lib.rs | 3 ++- lib/opte-ioctl/src/lib.rs | 3 ++- lib/oxide-vpc/src/api.rs | 2 ++ xde-tests/src/lib.rs | 1 + xde/src/xde.rs | 5 ++++- 6 files changed, 17 insertions(+), 4 deletions(-) diff --git a/bin/opteadm/src/bin/opteadm.rs b/bin/opteadm/src/bin/opteadm.rs index 1d353d59..b74ad3ab 100644 --- a/bin/opteadm/src/bin/opteadm.rs +++ b/bin/opteadm/src/bin/opteadm.rs @@ -26,6 +26,7 @@ use oxide_vpc::api::AddRouterEntryReq; use oxide_vpc::api::Address; use oxide_vpc::api::ClearVirt2BoundaryReq; use oxide_vpc::api::DelRouterEntryReq; +use oxide_vpc::api::DelRouterEntryResp; use oxide_vpc::api::DhcpCfg; use oxide_vpc::api::ExternalIpCfg; use oxide_vpc::api::Filters as FirewallFilters; @@ -711,7 +712,11 @@ fn main() -> anyhow::Result<()> { } => { let req = DelRouterEntryReq { port_name: port, dest, target, class }; - hdl.del_router_entry(&req)?; + if let DelRouterEntryResp::NotFound = hdl.del_router_entry(&req)? { + anyhow::bail!( + "could not delete entry -- no matching rule found" + ); + } } Command::SetExternalIps { port, external_net } => { diff --git a/bin/opteadm/src/lib.rs b/bin/opteadm/src/lib.rs index 44608b61..0a3f1be2 100644 --- a/bin/opteadm/src/lib.rs +++ b/bin/opteadm/src/lib.rs @@ -17,6 +17,7 @@ use oxide_vpc::api::AddRouterEntryReq; use oxide_vpc::api::ClearVirt2BoundaryReq; use oxide_vpc::api::CreateXdeReq; use oxide_vpc::api::DelRouterEntryReq; +use oxide_vpc::api::DelRouterEntryResp; use oxide_vpc::api::DeleteXdeReq; use oxide_vpc::api::DhcpCfg; use oxide_vpc::api::FirewallRule; @@ -263,7 +264,7 @@ impl OpteAdm { pub fn del_router_entry( &self, req: &DelRouterEntryReq, - ) -> Result { + ) -> Result { let cmd = OpteCmd::DelRouterEntry; run_cmd_ioctl(self.device.as_raw_fd(), cmd, Some(&req)) } diff --git a/lib/opte-ioctl/src/lib.rs b/lib/opte-ioctl/src/lib.rs index 5c6abf91..e1dbc392 100644 --- a/lib/opte-ioctl/src/lib.rs +++ b/lib/opte-ioctl/src/lib.rs @@ -16,6 +16,7 @@ use oxide_vpc::api::AddRouterEntryReq; use oxide_vpc::api::ClearVirt2BoundaryReq; use oxide_vpc::api::CreateXdeReq; use oxide_vpc::api::DelRouterEntryReq; +use oxide_vpc::api::DelRouterEntryResp; use oxide_vpc::api::DeleteXdeReq; use oxide_vpc::api::DhcpCfg; use oxide_vpc::api::ListPortsResp; @@ -183,7 +184,7 @@ impl OpteHdl { pub fn del_router_entry( &self, req: &DelRouterEntryReq, - ) -> Result { + ) -> Result { let cmd = OpteCmd::DelRouterEntry; run_cmd_ioctl(self.device.as_raw_fd(), cmd, Some(&req)) } diff --git a/lib/oxide-vpc/src/api.rs b/lib/oxide-vpc/src/api.rs index 30942267..0bef3b11 100644 --- a/lib/oxide-vpc/src/api.rs +++ b/lib/oxide-vpc/src/api.rs @@ -556,6 +556,8 @@ pub enum DelRouterEntryResp { NotFound, } +impl opte::api::cmd::CmdOk for DelRouterEntryResp {} + #[derive(Clone, Debug, Deserialize, Serialize)] pub struct SetExternalIpsReq { pub port_name: String, diff --git a/xde-tests/src/lib.rs b/xde-tests/src/lib.rs index 2344decd..022c98ff 100644 --- a/xde-tests/src/lib.rs +++ b/xde-tests/src/lib.rs @@ -117,6 +117,7 @@ impl OptePort { port_name: self.name.clone(), dest: IpCidr::Ip4(format!("{}/32", dest).parse().unwrap()), target: RouterTarget::Ip(dest.parse().unwrap()), + class: oxide_vpc::api::RouterClass::System, })?; Ok(()) } diff --git a/xde/src/xde.rs b/xde/src/xde.rs index 572c8fe5..1731cc5e 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -74,6 +74,7 @@ use oxide_vpc::api::AddRouterEntryReq; use oxide_vpc::api::ClearVirt2BoundaryReq; use oxide_vpc::api::CreateXdeReq; use oxide_vpc::api::DelRouterEntryReq; +use oxide_vpc::api::DelRouterEntryResp; use oxide_vpc::api::DeleteXdeReq; use oxide_vpc::api::DhcpCfg; use oxide_vpc::api::ListPortsResp; @@ -2154,7 +2155,9 @@ fn add_router_entry_hdlr(env: &mut IoctlEnvelope) -> Result { } #[no_mangle] -fn del_router_entry_hdlr(env: &mut IoctlEnvelope) -> Result { +fn del_router_entry_hdlr( + env: &mut IoctlEnvelope, +) -> Result { let req: DelRouterEntryReq = env.copy_in_req()?; let devs = unsafe { xde_devs.read() }; let mut iter = devs.iter(); From a8c99db28083ae164881bc5a4dce38723f25fc5c Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Tue, 9 Apr 2024 14:02:44 +0100 Subject: [PATCH 03/11] Encode system/custom rule distinction in priority --- lib/opte-test-utils/src/lib.rs | 2 + lib/oxide-vpc/src/engine/router.rs | 70 ++++++++++++++---------- lib/oxide-vpc/tests/integration_tests.rs | 17 ++++++ xde-tests/src/lib.rs | 3 +- 4 files changed, 61 insertions(+), 31 deletions(-) diff --git a/lib/opte-test-utils/src/lib.rs b/lib/opte-test-utils/src/lib.rs index 250326d3..cabb4fc9 100644 --- a/lib/opte-test-utils/src/lib.rs +++ b/lib/opte-test-utils/src/lib.rs @@ -63,6 +63,7 @@ pub use oxide_vpc::api::IpCfg; pub use oxide_vpc::api::Ipv4Cfg; pub use oxide_vpc::api::Ipv6Cfg; pub use oxide_vpc::api::PhysNet; +pub use oxide_vpc::api::RouterClass; pub use oxide_vpc::api::RouterTarget; pub use oxide_vpc::api::SNat4Cfg; pub use oxide_vpc::api::SNat6Cfg; @@ -333,6 +334,7 @@ pub fn oxide_net_setup2( &port, IpCidr::Ip4(cfg.ipv4().vpc_subnet), RouterTarget::VpcSubnet(IpCidr::Ip4(cfg.ipv4().vpc_subnet)), + RouterClass::System, ) .unwrap(); diff --git a/lib/oxide-vpc/src/engine/router.rs b/lib/oxide-vpc/src/engine/router.rs index 3be1ff20..0aa455d7 100644 --- a/lib/oxide-vpc/src/engine/router.rs +++ b/lib/oxide-vpc/src/engine/router.rs @@ -11,6 +11,7 @@ use super::firewall as fw; use super::VpcNetwork; use crate::api::DelRouterEntryResp; +use crate::api::RouterClass; use crate::api::RouterTarget; use crate::cfg::VpcCfg; use alloc::string::String; @@ -116,11 +117,11 @@ impl fmt::Display for RouterTargetInternal { } // Return the priority for a given IP subnet. The priority is based on -// the subnet's prefix length. Specifically, it is given the following -// value. +// the subnet's prefix length and the type of router the rule belongs to. +// Specifically, it is given the following value: // // ``` -// priroity = max_prefix_len - prefix len + 10 +// priority = (((max_prefix_len - prefix len) << 1) | is_system) + 10 // ``` // // `max_prefix_len` is the maximum prefix length for a given IP @@ -128,6 +129,9 @@ impl fmt::Display for RouterTargetInternal { // // `prefix_len` comes from the passed in `cidr` argument. // +// One bit is used to ensure that 'custom' router rules take precedence +// over 'system' rules when all other factors are equal. +// // The constant `10` displaces these rules so they start at a priority // of `10`. This allows placing higher priority rules (lower number) // to override them, if needed. @@ -135,27 +139,31 @@ impl fmt::Display for RouterTargetInternal { // # IPv4 // // ``` -// |Prefix Len |Priority | -// |-----------|--------------------| -// |32 |10 = 32 - 32 10 | -// |31 |11 = 32 - 31 10 | -// |30 |12 = 32 - 30 10 | -// |... |... | -// |0 |42 = 32 - 0 10 | +// |Prefix Len |System?|Priority | +// |-----------|-------|-------------------------------| +// |32 |0 |10 = ((32 - 32) << 1 | 0) + 10 | +// |32 |1 |11 = ((32 - 32) << 1 | 1) + 10 | +// |31 |0 |12 = ((32 - 31) << 1 | 0) + 10 | +// |30 |0 |14 = ((32 - 30) << 1 | 0) + 10 | +// |... |... |... | +// |0 |0 |74 = ((32 - 0) << 1 | 0) + 10 | +// |0 |1 |75 = ((32 - 0) << 1 | 1) + 10 | // ``` // // # IPv6 // // ``` -// |Prefix Len |Priority | -// |-----------|--------------------| -// |128 |10 = 128 - 128 10 | -// |127 |11 = 128 - 127 10 | -// |126 |12 = 128 - 126 10 | -// |... |... | -// |0 |138 = 128 - 0 10 | +// |Prefix Len |System?|Priority | +// |-----------|-------|---------------------------------| +// |128 |0 |10 = ((128 - 128) << 1 | 0) + 10 | +// |128 |1 |11 = ((128 - 128) << 1 | 1) + 10 | +// |127 |0 |12 = ((128 - 127) << 1 | 0) + 10 | +// |126 |0 |14 = ((128 - 126) << 1 | 0) + 10 | +// |... |... |... | +// |0 |0 |266 = ((128 - 0) << 1 | 0) + 10 | +// |0 |1 |267 = ((128 - 0) << 1 | 1) + 10 | // ``` -fn prefix_len_to_priority(cidr: &IpCidr) -> u16 { +fn compute_rule_priority(cidr: &IpCidr, class: RouterClass) -> u16 { use opte::api::ip::IpCidr::*; use opte::api::ip::Ipv4PrefixLen; use opte::api::ip::Ipv6PrefixLen; @@ -163,7 +171,11 @@ fn prefix_len_to_priority(cidr: &IpCidr) -> u16 { Ip4(ipv4) => (Ipv4PrefixLen::NETMASK_ALL.val(), ipv4.prefix_len()), Ip6(ipv6) => (Ipv6PrefixLen::NETMASK_ALL.val(), ipv6.prefix_len()), }; - (max_prefix_len - prefix_len) as u16 + 10 + let class_prio = match class { + RouterClass::Custom => 0, + RouterClass::System => 1, + }; + ((((max_prefix_len - prefix_len) as u16) << 1) | class_prio) + 10 } pub fn setup( @@ -209,6 +221,7 @@ fn valid_router_dest_target_pair(dest: &IpCidr, target: &RouterTarget) -> bool { fn make_rule( dest: IpCidr, target: RouterTarget, + class: RouterClass, ) -> Result, OpteError> { if !valid_router_dest_target_pair(&dest, &target) { return Err(OpteError::InvalidRouterEntry { @@ -280,7 +293,7 @@ fn make_rule( } }; - let priority = prefix_len_to_priority(&dest); + let priority = compute_rule_priority(&dest, class); let mut rule = Rule::new(priority, action); rule.add_predicate(predicate); Ok(rule.finalize()) @@ -294,8 +307,9 @@ pub fn del_entry( port: &Port, dest: IpCidr, target: RouterTarget, + class: RouterClass, ) -> Result { - let rule = make_rule(dest, target)?; + let rule = make_rule(dest, target, class)?; let maybe_id = port.find_rule(ROUTER_LAYER_NAME, Direction::Out, &rule)?; match maybe_id { Some(id) => { @@ -314,8 +328,9 @@ pub fn add_entry( port: &Port, dest: IpCidr, target: RouterTarget, + class: RouterClass, ) -> Result { - let rule = make_rule(dest, target)?; + let rule = make_rule(dest, target, class)?; port.add_rule(ROUTER_LAYER_NAME, Direction::Out, rule)?; Ok(NoResp::default()) } @@ -323,20 +338,17 @@ pub fn add_entry( /// Replace the current set of router entries with the set passed in. pub fn replace( port: &Port, - entries: Vec<(IpCidr, RouterTarget)>, + entries: Vec<(IpCidr, RouterTarget, RouterClass)>, ) -> Result { let mut out_rules = Vec::with_capacity(entries.len()); - for (cidr, target) in entries { - out_rules.push(make_rule(cidr, target)?); + for (cidr, target, class) in entries { + out_rules.push(make_rule(cidr, target, class)?); } port.set_rules(ROUTER_LAYER_NAME, vec![], out_rules)?; Ok(NoResp::default()) } -// TODO For each router table entry we should mark whether it came -// from system or custom. -// // TODO I may want to have different types of rule/flow tables a layer // can have. Up to this point the tables consist of `Rule` entires; // matching arbitrary header predicates to a `RuleAction`. I may want @@ -350,8 +362,6 @@ pub fn replace( // VFP ยง6.5 ("Packet Classification"), talks about the ability for // each condition type to use 1 of 4 different types of classifiers. pub struct RouterAction { - // system_table: RouterTable, - // subnet_table: Option, target: RouterTargetInternal, } diff --git a/lib/oxide-vpc/tests/integration_tests.rs b/lib/oxide-vpc/tests/integration_tests.rs index bee77e05..84b99a96 100644 --- a/lib/oxide-vpc/tests/integration_tests.rs +++ b/lib/oxide-vpc/tests/integration_tests.rs @@ -52,6 +52,7 @@ use opte::engine::udp::UdpMeta; use opte::engine::Direction; use oxide_vpc::api::ExternalIpCfg; use oxide_vpc::api::FirewallRule; +use oxide_vpc::api::RouterClass; use oxide_vpc::api::VpcCfg; use oxide_vpc::engine::overlay::BOUNDARY_SERVICES_VNI; use pcap::*; @@ -267,6 +268,7 @@ fn port_transition_pause() { RouterTarget::VpcSubnet(IpCidr::Ip4( g2_cfg.ipv4_cfg().unwrap().vpc_subnet )), + RouterClass::System, ), Err(OpteError::BadState(_)) )); @@ -444,6 +446,7 @@ fn guest_to_guest_no_route() { &g1.port, IpCidr::Ip4(g1_cfg.ipv4().vpc_subnet), RouterTarget::VpcSubnet(IpCidr::Ip4(g1_cfg.ipv4().vpc_subnet)), + RouterClass::System, ) .unwrap(); update!(g1, ["incr:epoch", "set:router.rules.out=0"]); @@ -707,6 +710,7 @@ fn guest_to_internet_ipv4() { &g1.port, IpCidr::Ip4("0.0.0.0/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["epoch", "router.rules.out"]); @@ -830,6 +834,7 @@ fn guest_to_internet_ipv6() { &g1.port, IpCidr::Ip6("::/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["epoch", "router.rules.out"]); @@ -1027,6 +1032,7 @@ fn multi_external_ip_setup( &g1.port, IpCidr::Ip6("::/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["epoch", "router.rules.out"]); @@ -1034,6 +1040,7 @@ fn multi_external_ip_setup( &g1.port, IpCidr::Ip4("0.0.0.0/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["epoch", "router.rules.out"]); @@ -1644,6 +1651,7 @@ fn snat_icmp_shared_echo_rewrite(dst_ip: IpAddr) { &g1.port, IpCidr::Ip6("::/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["epoch", "router.rules.out"]); @@ -1651,6 +1659,7 @@ fn snat_icmp_shared_echo_rewrite(dst_ip: IpAddr) { &g1.port, IpCidr::Ip4("0.0.0.0/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["epoch", "router.rules.out"]); @@ -2523,6 +2532,7 @@ fn outbound_ndp_dropped() { &g1.port, IpCidr::Ip6(ipv6.vpc_subnet), RouterTarget::VpcSubnet(IpCidr::Ip6(ipv6.vpc_subnet)), + RouterClass::System, ) .unwrap(); incr!(g1, ["router.rules.out", "epoch"]); @@ -2532,6 +2542,7 @@ fn outbound_ndp_dropped() { &g1.port, IpCidr::Ip6("::/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["router.rules.out", "epoch"]); @@ -3034,6 +3045,7 @@ fn uft_lft_invalidation_out() { &g1.port, IpCidr::Ip4("0.0.0.0/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["epoch", "router.rules.out"]); @@ -3120,6 +3132,7 @@ fn uft_lft_invalidation_in() { &g1.port, IpCidr::Ip4("0.0.0.0/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["epoch", "router.rules.out"]); @@ -3441,6 +3454,7 @@ fn tcp_outbound() { &g1.port, IpCidr::Ip4("0.0.0.0/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["epoch", "router.rules.out"]); @@ -3502,6 +3516,7 @@ fn early_tcp_invalidation() { &g1.port, IpCidr::Ip4("0.0.0.0/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["epoch", "router.rules.out"]); @@ -3684,6 +3699,7 @@ fn tcp_inbound() { &g1.port, IpCidr::Ip4("0.0.0.0/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["epoch", "router.rules.out"]); @@ -3949,6 +3965,7 @@ fn no_panic_on_flow_table_full() { &g1.port, IpCidr::Ip4("0.0.0.0/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["epoch", "router.rules.out"]); diff --git a/xde-tests/src/lib.rs b/xde-tests/src/lib.rs index 022c98ff..6ecdb97f 100644 --- a/xde-tests/src/lib.rs +++ b/xde-tests/src/lib.rs @@ -23,6 +23,7 @@ use oxide_vpc::api::Ipv6Addr; use oxide_vpc::api::MacAddr; use oxide_vpc::api::PhysNet; use oxide_vpc::api::Ports; +use oxide_vpc::api::RouterClass; use oxide_vpc::api::RouterTarget; use oxide_vpc::api::SNat4Cfg; use oxide_vpc::api::SetVirt2PhysReq; @@ -117,7 +118,7 @@ impl OptePort { port_name: self.name.clone(), dest: IpCidr::Ip4(format!("{}/32", dest).parse().unwrap()), target: RouterTarget::Ip(dest.parse().unwrap()), - class: oxide_vpc::api::RouterClass::System, + class: RouterClass::System, })?; Ok(()) } From aa08284580cf5ef18229a1f31a7d918c067d6052 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Tue, 9 Apr 2024 14:07:23 +0100 Subject: [PATCH 04/11] Iterating. --- xde/src/xde.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xde/src/xde.rs b/xde/src/xde.rs index 1731cc5e..c866539d 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -2151,7 +2151,7 @@ fn add_router_entry_hdlr(env: &mut IoctlEnvelope) -> Result { None => return Err(OpteError::PortNotFound(req.port_name)), }; - router::add_entry(&dev.port, req.dest, req.target) + router::add_entry(&dev.port, req.dest, req.target, req.class) } #[no_mangle] @@ -2166,7 +2166,7 @@ fn del_router_entry_hdlr( None => return Err(OpteError::PortNotFound(req.port_name)), }; - router::del_entry(&dev.port, req.dest, req.target) + router::del_entry(&dev.port, req.dest, req.target, req.class) } #[no_mangle] From 421c507481c9e877f6a8d5376dcceb9602ad88f9 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Tue, 9 Apr 2024 18:38:07 +0100 Subject: [PATCH 05/11] Integration test for custom router with add/delete --- lib/oxide-vpc/tests/integration_tests.rs | 121 +++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/lib/oxide-vpc/tests/integration_tests.rs b/lib/oxide-vpc/tests/integration_tests.rs index 84b99a96..4789f1fa 100644 --- a/lib/oxide-vpc/tests/integration_tests.rs +++ b/lib/oxide-vpc/tests/integration_tests.rs @@ -3998,3 +3998,124 @@ fn no_panic_on_flow_table_full() { let res2 = g1.port.process(Out, &mut pkt2, ActionMeta::new()); assert_drop!(res2, DropReason::TcpErr); } + +#[test] +fn intra_subnet_routes_with_custom() { + let g1_cfg = g1_cfg(); + let mut g1 = oxide_net_setup("g1_port", &g1_cfg, None, None); + g1.port.start(); + set!(g1, "port_state=running"); + + // This guest is 172.30.0.5 on 172.30.0.0/22. + // Suppose that we have a second subnet, 172.30.4.0/22. + // The control plane must insert a system route for as long + // as this subnet exists. + let cidr = IpCidr::Ip4("172.30.4.0/22".parse().unwrap()); + router::add_entry( + &g1.port, + cidr.clone(), + RouterTarget::VpcSubnet(cidr.clone()), + RouterClass::System, + ) + .unwrap(); + incr!(g1, ["epoch", "router.rules.out"]); + + // define a guest in this range... + let dst_ip: Ipv4Addr = "172.30.4.5".parse().unwrap(); + let other_guest_mac = ox_vpc_mac([0xF0, 0x00, 0x66]); + let other_guest_phys_ip = Ipv6Addr::from([ + 0xFD00, 0x0000, 0x00F7, 0x0116, 0x0000, 0x0000, 0x0000, 0x0001, + ]); + g1.vpc_map.add( + dst_ip.into(), + PhysNet { + ether: other_guest_mac, + ip: other_guest_phys_ip, + vni: g1_cfg.vni, + }, + ); + let data = b"1234\0"; + + // Send one ICMP packet to that guest. + let mut pkt1 = gen_icmpv4_echo_req( + g1_cfg.guest_mac, + g1_cfg.gateway_mac, + g1_cfg.ipv4().private_ip, + dst_ip, + 7777, + 1, + data, + 1, + ); + + // Process the packet through our port. It should be allowed through: + // we have a V2P mapping for the target guest, and a route for the other + // subnet. + let res = g1.port.process(Out, &mut pkt1, ActionMeta::new()); + assert!(matches!(res, Ok(ProcessResult::Modified))); + incr!( + g1, + [ + "firewall.flows.in, firewall.flows.out", + "stats.port.out_modified, stats.port.out_uft_miss, uft.out", + ] + ); + + // Suppose the user now installs a 'custom' route in the first subnet to + // drop traffic towards the second subnet. This rule must take priority. + router::add_entry( + &g1.port, + cidr.clone(), + RouterTarget::Drop, + RouterClass::Custom, + ) + .unwrap(); + incr!(g1, ["epoch", "router.rules.out"]); + let mut pkt2 = gen_icmpv4_echo_req( + g1_cfg.guest_mac, + g1_cfg.gateway_mac, + g1_cfg.ipv4().private_ip, + dst_ip, + 7777, + 1, + data, + 1, + ); + let res = g1.port.process(Out, &mut pkt2, ActionMeta::new()); + assert!(matches!( + res, + Ok(ProcessResult::Drop { + reason: DropReason::Layer { name: "router", .. } + }) + )); + update!( + g1, + [ + "incr:stats.port.out_drop, stats.port.out_drop_layer", + "incr:stats.port.out_uft_miss", + "decr:uft.out" + ] + ); + + // When the user removes this rule, traffic may flow again to subnet 2. + router::del_entry( + &g1.port, + cidr.clone(), + RouterTarget::Drop, + RouterClass::Custom, + ) + .unwrap(); + update!(g1, ["incr:epoch", "decr:router.rules.out"]); + let mut pkt3 = gen_icmpv4_echo_req( + g1_cfg.guest_mac, + g1_cfg.gateway_mac, + g1_cfg.ipv4().private_ip, + dst_ip, + 7777, + 1, + data, + 1, + ); + let res = g1.port.process(Out, &mut pkt3, ActionMeta::new()); + assert!(matches!(res, Ok(ProcessResult::Modified))); +} From d273b9f4d50d25aa24eca4ad0fafc7f10b0a935f Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Tue, 9 Apr 2024 18:44:50 +0100 Subject: [PATCH 06/11] Missed some routes in ubench. --- bench/src/packet.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bench/src/packet.rs b/bench/src/packet.rs index 4979ba03..44df84e0 100644 --- a/bench/src/packet.rs +++ b/bench/src/packet.rs @@ -295,6 +295,7 @@ impl BenchPacketInstance for UlpProcessInstance { &g1.port, IpCidr::Ip4("0.0.0.0/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["epoch", "router.rules.out"]); @@ -303,6 +304,7 @@ impl BenchPacketInstance for UlpProcessInstance { &g1.port, IpCidr::Ip6("::/0".parse().unwrap()), RouterTarget::InternetGateway, + RouterClass::System, ) .unwrap(); incr!(g1, ["epoch", "router.rules.out"]); From 87a84fe0b695d4d18521c5c3e4dd2f9134fe03ff Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 10 Apr 2024 13:04:54 +0100 Subject: [PATCH 07/11] Gateway: allow tx/rx of given CIDRs as required RFD 21 goes into some detail describing use cases where users might want to use one instance as a tunnel endpoint for a VPN, and how traffic towards an arbitrary CIDR can be routed to an instance for, e.g., software routing purposes. To enable this, we need to be able to setup holes in the anti-spoof filters in the gateway, which are added here as a new pair of ioctls to add and remove these holes. It's not clear from the RFD whether this is to be an on/off toggle or as fine-grained as the new ioctls allow, but we can suoport either case here. --- bin/opteadm/src/bin/opteadm.rs | 35 +++++++- bin/opteadm/src/lib.rs | 30 +++++++ crates/opte-api/src/cmd.rs | 4 + lib/opte-ioctl/src/lib.rs | 30 +++++++ lib/opte/src/engine/port.rs | 2 +- lib/oxide-vpc/src/api.rs | 26 ++++++ lib/oxide-vpc/src/engine/gateway/mod.rs | 91 ++++++++++++++++++++ lib/oxide-vpc/tests/integration_tests.rs | 103 +++++++++++++++++++++++ xde/src/xde.rs | 43 ++++++++++ 9 files changed, 362 insertions(+), 2 deletions(-) diff --git a/bin/opteadm/src/bin/opteadm.rs b/bin/opteadm/src/bin/opteadm.rs index b74ad3ab..ac8955c7 100644 --- a/bin/opteadm/src/bin/opteadm.rs +++ b/bin/opteadm/src/bin/opteadm.rs @@ -40,6 +40,7 @@ use oxide_vpc::api::PortInfo; use oxide_vpc::api::Ports; use oxide_vpc::api::ProtoFilter; use oxide_vpc::api::RemFwRuleReq; +use oxide_vpc::api::RemoveCidrResp; use oxide_vpc::api::RouterClass; use oxide_vpc::api::RouterTarget; use oxide_vpc::api::SNat4Cfg; @@ -223,13 +224,33 @@ enum Command { /// Configure external network addresses used by a port for VPC-external traffic. SetExternalIps { - /// The OPTE port to which the route is added + /// The OPTE port to configure #[arg(short)] port: String, #[command(flatten)] external_net: ExternalNetConfig, }, + + /// Allows a guest to send and receive traffic on a given CIDR block. + AllowCidr { + /// The OPTE port to configure + #[arg(short)] + port: String, + + /// The IP block to allow through the gateway. + prefix: IpCidr, + }, + + /// Prevents a guest from sending/receiving traffic on a given CIDR block. + RemoveCidr { + /// The OPTE port to configure + #[arg(short)] + port: String, + + /// The IP block to deny at the gateway. + prefix: IpCidr, + }, } #[derive(Debug, Parser)] @@ -743,6 +764,18 @@ fn main() -> anyhow::Result<()> { anyhow::bail!("expected IPv4 *or* IPv6 config."); } } + + Command::AllowCidr { port, prefix } => { + hdl.allow_cidr(&port, prefix)?; + } + + Command::RemoveCidr { port, prefix } => { + if let RemoveCidrResp::NotFound = hdl.remove_cidr(&port, prefix)? { + anyhow::bail!( + "could not remove cidr {prefix} from gateway -- not found" + ); + } + } } Ok(()) diff --git a/bin/opteadm/src/lib.rs b/bin/opteadm/src/lib.rs index 0a3f1be2..1ccf907d 100644 --- a/bin/opteadm/src/lib.rs +++ b/bin/opteadm/src/lib.rs @@ -6,6 +6,7 @@ //! OPTE driver administration library +use opte::api::IpCidr; use opte::api::NoResp; use opte::api::OpteCmd; use opte::api::SetXdeUnderlayReq; @@ -14,6 +15,7 @@ use opte_ioctl::run_cmd_ioctl; use opte_ioctl::Error; use oxide_vpc::api::AddFwRuleReq; use oxide_vpc::api::AddRouterEntryReq; +use oxide_vpc::api::AllowCidrReq; use oxide_vpc::api::ClearVirt2BoundaryReq; use oxide_vpc::api::CreateXdeReq; use oxide_vpc::api::DelRouterEntryReq; @@ -23,6 +25,8 @@ use oxide_vpc::api::DhcpCfg; use oxide_vpc::api::FirewallRule; use oxide_vpc::api::ListPortsResp; use oxide_vpc::api::RemFwRuleReq; +use oxide_vpc::api::RemoveCidrReq; +use oxide_vpc::api::RemoveCidrResp; use oxide_vpc::api::SetExternalIpsReq; use oxide_vpc::api::SetFwRulesReq; use oxide_vpc::api::SetVirt2BoundaryReq; @@ -276,4 +280,30 @@ impl OpteAdm { let cmd = OpteCmd::SetExternalIps; run_cmd_ioctl(self.device.as_raw_fd(), cmd, Some(&req)) } + + pub fn allow_cidr( + &self, + port_name: &str, + cidr: IpCidr, + ) -> Result { + let cmd = OpteCmd::AllowCidr; + run_cmd_ioctl( + self.device.as_raw_fd(), + cmd, + Some(&AllowCidrReq { cidr, port_name: port_name.into() }), + ) + } + + pub fn remove_cidr( + &self, + port_name: &str, + cidr: IpCidr, + ) -> Result { + let cmd = OpteCmd::RemoveCidr; + run_cmd_ioctl( + self.device.as_raw_fd(), + cmd, + Some(&RemoveCidrReq { cidr, port_name: port_name.into() }), + ) + } } diff --git a/crates/opte-api/src/cmd.rs b/crates/opte-api/src/cmd.rs index 48ddb663..c5b4608a 100644 --- a/crates/opte-api/src/cmd.rs +++ b/crates/opte-api/src/cmd.rs @@ -41,6 +41,8 @@ pub enum OpteCmd { DeleteXde = 71, // delete an xde device SetXdeUnderlay = 72, // set xde underlay devices SetExternalIps = 80, // set xde external IPs for a port + AllowCidr = 90, // allow ip block through gateway tx/rx + RemoveCidr = 91, // deny ip block through gateway tx/rx } impl TryFrom for OpteCmd { @@ -69,6 +71,8 @@ impl TryFrom for OpteCmd { 71 => Ok(Self::DeleteXde), 72 => Ok(Self::SetXdeUnderlay), 80 => Ok(Self::SetExternalIps), + 90 => Ok(Self::AllowCidr), + 91 => Ok(Self::RemoveCidr), _ => Err(()), } } diff --git a/lib/opte-ioctl/src/lib.rs b/lib/opte-ioctl/src/lib.rs index e1dbc392..1d860a2d 100644 --- a/lib/opte-ioctl/src/lib.rs +++ b/lib/opte-ioctl/src/lib.rs @@ -13,13 +13,17 @@ use opte::api::SetXdeUnderlayReq; use opte::api::API_VERSION; use opte::api::XDE_IOC_OPTE_CMD; use oxide_vpc::api::AddRouterEntryReq; +use oxide_vpc::api::AllowCidrReq; use oxide_vpc::api::ClearVirt2BoundaryReq; use oxide_vpc::api::CreateXdeReq; use oxide_vpc::api::DelRouterEntryReq; use oxide_vpc::api::DelRouterEntryResp; use oxide_vpc::api::DeleteXdeReq; use oxide_vpc::api::DhcpCfg; +use oxide_vpc::api::IpCidr; use oxide_vpc::api::ListPortsResp; +use oxide_vpc::api::RemoveCidrReq; +use oxide_vpc::api::RemoveCidrResp; use oxide_vpc::api::SetExternalIpsReq; use oxide_vpc::api::SetFwRulesReq; use oxide_vpc::api::SetVirt2BoundaryReq; @@ -201,6 +205,32 @@ impl OpteHdl { let cmd = OpteCmd::SetExternalIps; run_cmd_ioctl(self.device.as_raw_fd(), cmd, Some(&req)) } + + pub fn allow_cidr( + &self, + port_name: &str, + cidr: IpCidr, + ) -> Result { + let cmd = OpteCmd::AllowCidr; + run_cmd_ioctl( + self.device.as_raw_fd(), + cmd, + Some(&AllowCidrReq { cidr, port_name: port_name.into() }), + ) + } + + pub fn remove_cidr( + &self, + port_name: &str, + cidr: IpCidr, + ) -> Result { + let cmd = OpteCmd::RemoveCidr; + run_cmd_ioctl( + self.device.as_raw_fd(), + cmd, + Some(&RemoveCidrReq { cidr, port_name: port_name.into() }), + ) + } } #[cfg(target_os = "illumos")] diff --git a/lib/opte/src/engine/port.rs b/lib/opte/src/engine/port.rs index 614a7fcf..9d228239 100644 --- a/lib/opte/src/engine/port.rs +++ b/lib/opte/src/engine/port.rs @@ -710,7 +710,7 @@ macro_rules! check_state { impl Port { /// Return the [`NetworkImpl`] associated with this port. - pub fn network(&self) -> &dyn NetworkImpl { + pub fn network(&self) -> &N { &self.net } diff --git a/lib/oxide-vpc/src/api.rs b/lib/oxide-vpc/src/api.rs index 0bef3b11..7c0eaa78 100644 --- a/lib/oxide-vpc/src/api.rs +++ b/lib/oxide-vpc/src/api.rs @@ -542,6 +542,8 @@ pub struct AddRouterEntryReq { pub class: RouterClass, } +/// Remove an entry to the router. Addresses may be either IPv4 or IPv6, though the +/// destination and target must match in protocol version. #[derive(Clone, Debug, Deserialize, Serialize)] pub struct DelRouterEntryReq { pub port_name: String, @@ -890,6 +892,30 @@ impl Display for Ports { } } +/// Add an entry to the gateway allowing a port to send and receive +/// traffic on a CIDR other than its private IP. +#[derive(Clone, Debug, Deserialize, Serialize)] +pub struct AllowCidrReq { + pub port_name: String, + pub cidr: IpCidr, +} + +/// Remove entries from the gateway allowing a port to send and receive +/// traffic on a specific CIDR other than its private IP. +#[derive(Clone, Debug, Deserialize, Serialize)] +pub struct RemoveCidrReq { + pub port_name: String, + pub cidr: IpCidr, +} + +#[derive(Clone, Debug, Deserialize, Serialize)] +pub enum RemoveCidrResp { + Ok, + NotFound, +} + +impl opte::api::cmd::CmdOk for RemoveCidrResp {} + #[cfg(test)] pub mod tests { use super::*; diff --git a/lib/oxide-vpc/src/engine/gateway/mod.rs b/lib/oxide-vpc/src/engine/gateway/mod.rs index f76478d4..9c8cb15e 100644 --- a/lib/oxide-vpc/src/engine/gateway/mod.rs +++ b/lib/oxide-vpc/src/engine/gateway/mod.rs @@ -42,6 +42,7 @@ use crate::api::DhcpCfg; use crate::api::MacAddr; +use crate::api::RemoveCidrResp; use crate::cfg::Ipv4Cfg; use crate::cfg::Ipv6Cfg; use crate::cfg::VpcCfg; @@ -54,6 +55,8 @@ use core::fmt; use core::fmt::Display; use core::marker::PhantomData; use opte::api::Direction; +use opte::api::IpCidr; +use opte::api::NoResp; use opte::api::OpteError; use opte::engine::ether::EtherMod; use opte::engine::headers::HeaderAction; @@ -63,6 +66,7 @@ use opte::engine::layer::LayerActions; use opte::engine::packet::InnerFlowId; use opte::engine::packet::PacketMeta; use opte::engine::port::meta::ActionMeta; +use opte::engine::port::Port; use opte::engine::port::PortBuilder; use opte::engine::port::Pos; use opte::engine::predicate::DataPredicate; @@ -72,6 +76,7 @@ use opte::engine::predicate::Ipv6AddrMatch; use opte::engine::predicate::Predicate; use opte::engine::rule::Action; use opte::engine::rule::AllowOrDeny; +use opte::engine::rule::Finalized; use opte::engine::rule::GenHtResult; use opte::engine::rule::HdrTransform; use opte::engine::rule::MetaAction; @@ -79,6 +84,8 @@ use opte::engine::rule::ModMetaResult; use opte::engine::rule::Rule; use opte::engine::rule::StaticAction; +use super::VpcNetwork; + pub mod arp; pub mod dhcp; pub mod dhcpv6; @@ -275,3 +282,87 @@ impl Display for VpcMeta { write!(f, "vpc-meta") } } + +fn make_holepunch_rules( + guest_mac: MacAddr, + gateway_mac: MacAddr, + dest: IpCidr, + vpc_mappings: Arc, +) -> (Rule, Rule) { + let vpc_meta = Arc::new(VpcMeta::new(vpc_mappings)); + + let (cidr_in_pred, cidr_out_pred) = match dest { + IpCidr::Ip4(v4) => ( + Predicate::InnerDstIp4(vec![Ipv4AddrMatch::Prefix(v4)]), + Predicate::InnerSrcIp4(vec![Ipv4AddrMatch::Prefix(v4)]), + ), + IpCidr::Ip6(v6) => ( + Predicate::InnerDstIp6(vec![Ipv6AddrMatch::Prefix(v6)]), + Predicate::InnerSrcIp6(vec![Ipv6AddrMatch::Prefix(v6)]), + ), + }; + + let mut cidr_out = Rule::new(1000, Action::Meta(vpc_meta)); + cidr_out.add_predicate(Predicate::InnerEtherSrc(vec![ + EtherAddrMatch::Exact(guest_mac), + ])); + cidr_out.add_predicate(cidr_out_pred); + + let mut cidr_in = Rule::new( + 1000, + Action::Static(Arc::new(RewriteSrcMac { gateway_mac })), + ); + cidr_in.add_predicate(cidr_in_pred); + cidr_in.add_predicate(Predicate::InnerEtherDst(vec![ + EtherAddrMatch::Exact(guest_mac), + ])); + + (cidr_in.finalize(), cidr_out.finalize()) +} + +/// Allows a guest to send and receive traffic on a CIDR block +/// other than their private IP. +pub fn allow_cidr( + port: &Port, + dest: IpCidr, + vpc_mappings: Arc, +) -> Result { + let (in_rule, out_rule) = make_holepunch_rules( + port.mac_addr(), + port.network().cfg.gateway_mac, + dest, + vpc_mappings, + ); + port.add_rule(NAME, Direction::In, in_rule)?; + port.add_rule(NAME, Direction::Out, out_rule)?; + Ok(NoResp::default()) +} + +/// Prevents a guest from sending/receiving traffic on a CIDR block +/// other than their private IP. +pub fn remove_cidr( + port: &Port, + dest: IpCidr, + vpc_mappings: Arc, +) -> Result { + let (in_rule, out_rule) = make_holepunch_rules( + port.mac_addr(), + port.network().cfg.gateway_mac, + dest, + vpc_mappings, + ); + let maybe_in_id = port.find_rule(NAME, Direction::In, &in_rule)?; + let maybe_out_id = port.find_rule(NAME, Direction::Out, &out_rule)?; + if let Some(id) = maybe_in_id { + port.remove_rule(NAME, Direction::In, id)?; + } + if let Some(id) = maybe_out_id { + port.remove_rule(NAME, Direction::Out, id)?; + } + + Ok(if maybe_in_id.is_none() || maybe_out_id.is_none() { + RemoveCidrResp::NotFound + } else { + RemoveCidrResp::Ok + }) +} diff --git a/lib/oxide-vpc/tests/integration_tests.rs b/lib/oxide-vpc/tests/integration_tests.rs index 4789f1fa..5645417b 100644 --- a/lib/oxide-vpc/tests/integration_tests.rs +++ b/lib/oxide-vpc/tests/integration_tests.rs @@ -4119,3 +4119,106 @@ fn intra_subnet_routes_with_custom() { let res = g1.port.process(Out, &mut pkt3, ActionMeta::new()); assert!(matches!(res, Ok(ProcessResult::Modified))); } + +#[test] +fn port_as_router_target() { + // RFD 21 allows VPC routers to direct traffic on a subnet + // towards a given node. There are a few pieces here to consider: + // * Packet send from a node must send traffic to the correct + // PhysNet -- underlay and macaddr of the receiving VM's port. + // * A node must be able to receive traffic on such a block. + let g1_cfg = g1_cfg(); + let g2_cfg = g2_cfg(); + let mut g1 = oxide_net_setup("g1_port", &g1_cfg, None, None); + g1.vpc_map.add(g2_cfg.ipv4().private_ip.into(), g2_cfg.phys_addr()); + g1.port.start(); + set!(g1, "port_state=running"); + let mut g2 = + oxide_net_setup("g2_port", &g2_cfg, Some(g1.vpc_map.clone()), None); + g2.port.start(); + set!(g2, "port_state=running"); + + // Node G2 is configured to carry and soft-route VPN traffic on + // 192.168.0.0/16. + let cidr = IpCidr::Ip4("192.168.0.0/16".parse().unwrap()); + let dst_ip: Ipv4Addr = "192.168.0.1".parse().unwrap(); + router::add_entry( + &g1.port, + cidr.clone(), + RouterTarget::Ip(g2_cfg.ipv4().private_ip.into()), + RouterClass::Custom, + ) + .unwrap(); + incr!(g1, ["epoch", "router.rules.out"]); + + // This also requires that we allow g2 to send/recv on this CIDR. + gateway::allow_cidr(&g2.port, cidr, g2.vpc_map.clone()).unwrap(); + incr!(g2, ["epoch, epoch, gateway.rules.out, gateway.rules.in"]); + + let data = b"1234\0"; + + // Send one ICMP packet to that range. + let mut pkt1 = gen_icmpv4_echo_req( + g1_cfg.guest_mac, + g1_cfg.gateway_mac, + g1_cfg.ipv4().private_ip, + dst_ip, + 7777, + 1, + data, + 1, + ); + + // That packet should be allowed: the target IP resolves to a valid + // V2P Mapping. + let res = g1.port.process(Out, &mut pkt1, ActionMeta::new()); + assert!(matches!(res, Ok(ProcessResult::Modified))); + incr!( + g1, + [ + "firewall.flows.in, firewall.flows.out", + "stats.port.out_modified, stats.port.out_uft_miss, uft.out", + ] + ); + + // Encap routes between sleds correctly, inner IPs are not modified, + // and L2 dst matches the guest's NIC. + let v6_encap_meta = pkt1.meta().outer.ip.as_ref().unwrap().ip6().unwrap(); + assert_eq!(v6_encap_meta.src, g1_cfg.phys_ip); + assert_eq!(v6_encap_meta.dst, g2_cfg.phys_ip); + assert_eq!(pkt1.meta().inner_ether().dst, g2_cfg.guest_mac); + assert_eq!(pkt1.meta().inner_ether().src, g1_cfg.guest_mac); + assert_eq!(pkt1.meta().inner_ip4().unwrap().src, g1_cfg.ipv4().private_ip); + assert_eq!(pkt1.meta().inner_ip4().unwrap().dst, dst_ip); + + // Now deliver the packet to node g2. + let res = g2.port.process(In, &mut pkt1, ActionMeta::new()); + incr!( + g2, + [ + "firewall.flows.in, firewall.flows.out", + "stats.port.in_modified, stats.port.in_uft_miss, uft.in", + ] + ); + assert!(matches!(res, Ok(ProcessResult::Modified))); + + // A reply from that address must be allowed out by g2, and accepted + // by g1. + let mut pkt2 = gen_icmpv4_echo_reply( + g2_cfg.guest_mac, + g2_cfg.gateway_mac, + dst_ip, + g1_cfg.ipv4().private_ip, + 7777, + 1, + data, + 1, + ); + + let res = g2.port.process(Out, &mut pkt2, ActionMeta::new()); + incr!(g2, ["stats.port.out_modified, stats.port.out_uft_miss, uft.out",]); + assert!(matches!(res, Ok(ProcessResult::Modified))); + + let res = g1.port.process(In, &mut pkt2, ActionMeta::new()); + assert!(matches!(res, Ok(ProcessResult::Modified))); +} diff --git a/xde/src/xde.rs b/xde/src/xde.rs index c866539d..beb72815 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -68,6 +68,7 @@ use opte::engine::port::meta::ActionMeta; use opte::engine::port::Port; use opte::engine::port::PortBuilder; use opte::engine::port::ProcessResult; +use opte::engine::NetworkImpl; use opte::ExecCtx; use oxide_vpc::api::AddFwRuleReq; use oxide_vpc::api::AddRouterEntryReq; @@ -81,6 +82,7 @@ use oxide_vpc::api::ListPortsResp; use oxide_vpc::api::PhysNet; use oxide_vpc::api::PortInfo; use oxide_vpc::api::RemFwRuleReq; +use oxide_vpc::api::RemoveCidrResp; use oxide_vpc::api::SetFwRulesReq; use oxide_vpc::api::SetVirt2BoundaryReq; use oxide_vpc::api::SetVirt2PhysReq; @@ -598,6 +600,16 @@ unsafe extern "C" fn xde_ioc_opte_cmd(karg: *mut c_void, mode: c_int) -> c_int { let resp = set_external_ips_hdlr(&mut env); hdlr_resp(&mut env, resp) } + + OpteCmd::AllowCidr => { + let resp = allow_cidr_hdlr(&mut env); + hdlr_resp(&mut env, resp) + } + + OpteCmd::RemoveCidr => { + let resp = remove_cidr_hdlr(&mut env); + hdlr_resp(&mut env, resp) + } } } @@ -2355,6 +2367,37 @@ fn set_external_ips_hdlr(env: &mut IoctlEnvelope) -> Result { Ok(NoResp::default()) } +#[no_mangle] +fn allow_cidr_hdlr(env: &mut IoctlEnvelope) -> Result { + let req: oxide_vpc::api::AllowCidrReq = env.copy_in_req()?; + let devs = unsafe { xde_devs.read() }; + let mut iter = devs.iter(); + let dev = match iter.find(|x| x.devname == req.port_name) { + Some(dev) => dev, + None => return Err(OpteError::PortNotFound(req.port_name)), + }; + let state = get_xde_state(); + + gateway::allow_cidr(&dev.port, req.cidr, state.vpc_map.clone())?; + Ok(NoResp::default()) +} + +#[no_mangle] +fn remove_cidr_hdlr( + env: &mut IoctlEnvelope, +) -> Result { + let req: oxide_vpc::api::RemoveCidrReq = env.copy_in_req()?; + let devs = unsafe { xde_devs.read() }; + let mut iter = devs.iter(); + let dev = match iter.find(|x| x.devname == req.port_name) { + Some(dev) => dev, + None => return Err(OpteError::PortNotFound(req.port_name)), + }; + let state = get_xde_state(); + + gateway::remove_cidr(&dev.port, req.cidr, state.vpc_map.clone()) +} + #[no_mangle] fn list_ports_hdlr() -> Result { let mut resp = ListPortsResp { ports: vec![] }; From 2680fda4acefbce29009f2518c7f7a0ea8374764 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Fri, 31 May 2024 20:49:06 +0100 Subject: [PATCH 08/11] Review feedback. Breaks out the transit IPs/CIDR allowlist functions as requested. --- lib/oxide-vpc/src/engine/gateway/mod.rs | 95 +----------------- lib/oxide-vpc/src/engine/gateway/transit.rs | 101 ++++++++++++++++++++ 2 files changed, 104 insertions(+), 92 deletions(-) create mode 100644 lib/oxide-vpc/src/engine/gateway/transit.rs diff --git a/lib/oxide-vpc/src/engine/gateway/mod.rs b/lib/oxide-vpc/src/engine/gateway/mod.rs index fe67661e..a3b04065 100644 --- a/lib/oxide-vpc/src/engine/gateway/mod.rs +++ b/lib/oxide-vpc/src/engine/gateway/mod.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -// Copyright 2023 Oxide Computer Company +// Copyright 2024 Oxide Computer Company //! The Oxide VPC Virtual Gateway. //! @@ -42,7 +42,6 @@ use crate::api::DhcpCfg; use crate::api::MacAddr; -use crate::api::RemoveCidrResp; use crate::cfg::Ipv4Cfg; use crate::cfg::Ipv6Cfg; use crate::cfg::VpcCfg; @@ -55,8 +54,6 @@ use core::fmt; use core::fmt::Display; use core::marker::PhantomData; use opte::api::Direction; -use opte::api::IpCidr; -use opte::api::NoResp; use opte::api::OpteError; use opte::engine::ether::EtherMod; use opte::engine::headers::HeaderAction; @@ -66,7 +63,6 @@ use opte::engine::layer::LayerActions; use opte::engine::packet::InnerFlowId; use opte::engine::packet::PacketMeta; use opte::engine::port::meta::ActionMeta; -use opte::engine::port::Port; use opte::engine::port::PortBuilder; use opte::engine::port::Pos; use opte::engine::predicate::DataPredicate; @@ -76,7 +72,6 @@ use opte::engine::predicate::Ipv6AddrMatch; use opte::engine::predicate::Predicate; use opte::engine::rule::Action; use opte::engine::rule::AllowOrDeny; -use opte::engine::rule::Finalized; use opte::engine::rule::GenHtResult; use opte::engine::rule::HdrTransform; use opte::engine::rule::MetaAction; @@ -84,13 +79,13 @@ use opte::engine::rule::ModMetaResult; use opte::engine::rule::Rule; use opte::engine::rule::StaticAction; -use super::VpcNetwork; - pub mod arp; pub mod dhcp; pub mod dhcpv6; pub mod icmp; pub mod icmpv6; +mod transit; +pub use transit::*; pub const NAME: &str = "gateway"; @@ -282,87 +277,3 @@ impl Display for VpcMeta { write!(f, "vpc-meta") } } - -fn make_holepunch_rules( - guest_mac: MacAddr, - gateway_mac: MacAddr, - dest: IpCidr, - vpc_mappings: Arc, -) -> (Rule, Rule) { - let vpc_meta = Arc::new(VpcMeta::new(vpc_mappings)); - - let (cidr_in_pred, cidr_out_pred) = match dest { - IpCidr::Ip4(v4) => ( - Predicate::InnerDstIp4(vec![Ipv4AddrMatch::Prefix(v4)]), - Predicate::InnerSrcIp4(vec![Ipv4AddrMatch::Prefix(v4)]), - ), - IpCidr::Ip6(v6) => ( - Predicate::InnerDstIp6(vec![Ipv6AddrMatch::Prefix(v6)]), - Predicate::InnerSrcIp6(vec![Ipv6AddrMatch::Prefix(v6)]), - ), - }; - - let mut cidr_out = Rule::new(1000, Action::Meta(vpc_meta)); - cidr_out.add_predicate(Predicate::InnerEtherSrc(vec![ - EtherAddrMatch::Exact(guest_mac), - ])); - cidr_out.add_predicate(cidr_out_pred); - - let mut cidr_in = Rule::new( - 1000, - Action::Static(Arc::new(RewriteSrcMac { gateway_mac })), - ); - cidr_in.add_predicate(cidr_in_pred); - cidr_in.add_predicate(Predicate::InnerEtherDst(vec![ - EtherAddrMatch::Exact(guest_mac), - ])); - - (cidr_in.finalize(), cidr_out.finalize()) -} - -/// Allows a guest to send and receive traffic on a CIDR block -/// other than their private IP. -pub fn allow_cidr( - port: &Port, - dest: IpCidr, - vpc_mappings: Arc, -) -> Result { - let (in_rule, out_rule) = make_holepunch_rules( - port.mac_addr(), - port.network().cfg.gateway_mac, - dest, - vpc_mappings, - ); - port.add_rule(NAME, Direction::In, in_rule)?; - port.add_rule(NAME, Direction::Out, out_rule)?; - Ok(NoResp::default()) -} - -/// Prevents a guest from sending/receiving traffic on a CIDR block -/// other than their private IP. -pub fn remove_cidr( - port: &Port, - dest: IpCidr, - vpc_mappings: Arc, -) -> Result { - let (in_rule, out_rule) = make_holepunch_rules( - port.mac_addr(), - port.network().cfg.gateway_mac, - dest, - vpc_mappings, - ); - let maybe_in_id = port.find_rule(NAME, Direction::In, &in_rule)?; - let maybe_out_id = port.find_rule(NAME, Direction::Out, &out_rule)?; - if let Some(id) = maybe_in_id { - port.remove_rule(NAME, Direction::In, id)?; - } - if let Some(id) = maybe_out_id { - port.remove_rule(NAME, Direction::Out, id)?; - } - - Ok(if maybe_in_id.is_none() || maybe_out_id.is_none() { - RemoveCidrResp::NotFound - } else { - RemoveCidrResp::Ok - }) -} diff --git a/lib/oxide-vpc/src/engine/gateway/transit.rs b/lib/oxide-vpc/src/engine/gateway/transit.rs new file mode 100644 index 00000000..21b9fccd --- /dev/null +++ b/lib/oxide-vpc/src/engine/gateway/transit.rs @@ -0,0 +1,101 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +// Copyright 2024 Oxide Computer Company + +//! Utility functions to allow a port to permit traffic on an +//! additional set of CIDR blocks, e.g. to enable transit for +//! VPC-wide VPN traffic. + +use super::*; +use crate::api::RemoveCidrResp; +use crate::engine::VpcNetwork; +use opte::api::IpCidr; +use opte::api::NoResp; +use opte::engine::port::Port; +use opte::engine::rule::Finalized; + +fn make_holepunch_rules( + guest_mac: MacAddr, + gateway_mac: MacAddr, + dest: IpCidr, + vpc_mappings: Arc, +) -> (Rule, Rule) { + let vpc_meta = Arc::new(VpcMeta::new(vpc_mappings)); + + let (cidr_in_pred, cidr_out_pred) = match dest { + IpCidr::Ip4(v4) => ( + Predicate::InnerDstIp4(vec![Ipv4AddrMatch::Prefix(v4)]), + Predicate::InnerSrcIp4(vec![Ipv4AddrMatch::Prefix(v4)]), + ), + IpCidr::Ip6(v6) => ( + Predicate::InnerDstIp6(vec![Ipv6AddrMatch::Prefix(v6)]), + Predicate::InnerSrcIp6(vec![Ipv6AddrMatch::Prefix(v6)]), + ), + }; + + let mut cidr_out = Rule::new(1000, Action::Meta(vpc_meta)); + cidr_out.add_predicate(Predicate::InnerEtherSrc(vec![ + EtherAddrMatch::Exact(guest_mac), + ])); + cidr_out.add_predicate(cidr_out_pred); + + let mut cidr_in = Rule::new( + 1000, + Action::Static(Arc::new(RewriteSrcMac { gateway_mac })), + ); + cidr_in.add_predicate(cidr_in_pred); + cidr_in.add_predicate(Predicate::InnerEtherDst(vec![ + EtherAddrMatch::Exact(guest_mac), + ])); + + (cidr_in.finalize(), cidr_out.finalize()) +} + +/// Allows a guest to send and receive traffic on a CIDR block +/// other than their private IP. +pub fn allow_cidr( + port: &Port, + dest: IpCidr, + vpc_mappings: Arc, +) -> Result { + let (in_rule, out_rule) = make_holepunch_rules( + port.mac_addr(), + port.network().cfg.gateway_mac, + dest, + vpc_mappings, + ); + port.add_rule(NAME, Direction::In, in_rule)?; + port.add_rule(NAME, Direction::Out, out_rule)?; + Ok(NoResp::default()) +} + +/// Prevents a guest from sending/receiving traffic on a CIDR block +/// other than their private IP. +pub fn remove_cidr( + port: &Port, + dest: IpCidr, + vpc_mappings: Arc, +) -> Result { + let (in_rule, out_rule) = make_holepunch_rules( + port.mac_addr(), + port.network().cfg.gateway_mac, + dest, + vpc_mappings, + ); + let maybe_in_id = port.find_rule(NAME, Direction::In, &in_rule)?; + let maybe_out_id = port.find_rule(NAME, Direction::Out, &out_rule)?; + if let Some(id) = maybe_in_id { + port.remove_rule(NAME, Direction::In, id)?; + } + if let Some(id) = maybe_out_id { + port.remove_rule(NAME, Direction::Out, id)?; + } + + Ok(if maybe_in_id.is_none() || maybe_out_id.is_none() { + RemoveCidrResp::NotFound + } else { + RemoveCidrResp::Ok + }) +} From 4895f42fd5527e70fad9726a5f10c1e0cd051c6e Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 12 Jun 2024 15:57:32 +0100 Subject: [PATCH 09/11] Review feedback. --- bin/opteadm/src/bin/opteadm.rs | 22 +++++-- bin/opteadm/src/lib.rs | 7 ++- crates/opte-api/src/ip.rs | 7 +++ lib/opte-ioctl/src/lib.rs | 7 ++- lib/oxide-vpc/src/api.rs | 6 +- lib/oxide-vpc/src/engine/gateway/transit.rs | 69 ++++++++++++--------- lib/oxide-vpc/src/engine/router.rs | 11 +--- lib/oxide-vpc/tests/integration_tests.rs | 5 +- xde/src/xde.rs | 4 +- 9 files changed, 85 insertions(+), 53 deletions(-) diff --git a/bin/opteadm/src/bin/opteadm.rs b/bin/opteadm/src/bin/opteadm.rs index b882f145..2006d53b 100644 --- a/bin/opteadm/src/bin/opteadm.rs +++ b/bin/opteadm/src/bin/opteadm.rs @@ -244,7 +244,7 @@ enum Command { external_net: ExternalNetConfig, }, - /// Allows a guest to send and receive traffic on a given CIDR block. + /// Allows a guest to send or receive traffic on a given CIDR block. AllowCidr { /// The OPTE port to configure #[arg(short)] @@ -252,6 +252,11 @@ enum Command { /// The IP block to allow through the gateway. prefix: IpCidr, + + /// Control whether traffic on the CIDR should be allowed for + /// inbound or outbound traffic. + #[arg(long = "dir")] + direction: Direction, }, /// Prevents a guest from sending/receiving traffic on a given CIDR block. @@ -262,6 +267,11 @@ enum Command { /// The IP block to deny at the gateway. prefix: IpCidr, + + /// Control whether traffic on the CIDR should be allowed for + /// inbound or outbound traffic. + #[arg(long = "dir")] + direction: Direction, }, } @@ -788,12 +798,14 @@ fn main() -> anyhow::Result<()> { } } - Command::AllowCidr { port, prefix } => { - hdl.allow_cidr(&port, prefix)?; + Command::AllowCidr { port, prefix, direction } => { + hdl.allow_cidr(&port, prefix, direction)?; } - Command::RemoveCidr { port, prefix } => { - if let RemoveCidrResp::NotFound = hdl.remove_cidr(&port, prefix)? { + Command::RemoveCidr { port, prefix, direction } => { + if let RemoveCidrResp::NotFound = + hdl.remove_cidr(&port, prefix, direction)? + { anyhow::bail!( "could not remove cidr {prefix} from gateway -- not found" ); diff --git a/bin/opteadm/src/lib.rs b/bin/opteadm/src/lib.rs index f992f0a4..3339027f 100644 --- a/bin/opteadm/src/lib.rs +++ b/bin/opteadm/src/lib.rs @@ -7,6 +7,7 @@ //! OPTE driver administration library use opte::api::ClearXdeUnderlayReq; +use opte::api::Direction; use opte::api::IpCidr; use opte::api::NoResp; use opte::api::OpteCmd; @@ -302,12 +303,13 @@ impl OpteAdm { &self, port_name: &str, cidr: IpCidr, + dir: Direction, ) -> Result { let cmd = OpteCmd::AllowCidr; run_cmd_ioctl( self.device.as_raw_fd(), cmd, - Some(&AllowCidrReq { cidr, port_name: port_name.into() }), + Some(&AllowCidrReq { cidr, port_name: port_name.into(), dir }), ) } @@ -315,12 +317,13 @@ impl OpteAdm { &self, port_name: &str, cidr: IpCidr, + dir: Direction, ) -> Result { let cmd = OpteCmd::RemoveCidr; run_cmd_ioctl( self.device.as_raw_fd(), cmd, - Some(&RemoveCidrReq { cidr, port_name: port_name.into() }), + Some(&RemoveCidrReq { cidr, port_name: port_name.into(), dir }), ) } } diff --git a/crates/opte-api/src/ip.rs b/crates/opte-api/src/ip.rs index 9bd5deb0..bfdcf689 100644 --- a/crates/opte-api/src/ip.rs +++ b/crates/opte-api/src/ip.rs @@ -844,6 +844,13 @@ impl IpCidr { Self::Ip6(ip6) => ip6.prefix_len(), } } + + pub fn max_prefix_len(&self) -> u8 { + match self { + Self::Ip4(_) => Ipv4PrefixLen::NETMASK_ALL.val(), + Self::Ip6(_) => Ipv6PrefixLen::NETMASK_ALL.val(), + } + } } impl fmt::Display for IpCidr { diff --git a/lib/opte-ioctl/src/lib.rs b/lib/opte-ioctl/src/lib.rs index 22c02ced..085177a0 100644 --- a/lib/opte-ioctl/src/lib.rs +++ b/lib/opte-ioctl/src/lib.rs @@ -6,6 +6,7 @@ use opte::api::ClearXdeUnderlayReq; use opte::api::CmdOk; +use opte::api::Direction; use opte::api::NoResp; use opte::api::OpteCmd; use opte::api::OpteCmdIoctl; @@ -234,12 +235,13 @@ impl OpteHdl { &self, port_name: &str, cidr: IpCidr, + dir: Direction, ) -> Result { let cmd = OpteCmd::AllowCidr; run_cmd_ioctl( self.device.as_raw_fd(), cmd, - Some(&AllowCidrReq { cidr, port_name: port_name.into() }), + Some(&AllowCidrReq { cidr, port_name: port_name.into(), dir }), ) } @@ -247,12 +249,13 @@ impl OpteHdl { &self, port_name: &str, cidr: IpCidr, + dir: Direction, ) -> Result { let cmd = OpteCmd::RemoveCidr; run_cmd_ioctl( self.device.as_raw_fd(), cmd, - Some(&RemoveCidrReq { cidr, port_name: port_name.into() }), + Some(&RemoveCidrReq { cidr, port_name: port_name.into(), dir }), ) } } diff --git a/lib/oxide-vpc/src/api.rs b/lib/oxide-vpc/src/api.rs index 9827aa83..4b81c5be 100644 --- a/lib/oxide-vpc/src/api.rs +++ b/lib/oxide-vpc/src/api.rs @@ -939,20 +939,22 @@ impl Display for Ports { } } -/// Add an entry to the gateway allowing a port to send and receive +/// Add an entry to the gateway allowing a port to send or receive /// traffic on a CIDR other than its private IP. #[derive(Clone, Debug, Deserialize, Serialize)] pub struct AllowCidrReq { pub port_name: String, pub cidr: IpCidr, + pub dir: Direction, } -/// Remove entries from the gateway allowing a port to send and receive +/// Remove entries from the gateway allowing a port to send or receive /// traffic on a specific CIDR other than its private IP. #[derive(Clone, Debug, Deserialize, Serialize)] pub struct RemoveCidrReq { pub port_name: String, pub cidr: IpCidr, + pub dir: Direction, } #[derive(Clone, Debug, Deserialize, Serialize)] diff --git a/lib/oxide-vpc/src/engine/gateway/transit.rs b/lib/oxide-vpc/src/engine/gateway/transit.rs index 21b9fccd..92eae86d 100644 --- a/lib/oxide-vpc/src/engine/gateway/transit.rs +++ b/lib/oxide-vpc/src/engine/gateway/transit.rs @@ -16,14 +16,13 @@ use opte::api::NoResp; use opte::engine::port::Port; use opte::engine::rule::Finalized; -fn make_holepunch_rules( +fn make_holepunch_rule( guest_mac: MacAddr, gateway_mac: MacAddr, dest: IpCidr, + dir: Direction, vpc_mappings: Arc, -) -> (Rule, Rule) { - let vpc_meta = Arc::new(VpcMeta::new(vpc_mappings)); - +) -> Rule { let (cidr_in_pred, cidr_out_pred) = match dest { IpCidr::Ip4(v4) => ( Predicate::InnerDstIp4(vec![Ipv4AddrMatch::Prefix(v4)]), @@ -35,39 +34,48 @@ fn make_holepunch_rules( ), }; - let mut cidr_out = Rule::new(1000, Action::Meta(vpc_meta)); - cidr_out.add_predicate(Predicate::InnerEtherSrc(vec![ - EtherAddrMatch::Exact(guest_mac), - ])); - cidr_out.add_predicate(cidr_out_pred); + match dir { + Direction::In => { + let mut cidr_in = Rule::new( + 1000, + Action::Static(Arc::new(RewriteSrcMac { gateway_mac })), + ); + cidr_in.add_predicate(cidr_in_pred); + cidr_in.add_predicate(Predicate::InnerEtherDst(vec![ + EtherAddrMatch::Exact(guest_mac), + ])); - let mut cidr_in = Rule::new( - 1000, - Action::Static(Arc::new(RewriteSrcMac { gateway_mac })), - ); - cidr_in.add_predicate(cidr_in_pred); - cidr_in.add_predicate(Predicate::InnerEtherDst(vec![ - EtherAddrMatch::Exact(guest_mac), - ])); + cidr_in.finalize() + } + Direction::Out => { + let vpc_meta = Arc::new(VpcMeta::new(vpc_mappings)); + let mut cidr_out = Rule::new(1000, Action::Meta(vpc_meta)); + cidr_out.add_predicate(Predicate::InnerEtherSrc(vec![ + EtherAddrMatch::Exact(guest_mac), + ])); + cidr_out.add_predicate(cidr_out_pred); - (cidr_in.finalize(), cidr_out.finalize()) + cidr_out.finalize() + } + } } -/// Allows a guest to send and receive traffic on a CIDR block +/// Allows a guest to send or receive traffic on a CIDR block /// other than their private IP. pub fn allow_cidr( port: &Port, dest: IpCidr, + dir: Direction, vpc_mappings: Arc, ) -> Result { - let (in_rule, out_rule) = make_holepunch_rules( + let rule = make_holepunch_rule( port.mac_addr(), port.network().cfg.gateway_mac, dest, + dir, vpc_mappings, ); - port.add_rule(NAME, Direction::In, in_rule)?; - port.add_rule(NAME, Direction::Out, out_rule)?; + port.add_rule(NAME, dir, rule)?; Ok(NoResp::default()) } @@ -76,24 +84,23 @@ pub fn allow_cidr( pub fn remove_cidr( port: &Port, dest: IpCidr, + dir: Direction, vpc_mappings: Arc, ) -> Result { - let (in_rule, out_rule) = make_holepunch_rules( + let rule = make_holepunch_rule( port.mac_addr(), port.network().cfg.gateway_mac, dest, + dir, vpc_mappings, ); - let maybe_in_id = port.find_rule(NAME, Direction::In, &in_rule)?; - let maybe_out_id = port.find_rule(NAME, Direction::Out, &out_rule)?; - if let Some(id) = maybe_in_id { - port.remove_rule(NAME, Direction::In, id)?; - } - if let Some(id) = maybe_out_id { - port.remove_rule(NAME, Direction::Out, id)?; + + let maybe_id = port.find_rule(NAME, dir, &rule)?; + if let Some(id) = maybe_id { + port.remove_rule(NAME, dir, id)?; } - Ok(if maybe_in_id.is_none() || maybe_out_id.is_none() { + Ok(if maybe_id.is_none() { RemoveCidrResp::NotFound } else { RemoveCidrResp::Ok diff --git a/lib/oxide-vpc/src/engine/router.rs b/lib/oxide-vpc/src/engine/router.rs index 0aa455d7..a3b8b78b 100644 --- a/lib/oxide-vpc/src/engine/router.rs +++ b/lib/oxide-vpc/src/engine/router.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -// Copyright 2023 Oxide Computer Company +// Copyright 2024 Oxide Computer Company //! The Oxide Network VPC Router. //! @@ -164,13 +164,8 @@ impl fmt::Display for RouterTargetInternal { // |0 |1 |267 = ((128 - 0) << 1 | 1) + 10 | // ``` fn compute_rule_priority(cidr: &IpCidr, class: RouterClass) -> u16 { - use opte::api::ip::IpCidr::*; - use opte::api::ip::Ipv4PrefixLen; - use opte::api::ip::Ipv6PrefixLen; - let (max_prefix_len, prefix_len) = match cidr { - Ip4(ipv4) => (Ipv4PrefixLen::NETMASK_ALL.val(), ipv4.prefix_len()), - Ip6(ipv6) => (Ipv6PrefixLen::NETMASK_ALL.val(), ipv6.prefix_len()), - }; + let max_prefix_len = cidr.max_prefix_len(); + let prefix_len = cidr.prefix_len(); let class_prio = match class { RouterClass::Custom => 0, RouterClass::System => 1, diff --git a/lib/oxide-vpc/tests/integration_tests.rs b/lib/oxide-vpc/tests/integration_tests.rs index 5645417b..e22f3d94 100644 --- a/lib/oxide-vpc/tests/integration_tests.rs +++ b/lib/oxide-vpc/tests/integration_tests.rs @@ -4152,7 +4152,10 @@ fn port_as_router_target() { incr!(g1, ["epoch", "router.rules.out"]); // This also requires that we allow g2 to send/recv on this CIDR. - gateway::allow_cidr(&g2.port, cidr, g2.vpc_map.clone()).unwrap(); + gateway::allow_cidr(&g2.port, cidr, Direction::In, g2.vpc_map.clone()) + .unwrap(); + gateway::allow_cidr(&g2.port, cidr, Direction::Out, g2.vpc_map.clone()) + .unwrap(); incr!(g2, ["epoch, epoch, gateway.rules.out, gateway.rules.in"]); let data = b"1234\0"; diff --git a/xde/src/xde.rs b/xde/src/xde.rs index ee3a7239..a04baade 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -2494,7 +2494,7 @@ fn allow_cidr_hdlr(env: &mut IoctlEnvelope) -> Result { }; let state = get_xde_state(); - gateway::allow_cidr(&dev.port, req.cidr, state.vpc_map.clone())?; + gateway::allow_cidr(&dev.port, req.cidr, req.dir, state.vpc_map.clone())?; Ok(NoResp::default()) } @@ -2511,7 +2511,7 @@ fn remove_cidr_hdlr( }; let state = get_xde_state(); - gateway::remove_cidr(&dev.port, req.cidr, state.vpc_map.clone()) + gateway::remove_cidr(&dev.port, req.cidr, req.dir, state.vpc_map.clone()) } #[no_mangle] From 527d6ff35553bf511ddc8f835244f073ea989a5a Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Wed, 12 Jun 2024 17:04:26 +0100 Subject: [PATCH 10/11] Review feedback. --- lib/oxide-vpc/src/api.rs | 2 +- lib/oxide-vpc/src/engine/gateway/transit.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/oxide-vpc/src/api.rs b/lib/oxide-vpc/src/api.rs index 4b81c5be..011908bf 100644 --- a/lib/oxide-vpc/src/api.rs +++ b/lib/oxide-vpc/src/api.rs @@ -959,7 +959,7 @@ pub struct RemoveCidrReq { #[derive(Clone, Debug, Deserialize, Serialize)] pub enum RemoveCidrResp { - Ok, + Ok(IpCidr), NotFound, } diff --git a/lib/oxide-vpc/src/engine/gateway/transit.rs b/lib/oxide-vpc/src/engine/gateway/transit.rs index 92eae86d..9d58d3a1 100644 --- a/lib/oxide-vpc/src/engine/gateway/transit.rs +++ b/lib/oxide-vpc/src/engine/gateway/transit.rs @@ -103,6 +103,6 @@ pub fn remove_cidr( Ok(if maybe_id.is_none() { RemoveCidrResp::NotFound } else { - RemoveCidrResp::Ok + RemoveCidrResp::Ok(dest) }) } From f4366507c4ff30f67dd9f4e5ec5ec3992f688bf0 Mon Sep 17 00:00:00 2001 From: Kyle Simpson Date: Thu, 13 Jun 2024 11:21:34 +0100 Subject: [PATCH 11/11] Simultaneous in/out cidr manip on opteadm With rollback behaviour! --- bin/opteadm/src/bin/opteadm.rs | 53 ++++++++++++++++++++++++++++------ 1 file changed, 44 insertions(+), 9 deletions(-) diff --git a/bin/opteadm/src/bin/opteadm.rs b/bin/opteadm/src/bin/opteadm.rs index 2006d53b..ff5f2fd2 100644 --- a/bin/opteadm/src/bin/opteadm.rs +++ b/bin/opteadm/src/bin/opteadm.rs @@ -4,6 +4,7 @@ // Copyright 2024 Oxide Computer Company +use anyhow::Context; use clap::Args; use clap::Parser; use opte::api::Direction; @@ -256,7 +257,7 @@ enum Command { /// Control whether traffic on the CIDR should be allowed for /// inbound or outbound traffic. #[arg(long = "dir")] - direction: Direction, + direction: Option, }, /// Prevents a guest from sending/receiving traffic on a given CIDR block. @@ -271,7 +272,7 @@ enum Command { /// Control whether traffic on the CIDR should be allowed for /// inbound or outbound traffic. #[arg(long = "dir")] - direction: Direction, + direction: Option, }, } @@ -799,16 +800,50 @@ fn main() -> anyhow::Result<()> { } Command::AllowCidr { port, prefix, direction } => { - hdl.allow_cidr(&port, prefix, direction)?; + if let Some(dir) = direction { + hdl.allow_cidr(&port, prefix, dir)?; + } else { + hdl.allow_cidr(&port, prefix, Direction::In) + .context("failed to allow on inbound direction")?; + hdl.allow_cidr(&port, prefix, Direction::Out).inspect_err( + |e| { + hdl.remove_cidr(&port, prefix, Direction::In).expect( + &format!( + "FATAL: failed to rollback in-direction allow \ + of {prefix} after {e}" + ), + ); + }, + )?; + } } Command::RemoveCidr { port, prefix, direction } => { - if let RemoveCidrResp::NotFound = - hdl.remove_cidr(&port, prefix, direction)? - { - anyhow::bail!( - "could not remove cidr {prefix} from gateway -- not found" - ); + let remove_cidr = |dir: Direction| -> anyhow::Result<()> { + if let RemoveCidrResp::NotFound = + hdl.remove_cidr(&port, prefix, dir)? + { + anyhow::bail!( + "could not remove cidr {prefix} from gateway ({dir}) -- not found" + ); + } + + Ok(()) + }; + + if let Some(dir) = direction { + remove_cidr(dir)?; + } else { + remove_cidr(Direction::In) + .context("failed to deny on inbound direction")?; + remove_cidr(Direction::Out).inspect_err(|e| { + hdl.allow_cidr(&port, prefix, Direction::In).expect( + &format!( + "FATAL: failed to rollback in-direction remove \ + of {prefix} after {e}" + ), + ); + })?; } } }