Repository navigation
feat(envd): iptables DNAT backend for IPv4 port forwarding - #2697
Closed
ValentaTomas wants to merge 7 commits into
Closed
ValentaTomas wants to merge 7 commits into
ValentaTomas wants to merge 7 commits into
Conversation
…hysteresis
gopsutil's net.Connections walks /proc/<pid>/fd for every process every
second to attach PIDs to TCP sockets. The port forwarder never used the
PID for anything; it was a key component of the (pid, port) lookup map
but only because that's what the gopsutil call happened to provide. On a
sandbox running ~100 processes × ~20 FDs that's thousands of readlink
syscalls per scan.
- Replace the gopsutil call with a tiny /proc/net/tcp{,6} parser that
extracts (family, ip, port, status) for LISTEN sockets only. No /proc
walk; one read per protocol per scan.
- Drop the PID component of the forwarder's map key (family-ip-port is
enough — if a different process binds the same address the forwarded
path is still correct).
- Add a 3-scan hysteresis: a listener that disappears for a single scan
no longer drops its socat. Absorbs short flickers from things like
next.js HMR rebuilds and docker compose restarts.
macOS handler tests build via a `proc_listeners_other.go` no-op stub.
PR SummaryMedium Risk Overview Reviewed by Cursor Bugbot for commit 6b4f194. Bugbot is set up for automated code reviews on this repo. Configure here. |
Contributor
There was a problem hiding this comment.
Code Review
Ignoring errors from listListeningSockets can cause the scanner to broadcast an empty list, which may lead to the unintended termination of all active port forwardings during transient filesystem errors. The forwarder fails to clean up persistent iptables rules when the subscriber channel closes, resulting in kernel resource leaks. Furthermore, the iptables backend appends rules without verifying if they already exist, causing redundant rules to accumulate across service restarts.
❌ 9 Tests Failed:
View the full list of 9 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
Opt-in alternative to per-port socat for IPv4 listeners. IPv6 keeps socat. Toggled by ENVD_PORT_FORWARDER_IPV4_IPTABLES=1. - iptablesBackend adds/removes PREROUTING DNAT rules from sourceIP:port to 127.0.0.1:port. - route_localnet=1 enabled at forwarder start. - Falls back to socat if iptables fails.
ValentaTomas
force-pushed
the
feat/envd-port-forwarder-iptables
branch
from
May 17, 2026 10:01
fec454e to
cbb07f7
Compare
Member
Author
|
Superseded by a provision-time blanket DNAT rule (see follow-up PR). One rule covers all ports without envd-side iptables management. |
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.
Opt-in alternative to per-port socat for IPv4 listeners (IPv6 keeps socat). Toggled by
ENVD_PORT_FORWARDER_IPV4_IPTABLES=1.Stacks on #2693.