atunnel: #753 with the original-destination review suggestions applied - #8
Draft
ygao-g wants to merge 5 commits into
Draft
atunnel: #753 with the original-destination review suggestions applied#8ygao-g wants to merge 5 commits into
ygao-g wants to merge 5 commits into
Conversation
TCPOriginalDestination read only the IPv4 SOL_IP/SO_ORIGINAL_DST, so an actor's IPv6 connection redirected into the transparent egress listener had no destination to dial and the proxy failed it. Read IP6T_SO_ORIGINAL_DST too, falling back to it only when the IPv4 lookup returns ENOENT, so unrelated IPv4 failures keep their own error. One step towards dual-stack actor networking; the actor veth and its nftables rules are still IPv4-only. Co-authored-by: Yuan Gao <ypgao@google.com> (cherry picked from commit d8527b5) (cherry picked from commit 5ad28ad)
(cherry picked from commit cb7d66e)
(cherry picked from commit 583a7b8)
The IPv6 lookup ran only as a fallback, after an IPv4 lookup that a pure-IPv6 socket can never satisfy: its inet addresses are zeroed, so the conntrack tuple is all zeros and the query always misses with ENOENT. That made every IPv6 egress connection pay a guaranteed-to-fail syscall whose result was discarded, and left the code depending on that kernel detail holding. The connection's local address already says which family the flow is, so ask for that family's option directly. The failure message now names the family it asked for. atunnel logs it at warn level and it is the only signal an operator gets when a redirected connection cannot be resolved. (cherry picked from commit 13a1e4b)
The redirect tests built their veth, their nftables NAT table and both listeners in the host network namespace, so a machine with a default-deny INPUT policy dropped the redirected SYN and the test failed rather than skipped. Both sides now live in throwaway namespaces, which also removes the PID-derived addresses and interface names that could truncate or collide, and leaves no nftables state behind on the host. Coverage grew three ways: the IPv4 and IPv6 cases are one table built from the same ateomnet matchers production uses; that table gained the dual-stack socket the worker actually listens on, where an IPv4 actor arrives with a v4-mapped local address and is still only readable through the IPv4 socket option; and the address formatting, which was reachable only from root-gated tests, now has an ordinary one. Both lookups report a miss as ENOENT, so the failure-path cases assert the family named in the error rather than the errno alone. (cherry picked from commit 561cd0e)
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.
Reference only — this lives on a fork and is not proposed for merge. It exists to
give agent-substrate#753 a stable link for the review suggestions on that PR.
The first three commits are @lubingtan's from agent-substrate#753, unchanged and with their
original authorship. The last two are the suggested changes:
f7f12bc9f637e7c26c6884d9e6969fabdc915c6cThe dispatch commit replaces the ENOENT fallback with a branch on the family
already derived from the local address, and names that family in the error.
The test commit moves the redirect tests into throwaway network namespaces,
where a default-deny INPUT policy can no longer drop the redirected SYN, folds
the IPv4 and IPv6 cases into one table, adds the dual-stack socket the worker
actually listens on, and covers the address formatting without root.
Based on
f7b4f8f9against a pinned base branch, so the diff is exactly thesefive commits and stays that way as main moves. One difference from the version
that ran in CI:
ateomnet.IPv6SourceEqualdoes not exist on main yet, so theIPv6 source match is a local helper here rather than a call into ateomnet.
🤖 Generated with Claude Code