Merge "Prefer default Internet network for upstream tethering."

core/res/res/values/config.xml

@@ -495,6 +495,14 @@
         <item>0</item>
     </integer-array>
 
+    <!-- When true, the tethering upstream network follows the current default
+         Internet network (except when the current default network is mobile,
+         in which case a DUN network will be used if required).
+
+         When true, overrides the config_tether_upstream_types setting above.
+    -->
+    <bool translatable="false" name="config_tether_upstream_automatic">false</bool>
+
     <!-- If the DUN connection for this CDMA device supports more than just DUN -->
     <!-- traffic you should list them here. -->
     <!-- If this device is not CDMA this is ignored.  If this list is empty on -->

core/res/res/values/symbols.xml

@@ -1732,6 +1732,7 @@
   <java-symbol type="array" name="config_tether_bluetooth_regexs" />
   <java-symbol type="array" name="config_tether_dhcp_range" />
   <java-symbol type="array" name="config_tether_upstream_types" />
+  <java-symbol type="bool" name="config_tether_upstream_automatic" />
   <java-symbol type="array" name="config_tether_apndata" />
   <java-symbol type="array" name="config_tether_usb_regexs" />
   <java-symbol type="array" name="config_tether_wifi_regexs" />

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) {

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(
+            final TetheringConfiguration config = mConfig;
-                    mConfig.preferredUpstreamIfaceTypes);
+            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
+                // If we already have NetworkState for this network update it immediately.
-                // it immediately, because there likely will be no second
-                // EVENT_ON_AVAILABLE (it was already received).
                 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()) {

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<Integer> 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);

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;
+    }
 }

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   = 1;
-    public static final int EVENT_ON_CAPABILITIES   = 2;
+    public static final int EVENT_ON_LINKPROPERTIES = 2;
-    public static final int EVENT_ON_LINKPROPERTIES = 3;
+    public static final int EVENT_ON_LOST           = 3;
-    public static final int EVENT_ON_LOST           = 4;
     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);
+        if (defaultNetworkRequest != null) {
-        cm().registerDefaultNetworkCallback(mDefaultNetworkCallback, mHandler);
+            // 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<IpPrefix>) mLocalPrefixes.clone();
     }
 
-    private void handleAvailable(int callbackType, Network network) {
+    private void handleAvailable(Network network) {
-        if (VDBG) Log.d(TAG, "EVENT_ON_AVAILABLE for " + network);
+        if (mNetworkMap.containsKey(network)) return;
 
-        if (!mNetworkMap.containsKey(network)) {
+        if (VDBG) Log.d(TAG, "onAvailable for " + network);
-            mNetworkMap.put(network,
+        mNetworkMap.put(network, new NetworkState(null, null, null, network, null, null));
-                    new NetworkState(null, null, null, network, null, null));
     }
 
-        // Always request whatever extra information we can, in case this
+    private void handleNetCap(int callbackType, Network network, NetworkCapabilities newNc) {
-        // was already up when start() was called, in which case we would
+        if (callbackType == CALLBACK_DEFAULT_INTERNET) mDefaultInternetNetwork = network;
-        // 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.
-
-        // TODO: If sufficient information is available to select a more
-        // preferable upstream, do so now and notify the target.
-        notifyTarget(EVENT_ON_AVAILABLE, network);
-    }
-
-    private void handleNetCap(Network network, NetworkCapabilities newNc) {
         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
+            // There are few TODOs within ConnectivityService's rematching code
-            // mean the network is gone.  We wait for a separate notification
+            // pertaining to spurious onLost() notifications.
-            // on either the LISTEN_ALL or MOBILE_REQUEST callbacks before
+            //
-            // clearing all state.
+            // TODO: simplify this, probably if favor of code that:
-            return;
+            //     - 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<NetworkState> netStates) {
+        for (NetworkState ns : netStates) {
+            if (isCellular(ns) && hasCapability(ns, NET_CAPABILITY_DUN)) return ns;
+        }
+
+        return null;
+    }
 }

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);
 
@@ -655,6 +668,24 @@ public class TetheringTest {
         verify(mNMService, times(1)).startInterfaceForwarding(TEST_USB_IFNAME, TEST_MOBILE_IFNAME);
     }
 
+    @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();

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;
@@ -112,6 +118,13 @@ public class UpstreamNetworkMonitorTest {
         }
     }
 
+    @Test
+    public void testStopWithoutStartIsNonFatal() {
+        mUNM.stop();
+        mUNM.stop();
+        mUNM.stop();
+    }
+
     @Test
     public void testDoesNothingBeforeStarted() {
         assertTrue(mCM.hasNoCallbacks());
@@ -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();
+        mUNM.start(mDefaultRequest);
-        verify(mCM, times(1)).registerNetworkCallback(any(), any(), any());
+        verify(mCM, times(1)).registerNetworkCallback(
-        verify(mCM, times(1)).registerDefaultNetworkCallback(any(), any());
+                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();
+        mUNM.start(mDefaultRequest);
-        verify(mCM, Mockito.times(1)).registerNetworkCallback(
+        verify(mCM, times(1)).registerNetworkCallback(
                 any(NetworkRequest.class), any(NetworkCallback.class), any(Handler.class));
-        verify(mCM, Mockito.times(1)).registerDefaultNetworkCallback(
+        verify(mCM, times(1)).requestNetwork(
-                any(NetworkCallback.class), any(Handler.class));
+                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<Integer> 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));
 
@@ -333,9 +348,48 @@ public class UpstreamNetworkMonitorTest {
         assertTrue(netReq.networkCapabilities.hasCapability(NET_CAPABILITY_DUN));
     }
 
+    @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<IpPrefix> 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<NetworkCallback, Handler> allCallbacks = new HashMap<>();
         public Set<NetworkCallback> trackingDefault = new HashSet<>();
+        public TestNetworkAgent defaultNetwork = null;
         public Map<NetworkCallback, NetworkRequest> listening = new HashMap<>();
         public Map<NetworkCallback, NetworkRequest> requested = new HashMap<>();
         public Map<NetworkCallback, Integer> legacyTypeMap = new HashMap<>();
@@ -483,13 +539,35 @@ 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);
+            if (mDefaultRequest.equals(req)) {
+                assertFalse(trackingDefault.contains(cb));
+                trackingDefault.add(cb);
+            } else {
                 assertFalse(requested.containsKey(cb));
                 requested.put(cb, req);
             }
+        }
 
         @Override
         public void requestNetwork(NetworkRequest req, NetworkCallback cb) {
@@ -524,10 +602,7 @@ public class UpstreamNetworkMonitorTest {
 
         @Override
         public void registerDefaultNetworkCallback(NetworkCallback cb, Handler h) {
-            assertFalse(allCallbacks.containsKey(cb));
+            fail("Should never be called.");
-            allCallbacks.put(cb, h);
-            assertFalse(trackingDefault.contains(cb));
-            trackingDefault.add(cb);
         }
 
         @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<IpPrefix> prefixes, boolean expectation, String... expected) {
         final Set<String> expectedSet = new HashSet<>();
         Collections.addAll(expectedSet, expected);