From 72302908d42d8317b69da68285898d995523a33f Mon Sep 17 00:00:00 2001 From: Erik Kline Date: Thu, 14 Jun 2018 17:36:40 +0900 Subject: [PATCH] Prefer default Internet network for upstream tethering. Rather than use the crufty config.xml list of upstream transport types, use ConnectivityService's notion of the default network for the upstream. In cases where a DUN network is required and the default network is currently a mobile network, look for a DUN network (code in Tethering is currently responsible for requesting one). Test: as follows - built, flashed, booted - runtest frameworks-net - tethered via mobile, joined captive portal network, maintained laptop access via mobile until captive passed (then used wifi) - disabled client mode wifi, disabled mobile data, plugged in ethernet adapter, observed connectivity via ethernet Bug: 32163131 Bug: 62648872 Bug: 63282480 Bug: 109786760 Bug: 110118584 Bug: 110260419 Merged-In: I9cddf1fb7aa3b8d56bf048c563556244e74808c2 Merged-In: Icac3e5e20e99093ddb85aae1ca07ed7b5cf309fd Change-Id: I925b75994e31df8046f3ef9916a2457b4210485e (cherry picked from commit 4080a1bd15572caf149762e45c958627feceb74d) --- core/res/res/values/config.xml | 10 +- core/res/res/values/symbols.xml | 1 + .../com/android/server/ConnectivityService.java | 6 +- .../com/android/server/connectivity/Tethering.java | 22 +-- .../tethering/TetheringConfiguration.java | 14 ++ .../tethering/TetheringDependencies.java | 5 + .../tethering/UpstreamNetworkMonitor.java | 161 +++++++++++---------- .../android/server/connectivity/TetheringTest.java | 35 ++++- .../tethering/UpstreamNetworkMonitorTest.java | 147 +++++++++++++++---- 9 files changed, 273 insertions(+), 128 deletions(-) diff --git a/core/res/res/values/config.xml b/core/res/res/values/config.xml index d56c7260fb03..544c95351b7c 100644 --- a/core/res/res/values/config.xml +++ b/core/res/res/values/config.xml @@ -488,13 +488,21 @@ Note also: the order of this is important. The first upstream type for which a satisfying network exists is used. - --> + --> 1 7 0 + + false + diff --git a/core/res/res/values/symbols.xml b/core/res/res/values/symbols.xml index e52f0f818636..39cf36437a59 100644 --- a/core/res/res/values/symbols.xml +++ b/core/res/res/values/symbols.xml @@ -1732,6 +1732,7 @@ + diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 49d60a7aa6bb..0b24db00368f 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -868,6 +868,10 @@ public class ConnectivityService extends IConnectivityManager.Stub public boolean isTetheringSupported() { return ConnectivityService.this.isTetheringSupported(); } + @Override + public NetworkRequest getDefaultNetworkRequest() { + return mDefaultRequest; + } }; return new Tethering(mContext, mNetd, mStatsService, mPolicyManager, IoThread.get().getLooper(), new MockableSystemProperties(), @@ -885,7 +889,7 @@ public class ConnectivityService extends IConnectivityManager.Stub private NetworkRequest createDefaultInternetRequestForTransport( int transportType, NetworkRequest.Type type) { - NetworkCapabilities netCap = new NetworkCapabilities(); + final NetworkCapabilities netCap = new NetworkCapabilities(); netCap.addCapability(NET_CAPABILITY_INTERNET); netCap.addCapability(NET_CAPABILITY_NOT_RESTRICTED); if (transportType > -1) { diff --git a/services/core/java/com/android/server/connectivity/Tethering.java b/services/core/java/com/android/server/connectivity/Tethering.java index 62fe218da262..0230f75b8a91 100644 --- a/services/core/java/com/android/server/connectivity/Tethering.java +++ b/services/core/java/com/android/server/connectivity/Tethering.java @@ -1349,8 +1349,11 @@ public class Tethering extends BaseNetworkObserver { // do not currently know how to watch for changes in DUN settings. maybeUpdateConfiguration(); - final NetworkState ns = mUpstreamNetworkMonitor.selectPreferredUpstreamType( - mConfig.preferredUpstreamIfaceTypes); + final TetheringConfiguration config = mConfig; + final NetworkState ns = (config.chooseUpstreamAutomatically) + ? mUpstreamNetworkMonitor.getCurrentPreferredUpstream() + : mUpstreamNetworkMonitor.selectPreferredUpstreamType( + config.preferredUpstreamIfaceTypes); if (ns == null) { if (tryCell) { mUpstreamNetworkMonitor.registerMobileNetworkRequest(); @@ -1379,9 +1382,7 @@ public class Tethering extends BaseNetworkObserver { } notifyDownstreamsOfNewUpstreamIface(ifaces); if (ns != null && pertainsToCurrentUpstream(ns)) { - // If we already have NetworkState for this network examine - // it immediately, because there likely will be no second - // EVENT_ON_AVAILABLE (it was already received). + // If we already have NetworkState for this network update it immediately. handleNewUpstreamNetworkState(ns); } else if (mCurrentUpstreamIfaceSet == null) { // There are no available upstream networks. @@ -1497,15 +1498,6 @@ public class Tethering extends BaseNetworkObserver { } switch (arg1) { - case UpstreamNetworkMonitor.EVENT_ON_AVAILABLE: - // The default network changed, or DUN connected - // before this callback was processed. Updates - // for the current NetworkCapabilities and - // LinkProperties have been requested (default - // request) or are being sent shortly (DUN). Do - // nothing until they arrive; if no updates - // arrive there's nothing to do. - break; case UpstreamNetworkMonitor.EVENT_ON_CAPABILITIES: handleNewUpstreamNetworkState(ns); break; @@ -1538,7 +1530,7 @@ public class Tethering extends BaseNetworkObserver { } mSimChange.startListening(); - mUpstreamNetworkMonitor.start(); + mUpstreamNetworkMonitor.start(mDeps.getDefaultNetworkRequest()); // TODO: De-duplicate with updateUpstreamWanted() below. if (upstreamWanted()) { diff --git a/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java b/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java index 454c579ed0a7..dd9fe05b50ff 100644 --- a/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java +++ b/services/core/java/com/android/server/connectivity/tethering/TetheringConfiguration.java @@ -27,6 +27,7 @@ import static com.android.internal.R.array.config_tether_dhcp_range; import static com.android.internal.R.array.config_tether_usb_regexs; import static com.android.internal.R.array.config_tether_upstream_types; import static com.android.internal.R.array.config_tether_wifi_regexs; +import static com.android.internal.R.bool.config_tether_upstream_automatic; import static com.android.internal.R.string.config_mobile_hotspot_provision_app_no_ui; import android.content.Context; @@ -86,6 +87,7 @@ public class TetheringConfiguration { public final String[] tetherableBluetoothRegexs; public final int dunCheck; public final boolean isDunRequired; + public final boolean chooseUpstreamAutomatically; public final Collection preferredUpstreamIfaceTypes; public final String[] dhcpRanges; public final String[] defaultIPv4DNS; @@ -106,6 +108,7 @@ public class TetheringConfiguration { dunCheck = checkDunRequired(ctx); configLog.log("DUN check returned: " + dunCheckString(dunCheck)); + chooseUpstreamAutomatically = getResourceBoolean(ctx, config_tether_upstream_automatic); preferredUpstreamIfaceTypes = getUpstreamIfaceTypes(ctx, dunCheck); isDunRequired = preferredUpstreamIfaceTypes.contains(TYPE_MOBILE_DUN); @@ -142,6 +145,8 @@ public class TetheringConfiguration { pw.print("isDunRequired: "); pw.println(isDunRequired); + pw.print("chooseUpstreamAutomatically: "); + pw.println(chooseUpstreamAutomatically); dumpStringArray(pw, "preferredUpstreamIfaceTypes", preferredUpstreamNames(preferredUpstreamIfaceTypes)); @@ -160,6 +165,7 @@ public class TetheringConfiguration { sj.add(String.format("tetherableBluetoothRegexs:%s", makeString(tetherableBluetoothRegexs))); sj.add(String.format("isDunRequired:%s", isDunRequired)); + sj.add(String.format("chooseUpstreamAutomatically:%s", chooseUpstreamAutomatically)); sj.add(String.format("preferredUpstreamIfaceTypes:%s", makeString(preferredUpstreamNames(preferredUpstreamIfaceTypes)))); sj.add(String.format("provisioningApp:%s", makeString(provisioningApp))); @@ -286,6 +292,14 @@ public class TetheringConfiguration { } } + private static boolean getResourceBoolean(Context ctx, int resId) { + try { + return ctx.getResources().getBoolean(resId); + } catch (Resources.NotFoundException e404) { + return false; + } + } + private static String[] getResourceStringArray(Context ctx, int resId) { try { final String[] strArray = ctx.getResources().getStringArray(resId); diff --git a/services/core/java/com/android/server/connectivity/tethering/TetheringDependencies.java b/services/core/java/com/android/server/connectivity/tethering/TetheringDependencies.java index 0ac7a36e3ffc..605ee9c8ee2e 100644 --- a/services/core/java/com/android/server/connectivity/tethering/TetheringDependencies.java +++ b/services/core/java/com/android/server/connectivity/tethering/TetheringDependencies.java @@ -18,6 +18,7 @@ package com.android.server.connectivity.tethering; import android.content.Context; import android.net.INetd; +import android.net.NetworkRequest; import android.net.ip.RouterAdvertisementDaemon; import android.net.util.InterfaceParams; import android.net.util.NetdService; @@ -64,4 +65,8 @@ public class TetheringDependencies { public boolean isTetheringSupported() { return true; } + + public NetworkRequest getDefaultNetworkRequest() { + return null; + } } diff --git a/services/core/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitor.java b/services/core/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitor.java index 34132910ca86..f488be70af49 100644 --- a/services/core/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitor.java +++ b/services/core/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitor.java @@ -20,6 +20,9 @@ import static android.net.ConnectivityManager.getNetworkTypeName; import static android.net.ConnectivityManager.TYPE_NONE; import static android.net.ConnectivityManager.TYPE_MOBILE_DUN; import static android.net.ConnectivityManager.TYPE_MOBILE_HIPRI; +import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN; +import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_VPN; +import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import android.content.Context; import android.os.Handler; @@ -74,14 +77,13 @@ public class UpstreamNetworkMonitor { private static final boolean DBG = false; private static final boolean VDBG = false; - public static final int EVENT_ON_AVAILABLE = 1; - public static final int EVENT_ON_CAPABILITIES = 2; - public static final int EVENT_ON_LINKPROPERTIES = 3; - public static final int EVENT_ON_LOST = 4; + public static final int EVENT_ON_CAPABILITIES = 1; + public static final int EVENT_ON_LINKPROPERTIES = 2; + public static final int EVENT_ON_LOST = 3; public static final int NOTIFY_LOCAL_PREFIXES = 10; private static final int CALLBACK_LISTEN_ALL = 1; - private static final int CALLBACK_TRACK_DEFAULT = 2; + private static final int CALLBACK_DEFAULT_INTERNET = 2; private static final int CALLBACK_MOBILE_REQUEST = 3; private final Context mContext; @@ -117,7 +119,7 @@ public class UpstreamNetworkMonitor { mCM = cm; } - public void start() { + public void start(NetworkRequest defaultNetworkRequest) { stop(); final NetworkRequest listenAllRequest = new NetworkRequest.Builder() @@ -125,8 +127,16 @@ public class UpstreamNetworkMonitor { mListenAllCallback = new UpstreamNetworkCallback(CALLBACK_LISTEN_ALL); cm().registerNetworkCallback(listenAllRequest, mListenAllCallback, mHandler); - mDefaultNetworkCallback = new UpstreamNetworkCallback(CALLBACK_TRACK_DEFAULT); - cm().registerDefaultNetworkCallback(mDefaultNetworkCallback, mHandler); + if (defaultNetworkRequest != null) { + // This is not really a "request", just a way of tracking the system default network. + // It's guaranteed not to actually bring up any networks because it's the same request + // as the ConnectivityService default request, and thus shares fate with it. We can't + // use registerDefaultNetworkCallback because it will not track the system default + // network if there is a VPN that applies to our UID. + final NetworkRequest trackDefaultRequest = new NetworkRequest(defaultNetworkRequest); + mDefaultNetworkCallback = new UpstreamNetworkCallback(CALLBACK_DEFAULT_INTERNET); + cm().requestNetwork(trackDefaultRequest, mDefaultNetworkCallback, mHandler); + } } public void stop() { @@ -225,6 +235,20 @@ public class UpstreamNetworkMonitor { return typeStatePair.ns; } + // Returns null if no current upstream available. + public NetworkState getCurrentPreferredUpstream() { + final NetworkState dfltState = (mDefaultInternetNetwork != null) + ? mNetworkMap.get(mDefaultInternetNetwork) + : null; + if (!mDunRequired) return dfltState; + + if (isNetworkUsableAndNotCellular(dfltState)) return dfltState; + + // Find a DUN network. Note that code in Tethering causes a DUN request + // to be filed, but this might be moved into this class in future. + return findFirstDunNetwork(mNetworkMap.values()); + } + public void setCurrentUpstream(Network upstream) { mTetheringUpstreamNetwork = upstream; } @@ -233,72 +257,16 @@ public class UpstreamNetworkMonitor { return (Set) mLocalPrefixes.clone(); } - private void handleAvailable(int callbackType, Network network) { - if (VDBG) Log.d(TAG, "EVENT_ON_AVAILABLE for " + network); - - if (!mNetworkMap.containsKey(network)) { - mNetworkMap.put(network, - new NetworkState(null, null, null, network, null, null)); - } - - // Always request whatever extra information we can, in case this - // was already up when start() was called, in which case we would - // not have been notified of any information that had not changed. - switch (callbackType) { - case CALLBACK_LISTEN_ALL: - break; - - case CALLBACK_TRACK_DEFAULT: - if (mDefaultNetworkCallback == null) { - // The callback was unregistered in the interval between - // ConnectivityService enqueueing onAvailable() and our - // handling of it here on the mHandler thread. - // - // Clean-up of this network entry is deferred to the - // handling of onLost() by other callbacks. - // - // These request*() calls can be deleted post oag/339444. - return; - } - mDefaultInternetNetwork = network; - break; - - case CALLBACK_MOBILE_REQUEST: - if (mMobileNetworkCallback == null) { - // The callback was unregistered in the interval between - // ConnectivityService enqueueing onAvailable() and our - // handling of it here on the mHandler thread. - // - // Clean-up of this network entry is deferred to the - // handling of onLost() by other callbacks. - return; - } - break; - } - - // Requesting updates for mListenAllCallback is not currently possible - // because it's a "listen". Two possible solutions to getting updates - // about networks without waiting for a change (which might never come) - // are: - // - // [1] extend request{NetworkCapabilities,LinkProperties}() to - // take a Network argument and have ConnectivityService do - // what's required (if the network satisfies the request) - // - // [2] explicitly file a NetworkRequest for each connectivity type - // listed as a preferred upstream and wait for these callbacks - // to be notified (requires tracking many more callbacks). - // - // Until this is addressed, networks that exist prior to the "listen" - // registration and which do not subsequently change will not cause - // us to learn their NetworkCapabilities nor their LinkProperties. + private void handleAvailable(Network network) { + if (mNetworkMap.containsKey(network)) return; - // TODO: If sufficient information is available to select a more - // preferable upstream, do so now and notify the target. - notifyTarget(EVENT_ON_AVAILABLE, network); + if (VDBG) Log.d(TAG, "onAvailable for " + network); + mNetworkMap.put(network, new NetworkState(null, null, null, network, null, null)); } - private void handleNetCap(Network network, NetworkCapabilities newNc) { + private void handleNetCap(int callbackType, Network network, NetworkCapabilities newNc) { + if (callbackType == CALLBACK_DEFAULT_INTERNET) mDefaultInternetNetwork = network; + final NetworkState prev = mNetworkMap.get(network); if (prev == null || newNc.equals(prev.networkCapabilities)) { // Ignore notifications about networks for which we have not yet @@ -360,13 +328,17 @@ public class UpstreamNetworkMonitor { } private void handleLost(int callbackType, Network network) { - if (callbackType == CALLBACK_TRACK_DEFAULT) { + if (network.equals(mDefaultInternetNetwork)) { mDefaultInternetNetwork = null; - // Receiving onLost() for a default network does not necessarily - // mean the network is gone. We wait for a separate notification - // on either the LISTEN_ALL or MOBILE_REQUEST callbacks before - // clearing all state. - return; + // There are few TODOs within ConnectivityService's rematching code + // pertaining to spurious onLost() notifications. + // + // TODO: simplify this, probably if favor of code that: + // - selects a new upstream if mTetheringUpstreamNetwork has + // been lost (by any callback) + // - deletes the entry from the map only when the LISTEN_ALL + // callback gets notified. + if (callbackType == CALLBACK_DEFAULT_INTERNET) return; } if (!mNetworkMap.containsKey(network)) { @@ -416,17 +388,19 @@ public class UpstreamNetworkMonitor { @Override public void onAvailable(Network network) { - handleAvailable(mCallbackType, network); + handleAvailable(network); } @Override public void onCapabilitiesChanged(Network network, NetworkCapabilities newNc) { - handleNetCap(network, newNc); + handleNetCap(mCallbackType, network, newNc); } @Override public void onLinkPropertiesChanged(Network network, LinkProperties newLp) { handleLinkProp(network, newLp); + // TODO(b/110335330): reduce the number of times this is called by + // only recomputing on the LISTEN_ALL callback. recomputeLocalPrefixes(); } @@ -443,6 +417,8 @@ public class UpstreamNetworkMonitor { @Override public void onLost(Network network) { handleLost(mCallbackType, network); + // TODO(b/110335330): reduce the number of times this is called by + // only recomputing on the LISTEN_ALL callback. recomputeLocalPrefixes(); } } @@ -509,4 +485,31 @@ public class UpstreamNetworkMonitor { if (nc == null || !nc.hasSignalStrength()) return "unknown"; return Integer.toString(nc.getSignalStrength()); } + + private static boolean isCellular(NetworkState ns) { + return (ns != null) && isCellular(ns.networkCapabilities); + } + + private static boolean isCellular(NetworkCapabilities nc) { + return (nc != null) && nc.hasTransport(TRANSPORT_CELLULAR) && + nc.hasCapability(NET_CAPABILITY_NOT_VPN); + } + + private static boolean hasCapability(NetworkState ns, int netCap) { + return (ns != null) && (ns.networkCapabilities != null) && + ns.networkCapabilities.hasCapability(netCap); + } + + private static boolean isNetworkUsableAndNotCellular(NetworkState ns) { + return (ns != null) && (ns.networkCapabilities != null) && (ns.linkProperties != null) && + !isCellular(ns.networkCapabilities); + } + + private static NetworkState findFirstDunNetwork(Iterable netStates) { + for (NetworkState ns : netStates) { + if (isCellular(ns) && hasCapability(ns, NET_CAPABILITY_DUN)) return ns; + } + + return null; + } } diff --git a/tests/net/java/com/android/server/connectivity/TetheringTest.java b/tests/net/java/com/android/server/connectivity/TetheringTest.java index 9994cdd5c595..44d8f313f18e 100644 --- a/tests/net/java/com/android/server/connectivity/TetheringTest.java +++ b/tests/net/java/com/android/server/connectivity/TetheringTest.java @@ -71,6 +71,7 @@ import android.net.MacAddress; import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkInfo; +import android.net.NetworkRequest; import android.net.NetworkState; import android.net.NetworkUtils; import android.net.RouteInfo; @@ -128,6 +129,10 @@ public class TetheringTest { private static final String TEST_USB_IFNAME = "test_rndis0"; private static final String TEST_WLAN_IFNAME = "test_wlan0"; + // Actual contents of the request don't matter for this test. The lack of + // any specific TRANSPORT_* is sufficient to identify this request. + private static final NetworkRequest mDefaultRequest = new NetworkRequest.Builder().build(); + @Mock private ApplicationInfo mApplicationInfo; @Mock private Context mContext; @Mock private INetworkManagementService mNMService; @@ -238,6 +243,11 @@ public class TetheringTest { isTetheringSupportedCalls++; return true; } + + @Override + public NetworkRequest getDefaultNetworkRequest() { + return mDefaultRequest; + } } private static NetworkState buildMobileUpstreamState(boolean withIPv4, boolean withIPv6, @@ -305,6 +315,8 @@ public class TetheringTest { .thenReturn(new String[0]); when(mResources.getIntArray(com.android.internal.R.array.config_tether_upstream_types)) .thenReturn(new int[0]); + when(mResources.getBoolean(com.android.internal.R.bool.config_tether_upstream_automatic)) + .thenReturn(false); when(mNMService.listInterfaces()) .thenReturn(new String[] { TEST_MOBILE_IFNAME, TEST_WLAN_IFNAME, TEST_USB_IFNAME}); @@ -458,6 +470,7 @@ public class TetheringTest { } private void prepareUsbTethering(NetworkState upstreamState) { + when(mUpstreamNetworkMonitor.getCurrentPreferredUpstream()).thenReturn(upstreamState); when(mUpstreamNetworkMonitor.selectPreferredUpstreamType(any())) .thenReturn(upstreamState); @@ -519,7 +532,7 @@ public class TetheringTest { TEST_WLAN_IFNAME, WifiManager.IFACE_IP_MODE_LOCAL_ONLY); verifyNoMoreInteractions(mWifiManager); verifyTetheringBroadcast(TEST_WLAN_IFNAME, EXTRA_ACTIVE_LOCAL_ONLY); - verify(mUpstreamNetworkMonitor, times(1)).start(); + verify(mUpstreamNetworkMonitor, times(1)).start(any(NetworkRequest.class)); // TODO: Figure out why this isn't exactly once, for sendTetherStateChangedBroadcast(). assertTrue(1 <= mTetheringDependencies.isTetheringSupportedCalls); @@ -656,6 +669,24 @@ public class TetheringTest { } @Test + public void configTetherUpstreamAutomaticIgnoresConfigTetherUpstreamTypes() throws Exception { + when(mResources.getBoolean(com.android.internal.R.bool.config_tether_upstream_automatic)) + .thenReturn(true); + sendConfigurationChanged(); + + // Setup IPv6 + final NetworkState upstreamState = buildMobileIPv6UpstreamState(); + runUsbTethering(upstreamState); + + // UpstreamNetworkMonitor should choose upstream automatically + // (in this specific case: choose the default network). + verify(mUpstreamNetworkMonitor, times(1)).getCurrentPreferredUpstream(); + verify(mUpstreamNetworkMonitor, never()).selectPreferredUpstreamType(any()); + + verify(mUpstreamNetworkMonitor, times(1)).setCurrentUpstream(upstreamState.network); + } + + @Test public void workingLocalOnlyHotspotEnrichedApBroadcastWithIfaceChanged() throws Exception { workingLocalOnlyHotspotEnrichedApBroadcast(true); } @@ -718,7 +749,7 @@ public class TetheringTest { TEST_WLAN_IFNAME, WifiManager.IFACE_IP_MODE_TETHERED); verifyNoMoreInteractions(mWifiManager); verifyTetheringBroadcast(TEST_WLAN_IFNAME, EXTRA_ACTIVE_TETHER); - verify(mUpstreamNetworkMonitor, times(1)).start(); + verify(mUpstreamNetworkMonitor, times(1)).start(any(NetworkRequest.class)); // In tethering mode, in the default configuration, an explicit request // for a mobile network is also made. verify(mUpstreamNetworkMonitor, times(1)).registerMobileNetworkRequest(); diff --git a/tests/net/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitorTest.java b/tests/net/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitorTest.java index 9661dc24ca2e..3e21a2cfb7f9 100644 --- a/tests/net/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitorTest.java +++ b/tests/net/java/com/android/server/connectivity/tethering/UpstreamNetworkMonitorTest.java @@ -31,6 +31,7 @@ import static org.junit.Assert.fail; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyInt; import static org.mockito.Mockito.anyString; +import static org.mockito.Mockito.eq; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; @@ -73,6 +74,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Objects; import java.util.Set; @@ -84,6 +86,10 @@ public class UpstreamNetworkMonitorTest { private static final boolean INCLUDES = true; private static final boolean EXCLUDES = false; + // Actual contents of the request don't matter for this test. The lack of + // any specific TRANSPORT_* is sufficient to identify this request. + private static final NetworkRequest mDefaultRequest = new NetworkRequest.Builder().build(); + @Mock private Context mContext; @Mock private IConnectivityManager mCS; @Mock private SharedLog mLog; @@ -113,6 +119,13 @@ public class UpstreamNetworkMonitorTest { } @Test + public void testStopWithoutStartIsNonFatal() { + mUNM.stop(); + mUNM.stop(); + mUNM.stop(); + } + + @Test public void testDoesNothingBeforeStarted() { assertTrue(mCM.hasNoCallbacks()); assertFalse(mUNM.mobileNetworkRequested()); @@ -127,7 +140,7 @@ public class UpstreamNetworkMonitorTest { public void testDefaultNetworkIsTracked() throws Exception { assertEquals(0, mCM.trackingDefault.size()); - mUNM.start(); + mUNM.start(mDefaultRequest); assertEquals(1, mCM.trackingDefault.size()); mUNM.stop(); @@ -138,7 +151,7 @@ public class UpstreamNetworkMonitorTest { public void testListensForAllNetworks() throws Exception { assertTrue(mCM.listening.isEmpty()); - mUNM.start(); + mUNM.start(mDefaultRequest); assertFalse(mCM.listening.isEmpty()); assertTrue(mCM.isListeningForAll()); @@ -148,9 +161,11 @@ public class UpstreamNetworkMonitorTest { @Test public void testCallbacksRegistered() { - mUNM.start(); - verify(mCM, times(1)).registerNetworkCallback(any(), any(), any()); - verify(mCM, times(1)).registerDefaultNetworkCallback(any(), any()); + mUNM.start(mDefaultRequest); + verify(mCM, times(1)).registerNetworkCallback( + any(NetworkRequest.class), any(NetworkCallback.class), any(Handler.class)); + verify(mCM, times(1)).requestNetwork( + eq(mDefaultRequest), any(NetworkCallback.class), any(Handler.class)); mUNM.stop(); verify(mCM, times(2)).unregisterNetworkCallback(any(NetworkCallback.class)); @@ -161,7 +176,7 @@ public class UpstreamNetworkMonitorTest { assertFalse(mUNM.mobileNetworkRequested()); assertEquals(0, mCM.requested.size()); - mUNM.start(); + mUNM.start(mDefaultRequest); assertFalse(mUNM.mobileNetworkRequested()); assertEquals(0, mCM.requested.size()); @@ -184,17 +199,17 @@ public class UpstreamNetworkMonitorTest { assertFalse(mUNM.mobileNetworkRequested()); assertEquals(0, mCM.requested.size()); - mUNM.start(); - verify(mCM, Mockito.times(1)).registerNetworkCallback( + mUNM.start(mDefaultRequest); + verify(mCM, times(1)).registerNetworkCallback( any(NetworkRequest.class), any(NetworkCallback.class), any(Handler.class)); - verify(mCM, Mockito.times(1)).registerDefaultNetworkCallback( - any(NetworkCallback.class), any(Handler.class)); + verify(mCM, times(1)).requestNetwork( + eq(mDefaultRequest), any(NetworkCallback.class), any(Handler.class)); assertFalse(mUNM.mobileNetworkRequested()); assertEquals(0, mCM.requested.size()); mUNM.updateMobileRequiresDun(true); mUNM.registerMobileNetworkRequest(); - verify(mCM, Mockito.times(1)).requestNetwork( + verify(mCM, times(1)).requestNetwork( any(NetworkRequest.class), any(NetworkCallback.class), anyInt(), anyInt(), any(Handler.class)); @@ -222,7 +237,7 @@ public class UpstreamNetworkMonitorTest { assertFalse(mUNM.mobileNetworkRequested()); assertEquals(0, mCM.requested.size()); - mUNM.start(); + mUNM.start(mDefaultRequest); assertFalse(mUNM.mobileNetworkRequested()); assertEquals(0, mCM.requested.size()); @@ -242,7 +257,7 @@ public class UpstreamNetworkMonitorTest { @Test public void testUpdateMobileRequiresDun() throws Exception { - mUNM.start(); + mUNM.start(mDefaultRequest); // Test going from no-DUN to DUN correctly re-registers callbacks. mUNM.updateMobileRequiresDun(false); @@ -270,7 +285,7 @@ public class UpstreamNetworkMonitorTest { final Collection preferredTypes = new ArrayList<>(); preferredTypes.add(TYPE_WIFI); - mUNM.start(); + mUNM.start(mDefaultRequest); // There are no networks, so there is nothing to select. assertSatisfiesLegacyType(TYPE_NONE, mUNM.selectPreferredUpstreamType(preferredTypes)); @@ -334,8 +349,47 @@ public class UpstreamNetworkMonitorTest { } @Test + public void testGetCurrentPreferredUpstream() throws Exception { + mUNM.start(mDefaultRequest); + mUNM.updateMobileRequiresDun(false); + + // [0] Mobile connects, DUN not required -> mobile selected. + final TestNetworkAgent cellAgent = new TestNetworkAgent(mCM, TRANSPORT_CELLULAR); + cellAgent.fakeConnect(); + mCM.makeDefaultNetwork(cellAgent); + assertEquals(cellAgent.networkId, mUNM.getCurrentPreferredUpstream().network); + + // [1] WiFi connects but not validated/promoted to default -> mobile selected. + final TestNetworkAgent wifiAgent = new TestNetworkAgent(mCM, TRANSPORT_WIFI); + wifiAgent.fakeConnect(); + assertEquals(cellAgent.networkId, mUNM.getCurrentPreferredUpstream().network); + + // [2] WiFi validates and is promoted to the default network -> WiFi selected. + mCM.makeDefaultNetwork(wifiAgent); + assertEquals(wifiAgent.networkId, mUNM.getCurrentPreferredUpstream().network); + + // [3] DUN required, no other changes -> WiFi still selected + mUNM.updateMobileRequiresDun(true); + assertEquals(wifiAgent.networkId, mUNM.getCurrentPreferredUpstream().network); + + // [4] WiFi no longer validated, mobile becomes default, DUN required -> null selected. + mCM.makeDefaultNetwork(cellAgent); + assertEquals(null, mUNM.getCurrentPreferredUpstream()); + // TODO: make sure that a DUN request has been filed. This is currently + // triggered by code over in Tethering, but once that has been moved + // into UNM we should test for this here. + + // [5] DUN network arrives -> DUN selected + final TestNetworkAgent dunAgent = new TestNetworkAgent(mCM, TRANSPORT_CELLULAR); + dunAgent.networkCapabilities.addCapability(NET_CAPABILITY_DUN); + dunAgent.networkCapabilities.removeCapability(NET_CAPABILITY_INTERNET); + dunAgent.fakeConnect(); + assertEquals(dunAgent.networkId, mUNM.getCurrentPreferredUpstream().network); + } + + @Test public void testLocalPrefixes() throws Exception { - mUNM.start(); + mUNM.start(mDefaultRequest); // [0] Test minimum set of local prefixes. Set local = mUNM.getLocalPrefixes(); @@ -345,7 +399,7 @@ public class UpstreamNetworkMonitorTest { // [1] Pretend Wi-Fi connects. final TestNetworkAgent wifiAgent = new TestNetworkAgent(mCM, TRANSPORT_WIFI); - final LinkProperties wifiLp = new LinkProperties(); + final LinkProperties wifiLp = wifiAgent.linkProperties; wifiLp.setInterfaceName("wlan0"); final String[] WIFI_ADDRS = { "fe80::827a:bfff:fe6f:374d", "100.112.103.18", @@ -358,7 +412,7 @@ public class UpstreamNetworkMonitorTest { wifiLp.addLinkAddress(new LinkAddress(addrStr + cidr)); } wifiAgent.fakeConnect(); - wifiAgent.sendLinkProperties(wifiLp); + wifiAgent.sendLinkProperties(); local = mUNM.getLocalPrefixes(); assertPrefixSet(local, INCLUDES, alreadySeen); @@ -372,7 +426,7 @@ public class UpstreamNetworkMonitorTest { // [2] Pretend mobile connects. final TestNetworkAgent cellAgent = new TestNetworkAgent(mCM, TRANSPORT_CELLULAR); - final LinkProperties cellLp = new LinkProperties(); + final LinkProperties cellLp = cellAgent.linkProperties; cellLp.setInterfaceName("rmnet_data0"); final String[] CELL_ADDRS = { "10.102.211.48", "2001:db8:0:1:b50e:70d9:10c9:433d", @@ -382,7 +436,7 @@ public class UpstreamNetworkMonitorTest { cellLp.addLinkAddress(new LinkAddress(addrStr + cidr)); } cellAgent.fakeConnect(); - cellAgent.sendLinkProperties(cellLp); + cellAgent.sendLinkProperties(); local = mUNM.getLocalPrefixes(); assertPrefixSet(local, INCLUDES, alreadySeen); @@ -394,17 +448,18 @@ public class UpstreamNetworkMonitorTest { // [3] Pretend DUN connects. final TestNetworkAgent dunAgent = new TestNetworkAgent(mCM, TRANSPORT_CELLULAR); dunAgent.networkCapabilities.addCapability(NET_CAPABILITY_DUN); - final LinkProperties dunLp = new LinkProperties(); + dunAgent.networkCapabilities.removeCapability(NET_CAPABILITY_INTERNET); + final LinkProperties dunLp = dunAgent.linkProperties; dunLp.setInterfaceName("rmnet_data1"); final String[] DUN_ADDRS = { "192.0.2.48", "2001:db8:1:2:b50e:70d9:10c9:433d", }; for (String addrStr : DUN_ADDRS) { final String cidr = addrStr.contains(":") ? "/64" : "/27"; - cellLp.addLinkAddress(new LinkAddress(addrStr + cidr)); + dunLp.addLinkAddress(new LinkAddress(addrStr + cidr)); } dunAgent.fakeConnect(); - dunAgent.sendLinkProperties(dunLp); + dunAgent.sendLinkProperties(); local = mUNM.getLocalPrefixes(); assertPrefixSet(local, INCLUDES, alreadySeen); @@ -442,6 +497,7 @@ public class UpstreamNetworkMonitorTest { public static class TestConnectivityManager extends ConnectivityManager { public Map allCallbacks = new HashMap<>(); public Set trackingDefault = new HashSet<>(); + public TestNetworkAgent defaultNetwork = null; public Map listening = new HashMap<>(); public Map requested = new HashMap<>(); public Map legacyTypeMap = new HashMap<>(); @@ -483,12 +539,34 @@ public class UpstreamNetworkMonitorTest { int getNetworkId() { return ++mNetworkId; } + void makeDefaultNetwork(TestNetworkAgent agent) { + if (Objects.equals(defaultNetwork, agent)) return; + + final TestNetworkAgent formerDefault = defaultNetwork; + defaultNetwork = agent; + + for (NetworkCallback cb : trackingDefault) { + if (defaultNetwork != null) { + cb.onAvailable(defaultNetwork.networkId); + cb.onCapabilitiesChanged( + defaultNetwork.networkId, defaultNetwork.networkCapabilities); + cb.onLinkPropertiesChanged( + defaultNetwork.networkId, defaultNetwork.linkProperties); + } + } + } + @Override public void requestNetwork(NetworkRequest req, NetworkCallback cb, Handler h) { assertFalse(allCallbacks.containsKey(cb)); allCallbacks.put(cb, h); - assertFalse(requested.containsKey(cb)); - requested.put(cb, req); + if (mDefaultRequest.equals(req)) { + assertFalse(trackingDefault.contains(cb)); + trackingDefault.add(cb); + } else { + assertFalse(requested.containsKey(cb)); + requested.put(cb, req); + } } @Override @@ -524,10 +602,7 @@ public class UpstreamNetworkMonitorTest { @Override public void registerDefaultNetworkCallback(NetworkCallback cb, Handler h) { - assertFalse(allCallbacks.containsKey(cb)); - allCallbacks.put(cb, h); - assertFalse(trackingDefault.contains(cb)); - trackingDefault.add(cb); + fail("Should never be called."); } @Override @@ -561,6 +636,7 @@ public class UpstreamNetworkMonitorTest { public final Network networkId; public final int transportType; public final NetworkCapabilities networkCapabilities; + public final LinkProperties linkProperties; public TestNetworkAgent(TestConnectivityManager cm, int transportType) { this.cm = cm; @@ -569,12 +645,14 @@ public class UpstreamNetworkMonitorTest { networkCapabilities = new NetworkCapabilities(); networkCapabilities.addTransportType(transportType); networkCapabilities.addCapability(NET_CAPABILITY_INTERNET); + linkProperties = new LinkProperties(); } public void fakeConnect() { for (NetworkCallback cb : cm.listening.keySet()) { cb.onAvailable(networkId); cb.onCapabilitiesChanged(networkId, copy(networkCapabilities)); + cb.onLinkPropertiesChanged(networkId, copy(linkProperties)); } } @@ -584,11 +662,16 @@ public class UpstreamNetworkMonitorTest { } } - public void sendLinkProperties(LinkProperties lp) { + public void sendLinkProperties() { for (NetworkCallback cb : cm.listening.keySet()) { - cb.onLinkPropertiesChanged(networkId, lp); + cb.onLinkPropertiesChanged(networkId, copy(linkProperties)); } } + + @Override + public String toString() { + return String.format("TestNetworkAgent: %s %s", networkId, networkCapabilities); + } } public static class TestStateMachine extends StateMachine { @@ -618,6 +701,10 @@ public class UpstreamNetworkMonitorTest { return new NetworkCapabilities(nc); } + static LinkProperties copy(LinkProperties lp) { + return new LinkProperties(lp); + } + static void assertPrefixSet(Set prefixes, boolean expectation, String... expected) { final Set expectedSet = new HashSet<>(); Collections.addAll(expectedSet, expected); -- 2.11.0