thoughts on pr #1548

config/src/external/overlay/tests.rs

@@ -420,11 +420,7 @@ pub mod test {
 
         // Build overlay object and validate it
         let overlay = Overlay::new(vpc_table, peering_table);
-        assert!(
-            overlay
-                .validate()
-                .is_err_and(|e| e == ConfigError::IncompatibleNatModes("Peering-1".to_owned()))
-        );
+        assert!(overlay.validate().is_ok());
     }
 
     #[test]

config/src/external/overlay/validation_tests.rs

@@ -1209,7 +1209,7 @@ mod test {
         assert!(validate_overlay_with_peering(peering).is_ok());
     }
 
-    // Stateless + stateful rejected
+    // Stateless + stateful passes
     #[test]
     fn test_stateless_plus_stateful_rejected() {
         let peering = VpcPeering::with_default_group(
@@ -1237,15 +1237,10 @@ mod test {
                 ],
             ),
         );
-        let result = validate_overlay_with_peering(peering);
-        assert_eq!(
-            result,
-            Err(ConfigError::IncompatibleNatModes("Peering-1".to_owned())),
-            "{result:?}",
-        );
+        assert!(validate_overlay_with_peering(peering).is_ok());
     }
 
-    // Stateless + port forwarding rejected
+    // Stateless + port forwarding passes
     #[test]
     fn test_stateless_plus_port_forwarding_rejected() {
         let peering = VpcPeering::with_default_group(
@@ -1273,12 +1268,7 @@ mod test {
                 ],
             ),
         );
-        let result = validate_overlay_with_peering(peering);
-        assert_eq!(
-            result,
-            Err(ConfigError::IncompatibleNatModes("Peering-1".to_owned())),
-            "{result:?}",
-        );
+        assert!(validate_overlay_with_peering(peering).is_ok());
     }
 
     // Stateful + stateful rejected (across peering sides)
@@ -1389,6 +1379,112 @@ mod test {
         );
     }
 
