From ae3abdfa4be2e4867d4cad70be15f90ad36a0b32 Mon Sep 17 00:00:00 2001 From: Mark Date: Sat, 11 Mar 2023 05:01:48 +0000 Subject: [PATCH] Allow SAP and LOHS wifi clients exist at the same time This change store localOnly wifi clients in its own field so that tethered and localOnly hotspot clients can exist at the same time. Currently, there are no tethered and localOnly hotspot clients at the same time because PrivateAddressCoordinator does not support SAP + LOHS. A follow-up change will be made to allow this. When both SAP and LOHS are enabled, the SAP and LOHS clients from TetheringEventCallback#onClientsChanged are all TETHERING_WIFI. Currently, there is no way for the listeners to distinguish between SAP and LOHS clients. Bug: 233175023 Test: atest TetheringTests Change-Id: I01b0a6abb084f7135f7825e0c5303e49c16a4c39 --- .../tethering/ConnectedClientsTracker.java | 53 ++++++-- .../networkstack/tethering/Tethering.java | 58 ++++---- .../tethering/ConnectedClientsTrackerTest.kt | 124 ++++++++++++++---- 3 files changed, 168 insertions(+), 67 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/ConnectedClientsTracker.java b/Tethering/src/com/android/networkstack/tethering/ConnectedClientsTracker.java index 8a96988ae1..5b19c4e1fa 100644 --- a/Tethering/src/com/android/networkstack/tethering/ConnectedClientsTracker.java +++ b/Tethering/src/com/android/networkstack/tethering/ConnectedClientsTracker.java @@ -29,6 +29,8 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; +import com.android.modules.utils.build.SdkLevel; + import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -49,6 +51,8 @@ public class ConnectedClientsTracker { @NonNull private List mLastWifiClients = Collections.emptyList(); @NonNull + private List mLastLocalOnlyClients = Collections.emptyList(); + @NonNull private List mLastTetheredClients = Collections.emptyList(); @VisibleForTesting @@ -72,25 +76,44 @@ public class ConnectedClientsTracker { * *

