From 05ea62e83b063f45375e272ea110d5cfdeeb3b04 Mon Sep 17 00:00:00 2001 From: Ryan Zezeski Date: Mon, 1 May 2023 19:27:45 -0600 Subject: [PATCH] Remove the "external IP hack" (#236) (#311) * Remove the "external IP hack" * Remove xde_ext_ip_hack from xde.conf * Bump API version for EXT-IP-HACK free world. --------- Co-authored-by: Luqman Aden --- opte-api/src/lib.rs | 2 +- opte/src/engine/nat.rs | 52 +------ opte/src/engine/packet.rs | 14 -- opte/src/engine/snat.rs | 84 ++--------- opteadm/src/main.rs | 9 -- oxide-vpc/src/api.rs | 4 - oxide-vpc/src/engine/gateway/arp.rs | 30 +--- oxide-vpc/src/engine/gateway/mod.rs | 2 +- oxide-vpc/src/engine/mod.rs | 123 +++------------- oxide-vpc/src/engine/nat.rs | 17 +-- oxide-vpc/tests/common/mod.rs | 4 - oxide-vpc/tests/integration_tests.rs | 2 - xde/src/xde.rs | 213 ++++----------------------- xde/xde.conf | 8 - 14 files changed, 79 insertions(+), 485 deletions(-) diff --git a/opte-api/src/lib.rs b/opte-api/src/lib.rs index 9e86b725..a43c1813 100644 --- a/opte-api/src/lib.rs +++ b/opte-api/src/lib.rs @@ -58,7 +58,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 = 21; +pub const API_VERSION: u64 = 22; #[derive(Clone, Copy, Debug, Deserialize, Eq, PartialEq, Serialize)] pub enum Direction { diff --git a/opte/src/engine/nat.rs b/opte/src/engine/nat.rs index b70137e5..194a145b 100644 --- a/opte/src/engine/nat.rs +++ b/opte/src/engine/nat.rs @@ -6,7 +6,6 @@ //! 1:1 NAT. -use super::ether::EtherMod; use super::headers::HeaderAction; use super::headers::IpMod; use super::ip4::Ipv4Mod; @@ -27,7 +26,6 @@ use core::fmt; use core::marker::PhantomData; use opte_api::Direction; use opte_api::IpAddr; -use opte_api::MacAddr; cfg_if! { if #[cfg(all(not(feature = "std"), not(test)))] { @@ -46,22 +44,12 @@ cfg_if! { pub struct Nat { priv_ip: IpAddr, external_ip: IpAddr, - // XXX-EXT-IP Remove - phys_gw_mac: Option, } impl Nat { /// Create a new NAT mapping from a private to public IP address. - pub fn new( - priv_ip: T, - external_ip: T, - phys_gw_mac: Option, - ) -> Self { - Self { - priv_ip: priv_ip.into(), - external_ip: external_ip.into(), - phys_gw_mac, - } + pub fn new(priv_ip: T, external_ip: T) -> Self { + Self { priv_ip: priv_ip.into(), external_ip: external_ip.into() } } } @@ -78,15 +66,8 @@ impl StatefulAction for Nat { _pkt: &Packet, _meta: &mut ActionMeta, ) -> rule::GenDescResult { - let desc = NatDesc { - priv_ip: self.priv_ip, - external_ip: self.external_ip, - // XXX-EXT-IP This is assuming ext_ip_hack. All packets - // outbound for IG will have their dest mac rewritten to - // go to physical gateway, which will then properly route - // the destination IP. - phys_gw_mac: self.phys_gw_mac.clone(), - }; + let desc = + NatDesc { priv_ip: self.priv_ip, external_ip: self.external_ip }; Ok(AllowOrDeny::Allow(Arc::new(desc))) } @@ -103,8 +84,6 @@ impl StatefulAction for Nat { pub struct NatDesc { priv_ip: IpAddr, external_ip: IpAddr, - // XXX-EXT-IP - phys_gw_mac: Option, } pub const NAT_NAME: &'static str = "NAT"; @@ -123,26 +102,12 @@ impl ActionDesc for NatDesc { ..Default::default() }), }; - let mut ht = HdrTransform { + + HdrTransform { name: NAT_NAME.to_string(), inner_ip: HeaderAction::Modify(ip, PhantomData), ..Default::default() - }; - - // XXX-EXT-IP hack to rewrite destination MAC adress - // from virtual gateway addr to the real gateway addr - // on the same subnet as the external IP. - if self.phys_gw_mac.is_some() { - ht.inner_ether = HeaderAction::Modify( - EtherMod { - dst: Some(self.phys_gw_mac.unwrap()), - ..Default::default() - }, - core::marker::PhantomData, - ) } - - ht } Direction::In => { @@ -196,8 +161,7 @@ mod test { let pub_ip = "52.10.128.69".parse().unwrap(); let outside_ip = "76.76.21.21".parse().unwrap(); let outside_port = 80; - let gw_mac = MacAddr::from([0x78, 0x23, 0xae, 0x5d, 0x4f, 0x0d]); - let nat = Nat::new(priv_ip, pub_ip, Some(gw_mac)); + let nat = Nat::new(priv_ip, pub_ip); let mut ameta = ActionMeta::new(); // ================================================================ @@ -245,7 +209,7 @@ mod test { let ether_meta = pmo.inner.ether; assert_eq!(ether_meta.src, priv_mac); - assert_eq!(ether_meta.dst, gw_mac); + assert_eq!(ether_meta.dst, dest_mac); let ip4_meta = match pmo.inner.ip.as_ref().unwrap() { IpMeta::Ip4(v) => v, diff --git a/opte/src/engine/packet.rs b/opte/src/engine/packet.rs index c635c691..f86e747b 100644 --- a/opte/src/engine/packet.rs +++ b/opte/src/engine/packet.rs @@ -64,7 +64,6 @@ use super::Direction; use illumos_sys_hdrs::dblk_t; use illumos_sys_hdrs::mblk_t; use illumos_sys_hdrs::uintptr_t; -use opte_api::MacAddr; cfg_if! { if #[cfg(all(not(feature = "std"), not(test)))] { @@ -1009,19 +1008,6 @@ impl From for BodyTransformError { } impl Packet { - /// XXX-EXT-IP This is here purely for the use by the external IP - /// hack. - pub fn write_dst_mac(&mut self, addr: MacAddr) { - self.state.meta.inner.ether.dst = addr.into(); - let off = self.state.hdr_offsets.inner.ether.seg_pos; - let mut rdr = PacketReaderMut::new(&mut self.segs[0..1]); - // Unwrap: Assuming we didn't mess up calculating the offsets, - // we know we can seek forward by this amount. - rdr.seek(off).unwrap(); - let mut ether = EtherHdr::parse(&mut rdr).unwrap(); - ether.set_dst(addr); - } - pub fn body_csum(&self) -> Option { self.state.body_csum } diff --git a/opte/src/engine/snat.rs b/opte/src/engine/snat.rs index eef05411..304dd8d6 100644 --- a/opte/src/engine/snat.rs +++ b/opte/src/engine/snat.rs @@ -6,7 +6,6 @@ //! Types for working with IP Source NAT, both IPv4 and IPv6. -use super::ether::EtherMod; use super::headers::HeaderAction; use super::headers::IpMod; use super::headers::UlpGenericModify; @@ -44,7 +43,6 @@ use opte_api::Direction; use opte_api::IpAddr; use opte_api::Ipv4Addr; use opte_api::Ipv6Addr; -use opte_api::MacAddr; use opte_api::Protocol; use smoltcp::wire::Icmpv4Message; use smoltcp::wire::Icmpv4Packet; @@ -188,17 +186,11 @@ impl FiniteResource for NatPool { pub struct SNat { priv_ip: Ipv4Addr, ip_pool: Arc>, - // XXX-EXT-IP - phys_gw_mac: Option, } impl SNat { - pub fn new( - addr: Ipv4Addr, - ip_pool: Arc>, - phys_gw_mac: Option, - ) -> Self { - SNat { priv_ip: addr, ip_pool, phys_gw_mac } + pub fn new(addr: Ipv4Addr, ip_pool: Arc>) -> Self { + SNat { priv_ip: addr, ip_pool } } // A helper method for generating an SNAT + ICMP action descriptor. @@ -233,7 +225,6 @@ impl SNat { // Panic: We know this is safe because we make it here // only if this ICMP message is an Echo Request. echo_ident: icmp.echo_ident(), - phys_gw_mac: self.phys_gw_mac, }; Ok(AllowOrDeny::Allow(Arc::new(desc))) @@ -268,7 +259,6 @@ impl StatefulAction for SNat { pool: pool.clone(), priv_ip: self.priv_ip.into(), priv_port: priv_port, - phys_gw_mac: self.phys_gw_mac, nat, }; @@ -302,17 +292,11 @@ impl StatefulAction for SNat { pub struct SNat6 { priv_ip: Ipv6Addr, ip_pool: Arc>, - // XXX-EXT-IP - phys_gw_mac: Option, } impl SNat6 { - pub fn new( - addr: Ipv6Addr, - ip_pool: Arc>, - phys_gw_mac: Option, - ) -> Self { - SNat6 { priv_ip: addr, ip_pool, phys_gw_mac } + pub fn new(addr: Ipv6Addr, ip_pool: Arc>) -> Self { + SNat6 { priv_ip: addr, ip_pool } } } @@ -331,7 +315,6 @@ impl StatefulAction for SNat6 { pool: pool.clone(), priv_ip: self.priv_ip.into(), priv_port: priv_port, - phys_gw_mac: self.phys_gw_mac, nat, }; @@ -373,8 +356,6 @@ pub struct SNatDesc { nat: NatPoolEntry, priv_ip: T, priv_port: u16, - // XXX-EXT-IP - phys_gw_mac: Option, } pub const SNAT_NAME: &'static str = "SNAT"; @@ -389,7 +370,7 @@ impl ActionDesc for SNatDesc { ..Default::default() }); - let mut ht = HdrTransform { + HdrTransform { name: SNAT_NAME.to_string(), inner_ip: HeaderAction::Modify(ip, PhantomData), inner_ulp: UlpHeaderAction::Modify(UlpMetaModify { @@ -400,22 +381,7 @@ impl ActionDesc for SNatDesc { ..Default::default() }), ..Default::default() - }; - - // XXX-EXT-IP hack to rewrite destination MAC adress - // from virtual gateway addr to the real gateway addr - // on the same subnet as the external IP. - if self.phys_gw_mac.is_some() { - ht.inner_ether = HeaderAction::Modify( - EtherMod { - dst: Some(self.phys_gw_mac.unwrap()), - ..Default::default() - }, - PhantomData, - ); } - - ht } // Inbound traffic needs its destination IP and @@ -456,7 +422,8 @@ impl ActionDesc for SNatDesc { src: Some(self.nat.ip), ..Default::default() }); - let mut ht = HdrTransform { + + HdrTransform { name: SNAT_NAME.to_string(), inner_ip: HeaderAction::Modify(ip, PhantomData), inner_ulp: UlpHeaderAction::Modify(UlpMetaModify { @@ -467,22 +434,7 @@ impl ActionDesc for SNatDesc { ..Default::default() }), ..Default::default() - }; - - // XXX-EXT-IP hack to rewrite destination MAC adress - // from virtual gateway addr to the real gateway addr - // on the same subnet as the external IP. - if self.phys_gw_mac.is_some() { - ht.inner_ether = HeaderAction::Modify( - EtherMod { - dst: Some(self.phys_gw_mac.unwrap()), - ..Default::default() - }, - PhantomData, - ); } - - ht } // Inbound traffic needs its destination IP and @@ -526,8 +478,6 @@ pub struct SNatIcmpEchoDesc { nat: NatPoolEntry, priv_ip: Ipv4Addr, echo_ident: u16, - // XXX-EXT-IP - phys_gw_mac: Option, } pub const SNAT_ICMP_ECHO_NAME: &'static str = "SNAT_ICMP_ECHO"; @@ -541,26 +491,12 @@ impl ActionDesc for SNatIcmpEchoDesc { src: Some(self.nat.ip), ..Default::default() }); - let mut ht = HdrTransform { + + HdrTransform { name: SNAT_NAME.to_string(), inner_ip: HeaderAction::Modify(ip, PhantomData), ..Default::default() - }; - - // XXX-EXT-IP hack to rewrite destination MAC adress - // from virtual gateway addr to the real gateway addr - // on the same subnet as the external IP. - if self.phys_gw_mac.is_some() { - ht.inner_ether = HeaderAction::Modify( - EtherMod { - dst: Some(self.phys_gw_mac.unwrap()), - ..Default::default() - }, - PhantomData, - ); } - - ht } // Inbound traffic needs its destination IP and @@ -709,7 +645,7 @@ mod test { let pool = Arc::new(NatPool::new()); pool.add(priv_ip, pub_ip, 8765..=8765); - let snat = SNat::new(priv_ip, pool.clone(), None); + let snat = SNat::new(priv_ip, pool.clone()); let mut action_meta = ActionMeta::new(); assert!(pool.verify_available(priv_ip, pub_ip, pub_port)); diff --git a/opteadm/src/main.rs b/opteadm/src/main.rs index c822da38..cd590096 100644 --- a/opteadm/src/main.rs +++ b/opteadm/src/main.rs @@ -187,9 +187,6 @@ enum Command { #[structopt(long, parse(try_from_str))] domain_list: Vec, - #[structopt(long)] - phys_gw_mac: Option, - #[structopt(long)] external_ip: Option, @@ -355,7 +352,6 @@ fn main() -> anyhow::Result<()> { snat_start, snat_end, domain_list, - phys_gw_mac, external_ip, passthrough, } => { @@ -454,11 +450,6 @@ fn main() -> anyhow::Result<()> { mac: bsvc_mac, }, domain_list, - // XXX-EXT-IP: This is part of the external IP hack. We're - // removing this shortly, and won't be supporting creating OPTE - // ports through `opteadm` that use the hack. - proxy_arp_enable: false, - phys_gw_mac, }; hdl.create_xde(&name, cfg, passthrough)?; diff --git a/oxide-vpc/src/api.rs b/oxide-vpc/src/api.rs index 6e044681..69e3e13d 100644 --- a/oxide-vpc/src/api.rs +++ b/oxide-vpc/src/api.rs @@ -198,10 +198,6 @@ pub struct VpcCfg { /// Resolvers will use the provided list when resolving relative domain /// names. pub domain_list: Vec, - - // XXX-EXT-IP the following two fields are for the external IP hack. - pub proxy_arp_enable: bool, - pub phys_gw_mac: Option, } impl VpcCfg { diff --git a/oxide-vpc/src/engine/gateway/arp.rs b/oxide-vpc/src/engine/gateway/arp.rs index 7ced0fe4..c4082ce2 100644 --- a/oxide-vpc/src/engine/gateway/arp.rs +++ b/oxide-vpc/src/engine/gateway/arp.rs @@ -6,7 +6,6 @@ //! The ARP implementation of the Virtual Gateway. -use crate::api::Ipv4Cfg; use crate::api::VpcCfg; use core::result::Result; use opte::api::Direction; @@ -20,11 +19,7 @@ use opte::engine::predicate::Predicate; use opte::engine::rule::Action; use opte::engine::rule::Rule; -pub fn setup( - layer: &mut Layer, - cfg: &VpcCfg, - ip_cfg: &Ipv4Cfg, -) -> Result<(), OpteError> { +pub fn setup(layer: &mut Layer, cfg: &VpcCfg) -> Result<(), OpteError> { // ================================================================ // Outbound ARP Request for Gateway, from Guest // @@ -43,28 +38,5 @@ pub fn setup( ]); layer.add_rule(Direction::Out, rule.finalize()); - // ================================================================ - // Proxy ARP for any incoming requests for guest's externally - // visible IPs. - // - // XXX-EXT-IP This is a hack to get guest access working until we - // have boundary services integrated. - // ================================================================ - if ip_cfg.external_ips.as_ref().is_some() || ip_cfg.snat.is_some() { - if cfg.proxy_arp_enable { - let mut rule = Rule::new(1, Action::HandlePacket); - rule.add_predicates(vec![ - Predicate::InnerEtherType(vec![EtherTypeMatch::Exact( - ETHER_TYPE_ARP, - )]), - Predicate::InnerEtherDst(vec![EtherAddrMatch::Exact( - MacAddr::BROADCAST, - )]), - ]); - - layer.add_rule(Direction::In, rule.finalize()); - } - } - Ok(()) } diff --git a/oxide-vpc/src/engine/gateway/mod.rs b/oxide-vpc/src/engine/gateway/mod.rs index 85eb7030..46ac944b 100644 --- a/oxide-vpc/src/engine/gateway/mod.rs +++ b/oxide-vpc/src/engine/gateway/mod.rs @@ -167,7 +167,7 @@ fn setup_ipv4( ip_cfg: &Ipv4Cfg, vpc_mappings: Arc, ) -> Result<(), OpteError> { - arp::setup(layer, cfg, ip_cfg)?; + arp::setup(layer, cfg)?; dhcp::setup(layer, cfg, ip_cfg)?; icmp::setup(layer, cfg, ip_cfg)?; diff --git a/oxide-vpc/src/engine/mod.rs b/oxide-vpc/src/engine/mod.rs index 98de2672..02c7c001 100644 --- a/oxide-vpc/src/engine/mod.rs +++ b/oxide-vpc/src/engine/mod.rs @@ -40,14 +40,11 @@ use opte::engine::ether::ETHER_TYPE_IPV4; use opte::engine::ip4::Ipv4Addr; #[derive(Clone, Copy, Debug)] -pub struct VpcParser { - // XXX-EXT-IP hack - pub proxy_arp_enable: bool, -} +pub struct VpcParser {} impl VpcParser { pub fn new() -> Self { - Self { proxy_arp_enable: false } + Self {} } } @@ -96,62 +93,6 @@ impl VpcNetwork { Ok(HdlPktAction::Deny) } - - fn handle_arp_in( - &self, - pkt: &mut Packet, - ) -> Result { - let arp_start = pkt.hdr_offsets().inner.ether.hdr_len; - let mut rdr = pkt.get_rdr_mut(); - rdr.seek(arp_start).unwrap(); - let arp = ArpEthIpv4::parse(&mut rdr) - .map_err(|_| HdlPktError("inbound ARP"))?; - let proxy_arp = self.cfg.proxy_arp_enable; - let guest_mac = self.cfg.guest_mac; - let ip_cfg = self.cfg.ipv4_cfg().unwrap(); - - // ================================================================ - // Proxy ARP for any incoming requests for guest's external IP. - // - // XXX-EXT-IP This is a hack to get guest access working until - // we have boundary services integrated. - // ================================================================ - if let Some(external_ip) = ip_cfg.external_ips { - if proxy_arp && is_arp_req_for_tpa(external_ip, &arp) { - let hp = arp::gen_arp_reply( - guest_mac, - external_ip, - arp.sha, - arp.spa, - ); - return Ok(HdlPktAction::Hairpin(hp)); - } - } - - // ================================================================ - // Proxy ARP for any incoming requests for guest's SNAT IP. - // - // This is not great because once you have more than one guest - // it means there is an ARP battle for the same SNAT IP. One - // more rason why this hack needs to go away. - // - // XXX-EXT-IP This is a hack to get guest access working until - // we have boundary services integrated. - // ================================================================ - if let Some(snat) = ip_cfg.snat.as_ref() { - if proxy_arp && is_arp_req_for_tpa(snat.external_ip, &arp) { - let hp = arp::gen_arp_reply( - guest_mac, - snat.external_ip, - arp.sha, - arp.spa, - ); - return Ok(HdlPktAction::Hairpin(hp)); - } - } - - Ok(HdlPktAction::Deny) - } } impl NetworkImpl for VpcNetwork { @@ -167,15 +108,12 @@ impl NetworkImpl for VpcNetwork { match (dir, pkt.meta().inner.ether.ether_type) { (Direction::Out, EtherType::Arp) => self.handle_arp_out(pkt), - // XXX-EXT-IP This is only need for the hack. - (Direction::In, EtherType::Arp) => self.handle_arp_in(pkt), - _ => Ok(HdlPktAction::Deny), } } fn parser(&self) -> Self::Parser { - VpcParser { proxy_arp_enable: self.cfg.proxy_arp_enable } + VpcParser {} } } @@ -270,33 +208,29 @@ impl NetworkParser for VpcParser { let mut meta = PacketMeta::default(); let mut offsets = HeaderOffsets::default(); - // XXX-EXT-IP If proxy ARP is enabled, then we are not on the - // Oxide Rack Network and have no encap. - if !self.proxy_arp_enable { - let (outer_ether_hi, _hdr) = Packet::parse_ether(rdr)?; - meta.outer.ether = Some(outer_ether_hi.meta); - offsets.outer.ether = Some(outer_ether_hi.offset); - let outer_et = outer_ether_hi.meta.ether_type; + let (outer_ether_hi, _hdr) = Packet::parse_ether(rdr)?; + meta.outer.ether = Some(outer_ether_hi.meta); + offsets.outer.ether = Some(outer_ether_hi.offset); + let outer_et = outer_ether_hi.meta.ether_type; - // VPC traffic is delivered exclusively on an IPv6 + - // Geneve underlay. - let outer_ip_hi = match outer_et { - EtherType::Ipv6 => Packet::parse_ip6(rdr)?.0, + // VPC traffic is delivered exclusively on an IPv6 + + // Geneve underlay. + let outer_ip_hi = match outer_et { + EtherType::Ipv6 => Packet::parse_ip6(rdr)?.0, - _ => return Err(ParseError::UnexpectedEtherType(outer_et)), - }; + _ => return Err(ParseError::UnexpectedEtherType(outer_et)), + }; - meta.outer.ip = Some(outer_ip_hi.meta); - offsets.outer.ip = Some(outer_ip_hi.offset); + meta.outer.ip = Some(outer_ip_hi.meta); + offsets.outer.ip = Some(outer_ip_hi.offset); - let (geneve_hi, _geneve_hdr) = match outer_ip_hi.meta.proto() { - Protocol::UDP => Packet::parse_geneve(rdr)?, - proto => return Err(ParseError::UnexpectedProtocol(proto)), - }; + let (geneve_hi, _geneve_hdr) = match outer_ip_hi.meta.proto() { + Protocol::UDP => Packet::parse_geneve(rdr)?, + proto => return Err(ParseError::UnexpectedProtocol(proto)), + }; - meta.outer.encap = Some(EncapMeta::from(geneve_hi.meta)); - offsets.outer.encap = Some(geneve_hi.offset); - } + meta.outer.encap = Some(EncapMeta::from(geneve_hi.meta)); + offsets.outer.encap = Some(geneve_hi.offset); let (inner_ether_hi, _) = Packet::parse_ether(rdr)?; meta.inner.ether = inner_ether_hi.meta; @@ -314,21 +248,6 @@ impl NetworkParser for VpcParser { (ip_hi, hdr.pseudo_csum()) } - EtherType::Arp => { - // XXX-EXT-IP Need to allow inbound ARP for proxy ARP - // to work. - if self.proxy_arp_enable { - return Ok(PacketInfo { - meta, - offsets, - body_csum: None, - extra_hdr_space: None, - }); - } else { - return Err(ParseError::UnexpectedEtherType(inner_et)); - } - } - _ => return Err(ParseError::UnexpectedEtherType(inner_et)), }; diff --git a/oxide-vpc/src/engine/nat.rs b/oxide-vpc/src/engine/nat.rs index 84529e91..e58c7d02 100644 --- a/oxide-vpc/src/engine/nat.rs +++ b/oxide-vpc/src/engine/nat.rs @@ -18,7 +18,6 @@ use super::router::RouterTargetInternal; use super::router::ROUTER_LAYER_NAME; use crate::api::Ipv4Cfg; use crate::api::Ipv6Cfg; -use crate::api::MacAddr; use crate::api::VpcCfg; use core::num::NonZeroU32; use core::result::Result; @@ -62,10 +61,10 @@ pub fn setup( }; let mut layer = Layer::new(NAT_LAYER_NAME, pb.name(), actions, ft_limit); if let Some(ipv4_cfg) = cfg.ipv4_cfg() { - setup_ipv4_nat(&mut layer, ipv4_cfg, cfg.phys_gw_mac)?; + setup_ipv4_nat(&mut layer, ipv4_cfg)?; } if let Some(ipv6_cfg) = cfg.ipv6_cfg() { - setup_ipv6_nat(&mut layer, ipv6_cfg, cfg.phys_gw_mac)?; + setup_ipv6_nat(&mut layer, ipv6_cfg)?; } pb.add_layer(layer, Pos::After(ROUTER_LAYER_NAME)) } @@ -73,14 +72,12 @@ pub fn setup( fn setup_ipv4_nat( layer: &mut Layer, ip_cfg: &Ipv4Cfg, - // XXX-EXT-IP Remove - phys_gw_mac: Option, ) -> Result<(), OpteError> { // When it comes to NAT we always prefer using 1:1 NAT of external // IP to SNAT. To achieve this we place the NAT rules at a lower // priority than SNAT. if let Some(ip4) = ip_cfg.external_ips { - let nat = Arc::new(Nat::new(ip_cfg.private_ip, ip4, phys_gw_mac)); + let nat = Arc::new(Nat::new(ip_cfg.private_ip, ip4)); // 1:1 NAT outbound packets destined for internet gateway. let mut out_nat = @@ -110,7 +107,7 @@ fn setup_ipv4_nat( snat_cfg.external_ip, snat_cfg.ports.clone(), ); - let snat = SNat::new(ip_cfg.private_ip, Arc::new(pool), phys_gw_mac); + let snat = SNat::new(ip_cfg.private_ip, Arc::new(pool)); let mut rule = Rule::new(SNAT_PRIORITY, Action::Stateful(Arc::new(snat))); @@ -129,14 +126,12 @@ fn setup_ipv4_nat( fn setup_ipv6_nat( layer: &mut Layer, ip_cfg: &Ipv6Cfg, - // XXX-EXT-IP Remove - phys_gw_mac: Option, ) -> Result<(), OpteError> { // When it comes to NAT we always prefer using 1:1 NAT of external // IP to SNAT. To achieve this we place the NAT rules at a lower // priority than SNAT. if let Some(ip6) = ip_cfg.external_ips { - let nat = Arc::new(Nat::new(ip_cfg.private_ip, ip6, phys_gw_mac)); + let nat = Arc::new(Nat::new(ip_cfg.private_ip, ip6)); // 1:1 NAT outbound packets destined for internet gateway. let mut out_nat = @@ -166,7 +161,7 @@ fn setup_ipv6_nat( snat_cfg.external_ip, snat_cfg.ports.clone(), ); - let snat = SNat6::new(ip_cfg.private_ip, Arc::new(pool), phys_gw_mac); + let snat = SNat6::new(ip_cfg.private_ip, Arc::new(pool)); let mut rule = Rule::new(SNAT_PRIORITY, Action::Stateful(Arc::new(snat))); diff --git a/oxide-vpc/tests/common/mod.rs b/oxide-vpc/tests/common/mod.rs index 8405721b..eb8ddffc 100644 --- a/oxide-vpc/tests/common/mod.rs +++ b/oxide-vpc/tests/common/mod.rs @@ -147,8 +147,6 @@ pub fn g1_cfg2(ip_cfg: IpCfg) -> VpcCfg { vni: Vni::new(99u32).unwrap(), }, domain_list: vec!["oxide.computer".parse().unwrap()], - proxy_arp_enable: false, - phys_gw_mac: Some(MacAddr::from([0x78, 0x23, 0xae, 0x5d, 0x4f, 0x0d])), } } @@ -193,8 +191,6 @@ pub fn g2_cfg() -> VpcCfg { vni: Vni::new(99u32).unwrap(), }, domain_list: vec!["oxide.computer".parse().unwrap()], - proxy_arp_enable: false, - phys_gw_mac: Some(MacAddr::from([0x78, 0x23, 0xae, 0x5d, 0x4f, 0x0d])), } } diff --git a/oxide-vpc/tests/integration_tests.rs b/oxide-vpc/tests/integration_tests.rs index bf77111b..d0c3b0e0 100644 --- a/oxide-vpc/tests/integration_tests.rs +++ b/oxide-vpc/tests/integration_tests.rs @@ -113,8 +113,6 @@ fn lab_cfg() -> VpcCfg { vni: Vni::new(99u32).unwrap(), }, domain_list: vec!["oxide.computer".parse().unwrap()], - proxy_arp_enable: false, - phys_gw_mac: Some(MacAddr::from([0x78, 0x23, 0xae, 0x5d, 0x4f, 0x0d])), } } diff --git a/xde/src/xde.rs b/xde/src/xde.rs index fffab52d..e66cbf90 100644 --- a/xde/src/xde.rs +++ b/xde/src/xde.rs @@ -43,7 +43,6 @@ use core::time::Duration; use illumos_sys_hdrs::*; use opte::api::CmdOk; use opte::api::Direction; -use opte::api::MacAddr; use opte::api::NoResp; use opte::api::OpteCmd; use opte::api::OpteCmdIoctl; @@ -61,13 +60,11 @@ use opte::engine::ether::EtherAddr; use opte::engine::geneve::Vni; use opte::engine::headers::EncapMeta; use opte::engine::headers::IpAddr; -use opte::engine::headers::IpMeta; use opte::engine::ioctl::{self as api}; use opte::engine::ip6::Ipv6Addr; use opte::engine::packet::Initialized; use opte::engine::packet::Packet; use opte::engine::packet::PacketError; -use opte::engine::packet::PacketRead; use opte::engine::packet::Parsed; use opte::engine::port::meta::ActionMeta; use opte::engine::port::Port; @@ -120,10 +117,6 @@ static mut xde_devs: KRwLock>> = KRwLock::new(Vec::new()); /// DDI dev info pointer to the attached xde device. static mut xde_dip: *mut dev_info = 0 as *mut dev_info; -// XXX-EXT-IP -#[no_mangle] -pub static mut xde_ext_ip_hack: i32 = 0; - // This block is purely for SDT probes. extern "C" { pub fn __dtrace_probe_bad__packet( @@ -601,15 +594,6 @@ fn create_xde(req: &CreateXdeReq) -> Result { None => (), } - // XXX-EXT-IP Copy the configuration, modifying the external IP hack - // fields depending on the value of the `xde_ext_ip_hack` tunable. - let proxy_arp_enable = unsafe { xde_ext_ip_hack == 1 }; - let vpc_cfg = VpcCfg { - proxy_arp_enable, - phys_gw_mac: if proxy_arp_enable { cfg.phys_gw_mac } else { None }, - ..cfg.clone() - }; - // If this is the first guest in this VPC, then create a new // mapping for said VPC. Otherwise, pull the existing one. // @@ -618,7 +602,7 @@ fn create_xde(req: &CreateXdeReq) -> Result { // they're mapping both IP addresses to the same host. let phys_net = PhysNet { ether: cfg.guest_mac, ip: cfg.phys_ip, vni: cfg.vni }; - let port_v2p = match vpc_cfg.ip_cfg { + let port_v2p = match cfg.ip_cfg { IpCfg::Ipv4(ref ipv4) => { state.vpc_map.add(IpAddr::Ip4(ipv4.private_ip), phys_net) } @@ -633,7 +617,7 @@ fn create_xde(req: &CreateXdeReq) -> Result { let port = new_port( req.xde_devname.clone(), - &vpc_cfg, + &cfg, state.vpc_map.clone(), port_v2p.clone(), state.ectx.clone(), @@ -654,7 +638,7 @@ fn create_xde(req: &CreateXdeReq) -> Result { port, port_periodic, port_v2p, - vpc_cfg, + vpc_cfg: cfg.clone(), passthrough: req.passthrough, vni: cfg.vni, u1: underlay.u1.clone(), @@ -883,29 +867,6 @@ unsafe extern "C" fn xde_attach( assert!(xde_dip.is_null()); - // XXX-EXT-IP - if !driver_prop_exists(dip, "ext_ip_hack") { - warn!("failed to find 'ext_ip_hack' property in xde.conf"); - return DDI_FAILURE; - } - - match get_driver_prop_bool(dip, "ext_ip_hack") { - Some(true) => { - warn!("ext_ip_hack enabled: traffic will NOT be encapsulated"); - xde_ext_ip_hack = 1; - } - - Some(_) => { - warn!("ext_ip_hack disabled: traffic will be encapsulated"); - xde_ext_ip_hack = 0; - } - - None => { - warn!("failed to read 'ext_ip_hack' from xde.conf, disabled"); - xde_ext_ip_hack = 0; - } - }; - // We need to share the minor number space with the GLDv3 framework. // We'll use the first private minor number for our control device. XDE_CTL_MINOR = mac_private_minor(); @@ -1486,53 +1447,6 @@ unsafe extern "C" fn xde_mc_tx( let res = port.process(Direction::Out, &mut pkt, ActionMeta::new()); match res { Ok(ProcessResult::Modified) => { - // XXX-EXT-IP - // - // XXX Doing all these shenanigans because we don't - // have the overlay layer in place which would - // normally rewrite the dst MAC addr to that of the - // dest guest. - if xde_ext_ip_hack == 1 { - match pkt.meta().inner.ip { - Some(IpMeta::Ip4(ip4)) => { - let devs = xde_devs.read(); - for d in devs.iter() { - if let Some(cfg) = d.vpc_cfg.ipv4_cfg() { - if cfg.private_ip == ip4.dst { - pkt.write_dst_mac(d.vpc_cfg.guest_mac); - return guest_loopback( - src_dev, - pkt, - d.vpc_cfg.vni, - ); - } - } - } - } - - Some(IpMeta::Ip6(ip6)) => { - let devs = xde_devs.read(); - for d in devs.iter() { - if let Some(cfg) = d.vpc_cfg.ipv6_cfg() { - if cfg.private_ip == ip6.dst { - pkt.write_dst_mac(d.vpc_cfg.guest_mac); - return guest_loopback( - src_dev, - pkt, - d.vpc_cfg.vni, - ); - } - } - } - } - - None => (), - } - - mch.tx_drop_on_no_desc(pkt, hint, MacTxFlags::empty()); - return ptr::null_mut(); - } - let meta = pkt.meta(); // If the outer IPv6 destination is the same as the @@ -1995,14 +1909,7 @@ fn new_port( gateway::setup(&mut pb, &cfg, vpc_map, FT_LIMIT_ONE.unwrap())?; router::setup(&mut pb, &cfg, FT_LIMIT_ONE.unwrap())?; nat::setup(&mut pb, &cfg, NAT_FT_LIMIT.unwrap())?; - - // XXX-EXT-IP - if unsafe { xde_ext_ip_hack != 1 } { - warn!("enabling overlay for port: {}", name); - overlay::setup(&pb, &cfg, v2p, FT_LIMIT_ONE.unwrap())?; - } else { - warn!("disabling overlay for port: {}", name); - } + overlay::setup(&pb, &cfg, v2p, FT_LIMIT_ONE.unwrap())?; // Set the UFT and TCP flow table limits based on the guest's IP // configuration. @@ -2071,8 +1978,7 @@ unsafe extern "C" fn xde_rx( // We must first parse the packet in order to determine where it // is to be delivered. - let parser = - VpcParser { proxy_arp_enable: unsafe { xde_ext_ip_hack == 1 } }; + let parser = VpcParser {}; let mut pkt = match Packet::wrap_mblk_and_parse(mp_chain, Direction::In, parser) { Ok(pkt) => pkt, @@ -2093,91 +1999,34 @@ unsafe extern "C" fn xde_rx( let meta = pkt.meta(); let devs = xde_devs.read(); - let dev = if xde_ext_ip_hack == 0 { - // Determine where to send packet based on Geneve VNI and - // destination MAC address. - let geneve = match meta.outer.encap { - Some(EncapMeta::Geneve(geneve)) => geneve, - None => { - // TODO add stat - let msg = "no geneve header, dropping"; - bad_packet_probe(None, Direction::In, mp_chain, msg); - opte::engine::dbg(format!("{}", msg)); - return; - } - }; - - let vni = geneve.vni; - let ether_dst = meta.inner.ether.dst; - let dev = match devs - .iter() - .find(|x| x.vni == vni && x.port.mac_addr() == ether_dst) - { - Some(dev) => dev, - None => { - // TODO add SDT probe - // TODO add stat - opte::engine::dbg(format!( - "[encap] no device found for vni: {} mac: {}", - vni, ether_dst - )); - return; - } - }; - dev - } else { - // XXX-EXT-IP - let ether_dst = meta.inner.ether.dst; - - if ether_dst == MacAddr::from(MacAddr::BROADCAST) { - let bytes = pkt.get_rdr().copy_remaining(); - - for dev in devs.iter() { - // just go straight to overlay in passthrough mode - if (*dev).passthrough { - mac::mac_rx((*dev).mh, mrh, mp_chain); - } - - let port = &(*dev).port; - let parser = port.network().parser(); - // Unwrap: We are copying bytes from a packet that we - // previously parsed; we know it's good. - let mut pkt_copy = - Packet::copy(&bytes).parse(Direction::In, parser).unwrap(); - let res = port.process( - Direction::In, - &mut pkt_copy, - ActionMeta::new(), - ); - - match res { - Ok(ProcessResult::Modified) => { - mac::mac_rx((*dev).mh, mrh, pkt_copy.unwrap_mblk()); - } - Ok(ProcessResult::Hairpin(hppkt)) => { - mch.tx_drop_on_no_desc(hppkt, 0, MacTxFlags::empty()); - } - Ok(ProcessResult::Bypass) => { - mac::mac_rx((*dev).mh, mrh, mp_chain); - } - _ => {} - } - } + // Determine where to send packet based on Geneve VNI and + // destination MAC address. + let geneve = match meta.outer.encap { + Some(EncapMeta::Geneve(geneve)) => geneve, + None => { + // TODO add stat + let msg = "no geneve header, dropping"; + bad_packet_probe(None, Direction::In, mp_chain, msg); + opte::engine::dbg(format!("{}", msg)); + return; + } + }; + let vni = geneve.vni; + let ether_dst = meta.inner.ether.dst; + let dev = match devs + .iter() + .find(|x| x.vni == vni && x.port.mac_addr() == ether_dst) + { + Some(dev) => dev, + None => { + // TODO add SDT probe + // TODO add stat + opte::engine::dbg(format!( + "[encap] no device found for vni: {} mac: {}", + vni, ether_dst + )); return; - } else { - match devs.iter().find(|x| x.port.mac_addr() == ether_dst) { - Some(dev) => dev, - None => { - // TODO add SDT probe - // TODO add stat - opte::engine::dbg(format!( - "[ext_ip_hack] no device found for mac: {}", - ether_dst - )); - return; - } - } } }; diff --git a/xde/xde.conf b/xde/xde.conf index 2ad87abc..c168650b 100644 --- a/xde/xde.conf +++ b/xde/xde.conf @@ -1,11 +1,3 @@ # xde kernel module configuration file name="xde" parent="pseudo" instance=0; - -# -# Enable the "external IP hack". This disables encap and turns SNAT -# into plain NAT, along with performing proxy ARP for the NAT IP. This -# allows one to have network connectivty to their guests via the local -# IPv4 network that the sled is sitting on. -# -ext_ip_hack = 0; \ No newline at end of file