Replace old waitForTunnelUp function #7155
Conversation
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Rawa)
android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt line 141 at r1 (raw file):
val tunFd = vpnInterfaceFd.detachFd() waitForTunnelUp(tunFd, config.routes.any { route -> route.isIpv6 })
Were we able to find any historical reasons for adding this wait by looking in the git history? Would also be good to clarify in the git commit why we remove it as a future reference in case it causes any issues.
Code quote:
waitForTunnelUp
Rawa
force-pushed
the
remove-wait-for-tunnel-up
branch
from
7a9a0b5
to
d06185e
Compare
There was a problem hiding this comment.
Reviewed 2 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @albin-mullvad)
android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt line 141 at r1 (raw file):
Previously, albin-mullvad wrote…
Were we able to find any historical reasons for adding this wait by looking in the git history? Would also be good to clarify in the git commit why we remove it as a future reference in case it causes any issues.
💯% agree that we should try to document why this was removed. Doing so in the commit message ought to be enough
Rawa
force-pushed
the
remove-wait-for-tunnel-up
branch
from
d06185e
to
00a7fe1
Compare
Rawa
changed the title
After invoking VpnService.establish() we will get a tunnel file descriptor that corresponds to the interface that was created. However, this has no guarantee of the routing table beeing up to date, and we might thus send traffic outside the tunnel. Previously this was done through looking at the tunFd to see that traffic is sent to verify that the routing table has changed. If no traffic is seen some traffic is induced to a random IP address to ensure traffic can be seen. This new implementation is slower but won't risk sending UDP traffic to a random public address at the internet.
Rawa
force-pushed
the
remove-wait-for-tunnel-up
branch
from
00a7fe1
to
9c0127f
Compare
There was a problem hiding this comment.
Reviewable status: 2 of 6 files reviewed, 1 unresolved discussion (waiting on @albin-mullvad and @pinkisemils)
android/lib/talpid/src/main/kotlin/net/mullvad/talpid/TalpidVpnService.kt line 141 at r1 (raw file):
Previously, MarkusPettersson98 (Markus Pettersson) wrote…
💯% agree that we should try to document why this was removed. Doing so in the commit message ought to be enough
I've added some documentation, but I'll be putting this on hold for a bit since we want to consider doing this in the daemon instead. I talked with @pinkisemils whom proposed that we can setup a socket with NETLINK_ROUTE and then get events for when the routing tables changes. This would possible be faster than this implementation that uses connectivityManager.registerDefaultNetworkCallback(), which seem to take at least 50ms on a debug build.
Rawa
added
the
On hold
This change is