diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 7d1644ed3b..28f104e446 100755 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -1760,7 +1760,12 @@ public class ConnectivityService extends IConnectivityManager.Stub mUserAllContext.registerReceiver(mPackageIntentReceiver, packageIntentFilter, null /* broadcastPermission */, mHandler); - mNetworkActivityTracker = new LegacyNetworkActivityTracker(mContext, mNetd, mHandler); + // TrackMultiNetworkActivities feature should be enabled by trunk stable flag. + // But reading the trunk stable flags from mainline modules is not supported yet. + // So enabling this feature on V+ release. + mTrackMultiNetworkActivities = mDeps.isAtLeastV(); + mNetworkActivityTracker = new LegacyNetworkActivityTracker(mContext, mNetd, mHandler, + mTrackMultiNetworkActivities); final NetdCallback netdCallback = new NetdCallback(); try { @@ -3234,9 +3239,20 @@ public class ConnectivityService extends IConnectivityManager.Stub private void handleReportNetworkActivity(final NetworkActivityParams params) { mNetworkActivityTracker.handleReportNetworkActivity(params); + final boolean isCellNetworkActivity; + if (mTrackMultiNetworkActivities) { + final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(params.label); + // nai could be null if netd receives a netlink message and calls the network + // activity change callback after the network is unregistered from ConnectivityService. + isCellNetworkActivity = nai != null + && nai.networkCapabilities.hasTransport(TRANSPORT_CELLULAR); + } else { + isCellNetworkActivity = params.label == TRANSPORT_CELLULAR; + } + if (mDelayDestroyFrozenSockets && params.isActive - && params.label == TRANSPORT_CELLULAR + && isCellNetworkActivity && !mPendingFrozenUids.isEmpty()) { closePendingFrozenSockets(); } @@ -11710,8 +11726,8 @@ public class ConnectivityService extends IConnectivityManager.Stub */ private static final class NetworkActivityParams { public final boolean isActive; - // Label used for idle timer. Transport type is used as label. - // label is int since NMS was using the identifier as int, and it has not been changed + // If TrackMultiNetworkActivities is enabled, idleTimer label is netid. + // If TrackMultiNetworkActivities is disabled, idleTimer label is transport type. public final int label; public final long timestampNs; // Uid represents the uid that was responsible for waking the radio. @@ -11753,13 +11769,15 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + private final boolean mTrackMultiNetworkActivities; private final LegacyNetworkActivityTracker mNetworkActivityTracker; /** * Class used for updating network activity tracking with netd and notify network activity * changes. */ - private static final class LegacyNetworkActivityTracker { + @VisibleForTesting + public static final class LegacyNetworkActivityTracker { private static final int NO_UID = -1; private final Context mContext; private final INetd mNetd; @@ -11771,8 +11789,10 @@ public class ConnectivityService extends IConnectivityManager.Stub // If there is no default network, default network is considered active to keep the existing // behavior. Initial value is used until first connect to the default network. private volatile boolean mIsDefaultNetworkActive = true; + private Network mDefaultNetwork; // Key is netId. Value is configured idle timer information. private final SparseArray mActiveIdleTimers = new SparseArray<>(); + private final boolean mTrackMultiNetworkActivities; private static class IdleTimerParams { public final int timeout; @@ -11785,10 +11805,11 @@ public class ConnectivityService extends IConnectivityManager.Stub } LegacyNetworkActivityTracker(@NonNull Context context, @NonNull INetd netd, - @NonNull Handler handler) { + @NonNull Handler handler, boolean trackMultiNetworkActivities) { mContext = context; mNetd = netd; mHandler = handler; + mTrackMultiNetworkActivities = trackMultiNetworkActivities; } private void ensureRunningOnConnectivityServiceThread() { @@ -11798,19 +11819,58 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - public void handleReportNetworkActivity(NetworkActivityParams activityParams) { - ensureRunningOnConnectivityServiceThread(); + private void handleDefaultNetworkActivity(final int transportType, + final boolean isActive, final long timestampNs) { + mIsDefaultNetworkActive = isActive; + sendDataActivityBroadcast(transportTypeToLegacyType(transportType), + isActive, timestampNs); + if (isActive) { + reportNetworkActive(); + } + } + + private void handleReportNetworkActivityWithNetIdLabel( + NetworkActivityParams activityParams) { + if (mDefaultNetwork == null || mDefaultNetwork.netId != activityParams.label) { + // This activity change is not for the default network. + return; + } + + final IdleTimerParams idleTimerParams = mActiveIdleTimers.get(activityParams.label); + if (idleTimerParams == null) { + // This network activity change is not tracked anymore + // This can happen if netd callback post activity change event message but idle + // timer is removed before processing this message. + return; + } + // TODO: if a network changes transports, storing the transport type in the + // IdleTimerParams is not correct. Consider getting it from the network's + // NetworkCapabilities instead. + handleDefaultNetworkActivity(idleTimerParams.transportType, activityParams.isActive, + activityParams.timestampNs); + } + + private void handleReportNetworkActivityWithTransportTypeLabel( + NetworkActivityParams activityParams) { if (mActiveIdleTimers.size() == 0) { // This activity change is not for the current default network. // This can happen if netd callback post activity change event message but // the default network is lost before processing this message. return; } - sendDataActivityBroadcast(transportTypeToLegacyType(activityParams.label), - activityParams.isActive, activityParams.timestampNs); - mIsDefaultNetworkActive = activityParams.isActive; - if (mIsDefaultNetworkActive) { - reportNetworkActive(); + handleDefaultNetworkActivity(activityParams.label, activityParams.isActive, + activityParams.timestampNs); + } + + /** + * Handle network activity change + */ + public void handleReportNetworkActivity(NetworkActivityParams activityParams) { + ensureRunningOnConnectivityServiceThread(); + if (mTrackMultiNetworkActivities) { + handleReportNetworkActivityWithNetIdLabel(activityParams); + } else { + handleReportNetworkActivityWithTransportTypeLabel(activityParams); } } @@ -11866,6 +11926,15 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + /** + * Get idle timer label + */ + @VisibleForTesting + public static int getIdleTimerLabel(final boolean trackMultiNetworkActivities, + final int netId, final int transportType) { + return trackMultiNetworkActivities ? netId : transportType; + } + /** * Setup data activity tracking for the given network. * @@ -11902,7 +11971,9 @@ public class ConnectivityService extends IConnectivityManager.Stub if (timeout > 0 && iface != null) { try { mActiveIdleTimers.put(netId, new IdleTimerParams(timeout, type)); - mNetd.idletimerAddInterface(iface, timeout, Integer.toString(type)); + final String label = Integer.toString(getIdleTimerLabel( + mTrackMultiNetworkActivities, netId, type)); + mNetd.idletimerAddInterface(iface, timeout, label); return true; } catch (Exception e) { // You shall not crash! @@ -11939,9 +12010,10 @@ public class ConnectivityService extends IConnectivityManager.Stub return; } mActiveIdleTimers.remove(netId); - // The call fails silently if no idle timer setup for this interface - mNetd.idletimerRemoveInterface(iface, params.timeout, - Integer.toString(params.transportType)); + final String label = Integer.toString(getIdleTimerLabel( + mTrackMultiNetworkActivities, netId, params.transportType)); + // The call fails silently if no idle timer setup for this interface + mNetd.idletimerRemoveInterface(iface, params.timeout, label); } catch (Exception e) { // You shall not crash! loge("Exception in removeDataActivityTracking " + e); @@ -11951,12 +12023,14 @@ public class ConnectivityService extends IConnectivityManager.Stub private void updateDefaultNetworkActivity(NetworkAgentInfo defaultNetwork, boolean hasIdleTimer) { if (defaultNetwork != null) { + mDefaultNetwork = defaultNetwork.network(); mIsDefaultNetworkActive = true; // Callbacks are called only when the network has the idle timer. if (hasIdleTimer) { reportNetworkActive(); } } else { + mDefaultNetwork = null; // If there is no default network, default network is considered active to keep the // existing behavior. mIsDefaultNetworkActive = true; @@ -12006,7 +12080,9 @@ public class ConnectivityService extends IConnectivityManager.Stub } public void dump(IndentingPrintWriter pw) { + pw.print("mTrackMultiNetworkActivities="); pw.println(mTrackMultiNetworkActivities); pw.print("mIsDefaultNetworkActive="); pw.println(mIsDefaultNetworkActive); + pw.print("mDefaultNetwork="); pw.println(mDefaultNetwork); pw.println("Idle timers:"); try { for (int i = 0; i < mActiveIdleTimers.size(); i++) { diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index bafd450ff3..01fc3b55db 100755 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -10795,6 +10795,11 @@ public class ConnectivityServiceTest { expectNativeNetworkCreated(netId, permission, iface, null /* inOrder */); } + private int getIdleTimerLabel(int netId, int transportType) { + return ConnectivityService.LegacyNetworkActivityTracker.getIdleTimerLabel( + mDeps.isAtLeastV(), netId, transportType); + } + @Test public void testStackedLinkProperties() throws Exception { final LinkAddress myIpv4 = new LinkAddress("1.2.3.4/24"); @@ -11036,7 +11041,7 @@ public class ConnectivityServiceTest { networkCallback.expect(LOST, mCellAgent); networkCallback.assertNoCallback(); verify(mMockNetd, times(1)).idletimerRemoveInterface(eq(MOBILE_IFNAME), anyInt(), - eq(Integer.toString(TRANSPORT_CELLULAR))); + eq(Integer.toString(getIdleTimerLabel(cellNetId, TRANSPORT_CELLULAR)))); verify(mMockNetd).networkDestroy(cellNetId); if (mDeps.isAtLeastU()) { verify(mMockNetd).setNetworkAllowlist(any()); @@ -11095,7 +11100,7 @@ public class ConnectivityServiceTest { } verify(mMockNetd).idletimerRemoveInterface(eq(MOBILE_IFNAME), anyInt(), - eq(Integer.toString(TRANSPORT_CELLULAR))); + eq(Integer.toString(getIdleTimerLabel(cellNetId, TRANSPORT_CELLULAR)))); verify(mMockNetd).networkDestroy(cellNetId); if (mDeps.isAtLeastU()) { verify(mMockNetd).setNetworkAllowlist(any()); @@ -11352,6 +11357,7 @@ public class ConnectivityServiceTest { testAndCleanup(() -> { agent.connect(true); + final int idleTimerLabel = getIdleTimerLabel(agent.getNetwork().netId, transportType); // Network is considered active when the network becomes the default network. assertTrue(mCm.isDefaultNetworkActive()); @@ -11360,7 +11366,7 @@ public class ConnectivityServiceTest { // Interface goes to inactive state netdUnsolicitedEventListener.onInterfaceClassActivityChanged(false /* isActive */, - transportType, TIMESTAMP, NETWORK_ACTIVITY_NO_UID); + idleTimerLabel, TIMESTAMP, NETWORK_ACTIVITY_NO_UID); mServiceContext.expectDataActivityBroadcast(legacyType, false /* isActive */, TIMESTAMP); assertFalse(onNetworkActiveCv.block(TEST_CALLBACK_TIMEOUT_MS)); @@ -11368,7 +11374,7 @@ public class ConnectivityServiceTest { // Interface goes to active state netdUnsolicitedEventListener.onInterfaceClassActivityChanged(true /* isActive */, - transportType, TIMESTAMP, TEST_PACKAGE_UID); + idleTimerLabel, TIMESTAMP, TEST_PACKAGE_UID); mServiceContext.expectDataActivityBroadcast(legacyType, true /* isActive */, TIMESTAMP); assertTrue(onNetworkActiveCv.block(TEST_CALLBACK_TIMEOUT_MS)); assertTrue(mCm.isDefaultNetworkActive()); @@ -11455,15 +11461,19 @@ public class ConnectivityServiceTest { mCm.registerNetworkCallback(networkRequest, networkCallback); mCellAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR); + final String cellIdleTimerLabel = Integer.toString(getIdleTimerLabel( + mCellAgent.getNetwork().netId, TRANSPORT_CELLULAR)); final LinkProperties cellLp = new LinkProperties(); cellLp.setInterfaceName(MOBILE_IFNAME); mCellAgent.sendLinkProperties(cellLp); mCellAgent.connect(true); networkCallback.expectAvailableThenValidatedCallbacks(mCellAgent); verify(mMockNetd, times(1)).idletimerAddInterface(eq(MOBILE_IFNAME), anyInt(), - eq(Integer.toString(TRANSPORT_CELLULAR))); + eq(cellIdleTimerLabel)); mWiFiAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); + String wifiIdleTimerLabel = Integer.toString(getIdleTimerLabel( + mWiFiAgent.getNetwork().netId, TRANSPORT_WIFI)); final LinkProperties wifiLp = new LinkProperties(); wifiLp.setInterfaceName(WIFI_IFNAME); mWiFiAgent.sendLinkProperties(wifiLp); @@ -11474,9 +11484,9 @@ public class ConnectivityServiceTest { networkCallback.expectLosing(mCellAgent); networkCallback.expectCaps(mWiFiAgent, c -> c.hasCapability(NET_CAPABILITY_VALIDATED)); verify(mMockNetd, times(1)).idletimerAddInterface(eq(WIFI_IFNAME), anyInt(), - eq(Integer.toString(TRANSPORT_WIFI))); + eq(wifiIdleTimerLabel)); verify(mMockNetd, times(1)).idletimerRemoveInterface(eq(MOBILE_IFNAME), anyInt(), - eq(Integer.toString(TRANSPORT_CELLULAR))); + eq(cellIdleTimerLabel)); // Disconnect wifi and switch back to cell reset(mMockNetd); @@ -11484,13 +11494,15 @@ public class ConnectivityServiceTest { networkCallback.expect(LOST, mWiFiAgent); assertNoCallbacks(networkCallback); verify(mMockNetd, times(1)).idletimerRemoveInterface(eq(WIFI_IFNAME), anyInt(), - eq(Integer.toString(TRANSPORT_WIFI))); + eq(wifiIdleTimerLabel)); verify(mMockNetd, times(1)).idletimerAddInterface(eq(MOBILE_IFNAME), anyInt(), - eq(Integer.toString(TRANSPORT_CELLULAR))); + eq(cellIdleTimerLabel)); // reconnect wifi reset(mMockNetd); mWiFiAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI); + wifiIdleTimerLabel = Integer.toString(getIdleTimerLabel( + mWiFiAgent.getNetwork().netId, TRANSPORT_WIFI)); wifiLp.setInterfaceName(WIFI_IFNAME); mWiFiAgent.sendLinkProperties(wifiLp); mWiFiAgent.connect(true); @@ -11498,9 +11510,9 @@ public class ConnectivityServiceTest { networkCallback.expectLosing(mCellAgent); networkCallback.expectCaps(mWiFiAgent, c -> c.hasCapability(NET_CAPABILITY_VALIDATED)); verify(mMockNetd, times(1)).idletimerAddInterface(eq(WIFI_IFNAME), anyInt(), - eq(Integer.toString(TRANSPORT_WIFI))); + eq(wifiIdleTimerLabel)); verify(mMockNetd, times(1)).idletimerRemoveInterface(eq(MOBILE_IFNAME), anyInt(), - eq(Integer.toString(TRANSPORT_CELLULAR))); + eq(cellIdleTimerLabel)); // Disconnect cell reset(mMockNetd); @@ -11511,7 +11523,7 @@ public class ConnectivityServiceTest { // unexpectedly before network being removed. waitForIdle(); verify(mMockNetd, times(0)).idletimerRemoveInterface(eq(MOBILE_IFNAME), anyInt(), - eq(Integer.toString(TRANSPORT_CELLULAR))); + eq(cellIdleTimerLabel)); verify(mMockNetd, times(1)).networkDestroy(eq(mCellAgent.getNetwork().netId)); verify(mMockDnsResolver, times(1)).destroyNetworkCache(eq(mCellAgent.getNetwork().netId)); @@ -11520,7 +11532,7 @@ public class ConnectivityServiceTest { mWiFiAgent.disconnect(); b.expectBroadcast(); verify(mMockNetd, times(1)).idletimerRemoveInterface(eq(WIFI_IFNAME), anyInt(), - eq(Integer.toString(TRANSPORT_WIFI))); + eq(wifiIdleTimerLabel)); // Clean up mCm.unregisterNetworkCallback(networkCallback); @@ -18704,6 +18716,7 @@ public class ConnectivityServiceTest { final LinkProperties lp = new LinkProperties(); lp.setInterfaceName(transportToTestIfaceName(transportType)); final TestNetworkAgentWrapper agent = new TestNetworkAgentWrapper(transportType, lp); + final int idleTimerLabel = getIdleTimerLabel(agent.getNetwork().netId, transportType); testAndCleanup(() -> { final UidFrozenStateChangedCallback uidFrozenStateChangedCallback = getUidFrozenStateChangedCallback().get(); @@ -18716,7 +18729,7 @@ public class ConnectivityServiceTest { if (freezeWithNetworkInactive) { // Make network inactive netdUnsolicitedEventListener.onInterfaceClassActivityChanged(false /* isActive */, - transportType, TIMESTAMP, NETWORK_ACTIVITY_NO_UID); + idleTimerLabel, TIMESTAMP, NETWORK_ACTIVITY_NO_UID); } // Freeze TEST_FROZEN_UID and TEST_UNFROZEN_UID @@ -18740,7 +18753,7 @@ public class ConnectivityServiceTest { // Make network active netdUnsolicitedEventListener.onInterfaceClassActivityChanged(true /* isActive */, - transportType, TIMESTAMP, TEST_PACKAGE_UID); + idleTimerLabel, TIMESTAMP, TEST_PACKAGE_UID); waitForIdle(); if (expectDelay) { @@ -18759,8 +18772,8 @@ public class ConnectivityServiceTest { @Test @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) public void testDelayFrozenUidSocketDestroy_ActiveCellular() throws Exception { - doTestDelayFrozenUidSocketDestroy(TRANSPORT_CELLULAR, - false /* freezeWithNetworkInactive */, false /* expectDelay */); + doTestDelayFrozenUidSocketDestroy(TRANSPORT_CELLULAR, false /* freezeWithNetworkInactive */, + false /* expectDelay */); } @Test @@ -18768,22 +18781,22 @@ public class ConnectivityServiceTest { public void testDelayFrozenUidSocketDestroy_InactiveCellular() throws Exception { // When the default network is cellular and cellular network is inactive, closing socket // is delayed. - doTestDelayFrozenUidSocketDestroy(TRANSPORT_CELLULAR, - true /* freezeWithNetworkInactive */, true /* expectDelay */); + doTestDelayFrozenUidSocketDestroy(TRANSPORT_CELLULAR, true /* freezeWithNetworkInactive */, + true /* expectDelay */); } @Test @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) public void testDelayFrozenUidSocketDestroy_ActiveWifi() throws Exception { - doTestDelayFrozenUidSocketDestroy(TRANSPORT_WIFI, - false /* freezeWithNetworkInactive */, false /* expectDelay */); + doTestDelayFrozenUidSocketDestroy(TRANSPORT_WIFI, false /* freezeWithNetworkInactive */, + false /* expectDelay */); } @Test @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) public void testDelayFrozenUidSocketDestroy_InactiveWifi() throws Exception { - doTestDelayFrozenUidSocketDestroy(TRANSPORT_WIFI, - true /* freezeWithNetworkInactive */, false /* expectDelay */); + doTestDelayFrozenUidSocketDestroy(TRANSPORT_WIFI, true /* freezeWithNetworkInactive */, + false /* expectDelay */); } /** @@ -18804,6 +18817,8 @@ public class ConnectivityServiceTest { final LinkProperties cellLp = new LinkProperties(); cellLp.setInterfaceName(MOBILE_IFNAME); mCellAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, cellLp); + final int idleTimerLabel = + getIdleTimerLabel(mCellAgent.getNetwork().netId, TRANSPORT_CELLULAR); final TestNetworkCallback defaultCallback = new TestNetworkCallback(); mCm.registerDefaultNetworkCallback(defaultCallback); @@ -18813,7 +18828,7 @@ public class ConnectivityServiceTest { // Make cell network inactive netdUnsolicitedEventListener.onInterfaceClassActivityChanged(false /* isActive */, - TRANSPORT_CELLULAR, TIMESTAMP, NETWORK_ACTIVITY_NO_UID); + idleTimerLabel, TIMESTAMP, NETWORK_ACTIVITY_NO_UID); // Freeze TEST_FROZEN_UID final int[] uids = {TEST_FROZEN_UID}; diff --git a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt index 0ccbfc3e80..db70599abe 100644 --- a/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt +++ b/tests/unit/java/com/android/server/connectivityservice/base/CSTest.kt @@ -64,12 +64,12 @@ import com.android.server.connectivity.MultinetworkPolicyTrackerTestDependencies import com.android.server.connectivity.ProxyTracker import com.android.testutils.visibleOnHandlerThread import com.android.testutils.waitForIdle +import java.util.concurrent.Executors +import kotlin.test.fail import org.mockito.AdditionalAnswers.delegatesTo import org.mockito.Mockito.doAnswer import org.mockito.Mockito.doReturn import org.mockito.Mockito.mock -import java.util.concurrent.Executors -import kotlin.test.fail internal const val HANDLER_TIMEOUT_MS = 2_000 internal const val TEST_PACKAGE_NAME = "com.android.test.package"