android: cache default network and set callback for binding fn#754
android: cache default network and set callback for binding fn#754
Conversation
After switching from cellular to wifi without ipv6, ForeachInterface still sees rmnet prefixes, so HaveV6 stays true, and magicsock keeps attempting ipv6 connections that either route through cellular or time out for users on wifi without ipv6 This -Caches cachedDefaultNetwork on nevwork event in NetworkChangeCallbac -Implements BindSocketToNetwork, a callback that binds the socket to the currently selected Network -Sets the callback function whtne the VPN starts and clears it on disconnect Updates tailscale/tailscale#6152 Signed-off-by: kari-ts <kari@tailscale.com>
nickkhyl
left a comment
nickkhyl
left a comment
There was a problem hiding this comment.
This LGTM overall. I'm not approving it mainly because it requires an OSS bump and may require some changes (or at least a rebase) after the OSS changes land.
| TSLog.w(TAG, "bindSocketToActiveNetwork: bind failed fd=$fd net=$net iface=$iface: $e") | ||
| false | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: The indentation here seems off (the body has the same indentation level as the method declaration).
| val iface = NetworkChangeCallback.cachedDefaultInterfaceName | ||
| ?: NetworkChangeCallback.cachedDefaultNetworkInfo?.linkProps?.interfaceName |
There was a problem hiding this comment.
Do we actually need the fallback here? From reading the code, it seems that cachedDefaultInterfaceName is updated atomically with cachedDefaultNetworkInfo.
So if the interface name is immutable and cannot change after we cache it in cachedDefaultInterfaceName, then we should always use it:
| val iface = NetworkChangeCallback.cachedDefaultInterfaceName | |
| ?: NetworkChangeCallback.cachedDefaultNetworkInfo?.linkProps?.interfaceName | |
| val iface = NetworkChangeCallback.cachedDefaultInterfaceName |
Otherwise, we should remove cachedDefaultInterfaceName and always read it from linkProps:
| val iface = NetworkChangeCallback.cachedDefaultInterfaceName | |
| ?: NetworkChangeCallback.cachedDefaultNetworkInfo?.linkProps?.interfaceName | |
| val iface = NetworkChangeCallback.cachedDefaultNetworkInfo?.linkProps?.interfaceName |
The doc says the field was added for convenience, and there isn't much convenience in checking two fields 😸
2 participants
After switching from cellular to wifi without ipv6, ForeachInterface still sees rmnet prefixes, so HaveV6 stays true, and magicsock keeps attempting ipv6 connections that either route through cellular or time out for users on wifi without ipv6
This
-Caches cachedDefaultNetwork on nevwork event in NetworkChangeCallback
-Implements BindSocketToNetwork, a callback that binds the socket to the currently selected Network
-Sets the callback function whtne the VPN starts and clears it on disconnect
Updates tailscale/tailscale#6152