Repository navigation
Routed --forward-localhost reaches a host service that listens only on ::1 - #1032
Merged
Merged
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 42 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
…n ::1 In routed mode the host side of --forward-localhost dialled 127.0.0.1:<port> and nothing else. The guest's relay accepts on 127.0.0.1 and on ::1 and does not carry over which of the two its client dialled, so a host service bound only to ::1 could not be reached from the guest at all. The MySQL that WWW tests use on a devserver (mysqld@3300) is one: it listens on [::1]:3300. The flag forwards the host's localhost, which is both loopback addresses. connect_host_loopback dials 127.0.0.1, and ::1 when 127.0.0.1 refuses the connection. A refusal is the one error that says nothing listens there. A timeout, or no local port left to dial from, is the error of a service that is there, and that connection is not sent to whatever listens on ::1. With no service on either address the error names the port and the outcome of both dials in one message, because the relay logs an error's own text and not its chain. Rootless networking is unchanged: the guest's relay dials the gateway 10.0.2.2, which pasta maps to the host's 127.0.0.1 only (#1026). Red, each with the behaviour it pins put back and the test kept: the function dialling 127.0.0.1 only tcp_proxy::tests::a_host_service_on_ipv6_loopback_only_is_reached the service on ::1 accepts: connecting to host loopback every 127.0.0.1 error followed by a dial of ::1 tcp_proxy::tests::only_a_refusal_on_ipv4_loopback_is_followed_by_ipv6_loopback Connection timed out (os error 110) the ::1 outcome left to the error's chain tcp_proxy::tests::no_host_service_is_an_error_naming_both_loopback_addresses connecting to the host's loopback port 62485: 127.0.0.1:62485 gave Connection refused (os error 111), then [::1]:62485 the relay dialling 127.0.0.1 only (as root, in a network namespace) tcp_proxy::tests::test_localhost_forward_relay_reaches_ipv6_loopback the relay should reach the host's ::1 left: [] Green on this commit: make test-unit Summary [ 71.262s] 1375 tests run: 1375 passed (1 slow), 0 skipped make lint rc 0 As root on the build host, at the top of this stack: make _test-root FILTER="-p fcvm --lib -E 'test(/^network::tcp_proxy::tests::test_/)'" Summary [ 0.178s] 12 tests run: 12 passed, 736 skipped As root on the build host, at the top of this stack with the commit that reads --ipv6-prefix from FCVM_IPV6_PREFIX picked on top (the host's own addresses are not a routable /64, so the routed tests take its delegated subnet from that variable): make test-root FILTER="-E 'test(/test_port_forward_routed|test_clone_port_forward_routed|test_forward_localhost_routed/)'" Summary [ 58.010s] 3 tests run: 3 passed, 1719 skipped PASS [ 9.0s] fcvm::test_forward_localhost test_forward_localhost_routed PASS [ 11.5s] fcvm::test_port_forward test_port_forward_routed PASS [ 37.5s] fcvm::test_snapshot_clone test_clone_port_forward_routed The unit tests hold the other loopback address at the same port with a bound socket that does not listen, so the refusal there does not depend on what else runs on the machine. A bind error other than "address in use" fails those tests by name: they used to retry on any error, which never ends on a host without ::1.
start_localhost_forwards started a relay for each port as it went. When a
later port could not be bound it aborted the relays already started and
returned the error. abort only asks the runtime to drop a task, so their
listeners were still open when the error was returned. start_port_forwards had
the same shape until it bound every listener before starting any relay.
start_localhost_forwards now does the same. It binds every port first, and the
listeners are plain values until a relay takes them, so an error drops, and
with that closes, the ones already bound.
Red, with the function as it was (as root, in a network namespace):
make _test-root FILTER="-p fcvm --lib -E 'test(/^network::tcp_proxy::tests::test_/)'"
Summary [ 5.086s] 11 tests run: 10 passed, 1 failed, 731 skipped
tcp_proxy::tests::test_localhost_forward_bind_failure_leaves_no_listener
the first port must be free again once the start has failed: operation in namespace /var/run/netns/test-lfb-846982
Caused by: Address already in use (os error 98)
Green, the same command at the top of this stack:
Summary [ 0.178s] 12 tests run: 12 passed, 736 skipped
Green on this commit:
make test-unit
Summary [ 71.281s] 1375 tests run: 1375 passed (1 slow), 0 skipped
make lint rc 0
make _test-root FILTER=--no-run rc 0 (compiles the root-only tests, runs none)
ejc3
force-pushed
the
snapshot-run-publish
branch
from
October 1, 2026 13:31
93388d5 to
9ad4b7e
Compare
ejc3
force-pushed
the
forward-localhost-any-host-loopback
branch
from
October 1, 2026 13:31
dee5355 to
db197db
Compare
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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
1 participant
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.
Stacked on:
snapshot-run-publish(PR #1031).In routed mode the host side of
--forward-localhostdialled127.0.0.1:<port>and nothing else, so a host service bound only to::1could not be reached from the guest. The MySQL that WWW tests use on a devserver listens on[::1]:3300only.What changes
localhost, which is both loopback addresses. The host side dials 127.0.0.1, and ::1 when 127.0.0.1 refuses the connection. Only a refusal is followed by ::1: any other error is of a service that is there. With no service on either address, one message names the port and the outcome of both dials.--forward-localhoststart closes the listeners it had bound. It used to abort the relays already started without waiting for them, the pattern Routed networking binds a published port on the host address its mapping names #1030 removed from the port forwards.Contract and impact
Production code in routed networking's localhost relay. A host service on 127.0.0.1 is reached as before. New: when 127.0.0.1 refuses, a listener on
[::1]:<port>is reached, so the flag opens that port of both loopback addresses to the guest. The help text says so. Rootless networking is unchanged (#1026).Minimum evidence
Unit tests with a red each, the network namespace tests as root with a red for each commit, the unit suite and lint at both commits, and
test_forward_localhost_routedon a VM.Not in this PR
--forward-localhoststill reaches only the host's 127.0.0.1.Evidence
Routed --forward-localhost reaches a host service that listens only on ::1
A failed --forward-localhost start closes the listeners it had bound