Skip to content

Commit

Permalink
Remove the "external IP hack" (#236) (#311)
Browse files Browse the repository at this point in the history 
* 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 <[email protected]>
  • Loading branch information
@rzezeski
rzezeski authored May 2, 2023
1 parent c14412c commit 05ea62e
Show file tree
Hide file tree
Showing 14 changed files with 79 additions and 485 deletions.
2 changes: 1 addition & 1 deletion opte-api/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
52 changes: 8 additions & 44 deletions opte/src/engine/nat.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

//! 1:1 NAT.
use super::ether::EtherMod;
use super::headers::HeaderAction;
use super::headers::IpMod;
use super::ip4::Ipv4Mod;
Expand All @@ -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)))] {
Expand All @@ -46,22 +44,12 @@ cfg_if! {
pub struct Nat {
priv_ip: IpAddr,
external_ip: IpAddr,
// XXX-EXT-IP Remove
phys_gw_mac: Option<MacAddr>,
}

impl Nat {
/// Create a new NAT mapping from a private to public IP address.
pub fn new<T: ConcreteIpAddr>(
priv_ip: T,
external_ip: T,
phys_gw_mac: Option<MacAddr>,
) -> Self {
Self {
priv_ip: priv_ip.into(),
external_ip: external_ip.into(),
phys_gw_mac,
}
pub fn new<T: ConcreteIpAddr>(priv_ip: T, external_ip: T) -> Self {
Self { priv_ip: priv_ip.into(), external_ip: external_ip.into() }
}
}

Expand All @@ -78,15 +66,8 @@ impl StatefulAction for Nat {
_pkt: &Packet<Parsed>,
_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)))
}

Expand All @@ -103,8 +84,6 @@ impl StatefulAction for Nat {
pub struct NatDesc {
priv_ip: IpAddr,
external_ip: IpAddr,
// XXX-EXT-IP
phys_gw_mac: Option<MacAddr>,
}

pub const NAT_NAME: &'static str = "NAT";
Expand All @@ -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 => {
Expand Down Expand Up @@ -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();

// ================================================================
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 0 additions & 14 deletions opte/src/engine/packet.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)))] {
Expand Down Expand Up @@ -1009,19 +1008,6 @@ impl From<smoltcp::Error> for BodyTransformError {
}

impl Packet<Parsed> {
/// 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<Checksum> {
self.state.body_csum
}
Expand Down
84 changes: 10 additions & 74 deletions opte/src/engine/snat.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -188,17 +186,11 @@ impl<T: ConcreteIpAddr> FiniteResource for NatPool<T> {
pub struct SNat {
priv_ip: Ipv4Addr,
ip_pool: Arc<NatPool<Ipv4Addr>>,
// XXX-EXT-IP
phys_gw_mac: Option<MacAddr>,
}

impl SNat {
pub fn new(
addr: Ipv4Addr,
ip_pool: Arc<NatPool<Ipv4Addr>>,
phys_gw_mac: Option<MacAddr>,
) -> Self {
SNat { priv_ip: addr, ip_pool, phys_gw_mac }
pub fn new(addr: Ipv4Addr, ip_pool: Arc<NatPool<Ipv4Addr>>) -> Self {
SNat { priv_ip: addr, ip_pool }
}

// A helper method for generating an SNAT + ICMP action descriptor.
Expand Down Expand Up @@ -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)))
Expand Down Expand Up @@ -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,
};

Expand Down Expand Up @@ -302,17 +292,11 @@ impl StatefulAction for SNat {
pub struct SNat6 {
priv_ip: Ipv6Addr,
ip_pool: Arc<NatPool<Ipv6Addr>>,
// XXX-EXT-IP
phys_gw_mac: Option<MacAddr>,
}

impl SNat6 {
pub fn new(
addr: Ipv6Addr,
ip_pool: Arc<NatPool<Ipv6Addr>>,
phys_gw_mac: Option<MacAddr>,
) -> Self {
SNat6 { priv_ip: addr, ip_pool, phys_gw_mac }
pub fn new(addr: Ipv6Addr, ip_pool: Arc<NatPool<Ipv6Addr>>) -> Self {
SNat6 { priv_ip: addr, ip_pool }
}
}

Expand All @@ -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,
};

Expand Down Expand Up @@ -373,8 +356,6 @@ pub struct SNatDesc<T: ConcreteIpAddr> {
nat: NatPoolEntry<T>,
priv_ip: T,
priv_port: u16,
// XXX-EXT-IP
phys_gw_mac: Option<MacAddr>,
}

pub const SNAT_NAME: &'static str = "SNAT";
Expand All @@ -389,7 +370,7 @@ impl ActionDesc for SNatDesc<Ipv4Addr> {
..Default::default()
});

let mut ht = HdrTransform {
HdrTransform {
name: SNAT_NAME.to_string(),
inner_ip: HeaderAction::Modify(ip, PhantomData),
inner_ulp: UlpHeaderAction::Modify(UlpMetaModify {
Expand All @@ -400,22 +381,7 @@ impl ActionDesc for SNatDesc<Ipv4Addr> {
..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
Expand Down Expand Up @@ -456,7 +422,8 @@ impl ActionDesc for SNatDesc<Ipv6Addr> {
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 {
Expand All @@ -467,22 +434,7 @@ impl ActionDesc for SNatDesc<Ipv6Addr> {
..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
Expand Down Expand Up @@ -526,8 +478,6 @@ pub struct SNatIcmpEchoDesc {
nat: NatPoolEntry<Ipv4Addr>,
priv_ip: Ipv4Addr,
echo_ident: u16,
// XXX-EXT-IP
phys_gw_mac: Option<MacAddr>,
}

pub const SNAT_ICMP_ECHO_NAME: &'static str = "SNAT_ICMP_ECHO";
Expand All @@ -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
Expand Down Expand Up @@ -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));

Expand Down
9 changes: 0 additions & 9 deletions opteadm/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,9 +187,6 @@ enum Command {
#[structopt(long, parse(try_from_str))]
domain_list: Vec<DomainName>,

#[structopt(long)]
phys_gw_mac: Option<MacAddr>,

#[structopt(long)]
external_ip: Option<IpAddr>,

Expand Down Expand Up @@ -355,7 +352,6 @@ fn main() -> anyhow::Result<()> {
snat_start,
snat_end,
domain_list,
phys_gw_mac,
external_ip,
passthrough,
} => {
Expand Down Expand Up @@ -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)?;
Expand Down
4 changes: 0 additions & 4 deletions oxide-vpc/src/api.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,6 @@ pub struct VpcCfg {
/// Resolvers will use the provided list when resolving relative domain
/// names.
pub domain_list: Vec<DomainName>,

// XXX-EXT-IP the following two fields are for the external IP hack.
pub proxy_arp_enable: bool,
pub phys_gw_mac: Option<MacAddr>,
}

impl VpcCfg {
Expand Down
Loading

0 comments on commit 05ea62e

Please sign in to comment.