From d9fffc3510f6cc34288f39573687fe444afae778 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Fri, 11 May 2018 20:19:20 +0900 Subject: [PATCH] Destroy networks as soon as they are disconnected. ...as opposed to after the async channel finished disconnecting. Bug: 78308259 Test: runtest frameworks-net also used a device with this patch over the weekend and tried all I could think of Change-Id: I77ad6d97abb20815b801a794eaa9685acf2d1173 --- .../android/server/ConnectivityService.java | 175 ++++++++++-------- 1 file changed, 93 insertions(+), 82 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index e22a86e39d..c3f504f6c6 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2442,97 +2442,107 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + // This is a no-op if it's called with a message designating a network that has + // already been destroyed, because its reference will not be found in the relevant + // maps. private void handleAsyncChannelDisconnected(Message msg) { NetworkAgentInfo nai = mNetworkAgentInfos.get(msg.replyTo); if (nai != null) { - if (DBG) { - log(nai.name() + " got DISCONNECTED, was satisfying " + nai.numNetworkRequests()); - } - // A network agent has disconnected. - // TODO - if we move the logic to the network agent (have them disconnect - // because they lost all their requests or because their score isn't good) - // then they would disconnect organically, report their new state and then - // disconnect the channel. - if (nai.networkInfo.isConnected()) { - nai.networkInfo.setDetailedState(NetworkInfo.DetailedState.DISCONNECTED, - null, null); - } - final boolean wasDefault = isDefaultNetwork(nai); - if (wasDefault) { - mDefaultInetConditionPublished = 0; - // Log default network disconnection before required book-keeping. - // Let rematchAllNetworksAndRequests() below record a new default network event - // if there is a fallback. Taken together, the two form a X -> 0, 0 -> Y sequence - // whose timestamps tell how long it takes to recover a default network. - long now = SystemClock.elapsedRealtime(); - metricsLogger().defaultNetworkMetrics().logDefaultNetworkEvent(now, null, nai); - } - notifyIfacesChangedForNetworkStats(); - // TODO - we shouldn't send CALLBACK_LOST to requests that can be satisfied - // by other networks that are already connected. Perhaps that can be done by - // sending all CALLBACK_LOST messages (for requests, not listens) at the end - // of rematchAllNetworksAndRequests - notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_LOST); - mKeepaliveTracker.handleStopAllKeepalives(nai, - ConnectivityManager.PacketKeepalive.ERROR_INVALID_NETWORK); - for (String iface : nai.linkProperties.getAllInterfaceNames()) { - // Disable wakeup packet monitoring for each interface. - wakeupModifyInterface(iface, nai.networkCapabilities, false); - } - nai.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_DISCONNECTED); - mNetworkAgentInfos.remove(msg.replyTo); - nai.maybeStopClat(); - synchronized (mNetworkForNetId) { - // Remove the NetworkAgent, but don't mark the netId as - // available until we've told netd to delete it below. - mNetworkForNetId.remove(nai.network.netId); - } - // Remove all previously satisfied requests. - for (int i = 0; i < nai.numNetworkRequests(); i++) { - NetworkRequest request = nai.requestAt(i); - NetworkAgentInfo currentNetwork = getNetworkForRequest(request.requestId); - if (currentNetwork != null && currentNetwork.network.netId == nai.network.netId) { - clearNetworkForRequest(request.requestId); - sendUpdatedScoreToFactories(request, 0); - } - } - nai.clearLingerState(); - if (nai.isSatisfyingRequest(mDefaultRequest.requestId)) { - removeDataActivityTracking(nai); - notifyLockdownVpn(nai); - ensureNetworkTransitionWakelock(nai.name()); - } - mLegacyTypeTracker.remove(nai, wasDefault); - if (!nai.networkCapabilities.hasTransport(TRANSPORT_VPN)) { - updateAllVpnsCapabilities(); - } - rematchAllNetworksAndRequests(null, 0); - mLingerMonitor.noteDisconnect(nai); - if (nai.created) { - // Tell netd to clean up the configuration for this network - // (routing rules, DNS, etc). - // This may be slow as it requires a lot of netd shelling out to ip and - // ip[6]tables to flush routes and remove the incoming packet mark rule, so do it - // after we've rematched networks with requests which should make a potential - // fallback network the default or requested a new network from the - // NetworkFactories, so network traffic isn't interrupted for an unnecessarily - // long time. - try { - mNetd.removeNetwork(nai.network.netId); - } catch (Exception e) { - loge("Exception removing network: " + e); - } - mDnsManager.removeNetwork(nai.network); - } - synchronized (mNetworkForNetId) { - mNetIdInUse.delete(nai.network.netId); - } + disconnectAndDestroyNetwork(nai); } else { NetworkFactoryInfo nfi = mNetworkFactoryInfos.remove(msg.replyTo); if (DBG && nfi != null) log("unregisterNetworkFactory for " + nfi.name); } } + // Destroys a network, remove references to it from the internal state managed by + // ConnectivityService, free its interfaces and clean up. + // Must be called on the Handler thread. + private void disconnectAndDestroyNetwork(NetworkAgentInfo nai) { + if (DBG) { + log(nai.name() + " got DISCONNECTED, was satisfying " + nai.numNetworkRequests()); + } + // A network agent has disconnected. + // TODO - if we move the logic to the network agent (have them disconnect + // because they lost all their requests or because their score isn't good) + // then they would disconnect organically, report their new state and then + // disconnect the channel. + if (nai.networkInfo.isConnected()) { + nai.networkInfo.setDetailedState(NetworkInfo.DetailedState.DISCONNECTED, + null, null); + } + final boolean wasDefault = isDefaultNetwork(nai); + if (wasDefault) { + mDefaultInetConditionPublished = 0; + // Log default network disconnection before required book-keeping. + // Let rematchAllNetworksAndRequests() below record a new default network event + // if there is a fallback. Taken together, the two form a X -> 0, 0 -> Y sequence + // whose timestamps tell how long it takes to recover a default network. + long now = SystemClock.elapsedRealtime(); + metricsLogger().defaultNetworkMetrics().logDefaultNetworkEvent(now, null, nai); + } + notifyIfacesChangedForNetworkStats(); + // TODO - we shouldn't send CALLBACK_LOST to requests that can be satisfied + // by other networks that are already connected. Perhaps that can be done by + // sending all CALLBACK_LOST messages (for requests, not listens) at the end + // of rematchAllNetworksAndRequests + notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_LOST); + mKeepaliveTracker.handleStopAllKeepalives(nai, + ConnectivityManager.PacketKeepalive.ERROR_INVALID_NETWORK); + for (String iface : nai.linkProperties.getAllInterfaceNames()) { + // Disable wakeup packet monitoring for each interface. + wakeupModifyInterface(iface, nai.networkCapabilities, false); + } + nai.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_DISCONNECTED); + mNetworkAgentInfos.remove(nai.messenger); + nai.maybeStopClat(); + synchronized (mNetworkForNetId) { + // Remove the NetworkAgent, but don't mark the netId as + // available until we've told netd to delete it below. + mNetworkForNetId.remove(nai.network.netId); + } + // Remove all previously satisfied requests. + for (int i = 0; i < nai.numNetworkRequests(); i++) { + NetworkRequest request = nai.requestAt(i); + NetworkAgentInfo currentNetwork = getNetworkForRequest(request.requestId); + if (currentNetwork != null && currentNetwork.network.netId == nai.network.netId) { + clearNetworkForRequest(request.requestId); + sendUpdatedScoreToFactories(request, 0); + } + } + nai.clearLingerState(); + if (nai.isSatisfyingRequest(mDefaultRequest.requestId)) { + removeDataActivityTracking(nai); + notifyLockdownVpn(nai); + ensureNetworkTransitionWakelock(nai.name()); + } + mLegacyTypeTracker.remove(nai, wasDefault); + if (!nai.networkCapabilities.hasTransport(TRANSPORT_VPN)) { + updateAllVpnsCapabilities(); + } + rematchAllNetworksAndRequests(null, 0); + mLingerMonitor.noteDisconnect(nai); + if (nai.created) { + // Tell netd to clean up the configuration for this network + // (routing rules, DNS, etc). + // This may be slow as it requires a lot of netd shelling out to ip and + // ip[6]tables to flush routes and remove the incoming packet mark rule, so do it + // after we've rematched networks with requests which should make a potential + // fallback network the default or requested a new network from the + // NetworkFactories, so network traffic isn't interrupted for an unnecessarily + // long time. + try { + mNetd.removeNetwork(nai.network.netId); + } catch (Exception e) { + loge("Exception removing network: " + e); + } + mDnsManager.removeNetwork(nai.network); + } + synchronized (mNetworkForNetId) { + mNetIdInUse.delete(nai.network.netId); + } + } + // If this method proves to be too slow then we can maintain a separate // pendingIntent => NetworkRequestInfo map. // This method assumes that every non-null PendingIntent maps to exactly 1 NetworkRequestInfo. @@ -5618,6 +5628,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } updateUids(networkAgent, networkAgent.networkCapabilities, null); } + disconnectAndDestroyNetwork(networkAgent); } else if ((oldInfo != null && oldInfo.getState() == NetworkInfo.State.SUSPENDED) || state == NetworkInfo.State.SUSPENDED) { // going into or coming out of SUSPEND: rescore and notify