King should open ames ports via NAT-PMP#3261
Merged
Merged
Conversation
(Not yet hooked up to anything.)
There was a TODO in runShips about how the different layers of the RIO environment had to be changed, so that there was a layer between KingEnv and PierEnv for things shared between individual Piers, but which weren't used outside of any PierEnv. This addresses those TODOs by creating RunningEnv, which for now just owns MultiEyreApi and makes it so we don't have to pass the entire thing around explicitly. The IP handling stuff will go in RunningEnv in a following patch.
This sets up a separate thread to handle scheduling of lease renewals.
Tested with a comet trying to receive traffic from a planet in the cloud. (h/t ~master-morzod)
Also: error checking.
Contributor
|
How widely supported is NATPMP as opposed to PCP? It appears this has been a successor protocol since 2013: https://tools.ietf.org/html/rfc6887 |
Contributor
Author
|
I'm not aware of anyone that uses PCP. It's either NAT-PMP, or the older XML based UPnP IGD protocol. A quick peek at a two torrent clients show that neither supports PCP. |
Contributor
Author
|
Friendly ping. |
pilfer-pandex
suggested changes
Aug 13, 2020
pilfer-pandex
left a comment
Contributor
There was a problem hiding this comment.
Lots of comments, mostly nits.
| ":", err, ", disabling NAT-PMP") | ||
| loopErr q | ||
| Right _ -> do | ||
| let filteredPort = filterPort p nextRenew |
Contributor
There was a problem hiding this comment.
Does this amount to quadratic behavior?
Contributor
Author
|
Did most of the nits plus separated out the TQueue from the heap. The things I left unaddressed and unresolved were things I don't understand the point you're trying to make. |
| Left err -> do | ||
| ip <- likelyIPAddress | ||
| case ip of | ||
| Just (192, 168, c, d) -> do |
Contributor
There was a problem hiding this comment.
Yeah. IPv4 allocates two blocks for private networks: 10/8 and 192.168/16. You handle only the second, but we should be handling both.
Comment on lines
+70
to
+85
| typedef struct { | ||
| uint16_t type; /* NATPMP_RESPTYPE_* */ | ||
| uint16_t resultcode; /* NAT-PMP response code */ | ||
| uint32_t epoch; /* Seconds since start of epoch */ | ||
| union { | ||
| struct { | ||
| //in_addr_t addr; | ||
| struct in_addr addr; | ||
| } publicaddress; | ||
| struct { | ||
| uint16_t privateport; | ||
| uint16_t mappedpublicport; | ||
| uint32_t lifetime; | ||
| } newportmapping; | ||
| } pnu; | ||
| } natpmpresp_t; |
Contributor
There was a problem hiding this comment.
a language that has a proper datatype description language
pilfer-pandex
approved these changes
Aug 14, 2020
joemfb
requested changes
Aug 14, 2020
joemfb
requested changes
Aug 17, 2020
belisarius222
pushed a commit
that referenced
this pull request
Dec 1, 2020
King should open ames ports via NAT-PMP
pkova
added a commit
to urbit/vere
that referenced
this pull request
Apr 24, 2024
Same as urbit/urbit#3261 but for vere. From what I can tell this NAT-PMP stuff is fairly well supported by routers, works on my machine at least.
pkova
added a commit
to urbit/vere
that referenced
this pull request
May 10, 2024
Same as urbit/urbit#3261 but for vere. From what I can tell this NAT-PMP stuff is fairly well supported by routers, works on my machine at least.
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
4 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.
This vendors libnatpmp and writes Haskell bindings to it. It then adds support to king haskell to use this to open up the ames port to the world. This way, people behind consumer grade residential routers should be able to get direct routes.
This is on by default, but can be disabled in hosting contexts with
--no-port-forwarding.This patch also does a minor reorganization of the RIO environments, adding a middle "Running" environment between KingEnv and PierEnv, address some TODOs. I did this since this was the natural place to also put a PortControllerApi.
@joemfb: concept, along with bindings.c, the one new C file I added (though it's mostly code adapted from libnatpmp's test suite).
@pilfer-pandex: all the haskell code here. This is the first time I've written any hs2c or used the FFI.