interfaces: create android impl #11784
Conversation
| @@ -1,5 +1,6 @@ | |||
| // Copyright (c) Tailscale Inc & AUTHORS | |||
| // SPDX-License-Identifier: BSD-3-Clause | |||
| //go:build !android | |||
net/interfaces/interfaces_android.go
Outdated
| @@ -0,0 +1,186 @@ | |||
| // Copyright (c) Tailscale Inc & AUTHORS | |||
| // SPDX-License-Identifier: BSD-3-Clause | |||
| //go:build android | |||
There was a problem hiding this comment.
this line's not needed. Can delete. The filename's sufficient.
(but if you did need it, blank line above)
net/interfaces/interfaces_android.go
Outdated
| return | ||
| } | ||
| if old := lastKnownDefaultRouteIfName.Swap(ifName); old != ifName { | ||
| log.Printf("defaultroute_android: update from Android, ifName = %s (was %s)", ifName, old) |
| import ( | ||
| "errors" | ||
| ) |
|
Please copy the PR text to the commit message. |
|
Also it's not a "class" in Go. |
net/interfaces/interfaces_android.go
Outdated
| // UpdateLastKnownDefaultRouteInterface is called by libtailscale in the Android app when | ||
| // the connectivity manager detects a network path transition. | ||
| func UpdateLastKnownDefaultRouteInterface(ifName string) { | ||
| if ifName == "" { | ||
| return | ||
| } | ||
| if old := lastKnownDefaultRouteIfName.Swap(ifName); old != ifName { | ||
| log.Printf("defaultroute: update from Android, ifName = %s (was %s)", ifName, old) | ||
| } |
There was a problem hiding this comment.
can we document the meaning of the empty string? Does that mean no network? And shouldn't that transition be logged as well? Why's it not being recorded in the atomic?
Can you also document why this doesn't wake up something? Or why it's not a method on the Monitor?
When Android calls this, what happens later in the lifecycle that makes something back in Go read this?
There was a problem hiding this comment.
yup, no network should be recorded, thanks!
this doesn't wake anything up because debounce() is continuously monitoring and calls interfaces.GetState, which uses defaultRoute()
| /* | ||
| Parse 10.0.0.1 out of: | ||
|
|
||
| $ cat /proc/net/route |
There was a problem hiding this comment.
IIRC only now-ancient Androids permit reading this file.
Is it even worth keeping this proc net parsing code? I think we need to 100% rely on Android APIs at this point.
There was a problem hiding this comment.
yup, this will be removed in a follow up
net/interfaces/interfaces_android.go
Outdated
|
|
||
| // UpdateLastKnownDefaultRouteInterface is called by libtailscale in the Android app when | ||
| // the connectivity manager detects a network path transition. If ifName is "", network has been lost. | ||
| // interfaces.debounce continuously monitors for network changes and will trigger a link change |
There was a problem hiding this comment.
It doesn't continuously. This makes it sounds like it's busy looping polling this atomic.
I assume Android is calling Monitor.InjectEvent after the changes? Say that instead, if that's true?
-Move Android impl into interfaces_android.go -Instead of using ip route to get the interface name, use the one passed in by Android (ip route is restricted in Android 13+ per termux/termux-app#2993) Follow-up will be to do the same for router Fixes tailscale/corp#19215 Fixes tailscale/corp#19124 Signed-off-by: kari-ts <kari@tailscale.com>
-Move Android impl into interfaces_android.go
-Instead of using ip route to get the interface name, use the one passed in by Android (ip route is restricted in Android 13+ per termux/termux-app#2993)
Follow-up will be to do the same for router
Fixes tailscale/corp#19215
Fixes tailscale/corp#19124