Add support for port forwarding by Fredi-raspall · Pull Request #1262 · githedgehog/dataplane

Fredi-raspall · 2026-02-04T12:43:59Z

~~This goes on top of #1257~~

qmonnet · 2026-02-06T21:31:50Z

Note that I rebased the base branch on main. I didn't rebase this PR because I don't know whether you have local work in progress on your branch.

qmonnet

Some early feedback, even though I know it's still in progress.

Apologies if some of these comments are addressed in your -cont branch, I haven't been through that one yet.

qmonnet · 2026-02-20T14:10:18Z

nat/src/portfw/portfwtable.rs

+        if key.src_vpcd == entry.dst_vpcd {
+            return Err(PortFwTableError::Unsupported(
+                "Can't do port-forwarding within the same VPC".to_string(),
+            ));
+        }


What's the point of this check? Do we need it from a technical perspective for port forwarding? If not, and if we decide to forbid hairpinning, should it be at a more generic location (knowing that the other NAT modes don't enforce it, as far as I remember)?

That's a fair point. I can relax this check. However, we don't support peerings between VPC-A ------ VPC-A ..... do we? So, that won't be possible in any NAT or non-NAT case anyway, right?

I think you're correct

nat/src/portfw/portfwtable.rs

qmonnet · 2026-02-20T14:24:42Z

nat/src/portfw/packet.rs

