Use netId as idleTimer label on V+

Following CL adds idleTimer for multiple networks and track activities
including non default network on V+
Current LegacyNetworkActivityTracker uses transport type as an idleTimer
label but this makes it difficult to have multiple idleTimers.
So, this CL updates LegacyNetworkActivityTracker to support using netId
as an idleTimer label.

Bug: 267870186
Bug: 279380356
Test: atest FrameworksNetTests
Change-Id: I0039f5624ae1d142dbacba53aa90ca9bf6d43599

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.
+        // If TrackMultiNetworkActivities is enabled, idleTimer label is netid.
-        // label is int since NMS was using the identifier as int, and it has not been changed
+        // 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<IdleTimerParams> 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) {
+        private void handleDefaultNetworkActivity(final int transportType,
-            ensureRunningOnConnectivityServiceThread();
+                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),
+            handleDefaultNetworkActivity(activityParams.label, activityParams.isActive,
-                    activityParams.isActive, activityParams.timestampNs);
+                    activityParams.timestampNs);
-            mIsDefaultNetworkActive = activityParams.isActive;
+        }
-            if (mIsDefaultNetworkActive) {
+
-                reportNetworkActive();
+        /**
+         * 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
+                final String label = Integer.toString(getIdleTimerLabel(
-                mNetd.idletimerRemoveInterface(iface, params.timeout,
+                        mTrackMultiNetworkActivities, netId, params.transportType));
-                        Integer.toString(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++) {

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,
+        doTestDelayFrozenUidSocketDestroy(TRANSPORT_CELLULAR, false /* freezeWithNetworkInactive */,
-                false /* freezeWithNetworkInactive */, false /* expectDelay */);
+                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,
+        doTestDelayFrozenUidSocketDestroy(TRANSPORT_CELLULAR, true /* freezeWithNetworkInactive */,
-                true /* freezeWithNetworkInactive */, true /* expectDelay */);
+                true /* expectDelay */);
     }
 
     @Test
     @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU)
     public void testDelayFrozenUidSocketDestroy_ActiveWifi() throws Exception {
-        doTestDelayFrozenUidSocketDestroy(TRANSPORT_WIFI,
+        doTestDelayFrozenUidSocketDestroy(TRANSPORT_WIFI, false /* freezeWithNetworkInactive */,
-                false /* freezeWithNetworkInactive */, false /* expectDelay */);
+                false /* expectDelay */);
     }
 
     @Test
     @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU)
     public void testDelayFrozenUidSocketDestroy_InactiveWifi() throws Exception {
-        doTestDelayFrozenUidSocketDestroy(TRANSPORT_WIFI,
+        doTestDelayFrozenUidSocketDestroy(TRANSPORT_WIFI, true /* freezeWithNetworkInactive */,
-                true /* freezeWithNetworkInactive */, false /* expectDelay */);
+                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};

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"