Add interface-based routing matcher with kernel and DNS integration#3
Add interface-based routing matcher with kernel and DNS integration#3olicesx wants to merge 149 commits intooptimize/code-quality-fixesfrom
Conversation
- Implemented a concurrency limit in DnsController to manage simultaneous DNS queries. - Added a pipelined connection mechanism to optimize DNS request handling. - Introduced tests for concurrency limits and race conditions in DNS processing. - Enhanced error handling and logging in DNS listener and TCP relay functions. - Refactored DNS handling methods to support singleflight for duplicate requests. - Added benchmarks for pipelined connections and singleflight performance. - Improved resource management with context cancellation in TCP relay operations.
… packet detection - Implemented IsLikelyQuicInitialPacket to perform a fast header check on incoming UDP packets to filter out non-QUIC datagrams. - Updated Sniffer to utilize this function for early rejection of irrelevant packets. - Enhanced tests for IsLikelyQuicInitialPacket to ensure correct identification of QUIC initial packets. refactor(control): optimize DNS connection handling and routing cache - Improved connection pooling logic to prevent blocking on slow dials. - Replaced sync.Map with atomic operations for pending request slots in pipelined connections. - Added caching mechanism for UDP routing results with TTL to reduce redundant lookups. - Updated DNS controller to use sync.Map for forwarder cache, enhancing concurrency. test(control): add comprehensive tests for connection pool and routing cache - Introduced tests for connection pool to ensure non-blocking behavior during slow dials. - Added tests for response slot lifecycle to verify proper reuse and error handling. - Implemented tests for UDP endpoint routing cache to validate hit and expiration behavior.
…or failure scenarios
- guard DNS resolve against nil dialer to avoid panic paths in tests - initialize direct dialers in netutils tests and skip when network is unavailable - skip domain matcher geosite-dependent test when geosite.dat is absent - gate eBPF kernel tests behind explicit dae_bpf_tests build tag - remove fragile bitlist capacity assertions and validate tighten semantics - enhance config marshaller for repeatable function filters and int/uint values - make marshal test use secure temp files and assert round-trip idempotent output
- discard stale/mismatched UDP DNS responses and keep reading - close connection only after stale/malformed response threshold - add DoUDP regression tests for stale-discard and threshold-close
Revert DNS(53) goroutine fast-path introduced after run daeuniverse#697. This aligns packet handling semantics with the last known-good run and avoids kernel-test WAN IPv6 UDP instability.
Drop pre-singleflight cache short-circuit introduced at run daeuniverse#698 boundary. Restore the previous DNS handling flow to avoid WAN IPv6 UDP kernel-test regression.
- remove redundant EmitTask retry loop while preserving ordering semantics - simplify queue recycle path after idle GC - keep API and behavior unchanged
- add IPv4 fast path in hashAddrPort for sharded pools - reuse single timestamp in LookupDnsRespCache to reduce hot-path overhead - no API/behavior changes
Avoid waiting for secondary A/AAAA lookup when current query type is already preferred. Keep response semantics unchanged; secondary lookup still runs for cache warming.
- allocate/wait secondary-lookup done channel only when needed - early-return on canceled context in pipelined RoundTrip before write wait - no API or protocol semantics changes
Problem: - When DNS check option parsing fails or IP version is unavailable, CheckFunc returns (false, nil) to indicate 'skip check' - But Check() treated this as failure, marking Alive=false and adding Timeout latency - This caused all dialers to be marked unavailable when DNS check prerequisites weren't met, resulting in 'no alive dialer' errors Root Cause: Check() didn't distinguish between: 1. (true, nil) - success 2. (false, nil) - skip (should preserve state) 3. (false, err) - failure (should mark unavailable) Solution: Only update alive state on success (ok=true) or actual failure (err!=nil). When (ok=false, err=nil), preserve existing alive state instead of incorrectly marking as unavailable. This allows dialers to remain alive when certain check types are skipped due to configuration or network conditions.
Add regression tests for Dialer.Check state machine: - repeated (ok=false, err=nil) skip checks must not mark dialer unavailable - real failures (ok=false, err!=nil) must still mark dialer unavailable This guards against cascading no-alive-dialer collapse when a check path is temporarily skipped (e.g. DNS IP-version not available), while preserving existing failure semantics.
…ests for fallback scenarios
- Introduced batch reading for UDP packets using ipv4.NewPacketConn and ReadBatch, improving performance for high-throughput scenarios. - Refactored the Serve method in control_plane.go to handle both batch and single packet reads, with a fallback mechanism for single packet processing. - Enhanced tests to verify the correctness of GSO handling in Anyfrom methods, ensuring GSO is not applied to single UDP datagrams. - Added benchmarks for comparing single packet reads against batch reads, demonstrating performance improvements. - Updated BPF tests to reflect changes in routing logic and added runtime metadata for active routing rules. - Cleared LPM cache during rule reloads to prevent stale cache hits across different rule generations. - Optimized ingress buffer strategies to reduce unnecessary memory copies, improving overall efficiency.
This commit resolves CI failures by restoring do_tproxy_wan_ingress to match main branch behavior exactly. Changes: - Remove TCP early return (if l4proto != IPPROTO_UDP) - Remove DNS fast path optimization in wan_ingress - All UDP traffic now goes through conntrack (like main branch) Rationale: - The previous optimizations broke CI tests for WAN UDP scenarios - main branch behavior is correct and doesn't interfere with forwarding - Cloudflare origin pull continues to work because TCP is not processed in wan_ingress (only UDP conntrack is managed) This aligns with the main branch design where wan_ingress only manages UDP conntrack and doesn't make routing decisions or interfere with forwarded traffic.
This reverts commit de29c43.
- Refactored LPM cache clearing in RoutingMatcherBuilder to use a generic delete function. - Added GitHub Actions workflow for Go unit testing. - Generated eBPF constants and types in ebpf_generated.go from JSON specification. - Created ebpf_sync_spec.json to define match types, L4 protocols, IP versions, and outbound types. - Introduced bpf_stub.go to provide a stub implementation for eBPF objects when not building with real eBPF. - Added unit tests for validating required eBPF maps in control_plane_bpf_validation_test.go. - Defined eBPF synchronization constants in ebpf_sync_defs.h. - Created script gen_ebpf_sync.go to automate generation of eBPF synchronization files.
…lify error handling in _update_map_elem_by_cookie function
…d add bpf_stub.go for error handling
…t-practices perf(domain-matcher): precompile bruteforce matcher patterns
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 089817f44a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
There was a problem hiding this comment.
Pull request overview
This PR adds an interface(...) matcher that can be used in both traffic routing and DNS routing rules, and plumbs kernel-derived interface metadata (ifindex + direction) through to userspace match evaluation so routing decisions can be interface-aware end-to-end.
Changes:
- Introduces a new match type/function (
MatchType_Interface/interface(...)) and wires it into routing + DNS rule builders and matchers. - Extends eBPF routing/tproxy logic to record
ifindex+ direction and evaluate interface match rules in-kernel. - Updates docs/examples and adds unit tests for interface parsing/matching and DNS interface routing.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| hack/templates/example-config.md | Adds interface matcher examples to generated example config docs. |
| example.dae | Adds commented interface matcher examples for routing and DNS config. |
| control/utils.go | Passes direction + interface name context into userspace routing matcher; derives ifname from ifindex. |
| control/routing_matcher_userspace.go | Adds MatchWithInterface and evaluates MatchType_Interface in userspace matcher loop. |
| control/routing_matcher_builder.go | Registers interface function; builds interface match sets; resolves ifindex for kernel rules. |
| control/kern/tproxy.c | Adds ifindex + direction_in to routing result; adds kernel-side interface match evaluation; propagates skb ifindex. |
| control/kern/ebpf_sync_defs.h | Adds MatchType_Interface to the shared enum used for eBPF/userspace sync. |
| control/dns_control.go | Extracts interface context from BPF routing result and routes DNS requests/responses with interface-aware selectors. |
| control/bpf_stub.go | Extends Go stub bpfRoutingResult struct with Ifindex and DirectionIn fields. |
| config/desc.go | Documents the new interface function in routing and DNS sections. |
| component/routing/interface_matcher_test.go | Adds tests for interface matcher parsing and direction/name matching. |
| component/routing/interface_matcher.go | Implements parsing + matching logic for interface(wan:...) / interface(lan:...). |
| component/dns/response_routing.go | Adds interface-aware matching to DNS response routing builder + matcher (MatchWithInterface). |
| component/dns/request_routing.go | Adds interface-aware matching to DNS request routing builder + matcher (MatchWithInterface). |
| component/dns/interface_routing_test.go | Adds a DNS request routing test covering interface matcher behavior. |
| component/dns/dns.go | Adds RequestSelectWithInterface / ResponseSelectWithInterface and calls interface-aware matchers. |
| common/consts/routing.go | Adds Function_Interface = "interface". |
| common/consts/ebpf_sync_spec.json | Extends eBPF sync spec to include Interface match type. |
| common/consts/ebpf_generated.go | Adds generated MatchType_Interface constant. |
| README.md | Documents interface matcher availability and basic syntax. |
Comments suppressed due to low confidence (2)
control/utils.go:46
net.InterfaceByIndexis called on everyRoute()invocation to deriveifname. This is on a hot path and can be relatively expensive (netlink/syscall per call depending on platform), and it is now in the critical routing decision loop. Consider cachingifindex -> ifname(e.g., sync.Map with occasional refresh, or wiring into the existing interface/link monitoring) and only resolving on cache miss.
if routingResult.Ifindex > 0 {
if iface, e := net.InterfaceByIndex(int(routingResult.Ifindex)); e == nil {
ifname = iface.Name
}
control/dns_control.go:889
dnsInterfaceContextresolvesifindex -> ifnamevianet.InterfaceByIndexfor each UDP DNS request/response routing decision. Given DNS QPS can be high, this repeated lookup can add significant overhead. Consider sharing a cached resolver (e.g., sync.Map keyed by ifindex) with periodic invalidation or using the existing link monitor to keep names up to date.
iface, err := net.InterfaceByIndex(int(req.routingResult.Ifindex))
if err != nil {
return direction, ""
}
return direction, iface.Name
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (match_set->iface.zone == 1 && _is_wan) | ||
| direction_ok = true; | ||
| if (match_set->iface.zone == 2 && !_is_wan) |
There was a problem hiding this comment.
In kernel route evaluation, MatchType_Interface uses _is_wan to decide zone semantics (wan vs lan). But _is_wan is defined as ctx->params->flag[2] (the first byte of the process name buffer), and in WAN egress paths pid_pname can be NULL (cookie missing) so flag[2] stays 0. That will incorrectly treat WAN egress traffic as non-WAN and prevent interface(wan:...) rules from ever matching in kernspace for those flows. Consider adding an explicit boolean/enum field in route_params (or a dedicated flag byte) to represent direction/zone (WAN egress vs LAN ingress) and set it unconditionally before calling route(), then use that field here instead of _is_wan.
| if (match_set->iface.zone == 1 && _is_wan) | |
| direction_ok = true; | |
| if (match_set->iface.zone == 2 && !_is_wan) | |
| /* zone: 1 => WAN egress, 2 => LAN ingress */ | |
| if (match_set->iface.zone == 1 && match_outbound) | |
| direction_ok = true; | |
| if (match_set->iface.zone == 2 && !match_outbound) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e01de00390
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| } | ||
| ifname := "" | ||
| if routingResult.Ifindex > 0 { | ||
| if iface, e := net.InterfaceByIndex(int(routingResult.Ifindex)); e == nil { |
There was a problem hiding this comment.
Route now converts routingResult.Ifindex with net.InterfaceByIndex directly, but that ifindex is written by tc eBPF from skb->ifindex on host-side interfaces, while packet serving is started under GetDaeNetns().With(...) (cmd/run.go), so this lookup can run in daens and return empty/wrong names. In that case MatchWithInterface receives ifname="", and interface-based userspace rerouting decisions (including control-plane reroute paths) silently miss.
Useful? React with 👍 / 👎.
| if (match_set->iface.ifindex == 0) | ||
| break; |
There was a problem hiding this comment.
The interface matcher bails out immediately when match_set->iface.ifindex == 0, and builder can intentionally emit ifindex=0 when resolution fails (it logs a warning and keeps the rule). With current rule reduction, that makes good_subrule stay false, so negated rules like !interface(...) are treated as matched for all packets while unresolved, which can broadly misroute traffic until the next reload.
Useful? React with 👍 / 👎.
olicesx
force-pushed
the
optimize/code-quality-fixes
branch
2 times, most recently
from
834dfbb to
e1e101e
Compare
Code reviewFound 2 issues:
dae/component/dns/request_routing.go Lines 112 to 113 in 59020de dae/component/dns/response_routing.go Lines 162 to 163 in 59020de
Lines 87 to 100 in 59020de 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Update on code reviewAfter further analysis, both issues I reported appear to be false positives:
Please disregard my previous review - the code follows existing patterns correctly. |
olicesx
force-pushed
the
optimize/code-quality-fixes
branch
from
f83923b to
91f8500
Compare
olicesx
force-pushed
the
optimize/code-quality-fixes
branch
from
08ebe0a to
abdd21a
Compare
Motivation
interface(wan:0eth)andinterface(lan:3eth,4eth)can be used to distinguish WAN (out) and LAN (in) traffic.interfacefunction in routing and DNS rules.Description
MatchType_Interfaceand corresponding enum entries in generated constants and eBPF sync spec (common/consts/*,control/kern/ebpf_sync_defs.h).component/routing/interface_matcher.goand wires theinterfacefunction into rule builders for routing and DNS (RoutingMatcherBuilder,RequestMatcherBuilder,ResponseMatcherBuilder).ifindexand direction, store them in routing results, and match interface rules in kernel route evaluation (control/kern/tproxy.c,control/bpf_stub.go), and encodes interface data into BPF match sets.control/routing_matcher_builder.go,control/routing_matcher_userspace.go), and exposesMatchWithInterfacevariants for routing and DNS that acceptdirectionandifnamecontext.control/dns_control.go) and addingRequestSelectWithInterface/ResponseSelectWithInterfaceand related plumbing in DNS component (component/dns/*).interfacematcher syntax and semantics (README.md,config/desc.go,example.dae,hack/templates/example-config.md).component/routing/interface_matcher_test.go,component/dns/interface_routing_test.go).Testing
go test ./component/routing -run TestParseInterfaceMatchers,TestMatchInterfaceDirectionAndNameand the tests passed.go test ./component/dns -run TestRequestInterfaceMatcherand the test passed.go test ./...on the changed modules, which completed successfully.Codex Task