From 24344d7e72192d9f6c47b247c00fd52519851a5d Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 4 Dec 2019 13:32:31 +0900 Subject: [PATCH 1/3] [NS A41] Cut out a function to update NAI info Test: ConnectivityServiceTest Change-Id: I6d92fb1699eb0acb1bcd7baa4a5d9d3e1c0dc728 --- .../android/server/ConnectivityService.java | 60 +++++++++---------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 9e8ab4f20d..cb06d9da6f 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6636,39 +6636,39 @@ public class ConnectivityService extends IConnectivityManager.Stub final NetworkAgentInfo newSatisfier = entry.getValue(); changes.addRequestReassignment(new NetworkReassignment.RequestReassignment( nri, previousSatisfier, newSatisfier)); - if (newSatisfier != null) { - if (VDBG) log("rematch for " + newSatisfier.name()); - if (previousSatisfier != null) { - if (VDBG || DDBG) { - log(" accepting network in place of " + previousSatisfier.name()); - } - previousSatisfier.removeRequest(nri.request.requestId); - previousSatisfier.lingerRequest(nri.request, now, mLingerDelayMs); - } else { - if (VDBG || DDBG) log(" accepting network in place of null"); - } - newSatisfier.unlingerRequest(nri.request); - if (!newSatisfier.addRequest(nri.request)) { - Slog.wtf(TAG, "BUG: " + newSatisfier.name() + " already has " + nri.request); - } - } else { - // If "newNetwork" is listed as satisfying "nri" but no longer satisfies "nri", - // mark it as no longer satisfying "nri". Because networks are processed by - // rematchAllNetworksAndRequests() in descending score order, "currentNetwork" will - // match "newNetwork" before this loop will encounter a "currentNetwork" with higher - // score than "newNetwork" and where "currentNetwork" no longer satisfies "nri". - // This means this code doesn't have to handle the case where "currentNetwork" no - // longer satisfies "nri" when "currentNetwork" does not equal "newNetwork". - if (DBG) { - log("Network " + newNetwork.name() + " stopped satisfying" + - " request " + nri.request.requestId); - } - newNetwork.removeRequest(nri.request.requestId); - } - nri.mSatisfier = newSatisfier; + updateSatisfiersForRematchRequest(nri, previousSatisfier, newSatisfier, now); } } + private void updateSatisfiersForRematchRequest(@NonNull final NetworkRequestInfo nri, + @Nullable final NetworkAgentInfo previousSatisfier, + @Nullable final NetworkAgentInfo newSatisfier, + final long now) { + if (newSatisfier != null) { + if (VDBG) log("rematch for " + newSatisfier.name()); + if (previousSatisfier != null) { + if (VDBG || DDBG) { + log(" accepting network in place of " + previousSatisfier.name()); + } + previousSatisfier.removeRequest(nri.request.requestId); + previousSatisfier.lingerRequest(nri.request, now, mLingerDelayMs); + } else { + if (VDBG || DDBG) log(" accepting network in place of null"); + } + newSatisfier.unlingerRequest(nri.request); + if (!newSatisfier.addRequest(nri.request)) { + Slog.wtf(TAG, "BUG: " + newSatisfier.name() + " already has " + nri.request); + } + } else { + if (DBG) { + log("Network " + previousSatisfier.name() + " stopped satisfying" + + " request " + nri.request.requestId); + } + previousSatisfier.removeRequest(nri.request.requestId); + } + nri.mSatisfier = newSatisfier; + } + /** * Attempt to rematch all Networks with NetworkRequests. This may result in Networks * being disconnected. From 383f5ef79acfaeda4fa905e68e1663cbbbca65d0 Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 3 Dec 2019 23:37:28 +0900 Subject: [PATCH 2/3] [NS A42] Move writing to the reassignment to the computation ...instead of the side effect loop. This is a no-op refactoring. Test: ConnectivityServiceTest Change-Id: I8308d55eaff080efb22a7a43142492545e626cf5 --- .../core/java/com/android/server/ConnectivityService.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index cb06d9da6f..b382922c1a 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6591,9 +6591,13 @@ public class ConnectivityService extends IConnectivityManager.Stub } if (currentNetwork == null || currentNetwork.getCurrentScore() < score) { reassignedRequests.put(nri, newNetwork); + changes.addRequestReassignment(new NetworkReassignment.RequestReassignment( + nri, currentNetwork, newNetwork)); } } else if (newNetwork == currentNetwork) { reassignedRequests.put(nri, null); + changes.addRequestReassignment(new NetworkReassignment.RequestReassignment( + nri, currentNetwork, null)); } } return reassignedRequests; @@ -6634,8 +6638,6 @@ public class ConnectivityService extends IConnectivityManager.Stub final NetworkRequestInfo nri = entry.getKey(); final NetworkAgentInfo previousSatisfier = nri.mSatisfier; final NetworkAgentInfo newSatisfier = entry.getValue(); - changes.addRequestReassignment(new NetworkReassignment.RequestReassignment( - nri, previousSatisfier, newSatisfier)); updateSatisfiersForRematchRequest(nri, previousSatisfier, newSatisfier, now); } } From 8fd82aea7eb97f59fdc3ad1f780575ef0e6da32e Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Wed, 4 Dec 2019 18:49:18 +0900 Subject: [PATCH 3/3] [NS A43] Separate updating the linger timer and sending LOSING This will be necessary because updating the linger timer is what lets the network agent know whether the network is Test: ConnectivityServiceTest Change-Id: I9b5a36f6d5eac5a404eff6740cd8f4dcbb0c2786 --- .../android/server/ConnectivityService.java | 42 +++++++++++++++---- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index b382922c1a..bdd2007c9e 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3097,7 +3097,13 @@ public class ConnectivityService extends IConnectivityManager.Stub handleUpdateLinkProperties(nai, new LinkProperties(nai.linkProperties)); } - private void updateLingerState(NetworkAgentInfo nai, long now) { + /** + * Updates the linger state from the network requests inside the NAI. + * @param nai the agent info to update + * @param now the timestamp of the event causing this update + * @return whether the network was lingered as a result of this update + */ + private boolean updateLingerState(@NonNull final NetworkAgentInfo nai, final long now) { // 1. Update the linger timer. If it's changed, reschedule or cancel the alarm. // 2. If the network was lingering and there are now requests, unlinger it. // 3. If this network is unneeded (which implies it is not lingering), and there is at least @@ -3108,12 +3114,15 @@ public class ConnectivityService extends IConnectivityManager.Stub nai.unlinger(); logNetworkEvent(nai, NetworkEvent.NETWORK_UNLINGER); } else if (unneeded(nai, UnneededFor.LINGER) && nai.getLingerExpiry() > 0) { - int lingerTime = (int) (nai.getLingerExpiry() - now); - if (DBG) log("Lingering " + nai.name() + " for " + lingerTime + "ms"); + if (DBG) { + final int lingerTime = (int) (nai.getLingerExpiry() - now); + log("Lingering " + nai.name() + " for " + lingerTime + "ms"); + } nai.linger(); logNetworkEvent(nai, NetworkEvent.NETWORK_LINGER); - notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_LOSING, lingerTime); + return true; } + return false; } private void handleAsyncChannelHalfConnect(Message msg) { @@ -3457,7 +3466,10 @@ 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()); + final long now = SystemClock.elapsedRealtime(); + if (updateLingerState(nai, now)) { + notifyNetworkLosing(nai, now); + } if (unneeded(nai, UnneededFor.TEARDOWN)) { if (DBG) log("no live requests for " + nai.name() + "; disconnecting"); teardownUnneededNetwork(nai); @@ -6600,6 +6612,7 @@ public class ConnectivityService extends IConnectivityManager.Stub nri, currentNetwork, null)); } } + return reassignedRequests; } @@ -6746,13 +6759,20 @@ public class ConnectivityService extends IConnectivityManager.Stub processNewlySatisfiedListenRequests(event.mNetwork); } + final ArrayList lingeredNetworks = new ArrayList<>(); 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); + if (updateLingerState(nai, now)) { + lingeredNetworks.add(nai); + } + } + + for (final NetworkAgentInfo nai : lingeredNetworks) { + notifyNetworkLosing(nai, now); } updateLegacyTypeTrackerAndVpnLockdownForRematch(oldDefaultNetwork, newDefaultNetwork, nais); @@ -6768,7 +6788,9 @@ public class ConnectivityService extends IConnectivityManager.Stub // 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); + if (updateLingerState(nai, now)) { + notifyNetworkLosing(nai, now); + } } else { if (DBG) log("Reaping " + nai.name()); teardownUnneededNetwork(nai); @@ -7021,6 +7043,12 @@ public class ConnectivityService extends IConnectivityManager.Stub callCallbackForRequest(nri, nai, ConnectivityManager.CALLBACK_AVAILABLE, blocked ? 1 : 0); } + // Notify the requests on this NAI that the network is now lingered. + private void notifyNetworkLosing(@NonNull final NetworkAgentInfo nai, final long now) { + final int lingerTime = (int) (nai.getLingerExpiry() - now); + notifyNetworkCallbacks(nai, ConnectivityManager.CALLBACK_LOSING, lingerTime); + } + /** * Notify of the blocked state apps with a registered callback matching a given NAI. *