+    // ----------------------------------------------------------------------------------
+    // Matrix property test: NAT-mode allowlist across all (local, remote) pairings.
+    //
+    // The named tests above spot-check individual cells in the matrix. This test asserts
+    // the *entire* matrix matches the documented spec, in one shot: for every pair of
+    // (local mode, remote mode), the validator agrees with `expected_allowed`. The spec
+    // function below is the single source of truth -- when the allowlist changes, edit
+    // `expected_allowed` and the matrix re-validates against the implementation.
+    // ----------------------------------------------------------------------------------
+
+    #[derive(Debug, Clone, Copy)]
+    enum NatMode {
+        None,
+        Stateless,
+        Stateful,
+        PortForwarding,
+    }
+
+    impl NatMode {
+        const ALL: [NatMode; 4] = [
+            NatMode::None,
+            NatMode::Stateless,
+            NatMode::Stateful,
+            NatMode::PortForwarding,
+        ];
+
+        fn from_byte(b: u8) -> Self {
+            Self::ALL[(b as usize) % Self::ALL.len()]
+        }
+    }
+
+    /// The documented spec: which (local, remote) combinations are accepted by
+    /// `validate_nat_combinations` (see `config/src/external/overlay/vpc.rs`).
+    ///
+    /// Disallowed iff both sides use stateful or port-forwarding NAT.
+    fn expected_allowed(local: NatMode, remote: NatMode) -> bool {
+        let is_stateful_or_pf = |m| matches!(m, NatMode::Stateful | NatMode::PortForwarding);
+        !(is_stateful_or_pf(local) && is_stateful_or_pf(remote))
+    }
+
+    /// Build a single-expose `VpcManifest` for the given side and NAT mode. Each
+    /// side gets a distinct /16 to avoid prefix overlap across the two sides.
+    fn manifest_with_mode(vpc: &'static str, side: u8, mode: NatMode) -> VpcManifest {
+        // side 0 -> 1.x / 2.x ; side 1 -> 3.x / 4.x
+        let priv_octet = 1 + side * 2;
+        let pub_octet = 2 + side * 2;
+
+        let expose = match mode {
+            NatMode::None => {
+                VpcExpose::empty().ip(format!("{priv_octet}.0.0.0/16").as_str().into())
+            }
+            NatMode::Stateless => VpcExpose::empty()
+                .make_stateless_nat()
+                .unwrap()
+                .ip(format!("{priv_octet}.0.0.0/16").as_str().into())
+                .as_range(format!("{pub_octet}.0.0.0/16").as_str().into())
+                .unwrap(),
+            NatMode::Stateful => VpcExpose::empty()
+                .make_stateful_nat(None)
+                .unwrap()
+                .ip(format!("{priv_octet}.0.0.0/16").as_str().into())
+                .as_range(format!("{pub_octet}.0.0.0/16").as_str().into())
+                .unwrap(),
+            NatMode::PortForwarding => VpcExpose::empty()
+                .make_port_forwarding(None, None)
+                .unwrap()
+                .ip(prefix_with_ports(
+                    format!("{priv_octet}.0.0.1/32").as_str(),
+                    80,
+                    80,
+                ))
+                .as_range(prefix_with_ports(
+                    format!("{pub_octet}.0.0.1/32").as_str(),
+                    8080,
+                    8080,
+                ))
+                .unwrap(),
+        };
+        VpcManifest::with_exposes(vpc, vec![expose])
+    }
+
+    fn peering_with_modes(local: NatMode, remote: NatMode) -> VpcPeering {
+        VpcPeering::with_default_group(
+            "Peering-1",
+            manifest_with_mode("VPC-1", 0, local),
+            manifest_with_mode("VPC-2", 1, remote),
+        )
+    }
+
+    #[test]
+    fn nat_combination_matrix_matches_spec() {
+        bolero::check!()
+            .with_type::<(u8, u8)>()
+            .for_each(|(local_b, remote_b)| {
+                let local = NatMode::from_byte(*local_b);
+                let remote = NatMode::from_byte(*remote_b);
+                let result = validate_overlay_with_peering(peering_with_modes(local, remote));
+                let expected = expected_allowed(local, remote);
+                assert_eq!(
+                    result.is_ok(),
+                    expected,
+                    "validate_nat_combinations({local:?}, {remote:?}) -> {result:?}, expected ok={expected}",
+                );
+            });
+    }
+
     // ==================================================================================
     // Overlay-level tests (cross-peering)
     // ==================================================================================

config/src/external/overlay/vpc.rs

