From b9d9405225402504b5b7499b794549812562a13b Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 19 Nov 2019 19:16:48 +0900 Subject: [PATCH 1/3] [NS A15] Move legacy default broadcasts out of the loop Test: FrameworksNetTests NetworkStackTests Change-Id: Ib79b777b5efda3a4c85c30055f1a6d03d5d04c25 --- .../android/server/ConnectivityService.java | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 05ce2aef86..1bbd96a093 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6495,20 +6495,6 @@ public class ConnectivityService extends IConnectivityManager.Stub // Possibly unlinger newNetwork. Unlingering a network does not send any callbacks so it // does not need to be done in any particular order. updateLingerState(newNetwork, now); - - if (isNewDefault) { - // Maintain the illusion: since the legacy API only - // understands one network at a time, we must pretend - // that the current default network disconnected before - // the new one connected. - if (oldDefaultNetwork != null) { - mLegacyTypeTracker.remove(oldDefaultNetwork.networkInfo.getType(), - oldDefaultNetwork, true); - } - mDefaultInetConditionPublished = newNetwork.lastValidated ? 100 : 0; - mLegacyTypeTracker.add(newNetwork.networkInfo.getType(), newNetwork); - notifyLockdownVpn(newNetwork); - } } /** @@ -6522,6 +6508,8 @@ public class ConnectivityService extends IConnectivityManager.Stub // requests. Once the code has switched to a request-major iteration style, this can // be optimized to only do the processing needed. final long now = SystemClock.elapsedRealtime(); + final NetworkAgentInfo oldDefaultNetwork = getDefaultNetwork(); + final NetworkAgentInfo[] nais = mNetworkAgentInfos.values().toArray( new NetworkAgentInfo[mNetworkAgentInfos.size()]); // Rematch higher scoring networks first to prevent requests first matching a lower @@ -6532,6 +6520,27 @@ public class ConnectivityService extends IConnectivityManager.Stub rematchNetworkAndRequests(nai, now); } + final NetworkAgentInfo newDefaultNetwork = getDefaultNetwork(); + + // TODO : move the LegacyTypeTracker-related code to a separate function. + if (oldDefaultNetwork != newDefaultNetwork) { + // Maintain the illusion : since the legacy API only understands one network at a time, + // if the default network changed, apps should see a disconnected broadcast for the + // old default network before they see a connected broadcast for the new one. + if (oldDefaultNetwork != null) { + mLegacyTypeTracker.remove(oldDefaultNetwork.networkInfo.getType(), + oldDefaultNetwork, true); + } + if (newDefaultNetwork != null) { + // The new default network can be newly null if and only if the old default + // network doesn't satisfy the default request any more because it lost a + // capability. + mDefaultInetConditionPublished = newDefaultNetwork.lastValidated ? 100 : 0; + mLegacyTypeTracker.add(newDefaultNetwork.networkInfo.getType(), newDefaultNetwork); + notifyLockdownVpn(newDefaultNetwork); + } + } + // Now that all the callbacks have been sent, send the legacy network broadcasts // as needed. This is necessary so that legacy requests correctly bind dns // requests to this network. The legacy users are listening for this broadcast From f034453ccea9283762f1e6c8451b872f6167994a Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 19 Nov 2019 19:23:38 +0900 Subject: [PATCH 2/3] [NS A16] Cleanup Move LegacyTypeTracker work into a function for readability. Test: FrameworksNetTests NetworkStackTests Change-Id: I9539b8cc4422b3a0cc1d3d9b3a44d59dc1905b44 --- .../android/server/ConnectivityService.java | 51 +++++++++++-------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 1bbd96a093..1d5016e9d5 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6522,7 +6522,32 @@ public class ConnectivityService extends IConnectivityManager.Stub final NetworkAgentInfo newDefaultNetwork = getDefaultNetwork(); - // TODO : move the LegacyTypeTracker-related code to a separate function. + updateLegacyTypeTrackerAndVpnLockdownForRematch(oldDefaultNetwork, newDefaultNetwork, nais); + + // Tear down all unneeded networks. + for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) { + if (unneeded(nai, UnneededFor.TEARDOWN)) { + if (nai.getLingerExpiry() > 0) { + // This network has active linger timers and no requests, but is not + // lingering. Linger it. + // + // One way (the only way?) this can happen if this network is unvalidated + // and became unneeded due to another network improving its score to the + // point where this network will no longer be able to satisfy any requests + // even if it validates. + updateLingerState(nai, now); + } else { + if (DBG) log("Reaping " + nai.name()); + teardownUnneededNetwork(nai); + } + } + } + } + + private void updateLegacyTypeTrackerAndVpnLockdownForRematch( + @Nullable final NetworkAgentInfo oldDefaultNetwork, + @Nullable final NetworkAgentInfo newDefaultNetwork, + @NonNull final NetworkAgentInfo[] nais) { if (oldDefaultNetwork != newDefaultNetwork) { // Maintain the illusion : since the legacy API only understands one network at a time, // if the default network changed, apps should see a disconnected broadcast for the @@ -6537,6 +6562,11 @@ public class ConnectivityService extends IConnectivityManager.Stub // capability. mDefaultInetConditionPublished = newDefaultNetwork.lastValidated ? 100 : 0; mLegacyTypeTracker.add(newDefaultNetwork.networkInfo.getType(), newDefaultNetwork); + // If the legacy VPN is connected, notifyLockdownVpn may end up sending a broadcast + // to reflect the NetworkInfo of this new network. This broadcast has to be sent + // after the disconnect broadcasts above, but before the broadcasts sent by the + // legacy type tracker below. + // TODO : refactor this, it's too complex notifyLockdownVpn(newDefaultNetwork); } } @@ -6558,25 +6588,6 @@ public class ConnectivityService extends IConnectivityManager.Stub for (NetworkAgentInfo nai : nais) { addNetworkToLegacyTypeTracker(nai); } - - // Tear down all unneeded networks. - for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) { - if (unneeded(nai, UnneededFor.TEARDOWN)) { - if (nai.getLingerExpiry() > 0) { - // This network has active linger timers and no requests, but is not - // lingering. Linger it. - // - // One way (the only way?) this can happen if this network is unvalidated - // and became unneeded due to another network improving its score to the - // point where this network will no longer be able to satisfy any requests - // even if it validates. - updateLingerState(nai, now); - } else { - if (DBG) log("Reaping " + nai.name()); - teardownUnneededNetwork(nai); - } - } - } } private void addNetworkToLegacyTypeTracker(@NonNull final NetworkAgentInfo nai) { From 7807fa2b7fc7578370de638afacd1539b27664ac Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 19 Nov 2019 20:01:10 +0900 Subject: [PATCH 3/3] [NS A17] Update linger state after rematching. Test: FrameworksNetTests NetworkStackTests Change-Id: I720a1feb89088aa123201ef5867de444234343e8 --- .../android/server/ConnectivityService.java | 22 +++++++++---------- .../server/connectivity/NetworkAgentInfo.java | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 1d5016e9d5..245c7c7238 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6486,15 +6486,6 @@ public class ConnectivityService extends IConnectivityManager.Stub // do this after the default net is switched, but // before LegacyTypeTracker sends legacy broadcasts for (NetworkRequestInfo nri : addedRequests) notifyNetworkAvailable(newNetwork, nri); - - // Linger any networks that are no longer needed. This should be done after sending the - // available callback for newNetwork. - for (NetworkAgentInfo nai : removedRequests) { - updateLingerState(nai, now); - } - // Possibly unlinger newNetwork. Unlingering a network does not send any callbacks so it - // does not need to be done in any particular order. - updateLingerState(newNetwork, now); } /** @@ -6514,14 +6505,23 @@ public class ConnectivityService extends IConnectivityManager.Stub new NetworkAgentInfo[mNetworkAgentInfos.size()]); // Rematch higher scoring networks first to prevent requests first matching a lower // scoring network and then a higher scoring network, which could produce multiple - // callbacks and inadvertently unlinger networks. + // callbacks. Arrays.sort(nais); - for (NetworkAgentInfo nai : nais) { + for (final NetworkAgentInfo nai : nais) { rematchNetworkAndRequests(nai, now); } final NetworkAgentInfo newDefaultNetwork = getDefaultNetwork(); + for (final NetworkAgentInfo nai : nais) { + // Rematching may have altered the linger state of some networks, so update all linger + // timers. updateLingerState reads the state from the network agent and does nothing + // if the state has not changed : the source of truth is controlled with + // NetworkAgentInfo#lingerRequest and NetworkAgentInfo#unlingerRequest, which have been + // called while rematching the individual networks above. + updateLingerState(nai, now); + } + updateLegacyTypeTrackerAndVpnLockdownForRematch(oldDefaultNetwork, newDefaultNetwork, nais); // Tear down all unneeded networks. diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 24a5b7fa14..bb7f86233a 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -580,7 +580,7 @@ public class NetworkAgentInfo implements Comparable { // semantics of WakeupMessage guarantee that if cancel is called then the alarm will // never call its callback (handleLingerComplete), even if it has already fired. // WakeupMessage makes no such guarantees about rescheduling a message, so if mLingerMessage - // has already been dispatched, rescheduling to some time in the future it won't stop it + // has already been dispatched, rescheduling to some time in the future won't stop it // from calling its callback immediately. if (mLingerMessage != null) { mLingerMessage.cancel();