-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: self-healing NAT mappings with request deduplication #3367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@MarcoPolo @sukunrt I've resolved the conflicts (d4ac0de) caused by the grand log refactor that landed in master. Would it be possible to review this bugfix and ship in the next go-libp2p? 🙏 |
|
Yep! I'll take a look this week :)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor nits. Looks good. Thanks for this and for testing it!
5f61fa8 to
d59d597
Compare
|
I’ll deal with the conflicts and merge tomorrow |
Router restarts can cause UPnP/NAT-PMP services to change their listening ports, leading to connection refused errors. This fix implements automatic NAT rediscovery when consecutive connection failures are detected, restoring all existing port mappings on the new NAT instance. See #3224 (comment) for details on the router behavior that motivated this fix.
d59d597 to
c916a49
Compare
Multiple libp2p transports can share the same port (TCP, QUIC, WebTransport, WebRTC-direct), causing duplicate AddMapping calls for the same protocol/port combination. This fix adds deduplication in NAT.AddMapping to prevent redundant NAT device operations and reduce log spam.
c916a49 to
ff8b509
Compare
|
Is self-healing NAT a planned feature for Rust? Happy to begin working on a port if there's interest. |
|
@jxs might be able to comment on how self-healing NAT fits into rust-libp2p plans. |
This PR makes NAT port mappings self-healing after router restarts.
It also adds bunch of tests and eliminates duplicate mapping requests that occur on node start.
Changes
1. Automatic NAT recovery after router restart
Router restarts can cause UPnP (SOAP) services to change their listening ports, leading to "connection refused" errors that permanently break port mappings.
This is the problem described in #3224 (comment) (cc @sukunrt @MarcoPolo)
We had Kubo users impacted this since forever, impacting their ability to provide data:
This PR implements automatic NAT rediscovery that detects consecutive connection failures, triggers rediscovery after 3 failures, and restores all existing port mappings on the new NAT instance.
FWIW I've tested it extensively in my own LAN – see "Demo" at the end.
IIUC this
2. Port mapping deduplication
This is something I've discovered while testing (1), so also fixed it in this PR.
Multiple libp2p transports sharing the same port (TCP, QUIC, WebTransport, WebRTC-direct) were causing duplicate NAT mapping requests. The fix adds deduplication in
NAT.AddMapping()that checks if mapping already exists before making NAT API calls.In case of Kubo IPFS node:
Demo
The following log shows automatic NAT recovery in action. The router's UPnP service (miniupnpd) restarts and changes its SOAP/HTTP endpoint from port 45251 to a different port, causing "connection refused" errors. After 3 consecutive failures, go-libp2p automatically rediscovers the NAT and restores all mappings:
Key moments in the log:
Without this fix, the connection refused errors would continue indefinitely, requiring manual daemon restart.