@@ -122,55 +122,39 @@ impl ValidatedPeering {
     fn validate_nat_combinations(&self) -> ConfigResult {
         // If stateful NAT is set up on one side of the peering, we don't support NAT (stateless or
         // stateful) on the other side.
-        let mut local_has_stateless_nat = false;
-        let mut local_has_stateful_nat = false;
+        let mut local_has_masquerading = false;
         let mut local_has_port_forwarding = false;
         for expose in self.local.valexp() {
             match expose.nat_config() {
                 Some(VpcExposeNatConfig::Stateful { .. }) => {
-                    local_has_stateful_nat = true;
-                }
-                Some(VpcExposeNatConfig::Stateless { .. }) => {
-                    local_has_stateless_nat = true;
+                    local_has_masquerading = true;
                 }
                 Some(VpcExposeNatConfig::PortForwarding { .. }) => {
                     local_has_port_forwarding = true;
                 }
-                None => {}
+                Some(VpcExposeNatConfig::Stateless { .. }) | None => {}
             }
         }
-        let local_has_nat =
-            local_has_stateless_nat || local_has_stateful_nat || local_has_port_forwarding;
 
-        if !local_has_nat {
+        // No NAT or static NAT only is compatible with all other modes on the other side
+        if !(local_has_masquerading || local_has_port_forwarding) {
             return Ok(());
         }
 
-        let local_has_stateless_nat_only =
-            local_has_stateless_nat && !local_has_stateful_nat && !local_has_port_forwarding;
-
         // Allowed:
         //
         // - no NAT ------------ *
-        // - stateless NAT ----- stateless NAT
+        // - stateless NAT ----- *
         //
         // Disallowed (some of them may be supported in the future):
         //
-        // - stateful NAT ------ stateless NAT
-        // - stateful NAT ------ stateful NAT
-        // - stateful NAT ------ port forwarding
+        // - masquerading ------ masquerading
+        // - masquerading ------ port forwarding
         // - port forwarding --- port forwarding
-        // - port forwarding --- stateless NAT
-
         for remote_expose in self.remote.valexp() {
-            if !remote_expose.has_nat() {
-                continue;
-            }
-            if local_has_stateless_nat_only && remote_expose.has_stateless_nat() {
-                continue;
+            if remote_expose.has_stateful_nat() || remote_expose.has_port_forwarding() {
+                return Err(ConfigError::IncompatibleNatModes(self.name.clone()));
             }
-            // Other combinations are rejected
-            return Err(ConfigError::IncompatibleNatModes(self.name.clone()));
         }
         Ok(())
     }

config/src/external/overlay/vpcpeering.rs

@@ -535,13 +535,6 @@ impl ValidatedExpose {
         )
     }
 
-    #[must_use]
-    pub(crate) fn has_nat(&self) -> bool {
-        self.nat
-            .as_ref()
-            .is_some_and(|nat| !nat.as_range.is_empty())
-    }
-
     #[must_use]
     pub fn has_stateful_nat(&self) -> bool {
         self.nat.as_ref().is_some_and(VpcExposeNat::is_stateful)

dataplane/src/packet_processor/ipforward.rs

@@ -156,7 +156,9 @@ impl IpForwarder {
                 /* At this point decapsulation has already happened and `Packet` refers to
                 the innner packet. Annotate the incoming vni and the corresponding vrf to
                 make lookups from */
-                packet.meta_mut().src_vpcd = Some(VpcDiscriminant::VNI(vni));
+                packet
+                    .meta_mut()
+                    .set_src_vpcd(Some(VpcDiscriminant::VNI(vni)));
                 packet.meta_mut().vrf = Some(next_vrf);
                 packet.meta_mut().set_overlay(true);
             }

dataplane/src/packet_processor/mod.rs

@@ -98,7 +98,7 @@ pub(crate) fn start_router<Buf: PacketBufferMut>(
         let stage_egress = Egress::new("Egress", iftr_factory.handle(), atabler_factory.handle());
         let iprouter1 = IpForwarder::new("IP-Forward-1", fibtr_factory.handle());
         let iprouter2 = IpForwarder::new("IP-Forward-2", fibtr_factory.handle());
-        let stateless_nat = StatelessNat::with_reader("stateless-NAT", nattabler_factory.handle());
+        let static_nat = StatelessNat::with_reader("static-NAT-1", nattabler_factory.handle());
         let stateful_nat = StatefulNat::new(
             "stateful-NAT",
             flow_table_clone.clone(),
@@ -125,8 +125,8 @@ pub(crate) fn start_router<Buf: PacketBufferMut>(
             .add_stage(icmp_error_handler)
             .add_stage(flow_lookup)
             .add_stage(flow_filter)
+            .add_stage(static_nat)
             .add_stage(portfw)
-            .add_stage(stateless_nat)
             .add_stage(stateful_nat)
             .add_stage(iprouter2)
             .add_stage(stage_egress)

flow-entry/src/flow_table/nf_lookup.rs

@@ -62,7 +62,7 @@ mod test {
     use net::FlowKey;
     use net::buffer::PacketBufferMut;
     use net::buffer::TestBuffer;
-    use net::flows::FlowInfo;
+    use net::flows::{FlowInfo, FlowInfoFlags};
     use net::ip::NextHeader;
     use net::ip::UnicastIpAddr;
     use net::packet::Packet;
@@ -93,7 +93,7 @@ mod test {
 
         // Create a packet with the right info
         let mut packet = build_test_ipv4_packet_with_transport(100, Some(NextHeader::TCP)).unwrap();
-        packet.meta_mut().src_vpcd = Some(src_vpcd);
+        packet.meta_mut().set_src_vpcd(Some(src_vpcd));
         packet.set_ip_source(src_ip).unwrap();
         packet.set_ip_destination(dst_ip).unwrap();
         packet.set_tcp_source_port(src_port).unwrap();
@@ -198,7 +198,13 @@ mod test {
 
             // create a pair of related flow entries; flow_2 will get a longer timeout
             let expires_at = tokio::time::Instant::now().into_std() + Duration::from_secs(2);
-            let (flow_1, flow_2) = FlowInfo::related_pair(expires_at, key_1, key_2);
+            let (flow_1, flow_2) = FlowInfo::related_pair(
+                expires_at,
+                key_1,
+                FlowInfoFlags::default(),
+                key_2,
+                FlowInfoFlags::default(),
+            );
             assert_eq!(Arc::weak_count(&flow_1), 1);
             assert_eq!(Arc::weak_count(&flow_2), 1);
             assert_eq!(Arc::strong_count(&flow_1), 1);

flow-filter/Cargo.toml

@@ -6,6 +6,7 @@ publish.workspace = true
 version.workspace = true
 
 [dependencies]
+bitflags = { workspace = true }
 common = { workspace = true }
 concurrency = { workspace = true }
 config = { workspace = true }

flow-filter/src/lib.rs

@@ -307,8 +307,11 @@ impl FlowFilter {
         if data.requires_stateful_nat() {
             packet.meta_mut().set_stateful_nat(true);
         }
-        if data.requires_stateless_nat() {
-            packet.meta_mut().set_stateless_nat(true);
+        if data.requires_static_nat_src() {
+            packet.meta_mut().set_static_nat_src(true);
+        }
+        if data.requires_static_nat_dst() {
+            packet.meta_mut().set_static_nat_dst(true);
         }
         if data.requires_port_forwarding(get_l4_proto(packet)) {
             packet.meta_mut().set_port_forwarding(true);
@@ -319,22 +322,31 @@ impl FlowFilter {
     fn set_nat_requirements_from_flow_info<Buf: PacketBufferMut>(
         packet: &mut Packet<Buf>,
     ) -> Result<(), ()> {
-        let locked_info = packet.meta().flow_info.as_ref().ok_or(())?.locked.read();
+        let flow_info = packet.meta().flow_info.as_ref().ok_or(())?;
+        let needs_static_nat_src = flow_info.get_flags().requires_static_nat_src();
+        let needs_static_nat_dst = flow_info.get_flags().requires_static_nat_dst();
+
+        let locked_info = flow_info.locked.read();
         let needs_stateful_nat = locked_info.nat_state.is_some();
         let needs_port_forwarding = locked_info.port_fw_state.is_some();
         drop(locked_info);
 
         match (needs_stateful_nat, needs_port_forwarding) {
             (true, false) => {
                 packet.meta_mut().set_stateful_nat(true);
-                Ok(())
             }
             (false, true) => {
                 packet.meta_mut().set_port_forwarding(true);
-                Ok(())
             }
-            _ => Err(()),
+            _ => return Err(()),
         }
+        if needs_static_nat_src {
+            packet.meta_mut().set_static_nat_src(true);
+        }
+        if needs_static_nat_dst {
+            packet.meta_mut().set_static_nat_dst(true);
+        }
+        Ok(())
     }
 
     /// Process a packet.
@@ -357,7 +369,7 @@ impl FlowFilter {
             return;
         };
 
-        let Some(src_vpcd) = packet.meta().src_vpcd else {
+        let Some(src_vpcd) = packet.meta().src_vpcd() else {
             debug!("{nfi}: Missing source VPC discriminant, dropping packet");
             packet.done(DoneReason::Unroutable);
             return;

flow-filter/src/tables.rs

@@ -504,9 +504,12 @@ impl RemoteData {
             || self.dst_nat_req == Some(NatRequirement::Stateful)
     }
 
-    pub(crate) fn requires_stateless_nat(&self) -> bool {
+    pub(crate) fn requires_static_nat_src(&self) -> bool {
         self.src_nat_req == Some(NatRequirement::Stateless)
-            || self.dst_nat_req == Some(NatRequirement::Stateless)
+    }
+
+    pub(crate) fn requires_static_nat_dst(&self) -> bool {
+        self.dst_nat_req == Some(NatRequirement::Stateless)
     }
 
     pub(crate) fn requires_port_forwarding(&self, packet_proto: L4Protocol) -> bool {