The new list can be obtained through {@link #getLastTetheredClients()}. * @param ipServers The IpServers used to assign addresses to clients. - * @param wifiClients The list of L2-connected WiFi clients. Null for no change since last - * update. + * @param wifiClients The list of L2-connected WiFi clients that are connected to a global + * hotspot. Null for no change since last update. + * @param localOnlyClients The list of L2-connected WiFi clients that are connected to localOnly + * hotspot. Null for no change since last update. * @return True if the list of clients changed since the last calculation. */ public boolean updateConnectedClients( - Iterable ipServers, @Nullable List wifiClients) { + Iterable ipServers, @Nullable List wifiClients, + @Nullable List localOnlyClients) { final long now = mClock.elapsedRealtime(); - if (wifiClients != null) { - mLastWifiClients = wifiClients; - } + if (wifiClients != null) mLastWifiClients = wifiClients; + if (localOnlyClients != null) mLastLocalOnlyClients = localOnlyClients; + final Set wifiClientMacs = getClientMacs(mLastWifiClients); + final Set localOnlyClientMacs = getClientMacs(mLastLocalOnlyClients); // Build the list of non-expired leases from all IpServers, grouped by mac address final Map clientsMap = new HashMap<>(); for (IpServer server : ipServers) { + final Set connectedClientMacs; + switch (server.servingMode()) { + case IpServer.STATE_TETHERED: + connectedClientMacs = wifiClientMacs; + break; + case IpServer.STATE_LOCAL_ONLY: + // Before T, SAP and LOHS both use wifiClientMacs because + // registerLocalOnlyHotspotSoftApCallback didn't exist. + connectedClientMacs = SdkLevel.isAtLeastT() + ? localOnlyClientMacs : wifiClientMacs; + break; + default: + continue; + } + for (TetheredClient client : server.getAllLeases()) { if (client.getTetheringType() == TETHERING_WIFI - && !wifiClientMacs.contains(client.getMacAddress())) { + && !connectedClientMacs.contains(client.getMacAddress())) { // Skip leases of WiFi clients that are not (or no longer) L2-connected continue; } @@ -104,11 +127,8 @@ public class ConnectedClientsTracker { // TODO: add IPv6 addresses from netlink // Add connected WiFi clients that do not have any known address - for (MacAddress client : wifiClientMacs) { - if (clientsMap.containsKey(client)) continue; - clientsMap.put(client, new TetheredClient( - client, Collections.emptyList() /* addresses */, TETHERING_WIFI)); - } + addWifiClientsIfNoLeases(clientsMap, wifiClientMacs); + addWifiClientsIfNoLeases(clientsMap, localOnlyClientMacs); final HashSet clients = new HashSet<>(clientsMap.values()); final boolean clientsChanged = clients.size() != mLastTetheredClients.size() @@ -117,6 +137,15 @@ public class ConnectedClientsTracker { return clientsChanged; } + private static void addWifiClientsIfNoLeases( + final Map clientsMap, final Set clientMacs) { + for (MacAddress mac : clientMacs) { + if (clientsMap.containsKey(mac)) continue; + clientsMap.put(mac, new TetheredClient( + mac, Collections.emptyList() /* addresses */, TETHERING_WIFI)); + } + } + private static void addLease(Map clientsMap, TetheredClient lease) { final TetheredClient aggregateClient = clientsMap.getOrDefault( lease.getMacAddress(), lease); diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index 4c5bf4e81a..e5f644ed4a 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -58,6 +58,7 @@ import static android.net.wifi.WifiManager.IFACE_IP_MODE_CONFIGURATION_ERROR; import static android.net.wifi.WifiManager.IFACE_IP_MODE_LOCAL_ONLY; import static android.net.wifi.WifiManager.IFACE_IP_MODE_TETHERED; import static android.net.wifi.WifiManager.IFACE_IP_MODE_UNSPECIFIED; +import static android.net.wifi.WifiManager.SoftApCallback; import static android.net.wifi.WifiManager.WIFI_AP_STATE_DISABLED; import static android.telephony.CarrierConfigManager.ACTION_CARRIER_CONFIG_CHANGED; import static android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID; @@ -479,15 +480,15 @@ public class Tethering { mStateReceiver, noUpstreamFilter, PERMISSION_MAINLINE_NETWORK_STACK, mHandler); final WifiManager wifiManager = getWifiManager(); - TetheringSoftApCallback softApCallback = new TetheringSoftApCallback(); if (wifiManager != null) { - wifiManager.registerSoftApCallback(mExecutor, softApCallback); - } - if (SdkLevel.isAtLeastT() && wifiManager != null) { - // Although WifiManager#registerLocalOnlyHotspotSoftApCallback document that it need - // NEARBY_WIFI_DEVICES permission, but actually a caller who have NETWORK_STACK - // or MAINLINE_NETWORK_STACK permission would also able to use this API. - wifiManager.registerLocalOnlyHotspotSoftApCallback(mExecutor, softApCallback); + wifiManager.registerSoftApCallback(mExecutor, new TetheringSoftApCallback()); + if (SdkLevel.isAtLeastT()) { + // Although WifiManager#registerLocalOnlyHotspotSoftApCallback document that it need + // NEARBY_WIFI_DEVICES permission, but actually a caller who have NETWORK_STACK + // or MAINLINE_NETWORK_STACK permission can also use this API. + wifiManager.registerLocalOnlyHotspotSoftApCallback(mExecutor, + new LocalOnlyHotspotCallback()); + } } startTrackDefaultNetwork(); @@ -573,26 +574,17 @@ public class Tethering { } } - private class TetheringSoftApCallback implements WifiManager.SoftApCallback { - // TODO: Remove onStateChanged override when this method has default on - // WifiManager#SoftApCallback interface. - // Wifi listener for state change of the soft AP - @Override - public void onStateChanged(final int state, final int failureReason) { - // Nothing - } - - // Called by wifi when the number of soft AP clients changed. - // Currently multiple softAp would not behave well in PrivateAddressCoordinator - // (where it gets the address from cache), it ensure tethering only support one ipServer for - // TETHERING_WIFI. Once tethering support multiple softAp enabled simultaneously, - // onConnectedClientsChanged should also be updated to support tracking different softAp's - // clients individually. - // TODO: Add wtf log and have check to reject request duplicated type with different - // interface. + private class TetheringSoftApCallback implements SoftApCallback { @Override public void onConnectedClientsChanged(final List clients) { - updateConnectedClients(clients); + updateConnectedClients(clients, null); + } + } + + private class LocalOnlyHotspotCallback implements SoftApCallback { + @Override + public void onConnectedClientsChanged(final List clients) { + updateConnectedClients(null, clients); } } @@ -1968,7 +1960,7 @@ public class Tethering { mIPv6TetheringCoordinator.removeActiveDownstream(who); mOffload.excludeDownstreamInterface(who.interfaceName()); mForwardedDownstreams.remove(who); - updateConnectedClients(null /* wifiClients */); + maybeDhcpLeasesChanged(); // If this is a Wi-Fi interface, tell WifiManager of any errors // or the inactive serving state. @@ -2710,9 +2702,15 @@ public class Tethering { if (e != null) throw e; } - private void updateConnectedClients(final List wifiClients) { + private void maybeDhcpLeasesChanged() { + // null means wifi clients did not change. + updateConnectedClients(null, null); + } + + private void updateConnectedClients(final List wifiClients, + final List localOnlyClients) { if (mConnectedClientsTracker.updateConnectedClients(mTetherMainSM.getAllDownstreams(), - wifiClients)) { + wifiClients, localOnlyClients)) { reportTetherClientsChanged(mConnectedClientsTracker.getLastTetheredClients()); } } @@ -2731,7 +2729,7 @@ public class Tethering { @Override public void dhcpLeasesChanged() { - updateConnectedClients(null /* wifiClients */); + maybeDhcpLeasesChanged(); } @Override diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/ConnectedClientsTrackerTest.kt b/Tethering/tests/unit/src/com/android/networkstack/tethering/ConnectedClientsTrackerTest.kt index d915354b0c..2dd9f918e3 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/ConnectedClientsTrackerTest.kt +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/ConnectedClientsTrackerTest.kt @@ -24,19 +24,25 @@ import android.net.TetheringManager.TETHERING_USB import android.net.TetheringManager.TETHERING_WIFI import android.net.ip.IpServer import android.net.wifi.WifiClient +import android.os.Build import androidx.test.filters.SmallTest import androidx.test.runner.AndroidJUnit4 +import com.android.testutils.DevSdkIgnoreRule +import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue +import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.mockito.Mockito.doReturn import org.mockito.Mockito.mock -import kotlin.test.assertEquals -import kotlin.test.assertFalse -import kotlin.test.assertTrue @RunWith(AndroidJUnit4::class) @SmallTest class ConnectedClientsTrackerTest { + @get:Rule + val ignoreRule = DevSdkIgnoreRule() private val server1 = mock(IpServer::class.java) private val server2 = mock(IpServer::class.java) @@ -70,55 +76,122 @@ class ConnectedClientsTrackerTest { @Test fun testUpdateConnectedClients() { + doReturn(IpServer.STATE_TETHERED).`when`(server1).servingMode() + doReturn(IpServer.STATE_TETHERED).`when`(server2).servingMode() + runUpdateConnectedClientsTest(isGlobal = true) + } + + @Test @IgnoreUpTo(Build.VERSION_CODES.S_V2) + fun testUpdateConnectedClients_LocalOnly() { + doReturn(IpServer.STATE_LOCAL_ONLY).`when`(server1).servingMode() + doReturn(IpServer.STATE_LOCAL_ONLY).`when`(server2).servingMode() + runUpdateConnectedClientsTest(isGlobal = false) + } + + fun runUpdateConnectedClientsTest(isGlobal: Boolean) { doReturn(emptyList()).`when`(server1).allLeases doReturn(emptyList()).`when`(server2).allLeases val tracker = ConnectedClientsTracker(clock) - assertFalse(tracker.updateConnectedClients(servers, null)) + assertFalse(tracker.updateConnectedClients(servers, null, null)) // Obtain a lease for client 1 doReturn(listOf(client1)).`when`(server1).allLeases - assertSameClients(listOf(client1), assertNewClients(tracker, servers, listOf(wifiClient1))) + if (isGlobal) { + assertSameClients(listOf(client1), assertNewClients(tracker, servers, + wifiClients = listOf(wifiClient1))) + } else { + assertSameClients(listOf(client1), assertNewClients(tracker, servers, + localOnlyClients = listOf(wifiClient1))) + } // Client 2 L2-connected, no lease yet val client2WithoutAddr = TetheredClient(client2Addr, emptyList(), TETHERING_WIFI) - assertSameClients(listOf(client1, client2WithoutAddr), - assertNewClients(tracker, servers, listOf(wifiClient1, wifiClient2))) + if (isGlobal) { + assertSameClients(listOf(client1, client2WithoutAddr), assertNewClients( + tracker, servers, wifiClients = listOf(wifiClient1, wifiClient2))) + } else { + assertSameClients(listOf(client1, client2WithoutAddr), assertNewClients( + tracker, servers, localOnlyClients = listOf(wifiClient1, wifiClient2))) + } // Client 2 lease obtained doReturn(listOf(client1, client2)).`when`(server1).allLeases - assertSameClients(listOf(client1, client2), assertNewClients(tracker, servers, null)) + assertSameClients(listOf(client1, client2), assertNewClients(tracker, servers)) // Client 3 lease obtained doReturn(listOf(client3)).`when`(server2).allLeases - assertSameClients(listOf(client1, client2, client3), - assertNewClients(tracker, servers, null)) + assertSameClients(listOf(client1, client2, client3), assertNewClients(tracker, servers)) - // Client 2 L2-disconnected - assertSameClients(listOf(client1, client3), - assertNewClients(tracker, servers, listOf(wifiClient1))) - - // Client 1 L2-disconnected - assertSameClients(listOf(client3), assertNewClients(tracker, servers, emptyList())) - - // Client 1 comes back - assertSameClients(listOf(client1, client3), - assertNewClients(tracker, servers, listOf(wifiClient1))) + if (isGlobal) { + // Client 2 L2-disconnected + assertSameClients(listOf(client1, client3), + assertNewClients(tracker, servers, wifiClients = listOf(wifiClient1))) + // Client 1 L2-disconnected + assertSameClients(listOf(client3), assertNewClients(tracker, servers, + wifiClients = emptyList())) + // Client 1 comes back + assertSameClients(listOf(client1, client3), + assertNewClients(tracker, servers, wifiClients = listOf(wifiClient1))) + } else { + // Client 2 L2-disconnected + assertSameClients(listOf(client1, client3), + assertNewClients(tracker, servers, localOnlyClients = listOf(wifiClient1))) + // Client 1 L2-disconnected + assertSameClients(listOf(client3), + assertNewClients(tracker, servers, localOnlyClients = emptyList())) + // Client 1 comes back + assertSameClients(listOf(client1, client3), + assertNewClients(tracker, servers, localOnlyClients = listOf(wifiClient1))) + } // Leases lost, client 1 still L2-connected doReturn(emptyList()).`when`(server1).allLeases doReturn(emptyList()).`when`(server2).allLeases assertSameClients(listOf(TetheredClient(client1Addr, emptyList(), TETHERING_WIFI)), - assertNewClients(tracker, servers, null)) + assertNewClients(tracker, servers)) + } + + @Test @IgnoreUpTo(Build.VERSION_CODES.S_V2) + fun testLocalOnlyAndTetheredHotspotClients() { + val tracker = ConnectedClientsTracker(clock) + doReturn(IpServer.STATE_LOCAL_ONLY).`when`(server1).servingMode() + doReturn(IpServer.STATE_TETHERED).`when`(server2).servingMode() + + // Client 1 connected to server1 (LOHS) + doReturn(listOf(client1)).`when`(server1).allLeases + doReturn(emptyList()).`when`(server2).allLeases + assertSameClients(listOf(client1), assertNewClients(tracker, servers, + localOnlyClients = listOf(wifiClient1))) + + // Client 2 connected to server2 (wifi Tethering) + doReturn(listOf(client2)).`when`(server2).allLeases + assertSameClients(listOf(client1, client2), assertNewClients(tracker, servers, + listOf(wifiClient2), listOf(wifiClient1))) + + // Client 2 L2-disconnected but lease doesn't expired yet + assertSameClients(listOf(client1), assertNewClients(tracker, servers, + wifiClients = emptyList())) + + // Client 1 lease lost but still L2-connected + doReturn(emptyList()).`when`(server1).allLeases + val client1WithoutAddr = TetheredClient(client1Addr, emptyList(), TETHERING_WIFI) + assertSameClients(listOf(client1WithoutAddr), assertNewClients(tracker, servers)) + + // Client 1 L2-disconnected + assertSameClients(emptyList(), assertNewClients(tracker, servers, + localOnlyClients = emptyList())) } @Test fun testUpdateConnectedClients_LeaseExpiration() { + doReturn(IpServer.STATE_TETHERED).`when`(server1).servingMode() + doReturn(IpServer.STATE_TETHERED).`when`(server2).servingMode() val tracker = ConnectedClientsTracker(clock) doReturn(listOf(client1, client2)).`when`(server1).allLeases doReturn(listOf(client3)).`when`(server2).allLeases assertSameClients(listOf(client1, client2, client3), assertNewClients( - tracker, servers, listOf(wifiClient1, wifiClient2))) + tracker, servers, wifiClients = listOf(wifiClient1, wifiClient2))) clock.time += 20 // Client 3 has no remaining lease: removed @@ -131,15 +204,16 @@ class ConnectedClientsTrackerTest { // Only the "t + 30" address is left, the "t + 10" address expired listOf(client2Exp30AddrInfo), TETHERING_WIFI)) - assertSameClients(expectedClients, assertNewClients(tracker, servers, null)) + assertSameClients(expectedClients, assertNewClients(tracker, servers)) } private fun assertNewClients( tracker: ConnectedClientsTracker, ipServers: Iterable, - wifiClients: List? + wifiClients: List? = null, + localOnlyClients: List? = null ): List { - assertTrue(tracker.updateConnectedClients(ipServers, wifiClients)) + assertTrue(tracker.updateConnectedClients(ipServers, wifiClients, localOnlyClients)) return tracker.lastTetheredClients }