Skip to content

Pr/fredi/timeouts masquerading#1502

Merged
qmonnet merged 16 commits intomainfrom
pr/fredi/timeouts_masquerading
Apr 28, 2026
Merged

Pr/fredi/timeouts masquerading#1502
qmonnet merged 16 commits intomainfrom
pr/fredi/timeouts_masquerading

Conversation

@Fredi-raspall
Copy link
Copy Markdown
Contributor

@Fredi-raspall Fredi-raspall commented Apr 27, 2026

  • further cleanup stateful implementation
  • unify some types with port forwarding implementation
  • rewrite utils with new packet header matching support
  • add flowstatus + state machine for masqueraded flows
  • set timeouts for masqueraded flows accordingly.
  • The behavior is
    • configured timeout is ignored for ICMP. The timeout is set internally
    • UDP traffic: configured timeout is only applied until traffic gets to "established" state (2-way communication + 1 packet).
    • In the case of DNS (udp 53), the flow is closed (and flows cancelled) on the response.
    • TCP: user timeout ignored until flow becomes established.

Fixes: https://github.com/githedgehog/internal/issues/364

NOTE: this targets #1499
NOTE: this recreates #1501, which can be ignored

@Fredi-raspall Fredi-raspall requested a review from a team as a code owner April 27, 2026 15:51
@Fredi-raspall Fredi-raspall requested review from Copilot and sergeymatov and removed request for a team April 27, 2026 15:51
Copilot AI reviewed Apr 27, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@Fredi-raspall Fredi-raspall force-pushed the pr/fredi/timeouts_masquerading branch from e37d8ef to 1d6792c Compare April 27, 2026 17:57
@Fredi-raspall Fredi-raspall added the dont-merge Do not merge this Pull Request label Apr 27, 2026
Base automatically changed from pr/fredi/fix_dockerfile to main April 27, 2026 22:14
@Fredi-raspall Fredi-raspall requested a review from Copilot April 28, 2026 07:03
Copilot AI reviewed Apr 28, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.

Comment thread nat/src/stateful/nf.rs
Comment thread nat/src/portfw/protocol.rs Outdated
Comment thread nat/src/stateful/protocol.rs
Comment thread flow-entry/src/flow_table/mod.rs
@Fredi-raspall
feat(port-forwarding): use nat_packet() instead of dnat_packet()
f844c57 
Nat the first packet of a port-forwarded flow using the state
created. This helps in consistency and in exposing a single method
to nat a packet.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(port-forwarding): rename PortFwAction -> NatAction
ebd1da3 
Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(nat): add common module and unify nat actions
7b8bcfb 
Let masquerading and port forwarding use the same type for actions.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(net): do not obfuscate Headers type
b2dd93c 
Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(port-forwarding): rewrite packet utils
c599a09 
Write packet port-forwarding utils using the new pattern
matching. Return failures as errors and log them.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(stateful-nat): add new packet utils for masquerading
99524a0 
The utils:
  - use the new pattern matching
  - distinguish between source and destination address/port nat.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(stateful-nat): masquerade using packet::masquerade()
af3db2c 
Use the new functions based on header pattern matching to masquerade
flows.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(port-forwarding): rename types for flow status
c4dbb84 
Rename:
  PortFwFlowStatus       -> NatFlowStatus
  AtomicPortFwFlowStatus -> AtomicNatFlowStatus

.. so that the same types can be used for masquerading

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(port-forwarding): move flow status types to common
3d51a32 
Moves the types renamed in the prior commit to common/ so that
they can be used for masquerading. Note that while the types are
moved, the uses of the types and semantics may be NAT flavor
specific. In other words, only the types are moved but not some
of the implementations.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(stateful-nat): add AtomicNatFlowStatus to masquerade state
2672e27 
Add field status with type AtomicNatFlowStatus to the masquerade
states created for the flow pair to masquerade traffic.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(stateful-nat): add protocol submodule
3e67a72 
Adds simple flow status state machines for masqueraded flows.
With the state machine defined:
  1) icmp traffic can only be oneway or twoway. The timeout for
     icmp traffic will not configurable by the user.
  2) UDP traffic can be in oneway, twoway or established (3 way).
     Only when reaching established flow timeouts will be the ones
     configured by the user.
  3) TCP traffic uses the same SM as for port-forwarding, reversed.
     Until flows reach established state, their timeouts will not
     be the user configured ones.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(stateful-nat): set timeouts for masqueraded flows
a598347 
- Set the initial timeout for a masqueraded flow.
- Update FlowStatus of masqueraded flows according to SM and proto.
- Set subsequent flow timeouts depending on the FlowStatus.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(stateful-nat): allow refining status based on application
3fff9f2 
Allow "patching" the flow status depending on the application.
This is only implemented for DNS and relying on transport ports as
application identifiers.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(nat,net): misc tracing changes
f57255c 
- fix displays for clearer logs
- remove duplicate logs
- add tracing target to net/flows. We can't have linkme as a dependency
  of net, so we declare the target in flow-entry on behalf of net.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(net): remove doctest which is useless at this point
c1fe18d 
Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall
feat(stateful-nat): work around type issue addresses
b9efa1e 
Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall Fredi-raspall force-pushed the pr/fredi/timeouts_masquerading branch from 1d6792c to b9efa1e Compare April 28, 2026 09:02
qmonnet
qmonnet approved these changes Apr 28, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK to me. This is another large PR and I can't claim to have looked into every single detail, but I went through all commits and they make sense. Given that unit tests are still passing, we're reasonably confident that we don't break everything, so the PR seems good to go. 🚢

@Fredi-raspall Fredi-raspall removed the dont-merge Do not merge this Pull Request label Apr 28, 2026
@Fredi-raspall Fredi-raspall enabled auto-merge April 28, 2026 14:11
@Fredi-raspall Fredi-raspall added this pull request to the merge queue Apr 28, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 28, 2026
@qmonnet qmonnet added this pull request to the merge queue Apr 28, 2026
Merged via the queue into main with commit 7722938 Apr 28, 2026
30 of 31 checks passed
@qmonnet qmonnet deleted the pr/fredi/timeouts_masquerading branch April 28, 2026 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@qmonnet qmonnet qmonnet approved these changes

@sergeymatov sergeymatov Awaiting requested review from sergeymatov sergeymatov is a code owner automatically assigned from githedgehog/dataplane-reviewers

Assignees

@Fredi-raspall Fredi-raspall

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Fredi-raspall @qmonnet