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/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<WifiClient> mLastWifiClients = Collections.emptyList();
     @NonNull
+    private List<WifiClient> mLastLocalOnlyClients = Collections.emptyList();
+    @NonNull
     private List<TetheredClient> mLastTetheredClients = Collections.emptyList();
 
     @VisibleForTesting
@@ -72,25 +76,44 @@ public class ConnectedClientsTracker {
      *
      * <p>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
+     * @param wifiClients The list of L2-connected WiFi clients that are connected to a global
-     *                    update.
+     *                    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<IpServer> ipServers, @Nullable List<WifiClient> wifiClients) {
+            Iterable<IpServer> ipServers, @Nullable List<WifiClient> wifiClients,
+            @Nullable List<WifiClient> localOnlyClients) {
         final long now = mClock.elapsedRealtime();
 
-        if (wifiClients != null) {
+        if (wifiClients != null) mLastWifiClients = wifiClients;
-            mLastWifiClients = wifiClients;
+        if (localOnlyClients != null) mLastLocalOnlyClients = localOnlyClients;
-        }
+
         final Set<MacAddress> wifiClientMacs = getClientMacs(mLastWifiClients);
+        final Set<MacAddress> localOnlyClientMacs = getClientMacs(mLastLocalOnlyClients);
 
         // Build the list of non-expired leases from all IpServers, grouped by mac address
         final Map<MacAddress, TetheredClient> clientsMap = new HashMap<>();
         for (IpServer server : ipServers) {
+            final Set<MacAddress> 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) {
+        addWifiClientsIfNoLeases(clientsMap, wifiClientMacs);
-            if (clientsMap.containsKey(client)) continue;
+        addWifiClientsIfNoLeases(clientsMap, localOnlyClientMacs);
-            clientsMap.put(client, new TetheredClient(
-                    client, Collections.emptyList() /* addresses */, TETHERING_WIFI));
-        }
 
         final HashSet<TetheredClient> 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<MacAddress, TetheredClient> clientsMap, final Set<MacAddress> 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<MacAddress, TetheredClient> clientsMap, TetheredClient lease) {
         final TetheredClient aggregateClient = clientsMap.getOrDefault(
                 lease.getMacAddress(), lease);

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);
+            wifiManager.registerSoftApCallback(mExecutor, new TetheringSoftApCallback());
-        }
+            if (SdkLevel.isAtLeastT()) {
-        if (SdkLevel.isAtLeastT() && wifiManager != null) {
+                // Although WifiManager#registerLocalOnlyHotspotSoftApCallback document that it need
-            // Although WifiManager#registerLocalOnlyHotspotSoftApCallback document that it need
+                // NEARBY_WIFI_DEVICES permission, but actually a caller who have NETWORK_STACK
-            // NEARBY_WIFI_DEVICES permission, but actually a caller who have NETWORK_STACK
+                // or MAINLINE_NETWORK_STACK permission can also use this API.
-            // or MAINLINE_NETWORK_STACK permission would also able to use this API.
+                wifiManager.registerLocalOnlyHotspotSoftApCallback(mExecutor,
-            wifiManager.registerLocalOnlyHotspotSoftApCallback(mExecutor, softApCallback);
+                        new LocalOnlyHotspotCallback());
+            }
         }
 
         startTrackDefaultNetwork();
@@ -573,26 +574,17 @@ public class Tethering {
         }
     }
 
-    private class TetheringSoftApCallback implements WifiManager.SoftApCallback {
+    private class TetheringSoftApCallback implements 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.
         @Override
         public void onConnectedClientsChanged(final List<WifiClient> clients) {
-            updateConnectedClients(clients);
+            updateConnectedClients(clients, null);
+        }
+    }
+
+    private class LocalOnlyHotspotCallback implements SoftApCallback {
+        @Override
+        public void onConnectedClientsChanged(final List<WifiClient> 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<WifiClient> wifiClients) {
+    private void maybeDhcpLeasesChanged() {
+        // null means wifi clients did not change.
+        updateConnectedClients(null, null);
+    }
+
+    private void updateConnectedClients(final List<WifiClient> wifiClients,
+            final List<WifiClient> 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

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<TetheredClient>()).`when`(server1).allLeases
         doReturn(emptyList<TetheredClient>()).`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),
+        if (isGlobal) {
-                assertNewClients(tracker, servers, listOf(wifiClient1, wifiClient2)))
+            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),
+        assertSameClients(listOf(client1, client2, client3), assertNewClients(tracker, servers))
-                assertNewClients(tracker, servers, null))
 
-        // Client 2 L2-disconnected
+        if (isGlobal) {
-        assertSameClients(listOf(client1, client3),
+            // Client 2 L2-disconnected
-                assertNewClients(tracker, servers, listOf(wifiClient1)))
+            assertSameClients(listOf(client1, client3),
-
+                    assertNewClients(tracker, servers, wifiClients = listOf(wifiClient1)))
-        // Client 1 L2-disconnected
+            // Client 1 L2-disconnected
-        assertSameClients(listOf(client3), assertNewClients(tracker, servers, emptyList()))
+            assertSameClients(listOf(client3), assertNewClients(tracker, servers,
-
+                    wifiClients = emptyList()))
-        // Client 1 comes back
+            // Client 1 comes back
-        assertSameClients(listOf(client1, client3),
+            assertSameClients(listOf(client1, client3),
-                assertNewClients(tracker, servers, listOf(wifiClient1)))
+                    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<TetheredClient>()).`when`(server1).allLeases
         doReturn(emptyList<TetheredClient>()).`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<TetheredClient>()).`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<TetheredClient>()).`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<IpServer>,
-        wifiClients: List<WifiClient>?
+        wifiClients: List<WifiClient>? = null,
+        localOnlyClients: List<WifiClient>? = null
     ): List<TetheredClient> {
-        assertTrue(tracker.updateConnectedClients(ipServers, wifiClients))
+        assertTrue(tracker.updateConnectedClients(ipServers, wifiClients, localOnlyClients))
         return tracker.lastTetheredClients
     }