Skip to content

Conversation

@ibigbug
Copy link
Member

@ibigbug ibigbug commented Jul 14, 2025

🤔 This is a ...

  • New feature
  • Bug fix
  • Performance optimization
  • Enhancement feature
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Workflow
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

best effort to create proper family of socket for UDP connections

📝 Changelog

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Changelog is provided or not needed

@ibigbug ibigbug requested a review from Copilot July 14, 2025 16:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds an optional family_hint parameter to new_udp_socket so callers can explicitly specify the socket address family, and updates all existing call sites to pass either a computed hint or None. It also refactors binding logic in TUN creation, adjusts DNS runtime UDP binding, and makes the Cargo doc package opt out of default features.

  • Extend new_udp_socket signature with family_hint and incorporate it into domain selection.
  • Update all proxy modules, DNS runtime, and DHCP to supply or default the new family_hint.
  • Refactor TUN builder to avoid double build_async calls and tighten Cargo features in clash-doc.

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
proxy/utils/socket_helpers.rs Added family_hint to choose socket domain; switched android guard to runtime cfg!.
proxy/utils/proxy_connector.rs Changed _destination to destination and passed its address as family_hint.
proxy/tun/inbound.rs Cached tun_builder.build_async() into dev to prevent duplicate builds.
proxy/tuic/types.rs Passed server.ip as family_hint for TUIC endpoints.
proxy/tuic/mod.rs Temporarily passed None for family_hint; added TODO.
proxy/socks/outbound/mod.rs Passed bind address as family_hint for SOCKS outbound.
proxy/socks/inbound/stream.rs Added None placeholder for new family_hint parameter.
proxy/shadowsocks/inbound/mod.rs Added None placeholder for new family_hint parameter.
proxy/shadowquic/mod.rs Replaced manual bind-addr parsing with None src and Some(addr) hint.
proxy/hysteria2/mod.rs Replaced src hint with server_socket_addr hint.
proxy/direct/mod.rs Imported and used family_hint_for_session to compute hint.
app/dns/runtime.rs Renamed _server_addr to server_addr and passed it as hint.
app/dns/resolver/enhanced.rs Simplified error logging in DNS client error path.
app/dns/dhcp.rs Added None placeholder for new family_hint in DHCP listener.
clash-doc/Cargo.toml Disabled default features for clash-lib in the docs package.
Comments suppressed due to low confidence (3)

clash-lib/src/proxy/utils/socket_helpers.rs:94

  • [nitpick] The parameter family_hint holds a full SocketAddr, which may be misleading since only the address family is used. Consider renaming it to addr_hint or socket_hint for clearer intent.
    family_hint: Option<std::net::SocketAddr>,

clash-lib/src/proxy/utils/socket_helpers.rs:91

  • [nitpick] Clarify in this comment that only the IP component of the SocketAddr is used to derive the socket domain and that the port number is ignored when selecting the family.
    // Optional family hint for the socket.

clash-lib/src/proxy/utils/socket_helpers.rs:146

  • Switching from a compile-time #[cfg(not(target_os = "android"))] to a runtime if !cfg!(...) means the must_bind_socket_on_interface import is omitted on Android but its calls are still compiled, causing build errors. Use a compile-time #[cfg] on the block or import the function unconditionally and keep a runtime guard inside.
    if !cfg!(target_os = "android") {

@ibigbug ibigbug merged commit 4db1bc0 into master Jul 15, 2025
34 of 35 checks passed
@ibigbug ibigbug deleted the fix-dns-v6 branch July 15, 2025 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants