From 6e7edee44b7a7e7ac641a17981764b2a37024271 Mon Sep 17 00:00:00 2001 From: Treehugger Robot Date: Wed, 23 Dec 2020 12:31:53 +0000 Subject: [PATCH] Also update connected clients for local only tethering mForwardedDownstreams is the set of downstreams who wanted upstream. In other word, it don't contains localOnly tethering(e.g. local only hotspot, wifi p2p tethering). Changing the list from mForwardedDownstreams to mNotifyList make both tethered and localOnly tethering have connected clients callback. Bug: 172290164 Test: atest TetheringTests Original-Change: https://android-review.googlesource.com/1531561 Merged-In: I58fdb28efc616b00d63a1c237ea93aee4d8f2dcd Change-Id: I58fdb28efc616b00d63a1c237ea93aee4d8f2dcd --- .../networkstack/tethering/Tethering.java | 22 ++- .../networkstack/tethering/TetheringTest.java | 140 +++++++++++++++++- 2 files changed, 154 insertions(+), 8 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index 2c91d100ec..62ae88c24b 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -140,8 +140,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.Iterator; -import java.util.LinkedHashSet; import java.util.List; import java.util.Set; import java.util.concurrent.Executor; @@ -216,13 +216,12 @@ public class Tethering { private final ArrayMap mTetherStates; private final BroadcastReceiver mStateReceiver; private final Looper mLooper; - private final StateMachine mTetherMainSM; + private final TetherMainSM mTetherMainSM; private final OffloadController mOffloadController; private final UpstreamNetworkMonitor mUpstreamNetworkMonitor; // TODO: Figure out how to merge this and other downstream-tracking objects // into a single coherent structure. - // Use LinkedHashSet for predictable ordering order for ConnectedClientsTracker. - private final LinkedHashSet mForwardedDownstreams; + private final HashSet mForwardedDownstreams; private final VersionedBroadcastListener mCarrierConfigChange; private final TetheringDependencies mDeps; private final EntitlementManager mEntitlementMgr; @@ -287,7 +286,7 @@ public class Tethering { }); mUpstreamNetworkMonitor = mDeps.getUpstreamNetworkMonitor(mContext, mTetherMainSM, mLog, TetherMainSM.EVENT_UPSTREAM_CALLBACK); - mForwardedDownstreams = new LinkedHashSet<>(); + mForwardedDownstreams = new HashSet<>(); IntentFilter filter = new IntentFilter(); filter.addAction(ACTION_CARRIER_CONFIG_CHANGED); @@ -1423,6 +1422,7 @@ public class Tethering { // interfaces. // 2) mNotifyList contains all state machines that may have outstanding tethering state // that needs to be torn down. + // 3) Use mNotifyList for predictable ordering order for ConnectedClientsTracker. // // Because we excise interfaces immediately from mTetherStates, we must maintain mNotifyList // so that the garbage collector does not clean up the state machine before it has a chance @@ -1459,6 +1459,15 @@ public class Tethering { setInitialState(mInitialState); } + /** + * Returns all downstreams that are serving clients, regardless of they are actually + * tethered or localOnly. This must be called on the tethering thread (not thread-safe). + */ + @NonNull + public List getAllDownstreams() { + return mNotifyList; + } + class InitialState extends State { @Override public boolean processMessage(Message message) { @@ -2300,7 +2309,8 @@ public class Tethering { } private void updateConnectedClients(final List wifiClients) { - if (mConnectedClientsTracker.updateConnectedClients(mForwardedDownstreams, wifiClients)) { + if (mConnectedClientsTracker.updateConnectedClients(mTetherMainSM.getAllDownstreams(), + wifiClients)) { reportTetherClientsChanged(mConnectedClientsTracker.getLastTetheredClients()); } } diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java index 89146b55d1..bdc695c08f 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java @@ -16,6 +16,7 @@ package com.android.networkstack.tethering; +import static android.Manifest.permission.NETWORK_SETTINGS; import static android.content.pm.PackageManager.GET_ACTIVITIES; import static android.hardware.usb.UsbManager.USB_CONFIGURED; import static android.hardware.usb.UsbManager.USB_CONNECTED; @@ -36,6 +37,7 @@ import static android.net.TetheringManager.TETHERING_ETHERNET; import static android.net.TetheringManager.TETHERING_NCM; import static android.net.TetheringManager.TETHERING_USB; import static android.net.TetheringManager.TETHERING_WIFI; +import static android.net.TetheringManager.TETHERING_WIFI_P2P; import static android.net.TetheringManager.TETHER_ERROR_IFACE_CFG_ERROR; import static android.net.TetheringManager.TETHER_ERROR_NO_ERROR; import static android.net.TetheringManager.TETHER_ERROR_UNKNOWN_IFACE; @@ -49,12 +51,15 @@ import static android.net.wifi.WifiManager.EXTRA_WIFI_AP_STATE; 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.WIFI_AP_STATE_ENABLED; +import static android.system.OsConstants.RT_SCOPE_UNIVERSE; import static android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID; +import static com.android.net.module.util.Inet4AddressUtils.inet4AddressToIntHTH; import static com.android.net.module.util.Inet4AddressUtils.intToInet4AddressHTH; import static com.android.networkstack.tethering.Tethering.UserRestrictionActionListener; import static com.android.networkstack.tethering.TetheringNotificationUpdater.DOWNSTREAM_NONE; import static com.android.networkstack.tethering.UpstreamNetworkMonitor.EVENT_ON_CAPABILITIES; +import static com.android.testutils.TestPermissionUtil.runAsShell; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; @@ -108,11 +113,14 @@ import android.net.NetworkRequest; import android.net.RouteInfo; import android.net.TetherStatesParcel; import android.net.TetheredClient; +import android.net.TetheredClient.AddressInfo; import android.net.TetheringCallbackStartedParcel; import android.net.TetheringConfigurationParcel; import android.net.TetheringRequestParcel; +import android.net.dhcp.DhcpLeaseParcelable; import android.net.dhcp.DhcpServerCallbacks; import android.net.dhcp.DhcpServingParamsParcel; +import android.net.dhcp.IDhcpEventCallbacks; import android.net.dhcp.IDhcpServer; import android.net.ip.DadProxy; import android.net.ip.IpNeighborMonitor; @@ -122,7 +130,9 @@ import android.net.util.InterfaceParams; import android.net.util.NetworkConstants; import android.net.util.SharedLog; import android.net.wifi.SoftApConfiguration; +import android.net.wifi.WifiClient; import android.net.wifi.WifiManager; +import android.net.wifi.WifiManager.SoftApCallback; import android.net.wifi.p2p.WifiP2pGroup; import android.net.wifi.p2p.WifiP2pInfo; import android.net.wifi.p2p.WifiP2pManager; @@ -167,6 +177,7 @@ import java.net.Inet6Address; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Vector; @@ -236,6 +247,7 @@ public class TetheringTest { private EntitlementManager mEntitleMgr; private OffloadController mOffloadCtrl; private PrivateAddressCoordinator mPrivateAddressCoordinator; + private SoftApCallback mSoftApCallback; private class TestContext extends BroadcastInterceptingContext { TestContext(Context base) { @@ -567,8 +579,12 @@ public class TetheringTest { ArgumentCaptor.forClass(PhoneStateListener.class); verify(mTelephonyManager).listen(phoneListenerCaptor.capture(), eq(PhoneStateListener.LISTEN_ACTIVE_DATA_SUBSCRIPTION_ID_CHANGE)); - verify(mWifiManager).registerSoftApCallback(any(), any()); mPhoneStateListener = phoneListenerCaptor.getValue(); + + final ArgumentCaptor softApCallbackCaptor = + ArgumentCaptor.forClass(SoftApCallback.class); + verify(mWifiManager).registerSoftApCallback(any(), softApCallbackCaptor.capture()); + mSoftApCallback = softApCallbackCaptor.getValue(); } private void setTetheringSupported(final boolean supported) { @@ -1284,6 +1300,7 @@ public class TetheringTest { new ArrayList<>(); private final ArrayList mTetherStates = new ArrayList<>(); private final ArrayList mOffloadStatus = new ArrayList<>(); + private final ArrayList> mTetheredClients = new ArrayList<>(); // This function will remove the recorded callbacks, so it must be called once for // each callback. If this is called after multiple callback, the order matters. @@ -1329,6 +1346,13 @@ public class TetheringTest { return mTetherStates.remove(0); } + public void expectTetheredClientChanged(List leases) { + assertFalse(mTetheredClients.isEmpty()); + final List result = mTetheredClients.remove(0); + assertEquals(leases.size(), result.size()); + assertTrue(leases.containsAll(result)); + } + @Override public void onUpstreamChanged(Network network) { mActualUpstreams.add(network); @@ -1346,7 +1370,7 @@ public class TetheringTest { @Override public void onTetherClientsChanged(List clients) { - // TODO: check this + mTetheredClients.add(clients); } @Override @@ -1360,6 +1384,7 @@ public class TetheringTest { mTetheringConfigs.add(parcel.config); mTetherStates.add(parcel.states); mOffloadStatus.add(parcel.offloadStatus); + mTetheredClients.add(parcel.tetheredClients); } @Override @@ -1389,6 +1414,7 @@ public class TetheringTest { assertNoUpstreamChangeCallback(); assertNoConfigChangeCallback(); assertNoStateChangeCallback(); + assertTrue(mTetheredClients.isEmpty()); } private void assertTetherConfigParcelEqual(@NonNull TetheringConfigurationParcel actual, @@ -1428,6 +1454,7 @@ public class TetheringTest { // 1. Register one callback before running any tethering. mTethering.registerTetheringEventCallback(callback); mLooper.dispatchAll(); + callback.expectTetheredClientChanged(Collections.emptyList()); callback.expectUpstreamChanged(new Network[] {null}); callback.expectConfigurationChanged( mTethering.getTetheringConfiguration().toStableParcelable()); @@ -1454,6 +1481,7 @@ public class TetheringTest { // 3. Register second callback. mTethering.registerTetheringEventCallback(callback2); mLooper.dispatchAll(); + callback2.expectTetheredClientChanged(Collections.emptyList()); callback2.expectUpstreamChanged(upstreamState.network); callback2.expectConfigurationChanged( mTethering.getTetheringConfiguration().toStableParcelable()); @@ -2045,6 +2073,114 @@ public class TetheringTest { verify(mPackageManager).getPackageInfo(PROVISIONING_APP_NAME[0], GET_ACTIVITIES); } + @Test + public void testUpdateConnectedClients() throws Exception { + TestTetheringEventCallback callback = new TestTetheringEventCallback(); + runAsShell(NETWORK_SETTINGS, () -> { + mTethering.registerTetheringEventCallback(callback); + mLooper.dispatchAll(); + }); + callback.expectTetheredClientChanged(Collections.emptyList()); + + IDhcpEventCallbacks eventCallbacks; + final ArgumentCaptor dhcpEventCbsCaptor = + ArgumentCaptor.forClass(IDhcpEventCallbacks.class); + // Run local only tethering. + mTethering.interfaceStatusChanged(TEST_P2P_IFNAME, true); + sendWifiP2pConnectionChanged(true, true, TEST_P2P_IFNAME); + mLooper.dispatchAll(); + verify(mDhcpServer, timeout(DHCPSERVER_START_TIMEOUT_MS)).startWithCallbacks( + any(), dhcpEventCbsCaptor.capture()); + eventCallbacks = dhcpEventCbsCaptor.getValue(); + // Update lease for local only tethering. + final MacAddress testMac1 = MacAddress.fromString("11:11:11:11:11:11"); + final ArrayList p2pLeases = new ArrayList<>(); + p2pLeases.add(createDhcpLeaseParcelable("clientId1", testMac1, "192.168.50.24", 24, + Long.MAX_VALUE, "test1")); + notifyDhcpLeasesChanged(p2pLeases, eventCallbacks); + final List clients = toTetheredClients(p2pLeases, TETHERING_WIFI_P2P); + callback.expectTetheredClientChanged(clients); + reset(mDhcpServer); + + // Run wifi tethering. + mTethering.interfaceStatusChanged(TEST_WLAN_IFNAME, true); + sendWifiApStateChanged(WIFI_AP_STATE_ENABLED, TEST_WLAN_IFNAME, IFACE_IP_MODE_TETHERED); + mLooper.dispatchAll(); + verify(mDhcpServer, timeout(DHCPSERVER_START_TIMEOUT_MS)).startWithCallbacks( + any(), dhcpEventCbsCaptor.capture()); + eventCallbacks = dhcpEventCbsCaptor.getValue(); + // Update mac address from softAp callback before getting dhcp lease. + final ArrayList wifiClients = new ArrayList<>(); + final MacAddress testMac2 = MacAddress.fromString("22:22:22:22:22:22"); + final WifiClient testClient = mock(WifiClient.class); + when(testClient.getMacAddress()).thenReturn(testMac2); + wifiClients.add(testClient); + mSoftApCallback.onConnectedClientsChanged(wifiClients); + final TetheredClient noAddrClient = new TetheredClient(testMac2, + Collections.emptyList() /* addresses */, TETHERING_WIFI); + clients.add(noAddrClient); + callback.expectTetheredClientChanged(clients); + + // Update dhcp lease for wifi tethering. + clients.remove(noAddrClient); + final ArrayList wifiLeases = new ArrayList<>(); + wifiLeases.add(createDhcpLeaseParcelable("clientId2", testMac2, "192.168.43.24", 24, + Long.MAX_VALUE, "test2")); + notifyDhcpLeasesChanged(wifiLeases, eventCallbacks); + clients.addAll(toTetheredClients(wifiLeases, TETHERING_WIFI)); + callback.expectTetheredClientChanged(clients); + + // Test onStarted callback that register second callback when tethering is running. + TestTetheringEventCallback callback2 = new TestTetheringEventCallback(); + runAsShell(NETWORK_SETTINGS, () -> { + mTethering.registerTetheringEventCallback(callback2); + mLooper.dispatchAll(); + }); + callback2.expectTetheredClientChanged(clients); + } + + private void notifyDhcpLeasesChanged(List leaseParcelables, + IDhcpEventCallbacks callback) throws Exception { + callback.onLeasesChanged(leaseParcelables); + mLooper.dispatchAll(); + } + + private List toTetheredClients(List leaseParcelables, + int type) throws Exception { + final ArrayList leases = new ArrayList<>(); + for (DhcpLeaseParcelable lease : leaseParcelables) { + final LinkAddress address = new LinkAddress( + intToInet4AddressHTH(lease.netAddr), lease.prefixLength, + 0 /* flags */, RT_SCOPE_UNIVERSE /* as per RFC6724#3.2 */, + lease.expTime /* deprecationTime */, lease.expTime /* expirationTime */); + + final MacAddress macAddress = MacAddress.fromBytes(lease.hwAddr); + + final AddressInfo addressInfo = new TetheredClient.AddressInfo(address, lease.hostname); + leases.add(new TetheredClient( + macAddress, + Collections.singletonList(addressInfo), + type)); + } + + return leases; + } + + private DhcpLeaseParcelable createDhcpLeaseParcelable(final String clientId, + final MacAddress hwAddr, final String netAddr, final int prefixLength, + final long expTime, final String hostname) throws Exception { + final DhcpLeaseParcelable lease = new DhcpLeaseParcelable(); + lease.clientId = clientId.getBytes(); + lease.hwAddr = hwAddr.toByteArray(); + lease.netAddr = inet4AddressToIntHTH( + (Inet4Address) InetAddresses.parseNumericAddress(netAddr)); + lease.prefixLength = prefixLength; + lease.expTime = expTime; + lease.hostname = hostname; + + return lease; + } + // TODO: Test that a request for hotspot mode doesn't interfere with an // already operating tethering mode interface. }