From 87cdb7c771061f7d928b25add51cc98202750598 Mon Sep 17 00:00:00 2001 From: Automerger Merge Worker Date: Tue, 25 Feb 2020 00:56:07 +0000 Subject: [PATCH 1/5] test: LinkProperties: Unique Route Destinations Routes will always have unique destinations. Update tests to use unique destinations when adding multiple routes. Bug: 142892223 Test: treehugger Change-Id: I238899b0643407a1be29eb66d28728ca5d5dbc80 Merged-In: I238899b0643407a1be29eb66d28728ca5d5dbc80 (cherry picked from commit 891ea460b10c3ee3c74298dad828bd550e66b81f) --- .../java/android/net/LinkPropertiesTest.java | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/net/common/java/android/net/LinkPropertiesTest.java b/tests/net/common/java/android/net/LinkPropertiesTest.java index f25fd4daf8..48b65e592f 100644 --- a/tests/net/common/java/android/net/LinkPropertiesTest.java +++ b/tests/net/common/java/android/net/LinkPropertiesTest.java @@ -315,7 +315,7 @@ public class LinkPropertiesTest { source.addDnsServer(DNS1); source.addDnsServer(DNS2); // set 2 gateways - source.addRoute(new RouteInfo(GATEWAY1)); + source.addRoute(new RouteInfo(LINKADDRV4, GATEWAY1)); source.addRoute(new RouteInfo(GATEWAY2)); source.setMtu(MTU); @@ -327,7 +327,7 @@ public class LinkPropertiesTest { target.addDnsServer(DNS2); target.addDnsServer(DNS1); target.addRoute(new RouteInfo(GATEWAY2)); - target.addRoute(new RouteInfo(GATEWAY1)); + target.addRoute(new RouteInfo(LINKADDRV4, GATEWAY1)); target.setMtu(MTU); assertLinkPropertiesEqual(source, target); @@ -364,12 +364,13 @@ public class LinkPropertiesTest { @Test public void testRouteInterfaces() { - LinkAddress prefix = new LinkAddress(address("2001:db8::"), 32); + LinkAddress prefix1 = new LinkAddress(address("2001:db8:1::"), 48); + LinkAddress prefix2 = new LinkAddress(address("2001:db8:2::"), 48); InetAddress address = ADDRV6; // Add a route with no interface to a LinkProperties with no interface. No errors. LinkProperties lp = new LinkProperties(); - RouteInfo r = new RouteInfo(prefix, address, null); + RouteInfo r = new RouteInfo(prefix1, address, null); assertTrue(lp.addRoute(r)); assertEquals(1, lp.getRoutes().size()); assertAllRoutesHaveInterface(null, lp); @@ -379,7 +380,7 @@ public class LinkPropertiesTest { assertEquals(1, lp.getRoutes().size()); // Add a route with an interface. Expect an exception. - r = new RouteInfo(prefix, address, "wlan0"); + r = new RouteInfo(prefix2, address, "wlan0"); try { lp.addRoute(r); fail("Adding wlan0 route to LP with no interface, expect exception"); @@ -398,7 +399,7 @@ public class LinkPropertiesTest { } catch (IllegalArgumentException expected) {} // If the interface name matches, the route is added. - r = new RouteInfo(prefix, null, "wlan0"); + r = new RouteInfo(prefix2, null, "wlan0"); lp.setInterfaceName("wlan0"); lp.addRoute(r); assertEquals(2, lp.getRoutes().size()); @@ -423,10 +424,12 @@ public class LinkPropertiesTest { assertEquals(3, lp.compareAllRoutes(lp2).added.size()); assertEquals(3, lp.compareAllRoutes(lp2).removed.size()); - // Check remove works - lp.removeRoute(new RouteInfo(prefix, address, null)); + // Remove route with incorrect interface, no route removed. + lp.removeRoute(new RouteInfo(prefix2, null, null)); assertEquals(3, lp.getRoutes().size()); - lp.removeRoute(new RouteInfo(prefix, address, "wlan0")); + + // Check remove works when interface is correct. + lp.removeRoute(new RouteInfo(prefix2, null, "wlan0")); assertEquals(2, lp.getRoutes().size()); assertAllRoutesHaveInterface("wlan0", lp); assertAllRoutesNotHaveInterface("p2p0", lp); From 13a85611f8732adb1a4c071ce410e70da8c27b23 Mon Sep 17 00:00:00 2001 From: Automerger Merge Worker Date: Thu, 27 Feb 2020 01:16:45 +0000 Subject: [PATCH 2/5] test: ConnectivityService: Validate Route Add/Del Validate route addition and deletion after linkProperties are changed. Bug: 142892223 Test: atest ConnectivityServiceTest#testStackedLinkProperties Change-Id: I18296b933e856a0f8a4c1dbd75bd35024853bfbb Merged-In: I18296b933e856a0f8a4c1dbd75bd35024853bfbb (cherry picked from commit a22a979a0caf3e3533ede410d48127084052aec5) --- .../server/ConnectivityServiceTest.java | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 4e75f2a273..4236b9d43e 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -5927,6 +5927,12 @@ public class ConnectivityServiceTest { final LinkAddress myIpv6 = new LinkAddress("2001:db8:1::1/64"); final String kNat64PrefixString = "2001:db8:64:64:64:64::"; final IpPrefix kNat64Prefix = new IpPrefix(InetAddress.getByName(kNat64PrefixString), 96); + final RouteInfo defaultRoute = new RouteInfo((IpPrefix) null, myIpv6.getAddress(), + MOBILE_IFNAME); + final RouteInfo hostRoute = new RouteInfo(myIpv6, null, MOBILE_IFNAME); + final RouteInfo ipv4Default = new RouteInfo(myIpv4, null, MOBILE_IFNAME); + final RouteInfo stackedDefault = new RouteInfo((IpPrefix) null, myIpv4.getAddress(), + CLAT_PREFIX + MOBILE_IFNAME); final NetworkRequest networkRequest = new NetworkRequest.Builder() .addTransportType(TRANSPORT_CELLULAR) @@ -5939,15 +5945,13 @@ public class ConnectivityServiceTest { final LinkProperties cellLp = new LinkProperties(); cellLp.setInterfaceName(MOBILE_IFNAME); cellLp.addLinkAddress(myIpv6); - cellLp.addRoute(new RouteInfo((IpPrefix) null, myIpv6.getAddress(), MOBILE_IFNAME)); - cellLp.addRoute(new RouteInfo(myIpv6, null, MOBILE_IFNAME)); + cellLp.addRoute(defaultRoute); + cellLp.addRoute(hostRoute); mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, cellLp); reset(mNetworkManagementService); reset(mMockDnsResolver); reset(mMockNetd); reset(mBatteryStatsService); - when(mNetworkManagementService.getInterfaceConfig(CLAT_PREFIX + MOBILE_IFNAME)) - .thenReturn(getClatInterfaceConfig(myIpv4)); // Connect with ipv6 link properties. Expect prefix discovery to be started. mCellNetworkAgent.connect(true); @@ -5955,6 +5959,8 @@ public class ConnectivityServiceTest { waitForIdle(); verify(mMockNetd, times(1)).networkCreatePhysical(eq(cellNetId), anyInt()); + verify(mNetworkManagementService, times(1)).addRoute(eq(cellNetId), eq(defaultRoute)); + verify(mNetworkManagementService, times(1)).addRoute(eq(cellNetId), eq(hostRoute)); verify(mMockDnsResolver, times(1)).createNetworkCache(eq(cellNetId)); verify(mBatteryStatsService).noteNetworkInterfaceType(cellLp.getInterfaceName(), TYPE_MOBILE); @@ -5980,12 +5986,14 @@ public class ConnectivityServiceTest { verifyNoMoreInteractions(mMockNetd); verifyNoMoreInteractions(mMockDnsResolver); + reset(mNetworkManagementService); reset(mMockNetd); reset(mMockDnsResolver); + when(mNetworkManagementService.getInterfaceConfig(CLAT_PREFIX + MOBILE_IFNAME)) + .thenReturn(getClatInterfaceConfig(myIpv4)); // Remove IPv4 address. Expect prefix discovery to be started again. cellLp.removeLinkAddress(myIpv4); - cellLp.removeRoute(new RouteInfo(myIpv4, null, MOBILE_IFNAME)); mCellNetworkAgent.sendLinkProperties(cellLp); networkCallback.expectCallback(CallbackEntry.LINK_PROPERTIES_CHANGED, mCellNetworkAgent); verify(mMockDnsResolver, times(1)).startPrefix64Discovery(cellNetId); @@ -6007,6 +6015,7 @@ public class ConnectivityServiceTest { List stackedLps = mCm.getLinkProperties(mCellNetworkAgent.getNetwork()) .getStackedLinks(); assertEquals(makeClatLinkProperties(myIpv4), stackedLps.get(0)); + verify(mNetworkManagementService).addRoute(eq(cellNetId), eq(stackedDefault)); // Change trivial linkproperties and see if stacked link is preserved. cellLp.addDnsServer(InetAddress.getByName("8.8.8.8")); @@ -6032,8 +6041,9 @@ public class ConnectivityServiceTest { // Add ipv4 address, expect that clatd and prefix discovery are stopped and stacked // linkproperties are cleaned up. cellLp.addLinkAddress(myIpv4); - cellLp.addRoute(new RouteInfo(myIpv4, null, MOBILE_IFNAME)); + cellLp.addRoute(ipv4Default); mCellNetworkAgent.sendLinkProperties(cellLp); + verify(mNetworkManagementService).addRoute(eq(cellNetId), eq(stackedDefault)); networkCallback.expectCallback(CallbackEntry.LINK_PROPERTIES_CHANGED, mCellNetworkAgent); verify(mMockNetd, times(1)).clatdStop(MOBILE_IFNAME); verify(mMockDnsResolver, times(1)).stopPrefix64Discovery(cellNetId); @@ -6052,8 +6062,11 @@ public class ConnectivityServiceTest { verifyNoMoreInteractions(mMockNetd); verifyNoMoreInteractions(mMockDnsResolver); + reset(mNetworkManagementService); reset(mMockNetd); reset(mMockDnsResolver); + when(mNetworkManagementService.getInterfaceConfig(CLAT_PREFIX + MOBILE_IFNAME)) + .thenReturn(getClatInterfaceConfig(myIpv4)); // Stopping prefix discovery causes netd to tell us that the NAT64 prefix is gone. mService.mNetdEventCallback.onNat64PrefixEvent(cellNetId, false /* added */, @@ -6067,6 +6080,7 @@ public class ConnectivityServiceTest { cellLp.removeDnsServer(InetAddress.getByName("8.8.8.8")); mCellNetworkAgent.sendLinkProperties(cellLp); networkCallback.expectCallback(CallbackEntry.LINK_PROPERTIES_CHANGED, mCellNetworkAgent); + verify(mNetworkManagementService, times(1)).removeRoute(eq(cellNetId), eq(ipv4Default)); verify(mMockDnsResolver, times(1)).startPrefix64Discovery(cellNetId); mService.mNetdEventCallback.onNat64PrefixEvent(cellNetId, true /* added */, kNat64PrefixString, 96); From 76eead56aae71f1f95d4d469efa2776c17fb43a3 Mon Sep 17 00:00:00 2001 From: Automerger Merge Worker Date: Sun, 8 Mar 2020 06:07:47 +0000 Subject: [PATCH 3/5] Remove the NetworkScore class. This class is useless at this point and introduces overhead. Bug: 113554781 Test: FrameworksNetTests Change-Id: Ib5f540070222865260c16c7182cc13c710a243c2 Merged-In: Ib5f540070222865260c16c7182cc13c710a243c2 (cherry picked from commit c3489ad3a6ce98218ce223cea877586781025b98) --- core/java/android/net/NetworkAgent.java | 15 +- core/java/android/net/NetworkScore.java | 162 ------------------ .../android/server/ConnectivityService.java | 16 +- .../server/connectivity/NetworkAgentInfo.java | 22 +-- .../server/ConnectivityServiceTest.java | 10 +- .../connectivity/LingerMonitorTest.java | 5 +- 6 files changed, 21 insertions(+), 209 deletions(-) delete mode 100644 core/java/android/net/NetworkScore.java diff --git a/core/java/android/net/NetworkAgent.java b/core/java/android/net/NetworkAgent.java index 7cc569a42b..fef353f604 100644 --- a/core/java/android/net/NetworkAgent.java +++ b/core/java/android/net/NetworkAgent.java @@ -126,7 +126,7 @@ public abstract class NetworkAgent { /** * Sent by the NetworkAgent to ConnectivityService to pass the current * network score. - * obj = network score Integer + * arg1 = network score int * @hide */ public static final int EVENT_NETWORK_SCORE_CHANGED = BASE + 4; @@ -650,18 +650,7 @@ public abstract class NetworkAgent { if (score < 0) { throw new IllegalArgumentException("Score must be >= 0"); } - final NetworkScore ns = new NetworkScore(); - ns.putIntExtension(NetworkScore.LEGACY_SCORE, score); - updateScore(ns); - } - - /** - * Must be called by the agent when it has a new {@link NetworkScore} for this network. - * @param ns the new score. - * @hide TODO: unhide the NetworkScore class, and rename to sendNetworkScore. - */ - public void updateScore(@NonNull NetworkScore ns) { - queueOrSendMessage(EVENT_NETWORK_SCORE_CHANGED, new NetworkScore(ns)); + queueOrSendMessage(EVENT_NETWORK_SCORE_CHANGED, score, 0); } /** diff --git a/core/java/android/net/NetworkScore.java b/core/java/android/net/NetworkScore.java deleted file mode 100644 index 13f2994110..0000000000 --- a/core/java/android/net/NetworkScore.java +++ /dev/null @@ -1,162 +0,0 @@ -/* - * Copyright (C) 2019 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package android.net; - -import android.annotation.NonNull; -import android.annotation.Nullable; -import android.os.Bundle; -import android.os.Parcel; -import android.os.Parcelable; - -import java.util.Objects; - -/** - * Object representing the quality of a network as perceived by the user. - * - * A NetworkScore object represents the characteristics of a network that affects how good the - * network is considered for a particular use. - * @hide - */ -public final class NetworkScore implements Parcelable { - - // The key of bundle which is used to get the legacy network score of NetworkAgentInfo. - // TODO: Remove this when the transition to NetworkScore is over. - public static final String LEGACY_SCORE = "LEGACY_SCORE"; - @NonNull - private final Bundle mExtensions; - - public NetworkScore() { - mExtensions = new Bundle(); - } - - public NetworkScore(@NonNull NetworkScore source) { - mExtensions = new Bundle(source.mExtensions); - } - - /** - * Put the value of parcelable inside the bundle by key. - */ - public void putExtension(@Nullable String key, @Nullable Parcelable value) { - mExtensions.putParcelable(key, value); - } - - /** - * Put the value of int inside the bundle by key. - */ - public void putIntExtension(@Nullable String key, int value) { - mExtensions.putInt(key, value); - } - - /** - * Get the value of non primitive type by key. - */ - public T getExtension(@Nullable String key) { - return mExtensions.getParcelable(key); - } - - /** - * Get the value of int by key. - */ - public int getIntExtension(@Nullable String key) { - return mExtensions.getInt(key); - } - - /** - * Remove the entry by given key. - */ - public void removeExtension(@Nullable String key) { - mExtensions.remove(key); - } - - @Override - public int describeContents() { - return 0; - } - - @Override - public void writeToParcel(@NonNull Parcel dest, int flags) { - synchronized (this) { - dest.writeBundle(mExtensions); - } - } - - public static final @NonNull Creator CREATOR = new Creator() { - @Override - public NetworkScore createFromParcel(@NonNull Parcel in) { - return new NetworkScore(in); - } - - @Override - public NetworkScore[] newArray(int size) { - return new NetworkScore[size]; - } - }; - - private NetworkScore(@NonNull Parcel in) { - mExtensions = in.readBundle(); - } - - // TODO: Modify this method once new fields are added into this class. - @Override - public boolean equals(@Nullable Object obj) { - if (!(obj instanceof NetworkScore)) { - return false; - } - final NetworkScore other = (NetworkScore) obj; - return bundlesEqual(mExtensions, other.mExtensions); - } - - @Override - public int hashCode() { - int result = 29; - for (String key : mExtensions.keySet()) { - final Object value = mExtensions.get(key); - // The key may be null, so call Objects.hash() is safer. - result += 31 * value.hashCode() + 37 * Objects.hash(key); - } - return result; - } - - // mExtensions won't be null since the constructor will create it. - private boolean bundlesEqual(@NonNull Bundle bundle1, @NonNull Bundle bundle2) { - if (bundle1 == bundle2) { - return true; - } - - // This is unlikely but it's fine to add this clause here. - if (null == bundle1 || null == bundle2) { - return false; - } - - if (bundle1.size() != bundle2.size()) { - return false; - } - - for (String key : bundle1.keySet()) { - final Object value1 = bundle1.get(key); - final Object value2 = bundle2.get(key); - if (!Objects.equals(value1, value2)) { - return false; - } - } - return true; - } - - /** Convert to a string */ - public String toString() { - return "NetworkScore[" + mExtensions.toString() + "]"; - } -} diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 16dd3ada12..ff41d1cc67 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -101,7 +101,6 @@ import android.net.NetworkPolicyManager; import android.net.NetworkProvider; import android.net.NetworkQuotaInfo; import android.net.NetworkRequest; -import android.net.NetworkScore; import android.net.NetworkSpecifier; import android.net.NetworkStack; import android.net.NetworkStackClient; @@ -2731,8 +2730,7 @@ public class ConnectivityService extends IConnectivityManager.Stub break; } case NetworkAgent.EVENT_NETWORK_SCORE_CHANGED: { - final NetworkScore ns = (NetworkScore) msg.obj; - updateNetworkScore(nai, ns); + updateNetworkScore(nai, msg.arg1); break; } case NetworkAgent.EVENT_SET_EXPLICITLY_SELECTED: { @@ -5819,12 +5817,10 @@ public class ConnectivityService extends IConnectivityManager.Stub // TODO: Instead of passing mDefaultRequest, provide an API to determine whether a Network // satisfies mDefaultRequest. final NetworkCapabilities nc = new NetworkCapabilities(networkCapabilities); - final NetworkScore ns = new NetworkScore(); - ns.putIntExtension(NetworkScore.LEGACY_SCORE, currentScore); final NetworkAgentInfo nai = new NetworkAgentInfo(messenger, new AsyncChannel(), new Network(mNetIdManager.reserveNetId()), new NetworkInfo(networkInfo), lp, nc, - ns, mContext, mTrackerHandler, new NetworkAgentConfig(networkAgentConfig), this, - mNetd, mDnsResolver, mNMS, providerId); + currentScore, mContext, mTrackerHandler, new NetworkAgentConfig(networkAgentConfig), + this, mNetd, mDnsResolver, mNMS, providerId); // Make sure the network capabilities reflect what the agent info says. nai.getAndSetNetworkCapabilities(mixInCapabilities(nai, nc)); final String extraInfo = networkInfo.getExtraInfo(); @@ -7082,9 +7078,9 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - private void updateNetworkScore(NetworkAgentInfo nai, NetworkScore ns) { - if (VDBG || DDBG) log("updateNetworkScore for " + nai.toShortString() + " to " + ns); - nai.setNetworkScore(ns); + private void updateNetworkScore(@NonNull final NetworkAgentInfo nai, final int score) { + if (VDBG || DDBG) log("updateNetworkScore for " + nai.toShortString() + " to " + score); + nai.setScore(score); rematchAllNetworksAndRequests(); sendUpdatedScoreToFactories(nai); } diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 58b5cba477..23b954c03c 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -32,7 +32,6 @@ import android.net.NetworkCapabilities; import android.net.NetworkInfo; import android.net.NetworkMonitorManager; import android.net.NetworkRequest; -import android.net.NetworkScore; import android.net.NetworkState; import android.os.Handler; import android.os.INetworkManagementService; @@ -236,10 +235,8 @@ public class NetworkAgentInfo implements Comparable { // validated). private boolean mLingering; - // This represents the characteristics of a network that affects how good the network is - // considered for a particular use. - @NonNull - private NetworkScore mNetworkScore; + // This represents the quality of the network with no clear scale. + private int mScore; // The list of NetworkRequests being satisfied by this Network. private final SparseArray mNetworkRequests = new SparseArray<>(); @@ -268,7 +265,7 @@ public class NetworkAgentInfo implements Comparable { private final Handler mHandler; public NetworkAgentInfo(Messenger messenger, AsyncChannel ac, Network net, NetworkInfo info, - LinkProperties lp, NetworkCapabilities nc, @NonNull NetworkScore ns, Context context, + LinkProperties lp, NetworkCapabilities nc, int score, Context context, Handler handler, NetworkAgentConfig config, ConnectivityService connService, INetd netd, IDnsResolver dnsResolver, INetworkManagementService nms, int factorySerialNumber) { this.messenger = messenger; @@ -277,7 +274,7 @@ public class NetworkAgentInfo implements Comparable { networkInfo = info; linkProperties = lp; networkCapabilities = nc; - mNetworkScore = ns; + mScore = score; clatd = new Nat464Xlat(this, netd, dnsResolver, nms); mConnService = connService; mContext = context; @@ -491,7 +488,7 @@ public class NetworkAgentInfo implements Comparable { return ConnectivityConstants.EXPLICITLY_SELECTED_NETWORK_SCORE; } - int score = mNetworkScore.getIntExtension(NetworkScore.LEGACY_SCORE); + int score = mScore; if (!lastValidated && !pretendValidated && !ignoreWifiUnvalidationPenalty() && !isVPN()) { score -= ConnectivityConstants.UNVALIDATED_SCORE_PENALTY; } @@ -520,13 +517,8 @@ public class NetworkAgentInfo implements Comparable { return getCurrentScore(true); } - public void setNetworkScore(@NonNull NetworkScore ns) { - mNetworkScore = ns; - } - - @NonNull - public NetworkScore getNetworkScore() { - return mNetworkScore; + public void setScore(final int score) { + mScore = score; } public NetworkState getNetworkState() { diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 4e75f2a273..f876a13e41 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -6715,7 +6715,7 @@ public class ConnectivityServiceTest { public void testCheckConnectivityDiagnosticsPermissionsNetworkStack() throws Exception { final NetworkAgentInfo naiWithoutUid = new NetworkAgentInfo( - null, null, null, null, null, new NetworkCapabilities(), null, + null, null, null, null, null, new NetworkCapabilities(), 0, mServiceContext, null, null, mService, null, null, null, 0); mServiceContext.setPermission( @@ -6731,7 +6731,7 @@ public class ConnectivityServiceTest { public void testCheckConnectivityDiagnosticsPermissionsNoLocationPermission() throws Exception { final NetworkAgentInfo naiWithoutUid = new NetworkAgentInfo( - null, null, null, null, null, new NetworkCapabilities(), null, + null, null, null, null, null, new NetworkCapabilities(), 0, mServiceContext, null, null, mService, null, null, null, 0); mServiceContext.setPermission(android.Manifest.permission.NETWORK_STACK, PERMISSION_DENIED); @@ -6747,7 +6747,7 @@ public class ConnectivityServiceTest { public void testCheckConnectivityDiagnosticsPermissionsActiveVpn() throws Exception { final NetworkAgentInfo naiWithoutUid = new NetworkAgentInfo( - null, null, null, null, null, new NetworkCapabilities(), null, + null, null, null, null, null, new NetworkCapabilities(), 0, mServiceContext, null, null, mService, null, null, null, 0); setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, @@ -6773,7 +6773,7 @@ public class ConnectivityServiceTest { nc.setAdministratorUids(Arrays.asList(Process.myUid())); final NetworkAgentInfo naiWithUid = new NetworkAgentInfo( - null, null, null, null, null, nc, null, mServiceContext, null, null, + null, null, null, null, null, nc, 0, mServiceContext, null, null, mService, null, null, null, 0); setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, @@ -6795,7 +6795,7 @@ public class ConnectivityServiceTest { nc.setAdministratorUids(Arrays.asList(Process.myUid())); final NetworkAgentInfo naiWithUid = new NetworkAgentInfo( - null, null, null, null, null, nc, null, mServiceContext, null, null, + null, null, null, null, null, nc, 0, mServiceContext, null, null, mService, null, null, null, 0); setupLocationPermissions(Build.VERSION_CODES.Q, true, AppOpsManager.OPSTR_FINE_LOCATION, diff --git a/tests/net/java/com/android/server/connectivity/LingerMonitorTest.java b/tests/net/java/com/android/server/connectivity/LingerMonitorTest.java index e863266c4b..24a8717722 100644 --- a/tests/net/java/com/android/server/connectivity/LingerMonitorTest.java +++ b/tests/net/java/com/android/server/connectivity/LingerMonitorTest.java @@ -38,7 +38,6 @@ import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkInfo; import android.net.NetworkProvider; -import android.net.NetworkScore; import android.os.INetworkManagementService; import android.text.format.DateUtils; @@ -353,10 +352,8 @@ public class LingerMonitorTest { NetworkCapabilities caps = new NetworkCapabilities(); caps.addCapability(0); caps.addTransportType(transport); - NetworkScore ns = new NetworkScore(); - ns.putIntExtension(NetworkScore.LEGACY_SCORE, 50); NetworkAgentInfo nai = new NetworkAgentInfo(null, null, new Network(netId), info, null, - caps, ns, mCtx, null, null /* config */, mConnService, mNetd, mDnsResolver, mNMS, + caps, 50, mCtx, null, null /* config */, mConnService, mNetd, mDnsResolver, mNMS, NetworkProvider.ID_NONE); nai.everValidated = true; return nai; From e58e7a4d858d6f628532478f0eaad586d0bb69b2 Mon Sep 17 00:00:00 2001 From: Automerger Merge Worker Date: Wed, 4 Mar 2020 04:53:17 +0000 Subject: [PATCH 4/5] Add more assertions to testStackedLinkProperties. Check all routes that are added and removed instead of just some of them. This is in preparation of an upcoming change that switches to adding and creating routes by issuing direct calls to netd. Also rename the misleading ipv4Default route variable to ipv4Subnet, which is what it actually is. Bug: 142892223 Test: test-only change Change-Id: I7d111382be215a926a7d7d4701bd3c3e94372b99 Merged-In: I7d111382be215a926a7d7d4701bd3c3e94372b99 (cherry picked from commit dcb35cb9a5d1c250e6564b6e66a4b436dd11c795) --- .../server/ConnectivityServiceTest.java | 39 ++++++++++++++----- 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 4236b9d43e..1efc2a5602 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -5929,8 +5929,8 @@ public class ConnectivityServiceTest { final IpPrefix kNat64Prefix = new IpPrefix(InetAddress.getByName(kNat64PrefixString), 96); final RouteInfo defaultRoute = new RouteInfo((IpPrefix) null, myIpv6.getAddress(), MOBILE_IFNAME); - final RouteInfo hostRoute = new RouteInfo(myIpv6, null, MOBILE_IFNAME); - final RouteInfo ipv4Default = new RouteInfo(myIpv4, null, MOBILE_IFNAME); + final RouteInfo ipv6Subnet = new RouteInfo(myIpv6, null, MOBILE_IFNAME); + final RouteInfo ipv4Subnet = new RouteInfo(myIpv4, null, MOBILE_IFNAME); final RouteInfo stackedDefault = new RouteInfo((IpPrefix) null, myIpv4.getAddress(), CLAT_PREFIX + MOBILE_IFNAME); @@ -5946,7 +5946,7 @@ public class ConnectivityServiceTest { cellLp.setInterfaceName(MOBILE_IFNAME); cellLp.addLinkAddress(myIpv6); cellLp.addRoute(defaultRoute); - cellLp.addRoute(hostRoute); + cellLp.addRoute(ipv6Subnet); mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, cellLp); reset(mNetworkManagementService); reset(mMockDnsResolver); @@ -5959,8 +5959,7 @@ public class ConnectivityServiceTest { waitForIdle(); verify(mMockNetd, times(1)).networkCreatePhysical(eq(cellNetId), anyInt()); - verify(mNetworkManagementService, times(1)).addRoute(eq(cellNetId), eq(defaultRoute)); - verify(mNetworkManagementService, times(1)).addRoute(eq(cellNetId), eq(hostRoute)); + assertRoutesAdded(cellNetId, ipv6Subnet, defaultRoute); verify(mMockDnsResolver, times(1)).createNetworkCache(eq(cellNetId)); verify(mBatteryStatsService).noteNetworkInterfaceType(cellLp.getInterfaceName(), TYPE_MOBILE); @@ -5976,6 +5975,7 @@ public class ConnectivityServiceTest { cellLp.addLinkAddress(myIpv4); mCellNetworkAgent.sendLinkProperties(cellLp); networkCallback.expectCallback(CallbackEntry.LINK_PROPERTIES_CHANGED, mCellNetworkAgent); + assertRoutesAdded(cellNetId, ipv4Subnet); verify(mMockDnsResolver, times(1)).stopPrefix64Discovery(cellNetId); verify(mMockDnsResolver, atLeastOnce()).setResolverConfiguration(any()); @@ -5997,6 +5997,7 @@ public class ConnectivityServiceTest { mCellNetworkAgent.sendLinkProperties(cellLp); networkCallback.expectCallback(CallbackEntry.LINK_PROPERTIES_CHANGED, mCellNetworkAgent); verify(mMockDnsResolver, times(1)).startPrefix64Discovery(cellNetId); + assertRoutesRemoved(cellNetId, ipv4Subnet); // When NAT64 prefix discovery succeeds, LinkProperties are updated and clatd is started. Nat464Xlat clat = getNat464Xlat(mCellNetworkAgent); @@ -6015,7 +6016,7 @@ public class ConnectivityServiceTest { List stackedLps = mCm.getLinkProperties(mCellNetworkAgent.getNetwork()) .getStackedLinks(); assertEquals(makeClatLinkProperties(myIpv4), stackedLps.get(0)); - verify(mNetworkManagementService).addRoute(eq(cellNetId), eq(stackedDefault)); + assertRoutesAdded(cellNetId, stackedDefault); // Change trivial linkproperties and see if stacked link is preserved. cellLp.addDnsServer(InetAddress.getByName("8.8.8.8")); @@ -6041,10 +6042,10 @@ public class ConnectivityServiceTest { // Add ipv4 address, expect that clatd and prefix discovery are stopped and stacked // linkproperties are cleaned up. cellLp.addLinkAddress(myIpv4); - cellLp.addRoute(ipv4Default); + cellLp.addRoute(ipv4Subnet); mCellNetworkAgent.sendLinkProperties(cellLp); - verify(mNetworkManagementService).addRoute(eq(cellNetId), eq(stackedDefault)); networkCallback.expectCallback(CallbackEntry.LINK_PROPERTIES_CHANGED, mCellNetworkAgent); + assertRoutesAdded(cellNetId, ipv4Subnet); verify(mMockNetd, times(1)).clatdStop(MOBILE_IFNAME); verify(mMockDnsResolver, times(1)).stopPrefix64Discovery(cellNetId); @@ -6055,6 +6056,7 @@ public class ConnectivityServiceTest { expected.setNat64Prefix(kNat64Prefix); assertEquals(expected, actualLpAfterIpv4); assertEquals(0, actualLpAfterIpv4.getStackedLinks().size()); + assertRoutesRemoved(cellNetId, stackedDefault); // The interface removed callback happens but has no effect after stop is called. clat.interfaceRemoved(CLAT_PREFIX + MOBILE_IFNAME); @@ -6080,7 +6082,7 @@ public class ConnectivityServiceTest { cellLp.removeDnsServer(InetAddress.getByName("8.8.8.8")); mCellNetworkAgent.sendLinkProperties(cellLp); networkCallback.expectCallback(CallbackEntry.LINK_PROPERTIES_CHANGED, mCellNetworkAgent); - verify(mNetworkManagementService, times(1)).removeRoute(eq(cellNetId), eq(ipv4Default)); + assertRoutesRemoved(cellNetId, ipv4Subnet); // Directly-connected routes auto-added. verify(mMockDnsResolver, times(1)).startPrefix64Discovery(cellNetId); mService.mNetdEventCallback.onNat64PrefixEvent(cellNetId, true /* added */, kNat64PrefixString, 96); @@ -6092,15 +6094,20 @@ public class ConnectivityServiceTest { clat.interfaceLinkStateChanged(CLAT_PREFIX + MOBILE_IFNAME, true); networkCallback.expectLinkPropertiesThat(mCellNetworkAgent, (lp) -> lp.getStackedLinks().size() == 1 && lp.getNat64Prefix() != null); + assertRoutesAdded(cellNetId, stackedDefault); // NAT64 prefix is removed. Expect that clat is stopped. mService.mNetdEventCallback.onNat64PrefixEvent(cellNetId, false /* added */, kNat64PrefixString, 96); networkCallback.expectLinkPropertiesThat(mCellNetworkAgent, (lp) -> lp.getStackedLinks().size() == 0 && lp.getNat64Prefix() == null); + assertRoutesRemoved(cellNetId, ipv4Subnet, stackedDefault); + + // Stop has no effect because clat is already stopped. verify(mMockNetd, times(1)).clatdStop(MOBILE_IFNAME); networkCallback.expectLinkPropertiesThat(mCellNetworkAgent, (lp) -> lp.getStackedLinks().size() == 0); + verifyNoMoreInteractions(mMockNetd); // Clean up. mCellNetworkAgent.disconnect(); @@ -6668,6 +6675,20 @@ public class ConnectivityServiceTest { } } + private void assertRoutesAdded(int netId, RouteInfo... routes) throws Exception { + InOrder inOrder = inOrder(mNetworkManagementService); + for (int i = 0; i < routes.length; i++) { + inOrder.verify(mNetworkManagementService).addRoute(eq(netId), eq(routes[i])); + } + } + + private void assertRoutesRemoved(int netId, RouteInfo... routes) throws Exception { + InOrder inOrder = inOrder(mNetworkManagementService); + for (int i = 0; i < routes.length; i++) { + inOrder.verify(mNetworkManagementService).removeRoute(eq(netId), eq(routes[i])); + } + } + @Test public void testRegisterUnregisterConnectivityDiagnosticsCallback() throws Exception { final NetworkRequest wifiRequest = From de03bda1c38f5efeef1fcc905cc98dacd8e72eda Mon Sep 17 00:00:00 2001 From: Automerger Merge Worker Date: Fri, 6 Mar 2020 00:38:43 +0000 Subject: [PATCH 5/5] Remove polling of TetheringManager in ConnectivityManager. Test: manual Bug: 144742179 Merged-In: I7d88b38eb3d741534e980b7d1e226a411b71fae2 (cherry picked from commit f8a55a19faa938b4e58310f9a90926276b7936ea) Change-Id: I5cc4231bfb9a0709d677acbb317ee98af31bd041 --- .../java/android/net/ConnectivityManager.java | 60 ++++++------------- 1 file changed, 19 insertions(+), 41 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 38ef814561..fc6954fb48 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -53,7 +53,6 @@ import android.os.RemoteException; import android.os.ResultReceiver; import android.os.ServiceManager; import android.os.ServiceSpecificException; -import android.os.SystemClock; import android.provider.Settings; import android.telephony.SubscriptionManager; import android.telephony.TelephonyManager; @@ -808,7 +807,7 @@ public class ConnectivityManager { private INetworkManagementService mNMService; private INetworkPolicyManager mNPManager; - private TetheringManager mTetheringManager; + private final TetheringManager mTetheringManager; /** * Tests if a given integer represents a valid network type. @@ -2274,6 +2273,7 @@ public class ConnectivityManager { public ConnectivityManager(Context context, IConnectivityManager service) { mContext = Preconditions.checkNotNull(context, "missing context"); mService = Preconditions.checkNotNull(service, "missing IConnectivityManager"); + mTetheringManager = (TetheringManager) mContext.getSystemService(Context.TETHERING_SERVICE); sInstance = this; } @@ -2347,28 +2347,6 @@ public class ConnectivityManager { return getInstanceOrNull(); } - private static final int TETHERING_TIMEOUT_MS = 60_000; - private final Object mTetheringLock = new Object(); - - private TetheringManager getTetheringManager() { - synchronized (mTetheringLock) { - if (mTetheringManager != null) { - return mTetheringManager; - } - final long before = System.currentTimeMillis(); - while ((mTetheringManager = (TetheringManager) mContext.getSystemService( - Context.TETHERING_SERVICE)) == null) { - if (System.currentTimeMillis() - before > TETHERING_TIMEOUT_MS) { - Log.e(TAG, "Timeout waiting tethering service not ready yet"); - throw new IllegalStateException("No tethering service yet"); - } - SystemClock.sleep(100); - } - - return mTetheringManager; - } - } - /** * Get the set of tetherable, available interfaces. This list is limited by * device configuration and current interface existence. @@ -2382,7 +2360,7 @@ public class ConnectivityManager { @UnsupportedAppUsage @Deprecated public String[] getTetherableIfaces() { - return getTetheringManager().getTetherableIfaces(); + return mTetheringManager.getTetherableIfaces(); } /** @@ -2397,7 +2375,7 @@ public class ConnectivityManager { @UnsupportedAppUsage @Deprecated public String[] getTetheredIfaces() { - return getTetheringManager().getTetheredIfaces(); + return mTetheringManager.getTetheredIfaces(); } /** @@ -2418,7 +2396,7 @@ public class ConnectivityManager { @UnsupportedAppUsage @Deprecated public String[] getTetheringErroredIfaces() { - return getTetheringManager().getTetheringErroredIfaces(); + return mTetheringManager.getTetheringErroredIfaces(); } /** @@ -2462,7 +2440,7 @@ public class ConnectivityManager { @UnsupportedAppUsage @Deprecated public int tether(String iface) { - return getTetheringManager().tether(iface); + return mTetheringManager.tether(iface); } /** @@ -2486,7 +2464,7 @@ public class ConnectivityManager { @UnsupportedAppUsage @Deprecated public int untether(String iface) { - return getTetheringManager().untether(iface); + return mTetheringManager.untether(iface); } /** @@ -2512,7 +2490,7 @@ public class ConnectivityManager { @RequiresPermission(anyOf = {android.Manifest.permission.TETHER_PRIVILEGED, android.Manifest.permission.WRITE_SETTINGS}) public boolean isTetheringSupported() { - return getTetheringManager().isTetheringSupported(); + return mTetheringManager.isTetheringSupported(); } /** @@ -2605,7 +2583,7 @@ public class ConnectivityManager { final TetheringRequest request = new TetheringRequest.Builder(type) .setSilentProvisioning(!showProvisioningUi).build(); - getTetheringManager().startTethering(request, executor, tetheringCallback); + mTetheringManager.startTethering(request, executor, tetheringCallback); } /** @@ -2624,7 +2602,7 @@ public class ConnectivityManager { @Deprecated @RequiresPermission(android.Manifest.permission.TETHER_PRIVILEGED) public void stopTethering(int type) { - getTetheringManager().stopTethering(type); + mTetheringManager.stopTethering(type); } /** @@ -2682,7 +2660,7 @@ public class ConnectivityManager { synchronized (mTetheringEventCallbacks) { mTetheringEventCallbacks.put(callback, tetherCallback); - getTetheringManager().registerTetheringEventCallback(executor, tetherCallback); + mTetheringManager.registerTetheringEventCallback(executor, tetherCallback); } } @@ -2704,7 +2682,7 @@ public class ConnectivityManager { synchronized (mTetheringEventCallbacks) { final TetheringEventCallback tetherCallback = mTetheringEventCallbacks.remove(callback); - getTetheringManager().unregisterTetheringEventCallback(tetherCallback); + mTetheringManager.unregisterTetheringEventCallback(tetherCallback); } } @@ -2724,7 +2702,7 @@ public class ConnectivityManager { @UnsupportedAppUsage @Deprecated public String[] getTetherableUsbRegexs() { - return getTetheringManager().getTetherableUsbRegexs(); + return mTetheringManager.getTetherableUsbRegexs(); } /** @@ -2742,7 +2720,7 @@ public class ConnectivityManager { @UnsupportedAppUsage @Deprecated public String[] getTetherableWifiRegexs() { - return getTetheringManager().getTetherableWifiRegexs(); + return mTetheringManager.getTetherableWifiRegexs(); } /** @@ -2761,7 +2739,7 @@ public class ConnectivityManager { @UnsupportedAppUsage @Deprecated public String[] getTetherableBluetoothRegexs() { - return getTetheringManager().getTetherableBluetoothRegexs(); + return mTetheringManager.getTetherableBluetoothRegexs(); } /** @@ -2785,7 +2763,7 @@ public class ConnectivityManager { @UnsupportedAppUsage @Deprecated public int setUsbTethering(boolean enable) { - return getTetheringManager().setUsbTethering(enable); + return mTetheringManager.setUsbTethering(enable); } /** @@ -2902,7 +2880,7 @@ public class ConnectivityManager { @UnsupportedAppUsage @Deprecated public int getLastTetherError(String iface) { - return getTetheringManager().getLastTetherError(iface); + return mTetheringManager.getLastTetherError(iface); } /** @hide */ @@ -2973,7 +2951,7 @@ public class ConnectivityManager { } }; - getTetheringManager().requestLatestTetheringEntitlementResult(type, wrappedListener, + mTetheringManager.requestLatestTetheringEntitlementResult(type, wrappedListener, showEntitlementUi); } @@ -4469,7 +4447,7 @@ public class ConnectivityManager { public void factoryReset() { try { mService.factoryReset(); - getTetheringManager().stopAllTethering(); + mTetheringManager.stopAllTethering(); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); }