From f79dcecf9bb9a16eb1850b112ac53accfc49cc1b Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 7 Mar 2022 17:48:54 +0900 Subject: [PATCH] Don't attempt to enable rate-limiting before T. Rate-limiting cannot work because the BPF program is in the mainline version of netd.c, which is placed into net_shared and thus cannot run pre-T. Disable it entirely to ensure no impact on S. Test: atest ConnectivityCoverageTests:com.android.server.ConnectivityServiceTest on AOSP Test: atest ConnectivityCoverageTests:com.android.server.ConnectivityServiceTest on S Change-Id: I47521a100f8287ecdece25e810db8f3cade46778 --- .../android/server/ConnectivityService.java | 3 ++ .../server/ConnectivityServiceTest.java | 34 ++++++++++++++++--- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index ab78104a51..6c45ab4b46 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -10699,6 +10699,9 @@ public class ConnectivityService extends IConnectivityManager.Stub } private boolean canNetworkBeRateLimited(@NonNull final NetworkAgentInfo networkAgent) { + // Rate-limiting cannot run correctly before T because the BPF program is not loaded. + if (!SdkLevel.isAtLeastT()) return false; + final NetworkCapabilities agentCaps = networkAgent.networkCapabilities; // Only test networks (they cannot hold NET_CAPABILITY_INTERNET) and networks that provide // internet connectivity can be rate limited. diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 777da1714c..c8dc1074be 100644 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -137,6 +137,9 @@ import static com.android.server.ConnectivityService.PREFERENCE_ORDER_VPN; import static com.android.server.ConnectivityServiceTestUtils.transportToLegacyType; import static com.android.testutils.ConcurrentUtils.await; import static com.android.testutils.ConcurrentUtils.durationOf; +import static com.android.testutils.DevSdkIgnoreRule.IgnoreAfter; +import static com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo; +import static com.android.testutils.DevSdkIgnoreRuleKt.SC_V2; import static com.android.testutils.ExceptionUtils.ignoreExceptions; import static com.android.testutils.HandlerUtils.waitForIdleSerialExecutor; import static com.android.testutils.MiscAsserts.assertContainsAll; @@ -358,6 +361,7 @@ import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Ignore; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.AdditionalAnswers; @@ -420,6 +424,9 @@ import kotlin.reflect.KClass; public class ConnectivityServiceTest { private static final String TAG = "ConnectivityServiceTest"; + @Rule + public final DevSdkIgnoreRule ignoreRule = new DevSdkIgnoreRule(); + private static final int TIMEOUT_MS = 2_000; // Broadcasts can take a long time to be delivered. The test will not wait for that long unless // there is a failure, so use a long timeout. @@ -15397,7 +15404,7 @@ public class ConnectivityServiceTest { null /* callingAttributionTag */)); } - @Test + @Test @IgnoreUpTo(SC_V2) public void testUpdateRateLimit_EnableDisable() throws Exception { final LinkProperties wifiLp = new LinkProperties(); wifiLp.setInterfaceName(WIFI_IFNAME); @@ -15436,7 +15443,7 @@ public class ConnectivityServiceTest { it -> it.first == cellLp.getInterfaceName() && it.second == -1)); } - @Test + @Test @IgnoreUpTo(SC_V2) public void testUpdateRateLimit_WhenNewNetworkIsAdded() throws Exception { final LinkProperties wifiLp = new LinkProperties(); wifiLp.setInterfaceName(WIFI_IFNAME); @@ -15462,7 +15469,7 @@ public class ConnectivityServiceTest { && it.second == rateLimitInBytesPerSec)); } - @Test + @Test @IgnoreUpTo(SC_V2) public void testUpdateRateLimit_OnlyAffectsInternetCapableNetworks() throws Exception { final LinkProperties wifiLp = new LinkProperties(); wifiLp.setInterfaceName(WIFI_IFNAME); @@ -15480,7 +15487,7 @@ public class ConnectivityServiceTest { assertNull(readHeadWifi.poll(TIMEOUT_MS, it -> it.first == wifiLp.getInterfaceName())); } - @Test + @Test @IgnoreUpTo(SC_V2) public void testUpdateRateLimit_DisconnectingResetsRateLimit() throws Exception { // Steps: @@ -15516,7 +15523,7 @@ public class ConnectivityServiceTest { assertNull(readHeadWifi.poll(TIMEOUT_MS, it -> it.first == wifiLp.getInterfaceName())); } - @Test + @Test @IgnoreUpTo(SC_V2) public void testUpdateRateLimit_UpdateExistingRateLimit() throws Exception { final LinkProperties wifiLp = new LinkProperties(); wifiLp.setInterfaceName(WIFI_IFNAME); @@ -15545,4 +15552,21 @@ public class ConnectivityServiceTest { it -> it.first == wifiLp.getInterfaceName() && it.second == 2000)); } + + @Test @IgnoreAfter(SC_V2) + public void testUpdateRateLimit_DoesNothingBeforeT() throws Exception { + final LinkProperties wifiLp = new LinkProperties(); + wifiLp.setInterfaceName(WIFI_IFNAME); + mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI, wifiLp); + mWiFiNetworkAgent.connect(true); + waitForIdle(); + + final ArrayTrackRecord>.ReadHead readHead = + mDeps.mRateLimitHistory.newReadHead(); + + setIngressRateLimit(1000); + waitForIdle(); + + assertNull(readHead.poll(TEST_CALLBACK_TIMEOUT_MS, it -> true)); + } }