From 3d2298960a54334777fbf5292dc83e5855da2428 Mon Sep 17 00:00:00 2001 From: James Mattis Date: Mon, 16 Nov 2020 16:46:28 -0800 Subject: [PATCH 1/4] Update to unneeded for multilayered requests Update to ConnectivityService.unneeded in order to support multilayered requests. Bug: 173432311 Bug: 171991028 Test: atest FrameworksNetTests atest FrameworksNetIntegrationTests atest CtsNetTestCasesLatestSdk Change-Id: If040d61e33d86a9f9bbc7d50ead596d210a4f82c --- .../android/server/ConnectivityService.java | 60 ++++++++++++++----- 1 file changed, 46 insertions(+), 14 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index a42c864744..3e66990507 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3560,30 +3560,58 @@ public class ConnectivityService extends IConnectivityManager.Stub return false; } for (NetworkRequestInfo nri : mNetworkRequests.values()) { - if (reason == UnneededFor.LINGER && nri.request.isBackgroundRequest()) { + if (reason == UnneededFor.LINGER + && !nri.isMultilayerRequest() + && nri.mRequests.get(0).isBackgroundRequest()) { // Background requests don't affect lingering. continue; } - // If this Network is already the highest scoring Network for a request, or if - // there is hope for it to become one if it validated, then it is needed. - if (nri.request.isRequest() && nai.satisfies(nri.request) && - (nai.isSatisfyingRequest(nri.request.requestId) || - // Note that this catches two important cases: - // 1. Unvalidated cellular will not be reaped when unvalidated WiFi - // is currently satisfying the request. This is desirable when - // cellular ends up validating but WiFi does not. - // 2. Unvalidated WiFi will not be reaped when validated cellular - // is currently satisfying the request. This is desirable when - // WiFi ends up validating and out scoring cellular. - nri.mSatisfier.getCurrentScore() - < nai.getCurrentScoreAsValidated())) { + if (isNetworkPotentialSatisfier(nai, nri)) { return false; } } return true; } + private boolean isNetworkPotentialSatisfier( + @NonNull final NetworkAgentInfo candidate, @NonNull final NetworkRequestInfo nri) { + // listen requests won't keep up a network satisfying it. If this is not a multilayer + // request, we can return immediately. For multilayer requests, we have to check to see if + // any of the multilayer requests may have a potential satisfier. + if (!nri.isMultilayerRequest() && nri.mRequests.get(0).isListen()) { + return false; + } + for (final NetworkRequest req : nri.mRequests) { + // As non-multilayer listen requests have already returned, the below would only happen + // for a multilayer request therefore continue to the next request if available. + if (req.isListen()) { + continue; + } + // If this Network is already the highest scoring Network for a request, or if + // there is hope for it to become one if it validated, then it is needed. + if (candidate.satisfies(req)) { + // As soon as a network is found that satisfies a request, return. Specifically for + // multilayer requests, returning as soon as a NetworkAgentInfo satisfies a request + // is important so as to not evaluate lower priority requests further in + // nri.mRequests. + final boolean isNetworkNeeded = candidate.isSatisfyingRequest(req.requestId) + // Note that this catches two important cases: + // 1. Unvalidated cellular will not be reaped when unvalidated WiFi + // is currently satisfying the request. This is desirable when + // cellular ends up validating but WiFi does not. + // 2. Unvalidated WiFi will not be reaped when validated cellular + // is currently satisfying the request. This is desirable when + // WiFi ends up validating and out scoring cellular. + || nri.mSatisfier.getCurrentScore() + < candidate.getCurrentScoreAsValidated(); + return isNetworkNeeded; + } + } + + return false; + } + private NetworkRequestInfo getNriForAppRequest( NetworkRequest request, int callingUid, String requestedOperation) { final NetworkRequestInfo nri = mNetworkRequests.get(request); @@ -5451,6 +5479,10 @@ public class ConnectivityService extends IConnectivityManager.Stub this(r, null); } + boolean isMultilayerRequest() { + return mRequests.size() > 1; + } + private List initializeRequests(NetworkRequest r) { final ArrayList tempRequests = new ArrayList<>(); tempRequests.add(new NetworkRequest(r)); From d951eeca2615b051fb7422959b3c0295c7b6c045 Mon Sep 17 00:00:00 2001 From: James Mattis Date: Wed, 18 Nov 2020 16:23:25 -0800 Subject: [PATCH 2/4] Update getSignalStrengthThresholds for multilayer Update ConnectivityService.getSignalStrengthThresholds so that thresholds are created for multilayer requests. Bug: 173650494 Bug: 171991028 Test: atest FrameworksNetTests atest FrameworksNetIntegrationTests atest CtsNetTestCasesLatestSdk Change-Id: Ib3b9f52cf371efd1c7d9b176565c7ea8c46375f1 --- .../java/com/android/server/ConnectivityService.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 3e66990507..be4464790c 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -5557,13 +5557,15 @@ public class ConnectivityService extends IConnectivityManager.Stub mAppOpsManager.checkPackage(callerUid, callerPackageName); } - private ArrayList getSignalStrengthThresholds(NetworkAgentInfo nai) { + private ArrayList getSignalStrengthThresholds(@NonNull final NetworkAgentInfo nai) { final SortedSet thresholds = new TreeSet<>(); synchronized (nai) { - for (NetworkRequestInfo nri : mNetworkRequests.values()) { - if (nri.request.networkCapabilities.hasSignalStrength() && - nai.satisfiesImmutableCapabilitiesOf(nri.request)) { - thresholds.add(nri.request.networkCapabilities.getSignalStrength()); + for (final NetworkRequestInfo nri : mNetworkRequests.values()) { + for (final NetworkRequest req : nri.mRequests) { + if (req.networkCapabilities.hasSignalStrength() + && nai.satisfiesImmutableCapabilitiesOf(req)) { + thresholds.add(req.networkCapabilities.getSignalStrength()); + } } } } From cd7634501f0a12710b5905c5eddc2ecd120c96fc Mon Sep 17 00:00:00 2001 From: James Mattis Date: Thu, 19 Nov 2020 17:06:18 -0800 Subject: [PATCH 3/4] maybeLogBlockedStatusChanged multilayer requests Updating maybeLogBlockedStatusChanged logging so that it outputs the currently active request's information as part of the multilayared requests updates. Bug: 173145245 Bug: 171991028 Test: atest FrameworksNetTests Change-Id: I68f364b457e0e5ac8f47df4a4356e4bc25360bca --- .../core/java/com/android/server/ConnectivityService.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index be4464790c..cf3ca65f31 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1376,8 +1376,11 @@ public class ConnectivityService extends IConnectivityManager.Stub return; } final String action = blocked ? "BLOCKED" : "UNBLOCKED"; + final NetworkRequest satisfiedRequest = nri.getSatisfiedRequest(); + int requestId = satisfiedRequest != null + ? satisfiedRequest.requestId : nri.mRequests.get(0).requestId; mNetworkInfoBlockingLogs.log(String.format( - "%s %d(%d) on netId %d", action, nri.mUid, nri.request.requestId, net.getNetId())); + "%s %d(%d) on netId %d", action, nri.mUid, requestId, net.getNetId())); } /** From a152f88e4fedb8d92ac2aca094efb7b6db0b9af5 Mon Sep 17 00:00:00 2001 From: James Mattis Date: Fri, 20 Nov 2020 16:08:10 -0800 Subject: [PATCH 4/4] nits removing extra space, change method name, etc Minor cleanup as per nit comments on approved CLs in the relation chain. Namely: - removing an extranous space - changing requestsSortedById() to be package private - changing releaseNetworkRequest() name to releaseNetworkRequests() - adding final in a couple spots - added some test requests in testDumpDoesNotCrash() Bug: 173145245 Bug: 173292541 Bug: 173146509 Bug: 171991028 Test: atest FrameworksNetTests Change-Id: I177ec6072a44acd247022b65b56e90cc231094b9 --- .../com/android/server/ConnectivityService.java | 8 ++++---- .../android/server/ConnectivityServiceTest.java | 15 ++++++++++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index cf3ca65f31..82f9e7d010 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1377,7 +1377,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } final String action = blocked ? "BLOCKED" : "UNBLOCKED"; final NetworkRequest satisfiedRequest = nri.getSatisfiedRequest(); - int requestId = satisfiedRequest != null + final int requestId = satisfiedRequest != null ? satisfiedRequest.requestId : nri.mRequests.get(0).requestId; mNetworkInfoBlockingLogs.log(String.format( "%s %d(%d) on netId %d", action, nri.mUid, requestId, net.getNetId())); @@ -2712,7 +2712,7 @@ public class ConnectivityService extends IConnectivityManager.Stub * Return an array of all current NetworkRequest sorted by request id. */ @VisibleForTesting - protected NetworkRequestInfo[] requestsSortedById() { + NetworkRequestInfo[] requestsSortedById() { NetworkRequestInfo[] requests = new NetworkRequestInfo[0]; requests = mNetworkRequests.values().toArray(requests); // Sort the array based off the NRI containing the min requestId in its requests. @@ -5527,7 +5527,7 @@ public class ConnectivityService extends IConnectivityManager.Stub public void binderDied() { log("ConnectivityService NetworkRequestInfo binderDied(" + mRequests + ", " + mBinder + ")"); - releaseNetworkRequest(mRequests); + releaseNetworkRequests(mRequests); } @Override @@ -5854,7 +5854,7 @@ public class ConnectivityService extends IConnectivityManager.Stub return mNextNetworkProviderId.getAndIncrement(); } - private void releaseNetworkRequest(List networkRequests) { + private void releaseNetworkRequests(List networkRequests) { for (int i = 0; i < networkRequests.size(); i++) { releaseNetworkRequest(networkRequests.get(i)); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index bb7505be14..8defa2d5df 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -7812,7 +7812,16 @@ public class ConnectivityServiceTest { @Test public void testDumpDoesNotCrash() { - StringWriter stringWriter = new StringWriter(); + // Filing a couple requests prior to testing the dump. + final TestNetworkCallback genericNetworkCallback = new TestNetworkCallback(); + final TestNetworkCallback wifiNetworkCallback = new TestNetworkCallback(); + final NetworkRequest genericRequest = new NetworkRequest.Builder() + .clearCapabilities().build(); + final NetworkRequest wifiRequest = new NetworkRequest.Builder() + .addTransportType(TRANSPORT_WIFI).build(); + mCm.registerNetworkCallback(genericRequest, genericNetworkCallback); + mCm.registerNetworkCallback(wifiRequest, wifiNetworkCallback); + final StringWriter stringWriter = new StringWriter(); mService.dump(new FileDescriptor(), new PrintWriter(stringWriter), new String[0]); @@ -7834,11 +7843,11 @@ public class ConnectivityServiceTest { mCm.registerNetworkCallback(wifiRequest, wifiNetworkCallback); mCm.registerNetworkCallback(cellRequest, cellNetworkCallback); - ConnectivityService.NetworkRequestInfo[] nriOutput = mService.requestsSortedById(); + final ConnectivityService.NetworkRequestInfo[] nriOutput = mService.requestsSortedById(); assertTrue(nriOutput.length > 1); for (int i = 0; i < nriOutput.length - 1; i++) { - boolean isRequestIdInOrder = + final boolean isRequestIdInOrder = nriOutput[i].mRequests.get(0).requestId < nriOutput[i + 1].mRequests.get(0).requestId; assertTrue(isRequestIdInOrder);