From 72bbf4829dffb5918b859867a63c5597cd461e1a Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Wed, 20 Jul 2016 02:39:22 +0900 Subject: [PATCH 1/2] Rematch requests first and listens second. This CL splits rematching in two parts: first rematch requests, then rematch listens. This will allow us to change a network's capabilities depending on what requests are on that network, and properly dispatch callbacks depending on those capabilities. The behaviour changes are as follows: - Before this CL, callbacks for requests and listens were sent intermingled. After this CL, all request callbacks will be grouped together, and all listen callbacks will be grouped together. - Before this CL, the order was: 1. Send onLost callbacks. 2. If applicable, switch the default network. 3. Send onAvailable callbacks. After this CL, the order is: 1. Send onLost callbacks for requests. 2. If applicable, switch the default network. 3. Send onLost callbacks for listens. 4. Send onAvailable callbacks for listens. 5. Send onAvailable callbacks for requests. These changes shouldn't affect any apps because: 1. The order of callbacks continues to be first all onLost, then all onAvailable. 2. Both the old and the new code send callbacks in no particular order. Thus, the possible ordering combinations of callbacks in the new code are a strict subset of the possible ordering combinations of the old code. 3. The default network is switched before any onAvailable callback is sent. 4. Even though the new code does not send all onLost callbacks before switching the default network, even before this CL there was no guarantee that those callbacks would be received before the default network switch anyway, because callbacks are asynchronous. Bug: 23113288 Change-Id: Ia08900c50db9ff43895047e2f4f36b6c6c31a1f9 --- .../android/server/ConnectivityService.java | 53 +++++++++++++------ 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 58431c856dcf6..0c23d1a3ab6b4 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -4631,6 +4631,27 @@ public class ConnectivityService extends IConnectivityManager.Stub setDefaultDnsSystemProperties(newNetwork.linkProperties.getDnsServers()); } + private void processListenRequests(NetworkAgentInfo nai) { + // For consistency with previous behaviour, send onLost callbacks before onAvailable. + for (NetworkRequestInfo nri : mNetworkRequests.values()) { + NetworkRequest nr = nri.request; + if (!nr.isListen()) continue; + if (nai.isSatisfyingRequest(nr.requestId) && !nai.satisfies(nr)) { + nai.removeRequest(nri.request.requestId); + callCallbackForRequest(nri, nai, ConnectivityManager.CALLBACK_LOST, 0); + } + } + + for (NetworkRequestInfo nri : mNetworkRequests.values()) { + NetworkRequest nr = nri.request; + if (!nr.isListen()) continue; + if (nai.satisfies(nr) && !nai.isSatisfyingRequest(nr.requestId)) { + nai.addRequest(nr); + notifyNetworkCallback(nai, nri); + } + } + } + // Handles a network appearing or improving its score. // // - Evaluates all current NetworkRequests that can be @@ -4671,6 +4692,12 @@ public class ConnectivityService extends IConnectivityManager.Stub ArrayList addedRequests = new ArrayList(); if (VDBG) log(" network has: " + newNetwork.networkCapabilities); for (NetworkRequestInfo nri : mNetworkRequests.values()) { + // Process requests in the first pass and listens in the second pass. This allows us to + // change a network's capabilities depending on which requests it has. This is only + // correct if the change in capabilities doesn't affect whether the network satisfies + // requests or not, and doesn't affect the network's score. + if (nri.request.isListen()) continue; + final NetworkAgentInfo currentNetwork = mNetworkForRequestId.get(nri.request.requestId); final boolean satisfies = newNetwork.satisfies(nri.request); if (newNetwork == currentNetwork && satisfies) { @@ -4685,13 +4712,6 @@ public class ConnectivityService extends IConnectivityManager.Stub // check if it satisfies the NetworkCapabilities if (VDBG) log(" checking if request is satisfied: " + nri.request); if (satisfies) { - if (nri.request.isListen()) { - // This is not a request, it's a callback listener. - // Add it to newNetwork regardless of score. - if (newNetwork.addRequest(nri.request)) addedRequests.add(nri); - continue; - } - // next check if it's better than any current network we're using for // this request if (VDBG) { @@ -4748,16 +4768,14 @@ public class ConnectivityService extends IConnectivityManager.Stub mNetworkForRequestId.remove(nri.request.requestId); sendUpdatedScoreToFactories(nri.request, 0); } else { - if (nri.request.isRequest()) { - Slog.wtf(TAG, "BUG: Removing request " + nri.request.requestId + " from " + - newNetwork.name() + - " without updating mNetworkForRequestId or factories!"); - } + Slog.wtf(TAG, "BUG: Removing request " + nri.request.requestId + " from " + + newNetwork.name() + + " without updating mNetworkForRequestId or factories!"); } - // TODO: technically, sending CALLBACK_LOST here is - // incorrect if nri is a request (not a listen) and there - // is a replacement network currently connected that can - // satisfy it. However, the only capability that can both + // TODO: Technically, sending CALLBACK_LOST here is + // incorrect if there is a replacement network currently + // connected that can satisfy nri, which is a request + // (not a listen). However, the only capability that can both // a) be requested and b) change is NET_CAPABILITY_TRUSTED, // so this code is only incorrect for a network that loses // the TRUSTED capability, which is a rare case. @@ -4782,6 +4800,9 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + // Second pass: process all listens. + processListenRequests(newNetwork); + // do this after the default net is switched, but // before LegacyTypeTracker sends legacy broadcasts for (NetworkRequestInfo nri : addedRequests) notifyNetworkCallback(newNetwork, nri); From 3d4a10617c6d967aa0f07e129264e07320e99073 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Fri, 9 Sep 2016 18:48:56 +0900 Subject: [PATCH 2/2] Add a background NetworkRequest type for mobile data always on. Like a normal (foreground) request, a background request is only satisfied by one network and will keep that network up. Unlike a foreground request, when a network only has background requests, it will linger, and after lingering is complete, it will become a background network. Future CLs will cause the system to treat background networks differently, e.g., by requiring different permissions. Bug: 23113288 Change-Id: I40f735269dad1042eb04fea15e64584fc903ccb3 --- core/java/android/net/NetworkRequest.java | 56 ++++++++++-- .../android/server/ConnectivityService.java | 71 ++++++++++----- .../server/connectivity/NetworkAgentInfo.java | 87 ++++++++++++++++--- 3 files changed, 173 insertions(+), 41 deletions(-) diff --git a/core/java/android/net/NetworkRequest.java b/core/java/android/net/NetworkRequest.java index 4501f7b089b96..ae724709c6c62 100644 --- a/core/java/android/net/NetworkRequest.java +++ b/core/java/android/net/NetworkRequest.java @@ -49,7 +49,7 @@ public class NetworkRequest implements Parcelable { public final int legacyType; /** - * A NetworkRequest as used by the system can be one of three types: + * A NetworkRequest as used by the system can be one of the following types: * * - LISTEN, for which the framework will issue callbacks about any * and all networks that match the specified NetworkCapabilities, @@ -64,7 +64,20 @@ public class NetworkRequest implements Parcelable { * current network (if any) that matches the capabilities of the * default Internet request (mDefaultRequest), but which cannot cause * the framework to either create or retain the existence of any - * specific network. + * specific network. Note that from the point of view of the request + * matching code, TRACK_DEFAULT is identical to REQUEST: its special + * behaviour is not due to different semantics, but to the fact that + * the system will only ever create a TRACK_DEFAULT with capabilities + * that are identical to the default request's capabilities, thus + * causing it to share fate in every way with the default request. + * + * - BACKGROUND_REQUEST, like REQUEST but does not cause any networks + * to retain the NET_CAPABILITY_FOREGROUND capability. A network with + * no foreground requests is in the background. A network that has + * one or more background requests and loses its last foreground + * request to a higher-scoring network will not go into the + * background immediately, but will linger and go into the background + * after the linger timeout. * * - The value NONE is used only by applications. When an application * creates a NetworkRequest, it does not have a type; the type is set @@ -77,7 +90,8 @@ public class NetworkRequest implements Parcelable { NONE, LISTEN, TRACK_DEFAULT, - REQUEST + REQUEST, + BACKGROUND_REQUEST, }; /** @@ -140,7 +154,7 @@ public class NetworkRequest implements Parcelable { * Add the given capability requirement to this builder. These represent * the requested network's required capabilities. Note that when searching * for a network to satisfy a request, all capabilities requested must be - * satisfied. See {@link NetworkCapabilities} for {@code NET_CAPABILITIY_*} + * satisfied. See {@link NetworkCapabilities} for {@code NET_CAPABILITY_*} * definitions. * * @param capability The {@code NetworkCapabilities.NET_CAPABILITY_*} to add. @@ -284,7 +298,7 @@ public class NetworkRequest implements Parcelable { }; /** - * Returns true iff. the contained NetworkRequest is of type LISTEN. + * Returns true iff. this NetworkRequest is of type LISTEN. * * @hide */ @@ -298,8 +312,9 @@ public class NetworkRequest implements Parcelable { * - should be associated with at most one satisfying network * at a time; * - * - should cause a network to be kept up if it is the best network - * which can satisfy the NetworkRequest. + * - should cause a network to be kept up, but not necessarily in + * the foreground, if it is the best network which can satisfy the + * NetworkRequest. * * For full detail of how isRequest() is used for pairing Networks with * NetworkRequests read rematchNetworkAndRequests(). @@ -307,9 +322,36 @@ public class NetworkRequest implements Parcelable { * @hide */ public boolean isRequest() { + return isForegroundRequest() || isBackgroundRequest(); + } + + /** + * Returns true iff. the contained NetworkRequest is one that: + * + * - should be associated with at most one satisfying network + * at a time; + * + * - should cause a network to be kept up and in the foreground if + * it is the best network which can satisfy the NetworkRequest. + * + * For full detail of how isRequest() is used for pairing Networks with + * NetworkRequests read rematchNetworkAndRequests(). + * + * @hide + */ + public boolean isForegroundRequest() { return type == Type.TRACK_DEFAULT || type == Type.REQUEST; } + /** + * Returns true iff. this NetworkRequest is of type BACKGROUND_REQUEST. + * + * @hide + */ + public boolean isBackgroundRequest() { + return type == Type.BACKGROUND_REQUEST; + } + public String toString() { return "NetworkRequest [ " + type + " id=" + requestId + (legacyType != ConnectivityManager.TYPE_NONE ? ", legacyType=" + legacyType : "") + diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 0c23d1a3ab6b4..eddc4f40949e2 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -262,6 +262,11 @@ public class ConnectivityService extends IConnectivityManager.Stub DONT_REAP }; + private enum UnneededFor { + LINGER, // Determine whether this network is unneeded and should be lingered. + TEARDOWN, // Determine whether this network is unneeded and should be torn down. + } + /** * used internally to change our mobile data enabled flag */ @@ -691,13 +696,13 @@ public class ConnectivityService extends IConnectivityManager.Stub if (DBG) log("ConnectivityService starting up"); mMetricsLog = logger; - mDefaultRequest = createInternetRequestForTransport(-1); + mDefaultRequest = createInternetRequestForTransport(-1, NetworkRequest.Type.REQUEST); NetworkRequestInfo defaultNRI = new NetworkRequestInfo(null, mDefaultRequest, new Binder()); mNetworkRequests.put(mDefaultRequest, defaultNRI); mNetworkRequestInfoLogs.log("REGISTER " + defaultNRI); mDefaultMobileDataRequest = createInternetRequestForTransport( - NetworkCapabilities.TRANSPORT_CELLULAR); + NetworkCapabilities.TRANSPORT_CELLULAR, NetworkRequest.Type.BACKGROUND_REQUEST); mHandlerThread = createHandlerThread(); mHandlerThread.start(); @@ -848,15 +853,15 @@ public class ConnectivityService extends IConnectivityManager.Stub mLingerMonitor = new LingerMonitor(mContext, mNotifier, dailyLimit, rateLimit); } - private NetworkRequest createInternetRequestForTransport(int transportType) { + private NetworkRequest createInternetRequestForTransport( + int transportType, NetworkRequest.Type type) { NetworkCapabilities netCap = new NetworkCapabilities(); netCap.addCapability(NET_CAPABILITY_INTERNET); netCap.addCapability(NET_CAPABILITY_NOT_RESTRICTED); if (transportType > -1) { netCap.addTransportType(transportType); } - return new NetworkRequest(netCap, TYPE_NONE, nextNetworkRequestId(), - NetworkRequest.Type.REQUEST); + return new NetworkRequest(netCap, TYPE_NONE, nextNetworkRequestId(), type); } // Used only for testing. @@ -1970,8 +1975,12 @@ public class ConnectivityService extends IConnectivityManager.Stub for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) { pw.println(nai.toString()); pw.increaseIndent(); - pw.println(String.format("Requests: %d request/%d total", - nai.numRequestNetworkRequests(), nai.numNetworkRequests())); + pw.println(String.format( + "Requests: REQUEST:%d LISTEN:%d BACKGROUND_REQUEST:%d total:%d", + nai.numForegroundNetworkRequests(), + nai.numNetworkRequests() - nai.numRequestNetworkRequests(), + nai.numBackgroundNetworkRequests(), + nai.numNetworkRequests())); pw.increaseIndent(); for (int i = 0; i < nai.numNetworkRequests(); i++) { pw.println(nai.requestAt(i).toString()); @@ -2292,15 +2301,13 @@ public class ConnectivityService extends IConnectivityManager.Stub // 3. If this network is unneeded (which implies it is not lingering), and there is at least // one lingered request, start lingering. nai.updateLingerTimer(); - if (nai.isLingering() && nai.numRequestNetworkRequests() > 0) { + if (nai.isLingering() && nai.numForegroundNetworkRequests() > 0) { if (DBG) log("Unlingering " + nai.name()); nai.unlinger(); logNetworkEvent(nai, NetworkEvent.NETWORK_UNLINGER); - } else if (unneeded(nai) && nai.getLingerExpiry() > 0) { // unneeded() calls isLingering() + } else if (unneeded(nai, UnneededFor.LINGER) && nai.getLingerExpiry() > 0) { int lingerTime = (int) (nai.getLingerExpiry() - now); - if (DBG) { - Log.d(TAG, "Lingering " + nai.name() + " for " + lingerTime + "ms"); - } + if (DBG) log("Lingering " + nai.name() + " for " + lingerTime + "ms"); nai.linger(); logNetworkEvent(nai, NetworkEvent.NETWORK_LINGER); notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_LOSING, lingerTime); @@ -2479,15 +2486,37 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - // Is nai unneeded by all NetworkRequests (and should be disconnected)? - // This is whether it is satisfying any NetworkRequests or were it to become validated, - // would it have a chance of satisfying any NetworkRequests. - private boolean unneeded(NetworkAgentInfo nai) { - if (!nai.everConnected || nai.isVPN() || - nai.isLingering() || nai.numRequestNetworkRequests() > 0) { + // Determines whether the network is the best (or could become the best, if it validated), for + // none of a particular type of NetworkRequests. The type of NetworkRequests considered depends + // on the value of reason: + // + // - UnneededFor.TEARDOWN: non-listen NetworkRequests. If a network is unneeded for this reason, + // then it should be torn down. + // - UnneededFor.LINGER: foreground NetworkRequests. If a network is unneeded for this reason, + // then it should be lingered. + private boolean unneeded(NetworkAgentInfo nai, UnneededFor reason) { + final int numRequests; + switch (reason) { + case TEARDOWN: + numRequests = nai.numRequestNetworkRequests(); + break; + case LINGER: + numRequests = nai.numForegroundNetworkRequests(); + break; + default: + Slog.wtf(TAG, "Invalid reason. Cannot happen."); + return true; + } + + if (!nai.everConnected || nai.isVPN() || nai.isLingering() || numRequests > 0) { return false; } for (NetworkRequestInfo nri : mNetworkRequests.values()) { + if (reason == UnneededFor.LINGER && nri.request.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) && @@ -2583,7 +2612,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // If there are still lingered requests on this network, don't tear it down, // but resume lingering instead. updateLingerState(nai, SystemClock.elapsedRealtime()); - if (unneeded(nai)) { + if (unneeded(nai, UnneededFor.TEARDOWN)) { if (DBG) log("no live requests for " + nai.name() + "; disconnecting"); teardownUnneededNetwork(nai); } else { @@ -4612,7 +4641,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // must be no other active linger timers, and we must stop lingering. oldNetwork.clearLingerState(); - if (unneeded(oldNetwork)) { + if (unneeded(oldNetwork, UnneededFor.TEARDOWN)) { teardownUnneededNetwork(oldNetwork); } } @@ -4880,7 +4909,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } if (reapUnvalidatedNetworks == ReapUnvalidatedNetworks.REAP) { for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) { - if (unneeded(nai)) { + if (unneeded(nai, UnneededFor.TEARDOWN)) { if (nai.getLingerExpiry() > 0) { // This network has active linger timers and no requests, but is not // lingering. Linger it. diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index b0330b94a6786..7044e60ece92e 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -104,14 +104,16 @@ import java.util.TreeSet; // ----------------------------------------------- // If a network has no chance of satisfying any requests (even if it were to become validated // and enter state #5), ConnectivityService will disconnect the NetworkAgent's AsyncChannel. -// If the network ever for any period of time had satisfied a NetworkRequest (i.e. had been -// the highest scoring that satisfied the NetworkRequest's constraints), but is no longer the -// highest scoring network for any NetworkRequest, then there will be a 30s pause before -// ConnectivityService disconnects the NetworkAgent's AsyncChannel. During this pause the -// network is considered "lingering". This pause exists to allow network communication to be -// wrapped up rather than abruptly terminated. During this pause if the network begins satisfying -// a NetworkRequest, ConnectivityService will cancel the future disconnection of the NetworkAgent's -// AsyncChannel, and the network is no longer considered "lingering". +// +// If the network was satisfying a foreground NetworkRequest (i.e. had been the highest scoring that +// satisfied the NetworkRequest's constraints), but is no longer the highest scoring network for any +// foreground NetworkRequest, then there will be a 30s pause to allow network communication to be +// wrapped up rather than abruptly terminated. During this pause the network is said to be +// "lingering". During this pause if the network begins satisfying a foreground NetworkRequest, +// ConnectivityService will cancel the future disconnection of the NetworkAgent's AsyncChannel, and +// the network is no longer considered "lingering". After the linger timer expires, if the network +// is satisfying one or more background NetworkRequests it is kept up in the background. If it is +// not, ConnectivityService disconnects the NetworkAgent's AsyncChannel. public class NetworkAgentInfo implements Comparable { public NetworkInfo networkInfo; // This Network object should always be used if possible, so as to encourage reuse of the @@ -227,11 +229,13 @@ public class NetworkAgentInfo implements Comparable { // The list of NetworkRequests being satisfied by this Network. private final SparseArray mNetworkRequests = new SparseArray<>(); - // The list of NetworkRequests that this Network previously satisfied with the highest - // score. A non-empty list indicates that if this Network was validated it is lingered. + // How many of the satisfied requests are actual requests and not listens. private int mNumRequestNetworkRequests = 0; + // How many of the satisfied requests are of type BACKGROUND_REQUEST. + private int mNumBackgroundNetworkRequests = 0; + public final Messenger messenger; public final AsyncChannel asyncChannel; @@ -265,6 +269,32 @@ public class NetworkAgentInfo implements Comparable { // // These functions must only called on ConnectivityService's main thread. + private static final boolean ADD = true; + private static final boolean REMOVE = false; + + private void updateRequestCounts(boolean add, NetworkRequest request) { + int delta = add ? +1 : -1; + switch (request.type) { + case REQUEST: + case TRACK_DEFAULT: + mNumRequestNetworkRequests += delta; + break; + + case BACKGROUND_REQUEST: + mNumRequestNetworkRequests += delta; + mNumBackgroundNetworkRequests += delta; + break; + + case LISTEN: + break; + + case NONE: + default: + Log.wtf(TAG, "Unhandled request type " + request.type); + break; + } + } + /** * Add {@code networkRequest} to this network as it's satisfied by this network. * @return true if {@code networkRequest} was added or false if {@code networkRequest} was @@ -273,9 +303,15 @@ public class NetworkAgentInfo implements Comparable { public boolean addRequest(NetworkRequest networkRequest) { NetworkRequest existing = mNetworkRequests.get(networkRequest.requestId); if (existing == networkRequest) return false; - if (existing != null && existing.isRequest()) mNumRequestNetworkRequests--; + if (existing != null) { + // Should only happen if the requestId wraps. If that happens lots of other things will + // be broken as well. + Log.wtf(TAG, String.format("Duplicate requestId for %s and %s on %s", + networkRequest, existing, name())); + updateRequestCounts(REMOVE, existing); + } mNetworkRequests.put(networkRequest.requestId, networkRequest); - if (networkRequest.isRequest()) mNumRequestNetworkRequests++; + updateRequestCounts(ADD, networkRequest); return true; } @@ -285,9 +321,9 @@ public class NetworkAgentInfo implements Comparable { public void removeRequest(int requestId) { NetworkRequest existing = mNetworkRequests.get(requestId); if (existing == null) return; + updateRequestCounts(REMOVE, existing); mNetworkRequests.remove(requestId); if (existing.isRequest()) { - mNumRequestNetworkRequests--; unlingerRequest(existing); } } @@ -315,6 +351,21 @@ public class NetworkAgentInfo implements Comparable { return mNumRequestNetworkRequests; } + /** + * Returns the number of requests currently satisfied by this network of type + * {@link android.net.NetworkRequest.Type.BACKGROUND_REQUEST}. + */ + public int numBackgroundNetworkRequests() { + return mNumBackgroundNetworkRequests; + } + + /** + * Returns the number of foreground requests currently satisfied by this network. + */ + public int numForegroundNetworkRequests() { + return mNumRequestNetworkRequests - mNumBackgroundNetworkRequests; + } + /** * Returns the number of requests of any type currently satisfied by this network. */ @@ -322,6 +373,16 @@ public class NetworkAgentInfo implements Comparable { return mNetworkRequests.size(); } + /** + * Returns whether the network is a background network. A network is a background network if it + * is satisfying no foreground requests and at least one background request. (If it did not have + * a background request, it would be a speculative network that is only being kept up because + * it might satisfy a request if it validated). + */ + public boolean isBackgroundNetwork() { + return numForegroundNetworkRequests() == 0 && mNumBackgroundNetworkRequests > 0; + } + // Does this network satisfy request? public boolean satisfies(NetworkRequest request) { return created &&