feat(tests): Add a router with NAT functionality to proto tests#596
Open
feat(tests): Add a router with NAT functionality to proto tests#596
Conversation
This introduces a new router that simulates a very basic NAT traversal. Both client and server are behind a NAT that needs to be opened first. It makes the Pair have fixed SocketAddr for the client and server TestEndpoints. These SocketAddrs are never bound so we may as well keep them consistent and follow an easy pattern. It introduces a trait for the routing table, so as to not interfere with the routing table from the proptests. I did not know how to turn that routing table into NAT router that would be comprehensible. Creating a new one seemed the easier solution. We can now add more specific routers as well which could be handy.
This was some left over from when there was a method to bind sockes for real. Which we removed already.
flub
commented
Apr 16, 2026
matheus23
reviewed
Apr 17, 2026
matheus23
requested changes
Apr 17, 2026
Comment on lines
+129
to
+132
| /// Drive both endpoints once, optionally preventing them from receiving traffic. | ||
| /// | ||
| /// Returns `false` if the connection is idle after the step. | ||
| pub(super) fn blackhole_step( |
Member
There was a problem hiding this comment.
Can you undo moving this function? This seems like needless diff noise, no?
Collaborator
Author
There was a problem hiding this comment.
Hum, perhaps. I was hoping this would be fine, but I could do a separate PR if you insist. I found it really hard to have to go look somewhere seemingly random instead of where it is used. It seems like it just drifted away by accident.
Member
There was a problem hiding this comment.
It's fine if you really want to leave it. But IMO it just makes git blame harder for very little benefit (I mostly navigate by IDE references though, so YMMV).
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/noq/pr/596/docs/noq/ Last updated: 2026-04-17T16:04:17Z |
Performance Comparison Report
|
| Scenario | noq | upstream | Delta | CPU (avg/max) |
|---|---|---|---|---|
| large-single | 5627.3 Mbps | 8066.4 Mbps | -30.2% | 95.1% / 100.0% |
| medium-concurrent | 5526.4 Mbps | 7584.4 Mbps | -27.1% | 96.2% / 149.0% |
| medium-single | 4144.5 Mbps | 4631.0 Mbps | -10.5% | 99.6% / 150.0% |
| small-concurrent | 3910.8 Mbps | 5186.9 Mbps | -24.6% | 92.9% / 102.0% |
| small-single | 3519.4 Mbps | 4662.9 Mbps | -24.5% | 94.4% / 102.0% |
Netsim Benchmarks (network simulation)
| Condition | noq | upstream | Delta |
|---|---|---|---|
| ideal | 3031.3 Mbps | 4063.9 Mbps | -25.4% |
| lan | 782.4 Mbps | 810.3 Mbps | -3.4% |
| lossy | 69.8 Mbps | 55.9 Mbps | +25.1% |
| wan | 83.8 Mbps | 83.8 Mbps | ~0% |
Summary
noq is 24.0% slower on average
4e5b4085f58aa838d6fc3b95113bdfcbb98e12ce - artifacts
No results available
d8410f22d72fb28b7074046cbdad41edc201421c - artifacts
Raw Benchmarks (localhost)
| Scenario | noq | upstream | Delta | CPU (avg/max) |
|---|---|---|---|---|
| large-single | 5659.8 Mbps | 7975.4 Mbps | -29.0% | 98.2% / 148.0% |
| medium-concurrent | 5456.8 Mbps | 7736.6 Mbps | -29.5% | 98.0% / 149.0% |
| medium-single | 3867.1 Mbps | 4611.6 Mbps | -16.1% | 92.0% / 100.0% |
| small-concurrent | 3908.4 Mbps | 5286.9 Mbps | -26.1% | 96.0% / 102.0% |
| small-single | 3540.7 Mbps | 4737.7 Mbps | -25.3% | 97.5% / 151.0% |
Netsim Benchmarks (network simulation)
| Condition | noq | upstream | Delta |
|---|---|---|---|
| ideal | 3117.8 Mbps | 4064.7 Mbps | -23.3% |
| lan | 782.4 Mbps | 810.3 Mbps | -3.4% |
| lossy | 69.8 Mbps | 69.9 Mbps | ~0% |
| wan | 83.8 Mbps | 83.8 Mbps | ~0% |
Summary
noq is 25.1% slower on average
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.
Description
This introduces a new router that simulates a very basic NAT
traversal. Both client and server are behind a NAT that needs to be
opened first.
It makes the Pair have fixed SocketAddr for the client and server
TestEndpoints. These SocketAddrs are never bound so we may as well
keep them consistent and follow an easy pattern.
It introduces a trait for the routing table, so as to not interfere
with the routing table from the proptests. I did not know how to turn
that routing table into NAT router that would be
comprehensible. Creating a new one seemed the easier solution. We can
now add more specific routers as well which could be handy.
Finally it cleans up the CLIENT_PORTS, SERVER_PORTS globals, they
really were a left over from the real socket binding functionality
removed in #537.
Breaking Changes
n/a
Notes & open questions
The newly added test currently only demonstrates this. It does not yet
do anything useful. It should probably be removed and be part of the
follow up PR I will make that builds on this. But I thought it would
be better to split off this functionality into its own PR.
Change checklist