Extend support for port forwarding with masquerading (stateful NAT)#1303
Merged
Fredi-raspall merged 24 commits intomainfrom Mar 2, 2026
Merged
Extend support for port forwarding with masquerading (stateful NAT)#1303Fredi-raspall merged 24 commits intomainfrom
Fredi-raspall merged 24 commits intomainfrom
Conversation
Member
qmonnet
commented
Feb 24, 2026
qmonnet
commented
- Re-issue of Extend support for port forwarding with masquerading (stateful NAT) #1274
- On top of Add support for port forwarding #1262
Fredi-raspall
force-pushed
the
pr/qmonnet/port-forwarding
branch
from
7bc41bd to
4421d8f
Compare
Fredi-raspall
force-pushed
the
pr/fredi/port-forwarding
branch
from
9dabbcd to
f6566b2
Compare
qmonnet
commented
Feb 25, 2026
qmonnet
force-pushed
the
pr/qmonnet/port-forwarding
branch
from
4421d8f to
8f726d9
Compare
Fredi-raspall
force-pushed
the
pr/fredi/port-forwarding
branch
from
f6566b2 to
6fefef0
Compare
qmonnet
force-pushed
the
pr/qmonnet/port-forwarding
branch
3 times, most recently
from
f60437f to
f90f1e7
Compare
Fredi-raspall
force-pushed
the
pr/fredi/port-forwarding
branch
2 times, most recently
from
6a0aab8 to
6497a7c
Compare
Fredi-raspall
force-pushed
the
pr/qmonnet/port-forwarding
branch
from
f90f1e7 to
d54696f
Compare
Fredi-raspall
approved these changes
Mar 2, 2026
| } | ||
|
|
||
| #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] | ||
| pub enum L4Protocol { |
Contributor
There was a problem hiding this comment.
Do we need this type? Couldn't we just use Option<NextHeader> ?, ... with None indicating Any?
This way we'd not need:
- the translations.
- the intersections()
Also, does this need to live in the lpm crate?
Member
Author
There was a problem hiding this comment.
Do we need this type? Couldn't we just use
Option<NextHeader>?, ... with None indicating Any? This way we'd not need:* the translations. * the intersections()
It would probably work just as well with an Option<NextHeader>, but I'm not a fan of using None to actually mean “both protocols”. We're doing that with elsewhere for other types already, but I'm not particularly happy with it.
I agree we wouldn't need translation, I disagree for the intersection?
Also, does this need to live in the lpm crate?
No, it doesn't have to. Maybe not even the right place - I think I put it there because I initially considered adding the proto to the PrefixWithOptionalPorts, but in the end I didn't. I'm happy if we move it.
Contributor
There was a problem hiding this comment.
It would probably work just as well with an Option, but I'm not a fan of using None to actually mean “both protocols”. We're doing that with elsewhere for other types already, but I'm not particularly happy with it.
I see None as "any" or "don't care"; i.e. no need to restrict to any proto in particular.
Add the PortForwarding variant to struct VpcExposeNatConfig. We also add the related methods: - VpcExposeNat.is_port_forwarding() - VpcExpose.make_port_forwarding() - VpcExpose.has_port_forwarding() Consequence of the addition of the new variant, we also update function get_nat_requirement() in the flow-filter stage setup code, basing it on a new From<&VpcExposeNatConfig> for NatRequirement implementation. Signed-off-by: Quentin Monnet <qmo@qmon.net>
As part of the port forwarding support, we are able to change the structure of enum VpcdLookupResult, and we won't be able to derive the Copy trait for the new one. Let's remove it now, and adjust the code where necessary. Signed-off-by: Quentin Monnet <qmo@qmon.net>
As part of the work for port forwarding support, and in particular, to support both port forwarding and masquerading at the same time on the same end of a given VPC peering, we need to keep track of some destination-related information even when several destination blocks can match during the flow-filter lookup. To that end, we add a set of RemoteData objects to the MultipleMatches variant. This will be used in follow-up commit to handle overlap between the prefixes of masquerading and port forwarding. Signed-off-by: Quentin Monnet <qmo@qmon.net>
In preparation for adjusting tests for the flow-filter stage setup submodule, introduce (and use) small helpers to produce VpcDiscriminant or Vni objects in a less verbose way. Signed-off-by: Quentin Monnet <qmo@qmon.net>
As part of the work for port forwarding support, we want to update the flow-filter stage and have it work with a specific configuration, namely: when both port forwarding and masquerading are configured for distinct expose blocks, but on the same side of one given VPC peering. So far, when the flow-filter stage would find conflicting results for the destination VPC lookup (when several destinations may match), it would simply return the MultipleMatches variant and leave it at that. In a previous commit, we extended this variant to make it able to hold data: in the current commit, we populate this data to be able to validate whether a flow is legit, and whether the NAT requirement for the packet should be masquerading (stateful NAT) or port forwarding. Tests come in a follow-up commit. Signed-off-by: Quentin Monnet <qmo@qmon.net>
Now that we've updated, in a recent commit, the flow-filter table to contain information in case of overlap between expose blocks on the same side of a VPC peering with masquerading (stateful NAT) and port forwarding, we need to update the logic in the packet-processing code to adjust the flow-filter decisions accordingly. In particular, when we have multiple matches (for the same destination VPC), but not flow table entry, determine the NAT requirements based on the direction; when the flow table entry is present, determine the requirements based on the nature of that entry (whether it's been created for stateful NAT or for port forwarding). [ Fredi: Fix requires_port_forwarding() ] Signed-off-by: Quentin Monnet <qmo@qmon.net> Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
Signed-off-by: Quentin Monnet <qmo@qmon.net>
Remove the description of validation steps on top of VpcExpose.validate(): we haven't kept them up-to-date, and we don't really need to anyway, all checks are documented in the body of the method. Also remove numbers between the different checks. I find they make it clearer to figure out where we are in the process, and to designate a specific step when discussing the code, but it forces developers to make contortions (step 0) or to adjust many numbers when doing some changes. It's probably easier to just get rid of these step numbers. Signed-off-by: Quentin Monnet <qmo@qmon.net>
Make sure that the config is valid for the needs of port forwarding. Signed-off-by: Quentin Monnet <qmo@qmon.net>
As part of the Peering validation steps, reject incompatible NAT modes (when a Peering manifest has NAT configured for both sides, in a way that is not currently supported). This does not address multiple NAT modes on _one_ side of a peering. Signed-off-by: Quentin Monnet <qmo@qmon.net>
As part of the support for port forwarding, update the user configuration validation to support the case when a VPC peering manifest uses masquerading and port forwarding on a same end (via two expose blocks). We reject all other NAT combinations on a same end. Signed-off-by: Quentin Monnet <qmo@qmon.net>
We've been growing a decent portion of the vpcpeering.rs code to deal with overlap validation for the expose blocks. In order to keep this file clearer, move the overlap-related methods to a separate file under the utils submodule. Signed-off-by: Quentin Monnet <qmo@qmon.net>
Add some tests to check that the flow-filter stage works with port forwarding and with the additional prefix overlap use case, in particular when Expose objects on the same side of a peering use a combination of stateful NAT + port forwarding. Signed-off-by: Quentin Monnet <qmo@qmon.net>
We plan to re-use some of these types in the stateful NAT code. They're not proper to stateless NAT anyway; so let's move them to a dedicated module at the top of the crate. We could even move them to another crate in the future, if other components need to share them. For types where all members are public, and "new()" implementation is a straightforward struct build, remove the "new()" constructor and replace it with a direct object build, to avoid having several ways to build the objects. This change accounts for a good portion of the churn in the range_builder stateless NAT submodule. Signed-off-by: Quentin Monnet <qmo@qmon.net>
As part of the work to add support for port forwarding, we want to reserve some specific ports in the stateful NAT allocator, to prevent the allocator from using them. This is the case of ports that should be used for port forwarding only, when both stateful NAT and port forwarding are in use on the same side of a VPC peering manifest. Prepare the allocator for reserving these ports: compute the list of ports to reserve, and attach it to NatPool objects when creating them. [ Fredi: Add port_forwarding_exposes() ] Signed-off-by: Quentin Monnet <qmo@qmon.net> Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
In a configuration when VPC 1 is peered with VPC 2, and VPC 1 uses both port forwarding and masquerading (stateful NAT) on its side of the peering to communicate with VPC 2, we may need to block some ports from being allocated for stateful NAT. What happens if we don't reserve ports? For example: 1. Endpoint A in VPC 1 attempts to open a connection with endpoint X in VPC 2. 2. The connection gets masqueraded; bad luck, the allocated IP and port are exactly the same as those exposed via port forwarding. Flow table sessions are created for both directions, based on this IP and port. 3. The packet is translated with these IP and port, and is sent to endpoint X. 4. The packet is either dropped later on the path, or endpoint X closes the connection for some reason. 5. Endpoint X attempts to reach the service exposed on VPC 1 via port forwarding. It uses the port forwarding IP and port and sends its packet. Bad luck (again), it also uses the same source port as the previous packet targeted. 6. Because we have a matching flow table entry, we translate back the packet, and send it to endpoint A. But wait, endpoint X wanted to use the service behind port forwarding - we have no guarantee that it is endpoint A, it could well be some endpoint B on VPC 1 instead! To avoid this unlikely, but possible corner case, we need to block the ports used for port forwarding from being allocated for masquerading. Update the stateful NAT allocator to mark IPs (if all ports are concerned) or port ranges (otherwise) in a port block allocator as unavailable, when creating the object pools, so that stateful NAT can never assigns them when they should be "reserved" for port forwarding. Signed-off-by: Quentin Monnet <qmo@qmon.net>
This is in preparation for port forwarding support. When converting the K8s configuration into VpcExpose resources, we would produce internally one VpcExpose for each expose blocks in the configuration. Now that we're adding support for port forwarding, we're changing that: we want to be able to produce several VpcExpose from one expose blocks. The rationale for this is that in the new API, the "ports" block in the port forwarding configuration object "links" original and target port ranges for port forwarding _within_ an expose blocks - a structure that we don't otherwise support internally, hence the need to convert into separate VpcExpose objects. Signed-off-by: Quentin Monnet <qmo@qmon.net>
Add support for converting port forwarding configuration received from Kubernetes into the internal objects that we use to actually implement the feature. We need a second NAT-processing step for the case of port forwarding. This is because we need to initialise the VpcExpose's NAT object before collecting the target prefixes with process_as_block() (or we won't have a NAT block to attach these target prefixes to), so process_nat_block() comes first; but we need the target prefixes to expand the port range rules into several expose blocks, so we give it another pass with nat_expand_rules(). Also update the related Bolero generator and test accordingly. Note: Support is not complete yet, we do not parse and account for the L4 protocols (TCP/UDP) yet. Signed-off-by: Quentin Monnet <qmo@qmon.net>
Stop exposing the BTreeSet-based sets used to store the original and target (and exclusion) prefixes for expose blocks. Instead, encapsulate them into a wrapper type that will be easier to modify, if necessary, and extend. We introduce the new type in lpm/src/prefix/with_ports.rs, where PrefixWithOptionalPorts is already defined. Then we use it for the VpcExpose's lists in config/src/external/overlay/vpcpeering.rs; the rest of the changes are just the resulting propagation. Signed-off-by: Quentin Monnet <qmo@qmon.net>
Rather than having a function that works on two PrefixPortsSet to compute the resulting set of intersections, make it part of the implementation of PrefixPortsSet itself, now that the type exists. Move the related tests so that they remain close to the code. Signed-off-by: Quentin Monnet <qmo@qmon.net>
Port forwarding has been designed with the TCP/UDP dissociation in mind,
contrarily to stateless NAT for example, which postponed it to future
work and handles both protocols indifferently, without offering a way to
configure port translation for juts one of the two.
But to support the TCP/UDP split, we're currently missing a few
elements, in particular:
1. We need to adjust the VpcExpose structure (or its child structures)
to store information about the L4 protocol that a rules relates to.
2. We need to use this information to build the context for port
forwarding accordingly.
3. We need to use this information when reserving ports in the
stateful NAT allocator, in case of masquerading and port forwarding
superposition on the same end of a VPC peering's manifest.
4. We need to account for the TCP/UDP dissociation in the flow-filter,
when looking at the NAT requirements for a packet of a given
protocol.
This commit addresses item 1 by extending VpcExposeNat with L4 protocol
information. It will be reused in subsequent commits to address the
other items.
Signed-off-by: Quentin Monnet <qmo@qmon.net>
Given that port forwarding makes a distinction, in its rules, between TCP and UDP, we must account for the L4 protocol in use when reserving ports in the stateful NAT allocator, for the case of masquerading and port forwarding superposition on the same end of a VPC peering's manifest. It's actually not as big a change as one might fear: we need to adjust find_masquerade_portfw_overlap() to account for L4 protocol information stored in VpcExposeNat objects, and use it when building the allocators for TCP and UDP, that were already distinct. One consequence is that the allocators for TCP and UDP (and ICMP) are now different, we can no longer copy the TCP one into the other two to avoid repeating the creation process. This is cleaner, but slightly more expensive. We also extend the tests accordingly. Signed-off-by: Quentin Monnet <qmo@qmon.net>
Port forwarding accepts rules that apply only to TCP, or UDP, or to both protocols. When evaluating the NAT requirements for a given packet, we need to account for the protocols used for these rules. Let's update the flow-filter stage. To do so, we add L4 protocol information to the NatRequirement::PortForwarding enum variant, and take it into account when evaluating it to determine NAT requirements. Add associated unit tests (generated by Claude Opus 4.6). Signed-off-by: Quentin Monnet <qmo@qmon.net>
Do not consider prefixes as overlapping when the expose block they belong to has port forwarding set up, and they refer to different layer 4 protocols. Signed-off-by: Quentin Monnet <qmo@qmon.net>
Fredi-raspall
force-pushed
the
pr/qmonnet/port-forwarding
branch
from
d54696f to
299bb7f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.