+/// Perform source-nat/pat for a packet. Returns true if the packet could be source-natted and false otherwise
+pub(crate) fn snat_packet<Buf: PacketBufferMut>(
+    packet: &mut Packet<Buf>,
+    new_src_ip: UnicastIpAddr,
+    new_src_port: NonZero<u16>,
+) -> bool {


Nit: Can we return a Result rather than a bool? Same for dnat_packet()

Yeah, we could.

nat/src/portfw/nf.rs

nat/src/portfw/flow_state.rs

nat/src/portfw/test.rs

qmonnet · 2026-02-20T14:58:06Z

nat/src/portfw/flow_state.rs

-        debug!("Created flow entry with port-forwarding state; key={flow_key} info={flow_info}");
+        debug!("Created flow entry with port-forwarding state;\nkey={flow_key}\ninfo={flow_info}");


Is it OK to insert line breaks inside of formatted logs? I know we have line breaks when dumping large structures, but for something smaller I'd expect to use just one line so we keep the format with the timestamp and log level etc. on the line?

I believe it is up to us. For stuff that spans multiple lines, I like the beginning of the blob not to be indented for readability, but I guess this is subjective.

qmonnet · 2026-02-20T15:01:47Z

nat/src/portfw/test.rs

        assert_eq!(output.ip_destination().unwrap().to_string(), "192.168.1.2");
        assert_eq!(output.udp_source_port().unwrap().as_u16(), 9876);
        assert_eq!(output.udp_destination_port().unwrap().as_u16(), 53);
+        assert!(output.meta().flow_info.is_none());


Here you could also assert_eq!(flow_table.len(), 2);

Also, extend the metadata to 32 bits and the Display impl. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Extend the enum with PortForwarding. The expectation is that from the API, we'll be explicitly told about port forwarding. As a result, the contents of the flow-filter will include that infor- mation which will allow us to annotate the packet accordingly to steer the packet through the PortForwarder. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Do not unnecessarily propagate src/dst vpc discriminants. The stateful NAT NF requires packets to have been annotated with both of them, but it only needs them if there is no session. Instead of always retrieving them and passing them along, keep the check that they are present, but just retrieve them from the packet given that we need to pass a reference to it to modify it. Also, given that the flow-filter is the one responsible for determining src & dst vpcd, and annotating that nat is needed, the stateful nat function should never get a packet without those annotations. Therefore, add a debug assert to the existing check. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Adds generic types to support non-overlapping ranges in general and a specific variant to support sets of disjoint prefixes. The type allows storing a collection of prefixes and associated value for each, and perform membership queries. E.g. given an address, a query returns the prefix the address belongs to (if any) and the value associated to the prefix. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

The PortForwarder object is the entity responsible for port- forwarding all of the packets received from a VPC for udp or tcp. This entity is introduced to avoid linear lookups to search for matching prefixes and ports, that were introduced when migrating from single-address rules to prefix-based rules. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Move PortFwFlowStatus to a separate submodule, as we will refine it in the future. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Set the correct vpcd in the flow for the reverse flow with port- forwarding. This was broken when removing the dst-vpcd from the FlowKey. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

The state machine for port-forwarded TCP flows is intended, atm, solely to determine how much flow entries should be kept. Given that we cannot reduce the timeouts of existing entries, this patch extends the state machine to determine better if a TCP connection is closed, so that rather than reducing the flows lifetimes, these can be removed. Note that the approach is still very primitive since we do not track sequence/ack numbers. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

If a TCP connection reaches state Closed, invalidate the corres- ponding flows in each direction. This marks the flows entries as Cancelled. The way the flow table works at the momemnt, this will cause flows to be removed lazily, if a lookup occurs. Otherwise, the flows will remain until timed out. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Set distinct default timeouts for established state for UDP and TCP and adjust the tests accordingly and considering also that we invalidate flows when they transition to Closed state. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

This patch agressively removes flow entries when the flow table occupancy exceeds a threashold. This is a sanity to protect dataplane's flow table from unnecessarily growing, given that: - in some cases we don't detect that communications are over, as we don't analyze encaped protocols (e.g. UDP). - we don't explicitly remove flow entries even when we know, but instead cancel them. This could be alleviated by directly removing those, but that requires recovering or rebuilding flow keys, which we currently don't. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

qmonnet

Blanket approval from the escalator on my way out of the underground 🙈

Fredi-raspall requested a review from qmonnet February 4, 2026 12:44

qmonnet assigned Fredi-raspall and qmonnet Feb 4, 2026

qmonnet changed the title ~~Pr/fredi/port forwarding~~ Add support for port forwarding with masquerading (stateful NAT) Feb 4, 2026

qmonnet added the area/nat Related to Network Address Translation (NAT) label Feb 4, 2026

qmonnet linked an issue Feb 4, 2026 that may be closed by this pull request

Support port forwarding for masquerading (stateful NAT) #1259

Closed

Fredi-raspall added the dont-merge Do not merge this Pull Request label Feb 4, 2026

qmonnet force-pushed the pr/fredi/nat_use_cases branch from 50d68da to 5b0b995 Compare February 6, 2026 21:30

Fredi-raspall force-pushed the pr/fredi/port-forwarding branch 3 times, most recently from 4764633 to 7087c7c Compare February 9, 2026 16:39

qmonnet force-pushed the pr/fredi/nat_use_cases branch 2 times, most recently from c7b016c to 0ade936 Compare February 9, 2026 17:18

Fredi-raspall force-pushed the pr/fredi/port-forwarding branch 2 times, most recently from d46f0ac to 96886ec Compare February 10, 2026 10:42

qmonnet mentioned this pull request Feb 10, 2026

Extend support for port forwarding with masquerading (stateful NAT) #1274

Closed

Fredi-raspall force-pushed the pr/fredi/nat_use_cases branch 2 times, most recently from d1de973 to 8840886 Compare February 18, 2026 11:45

Fredi-raspall force-pushed the pr/fredi/port-forwarding branch from cf14551 to d46ff5c Compare February 18, 2026 11:52

Fredi-raspall changed the title ~~Add support for port forwarding with masquerading (stateful NAT)~~ Add support for port forwarding Feb 18, 2026

Fredi-raspall mentioned this pull request Feb 18, 2026

remove bidirectional flow keys in flow table #1295

Merged

Fredi-raspall force-pushed the pr/fredi/nat_use_cases branch from 8840886 to 1ad90dc Compare February 19, 2026 21:34

Fredi-raspall force-pushed the pr/fredi/port-forwarding branch from 6098287 to 6fd1ccd Compare February 19, 2026 21:34

Base automatically changed from pr/fredi/nat_use_cases to main February 20, 2026 00:33

qmonnet reviewed Feb 20, 2026

View reviewed changes

Fredi-raspall force-pushed the pr/fredi/port-forwarding branch from a339eaa to 66c898a Compare February 23, 2026 21:26

Fredi-raspall added 3 commits February 23, 2026 22:30

feat(net): add port-forwarding metadata flag

07a591a

Also, extend the metadata to 32 bits and the Display impl. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Fredi-raspall added 11 commits February 28, 2026 20:14

feat(lpm): add IpPrefixTrie get(), iter() and iter_mut()

0cd26a0

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

feat(port-forwarding): add lpm map

182ebb8

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

feat(port-forwarding): use the PortForwarder

9c89452

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

feat(port-forwarding): logging improvements

37a3c5c

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

feat(port-forwarding): move PortFwFlowStatus

34b1949

Move PortFwFlowStatus to a separate submodule, as we will refine it in the future. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

feat(port-forwarding): fix dst vpcd in flow

123767b

Set the correct vpcd in the flow for the reverse flow with port- forwarding. This was broken when removing the dst-vpcd from the FlowKey. Signed-off-by: Fredi Raspall <fredi@githedgehog.com>

Fredi-raspall force-pushed the pr/fredi/port-forwarding branch from 418c633 to 6a0aab8 Compare March 2, 2026 16:53

Fredi-raspall force-pushed the pr/fredi/port-forwarding branch from 6a0aab8 to 6497a7c Compare March 2, 2026 18:49

Fredi-raspall marked this pull request as ready for review March 2, 2026 19:04

Fredi-raspall requested a review from a team as a code owner March 2, 2026 19:04

Fredi-raspall requested review from daniel-noland and removed request for a team March 2, 2026 19:04

Fredi-raspall removed the dont-merge Do not merge this Pull Request label Mar 2, 2026

Fredi-raspall enabled auto-merge March 2, 2026 19:05

Fredi-raspall requested review from mvachhar and qmonnet and removed request for daniel-noland March 2, 2026 19:14

qmonnet approved these changes Mar 2, 2026

View reviewed changes

Fredi-raspall added this pull request to the merge queue Mar 2, 2026

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 2, 2026

Fredi-raspall added this pull request to the merge queue Mar 2, 2026

Merged via the queue into main with commit 076d20d Mar 2, 2026
21 of 22 checks passed

Fredi-raspall deleted the pr/fredi/port-forwarding branch March 2, 2026 20:28

		debug!("Created flow entry with port-forwarding state; key={flow_key} info={flow_info}");
		debug!("Created flow entry with port-forwarding state;\nkey={flow_key}\ninfo={flow_info}");

Conversation

Fredi-raspall commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qmonnet commented Feb 6, 2026

Uh oh!

qmonnet left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qmonnet left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fredi-raspall commented Feb 4, 2026 •

edited

Loading

qmonnet left a comment •

edited

Loading