Merge changes I1aca68f2,I896eb92c into main am: 18f80efe93 Original change: https://android-review.googlesource.com/c/platform/packages/modules/NetworkStack/+/2837655 Change-Id: I306c5fd51584fcdeb60c6cac51dfb365c6e469c7 Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/src/com/android/networkstack/netlink/TcpSocketTracker.java b/src/com/android/networkstack/netlink/TcpSocketTracker.java index cd400f4..658fe8a 100644 --- a/src/com/android/networkstack/netlink/TcpSocketTracker.java +++ b/src/com/android/networkstack/netlink/TcpSocketTracker.java
@@ -148,6 +148,7 @@ @NonNull private NetworkCapabilities mNetworkCapabilities; + private final boolean mShouldDisableInDeepDoze; private final boolean mShouldDisableInLightDoze; private final boolean mShouldIgnoreTcpInfoForBlockedUids; private final ConnectivityManager mCm; @@ -168,8 +169,9 @@ } }; - private static boolean isDeviceIdleModeChangedAction(Intent intent) { - return ACTION_DEVICE_IDLE_MODE_CHANGED.equals(intent.getAction()); + private boolean isDeviceIdleModeChangedAction(Intent intent) { + return mShouldDisableInDeepDoze + && ACTION_DEVICE_IDLE_MODE_CHANGED.equals(intent.getAction()); } @TargetApi(Build.VERSION_CODES.TIRAMISU) @@ -190,7 +192,8 @@ // For tcp polling mechanism, there is no difference between deep doze mode and // light doze mode. The deep doze mode and light doze mode block networking // for uids in the same way, use single variable to control. - final boolean deviceIdle = powerManager.isDeviceIdleMode() + final boolean deviceIdle = (mShouldDisableInDeepDoze + && powerManager.isDeviceIdleMode()) || (mShouldDisableInLightDoze && powerManager.isDeviceLightIdleMode()); setDozeMode(deviceIdle); } @@ -201,9 +204,16 @@ mDependencies = dps; mNetwork = network; mNetd = mDependencies.getNetd(); - mShouldDisableInLightDoze = mDependencies.shouldDisableInLightDoze(); mShouldIgnoreTcpInfoForBlockedUids = mDependencies.shouldIgnoreTcpInfoForBlockedUids(); + // Previous workarounds can be disabled if the device supports ignore blocked uids feature. + // To prevent inconsistencies and issues like broadcast receiver leaks, the feature flags + // are fixed after being read. + // TODO: Remove these workarounds when pre-T devices are no longer supported. + mShouldDisableInLightDoze = mDependencies.shouldDisableInLightDoze( + mShouldIgnoreTcpInfoForBlockedUids); + mShouldDisableInDeepDoze = !mShouldIgnoreTcpInfoForBlockedUids; + // If the parcel is null, nothing should be matched which is achieved by the combination of // {@code NetlinkUtils#NULL_MASK} and {@code NetlinkUtils#UNKNOWN_MARK}. final MarkMaskParcel parcel = getNetworkMarkMask(); @@ -216,7 +226,8 @@ family, InetDiagMessage.buildInetDiagReqForAliveTcpSockets(family)); } mDependencies.addDeviceConfigChangedListener(mConfigListener); - mDependencies.addDeviceIdleReceiver(mDeviceIdleReceiver, mShouldDisableInLightDoze); + mDependencies.addDeviceIdleReceiver(mDeviceIdleReceiver, mShouldDisableInDeepDoze, + mShouldDisableInLightDoze); mCm = mDependencies.getContext().getSystemService(ConnectivityManager.class); } @@ -551,7 +562,8 @@ /** Stops monitoring and releases resources. */ public void quit() { mDependencies.removeDeviceConfigChangedListener(mConfigListener); - mDependencies.removeBroadcastReceiver(mDeviceIdleReceiver); + mDependencies.removeBroadcastReceiver(mDeviceIdleReceiver, + mShouldDisableInDeepDoze, mShouldDisableInLightDoze); } /** @@ -736,8 +748,14 @@ /** Add receiver for detecting doze mode change to control TCP detection. */ @TargetApi(Build.VERSION_CODES.TIRAMISU) public void addDeviceIdleReceiver(@NonNull final BroadcastReceiver receiver, - boolean shouldDisableInLightDoze) { - final IntentFilter intentFilter = new IntentFilter(ACTION_DEVICE_IDLE_MODE_CHANGED); + boolean shouldDisableInDeepDoze, boolean shouldDisableInLightDoze) { + // No need to register receiver if no related feature is enabled. + if (!shouldDisableInDeepDoze && !shouldDisableInLightDoze) return; + + final IntentFilter intentFilter = new IntentFilter(); + if (shouldDisableInDeepDoze) { + intentFilter.addAction(ACTION_DEVICE_IDLE_MODE_CHANGED); + } if (shouldDisableInLightDoze) { intentFilter.addAction(ACTION_DEVICE_LIGHT_IDLE_MODE_CHANGED); } @@ -745,7 +763,9 @@ } /** Remove broadcast receiver. */ - public void removeBroadcastReceiver(@NonNull final BroadcastReceiver receiver) { + public void removeBroadcastReceiver(@NonNull final BroadcastReceiver receiver, + boolean shouldDisableInDeepDoze, boolean shouldDisableInLightDoze) { + if (!shouldDisableInDeepDoze && !shouldDisableInLightDoze) return; mContext.unregisterReceiver(receiver); } @@ -755,9 +775,14 @@ * to deal with flag values changing at runtime. */ @TargetApi(Build.VERSION_CODES.TIRAMISU) - public boolean shouldDisableInLightDoze() { + public boolean shouldDisableInLightDoze(boolean ignoreBlockedUidsSupported) { // Light doze mode status checking API is only available at T or later releases. - return SdkLevel.isAtLeastT() && DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut( + if (!SdkLevel.isAtLeastT()) return false; + + // Disable light doze mode design is replaced by ignoring blocked uids design. + if (ignoreBlockedUidsSupported) return false; + + return DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut( mContext, SKIP_TCP_POLL_IN_LIGHT_DOZE); }
diff --git a/src/com/android/server/NetworkStackService.java b/src/com/android/server/NetworkStackService.java index 347200b..40aee28 100644 --- a/src/com/android/server/NetworkStackService.java +++ b/src/com/android/server/NetworkStackService.java
@@ -22,6 +22,8 @@ import static com.android.net.module.util.DeviceConfigUtils.getResBooleanConfig; import static com.android.net.module.util.FeatureVersions.FEATURE_IS_UID_NETWORKING_BLOCKED; +import static com.android.networkstack.util.NetworkStackUtils.IGNORE_TCP_INFO_FOR_BLOCKED_UIDS; +import static com.android.networkstack.util.NetworkStackUtils.SKIP_TCP_POLL_IN_LIGHT_DOZE; import static com.android.server.util.PermissionUtil.checkDumpPermission; import android.app.Service; @@ -440,6 +442,20 @@ return; } + pw.println("Device Configs:"); + pw.increaseIndent(); + pw.println("SKIP_TCP_POLL_IN_LIGHT_DOZE=" + + DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut( + mContext, SKIP_TCP_POLL_IN_LIGHT_DOZE)); + pw.println("FEATURE_IS_UID_NETWORKING_BLOCKED=" + DeviceConfigUtils.isFeatureSupported( + mContext, FEATURE_IS_UID_NETWORKING_BLOCKED)); + pw.println("IGNORE_TCP_INFO_FOR_BLOCKED_UIDS=" + + DeviceConfigUtils.isNetworkStackFeatureNotChickenedOut(mContext, + IGNORE_TCP_INFO_FOR_BLOCKED_UIDS)); + pw.decreaseIndent(); + pw.println(); + + pw.println("NetworkStack logs:"); mLog.dump(fd, pw, args);
diff --git a/tests/unit/src/com/android/networkstack/netlink/TcpSocketTrackerTest.java b/tests/unit/src/com/android/networkstack/netlink/TcpSocketTrackerTest.java index 4e5302e..3857b04 100644 --- a/tests/unit/src/com/android/networkstack/netlink/TcpSocketTrackerTest.java +++ b/tests/unit/src/com/android/networkstack/netlink/TcpSocketTrackerTest.java
@@ -21,6 +21,7 @@ import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.util.DataStallUtils.CONFIG_TCP_PACKETS_FAIL_PERCENTAGE; import static android.net.util.DataStallUtils.DEFAULT_TCP_PACKETS_FAIL_PERCENTAGE; +import static android.os.PowerManager.ACTION_DEVICE_IDLE_MODE_CHANGED; import static android.os.PowerManager.ACTION_DEVICE_LIGHT_IDLE_MODE_CHANGED; import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY; import static android.system.OsConstants.AF_INET; @@ -41,6 +42,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.annotation.IntDef; import android.content.BroadcastReceiver; import android.content.Context; import android.content.Intent; @@ -78,6 +80,8 @@ import org.mockito.MockitoAnnotations; import java.io.FileDescriptor; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.net.InetAddress; import java.nio.ByteBuffer; import java.nio.ByteOrder; @@ -166,7 +170,7 @@ eq(NAMESPACE_CONNECTIVITY), eq(CONFIG_TCP_PACKETS_FAIL_PERCENTAGE), anyInt())).thenReturn(DEFAULT_TCP_PACKETS_FAIL_PERCENTAGE); - when(mDependencies.shouldDisableInLightDoze()).thenReturn(true); + when(mDependencies.shouldDisableInLightDoze(anyBoolean())).thenReturn(true); when(mNetd.getFwmarkForNetwork(eq(TEST_NETID1))) .thenReturn(makeMarkMaskParcel(NETID_MASK, TEST_NETID1_FWMARK)); @@ -671,50 +675,76 @@ + "0000000000000000"; // deliverRate = 0 } + private static final int DEEP_DOZE = 0; + private static final int LIGHT_DOZE = 1; + + @Retention(RetentionPolicy.SOURCE) + @IntDef(value = { + DEEP_DOZE, + LIGHT_DOZE + }) + private @interface DozeModeType {} + @Test - public void testTcpInfoParsingWithDozeMode() throws Exception { - final TcpSocketTracker tst = new TcpSocketTracker(mDependencies, mNetwork); - final ArgumentCaptor<BroadcastReceiver> receiverCaptor = - ArgumentCaptor.forClass(BroadcastReceiver.class); + public void testTcpInfoParsingWithDozeMode_enabled() throws Exception { + doReturn(false).when(mDependencies).shouldIgnoreTcpInfoForBlockedUids(); + doReturn(false).when(mDependencies).shouldDisableInLightDoze(anyBoolean()); + doTestTcpInfoDisableParsingWithDozeMode(DEEP_DOZE, true /* featureEnabled */); + } - verify(mDependencies).addDeviceIdleReceiver(receiverCaptor.capture(), anyBoolean()); - setupNormalTestTcpInfo(); - assertTrue(tst.pollSocketsInfo()); - - // Lower the threshold. - when(mDependencies.getDeviceConfigPropertyInt(any(), eq(CONFIG_TCP_PACKETS_FAIL_PERCENTAGE), - anyInt())).thenReturn(40); - - // Trigger a config update. - tst.mConfigListener.onPropertiesChanged(null /* properties */); - assertEquals(10, tst.getSentSinceLastRecv()); - assertEquals(50, tst.getLatestPacketFailPercentage()); - assertTrue(tst.isDataStallSuspected()); - - // Enable doze mode, verify counters are not updated. - doReturn(true).when(mPowerManager).isDeviceIdleMode(); - final BroadcastReceiver receiver = receiverCaptor.getValue(); - receiver.onReceive(mContext, new Intent(PowerManager.ACTION_DEVICE_IDLE_MODE_CHANGED)); - assertFalse(tst.pollSocketsInfo()); - assertEquals(10, tst.getSentSinceLastRecv()); - assertEquals(50, tst.getLatestPacketFailPercentage()); - assertFalse(tst.isDataStallSuspected()); + // Ignore blocked uids is supported on T. Thus, for pre-T device this feature is always + // needed since there is no replacement. + @IgnoreUpTo(Build.VERSION_CODES.S_V2) + @Test + public void testTcpInfoParsingWithDozeMode_disabled() throws Exception { + doReturn(true).when(mDependencies).shouldIgnoreTcpInfoForBlockedUids(); + doReturn(false).when(mDependencies).shouldDisableInLightDoze(anyBoolean()); + doTestTcpInfoDisableParsingWithDozeMode(DEEP_DOZE, false /* featureEnabled */); } @Test @IgnoreUpTo(Build.VERSION_CODES.S_V2) public void testTcpInfoDisableParsingWithLightDozeMode_enabled() throws Exception { + doReturn(true).when(mDependencies).shouldDisableInLightDoze(anyBoolean()); + doTestTcpInfoDisableParsingWithDozeMode(LIGHT_DOZE, true /* featureEnabled */); + } + + @Test @IgnoreUpTo(Build.VERSION_CODES.S_V2) + public void testTcpInfoDisableParsingWithLightDozeMode_disabled() throws Exception { + doReturn(false).when(mDependencies).shouldDisableInLightDoze(anyBoolean()); + doTestTcpInfoDisableParsingWithDozeMode(LIGHT_DOZE, false /* featureEnabled */); + } + + private void doTestTcpInfoDisableParsingWithDozeMode(@DozeModeType int dozeModeType, + boolean featureEnabled) throws Exception { final TcpSocketTracker tst = new TcpSocketTracker(mDependencies, mNetwork); + tst.setNetworkCapabilities(CELL_NOT_METERED_CAPABILITIES); final ArgumentCaptor<BroadcastReceiver> receiverCaptor = ArgumentCaptor.forClass(BroadcastReceiver.class); - // Enable light doze mode with 1 netlink message. - verify(mDependencies).addDeviceIdleReceiver(receiverCaptor.capture(), anyBoolean()); + // Enable doze mode with 1 netlink message. + verify(mDependencies).addDeviceIdleReceiver(receiverCaptor.capture(), + anyBoolean(), anyBoolean()); final BroadcastReceiver receiver = receiverCaptor.getValue(); - doReturn(true).when(mPowerManager).isDeviceLightIdleMode(); - receiver.onReceive(mContext, new Intent(ACTION_DEVICE_LIGHT_IDLE_MODE_CHANGED)); + if (dozeModeType == DEEP_DOZE) { + doReturn(true).when(mPowerManager).isDeviceIdleMode(); + receiver.onReceive(mContext, new Intent(ACTION_DEVICE_IDLE_MODE_CHANGED)); + } else { + doReturn(true).when(mPowerManager).isDeviceLightIdleMode(); + receiver.onReceive(mContext, new Intent(ACTION_DEVICE_LIGHT_IDLE_MODE_CHANGED)); + } doReturn(getByteBufferFromHexString(composeSockDiagTcpHex(9, 10) + NLMSG_DONE_HEX)).when(mDependencies).recvMessage(any()); + if (!featureEnabled) { + // Verify TcpInfo is still processed. + assertTrue(tst.pollSocketsInfo()); + assertEquals(10, tst.getSentSinceLastRecv()); + // Lost 4 + default 5 retrans / 10 sent. + assertEquals(90, tst.getLatestPacketFailPercentage()); + assertTrue(tst.isDataStallSuspected()); + return; + } + // Verify counters are not updated. assertFalse(tst.pollSocketsInfo()); assertEquals(0, tst.getSentSinceLastRecv()); @@ -722,9 +752,14 @@ assertEquals(-1, tst.getLatestPacketFailPercentage()); assertFalse(tst.isDataStallSuspected()); - // Disable light doze mode, verify polling are processed and counters are updated. - doReturn(false).when(mPowerManager).isDeviceLightIdleMode(); - receiver.onReceive(mContext, new Intent(ACTION_DEVICE_LIGHT_IDLE_MODE_CHANGED)); + // Disable deep/light doze mode, verify polling are processed and counters are updated. + if (dozeModeType == DEEP_DOZE) { + doReturn(false).when(mPowerManager).isDeviceIdleMode(); + receiver.onReceive(mContext, new Intent(ACTION_DEVICE_IDLE_MODE_CHANGED)); + } else { + doReturn(false).when(mPowerManager).isDeviceLightIdleMode(); + receiver.onReceive(mContext, new Intent(ACTION_DEVICE_LIGHT_IDLE_MODE_CHANGED)); + } assertTrue(tst.pollSocketsInfo()); assertEquals(10, tst.getSentSinceLastRecv()); // Lost 4 + default 5 retrans / 10 sent. @@ -732,28 +767,6 @@ assertTrue(tst.isDataStallSuspected()); } - @Test @IgnoreUpTo(Build.VERSION_CODES.S_V2) - public void testTcpInfoDisableParsingWithLightDozeMode_disabled() throws Exception { - when(mDependencies.shouldDisableInLightDoze()).thenReturn(false); - final TcpSocketTracker tst = new TcpSocketTracker(mDependencies, mNetwork); - final ArgumentCaptor<BroadcastReceiver> receiverCaptor = - ArgumentCaptor.forClass(BroadcastReceiver.class); - - // Enable light doze mode with 1 netlink message. - verify(mDependencies).addDeviceIdleReceiver(receiverCaptor.capture(), anyBoolean()); - final BroadcastReceiver receiver = receiverCaptor.getValue(); - doReturn(true).when(mPowerManager).isDeviceLightIdleMode(); - receiver.onReceive(mContext, new Intent(ACTION_DEVICE_LIGHT_IDLE_MODE_CHANGED)); - doReturn(getByteBufferFromHexString(composeSockDiagTcpHex(9, 10) - + NLMSG_DONE_HEX)).when(mDependencies).recvMessage(any()); - - // Verify TcpInfo is still processed. - assertTrue(tst.pollSocketsInfo()); - assertEquals(10, tst.getSentSinceLastRecv()); - assertEquals(90, tst.getLatestPacketFailPercentage()); - assertTrue(tst.isDataStallSuspected()); - } - private void setupNormalTestTcpInfo() throws Exception { final ByteBuffer tcpBufferV6 = getByteBuffer(TEST_RESPONSE_BYTES); final ByteBuffer tcpBufferV4 = getByteBuffer(TEST_RESPONSE_BYTES);