From 56b6a95256fd58737bf5b58df38d2121ea609a8e Mon Sep 17 00:00:00 2001 From: junyulai Date: Tue, 31 Dec 2019 13:40:15 +0800 Subject: [PATCH 1/7] [SM05] Enable record mobile network stats by collapsed rat type Switch on the recording in device side. Metrics will be collected in follow-up patches which can be independently enabled/disabled. This change also fix the fail in NetworkStatsCollectionTest which caused by enabling this feature, where the rounding problem happened when records are distributed into smaller buckets and categorized into more NetworkIdentity. Test: atest FrameworksNetTests Bug: 129082217 Change-Id: If330e85330a4ff713dd420c98d42fa741eabd90a Merged-In: If330e85330a4ff713dd420c98d42fa741eabd90a (cherry picked from commit 2d4fa2c0fae8c2d79a25093d9f732a33c2f91dd4) --- .../net/NetworkStatsCollectionTest.java | 90 +++++++-------- .../server/net/NetworkStatsServiceTest.java | 109 +++++++++++++++++- 2 files changed, 149 insertions(+), 50 deletions(-) diff --git a/tests/net/java/com/android/server/net/NetworkStatsCollectionTest.java b/tests/net/java/com/android/server/net/NetworkStatsCollectionTest.java index 8f90f13ce7..551498f2c0 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsCollectionTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsCollectionTest.java @@ -319,33 +319,33 @@ public class NetworkStatsCollectionTest { assertEntry(18322, 75, 15031, 75, history.getValues(i++, null)); assertEntry(527798, 761, 78570, 652, history.getValues(i++, null)); assertEntry(527797, 760, 78570, 651, history.getValues(i++, null)); - assertEntry(10747, 50, 16838, 55, history.getValues(i++, null)); - assertEntry(10747, 49, 16838, 54, history.getValues(i++, null)); + assertEntry(10747, 50, 16839, 55, history.getValues(i++, null)); + assertEntry(10747, 49, 16837, 54, history.getValues(i++, null)); assertEntry(89191, 151, 18021, 140, history.getValues(i++, null)); assertEntry(89190, 150, 18020, 139, history.getValues(i++, null)); - assertEntry(3821, 22, 4525, 26, history.getValues(i++, null)); - assertEntry(3820, 22, 4524, 26, history.getValues(i++, null)); - assertEntry(91686, 159, 18575, 146, history.getValues(i++, null)); - assertEntry(91685, 159, 18575, 146, history.getValues(i++, null)); - assertEntry(8289, 35, 6863, 38, history.getValues(i++, null)); - assertEntry(8289, 35, 6863, 38, history.getValues(i++, null)); + assertEntry(3821, 23, 4525, 26, history.getValues(i++, null)); + assertEntry(3820, 21, 4524, 26, history.getValues(i++, null)); + assertEntry(91686, 159, 18576, 146, history.getValues(i++, null)); + assertEntry(91685, 159, 18574, 146, history.getValues(i++, null)); + assertEntry(8289, 36, 6864, 39, history.getValues(i++, null)); + assertEntry(8289, 34, 6862, 37, history.getValues(i++, null)); assertEntry(113914, 174, 18364, 157, history.getValues(i++, null)); assertEntry(113913, 173, 18364, 157, history.getValues(i++, null)); - assertEntry(11378, 49, 9261, 49, history.getValues(i++, null)); - assertEntry(11377, 48, 9261, 49, history.getValues(i++, null)); - assertEntry(201765, 328, 41808, 291, history.getValues(i++, null)); - assertEntry(201765, 328, 41807, 290, history.getValues(i++, null)); - assertEntry(106106, 218, 39917, 201, history.getValues(i++, null)); - assertEntry(106105, 217, 39917, 201, history.getValues(i++, null)); + assertEntry(11378, 49, 9261, 50, history.getValues(i++, null)); + assertEntry(11377, 48, 9261, 48, history.getValues(i++, null)); + assertEntry(201766, 328, 41808, 291, history.getValues(i++, null)); + assertEntry(201764, 328, 41807, 290, history.getValues(i++, null)); + assertEntry(106106, 219, 39918, 202, history.getValues(i++, null)); + assertEntry(106105, 216, 39916, 200, history.getValues(i++, null)); assertEquals(history.size(), i); // Slice from middle should be untouched history = getHistory(collection, plan, TIME_B - HOUR_IN_MILLIS, TIME_B + HOUR_IN_MILLIS); i = 0; - assertEntry(3821, 22, 4525, 26, history.getValues(i++, null)); - assertEntry(3820, 22, 4524, 26, history.getValues(i++, null)); - assertEntry(91686, 159, 18575, 146, history.getValues(i++, null)); - assertEntry(91685, 159, 18575, 146, history.getValues(i++, null)); + assertEntry(3821, 23, 4525, 26, history.getValues(i++, null)); + assertEntry(3820, 21, 4524, 26, history.getValues(i++, null)); + assertEntry(91686, 159, 18576, 146, history.getValues(i++, null)); + assertEntry(91685, 159, 18574, 146, history.getValues(i++, null)); assertEquals(history.size(), i); } @@ -373,25 +373,25 @@ public class NetworkStatsCollectionTest { assertEntry(527797, 760, 78570, 651, history.getValues(i++, null)); // Cycle point; start data normalization assertEntry(7507, 0, 11763, 0, history.getValues(i++, null)); - assertEntry(7507, 0, 11763, 0, history.getValues(i++, null)); + assertEntry(7507, 0, 11762, 0, history.getValues(i++, null)); assertEntry(62309, 0, 12589, 0, history.getValues(i++, null)); assertEntry(62309, 0, 12588, 0, history.getValues(i++, null)); assertEntry(2669, 0, 3161, 0, history.getValues(i++, null)); assertEntry(2668, 0, 3160, 0, history.getValues(i++, null)); // Anchor point; end data normalization - assertEntry(91686, 159, 18575, 146, history.getValues(i++, null)); - assertEntry(91685, 159, 18575, 146, history.getValues(i++, null)); - assertEntry(8289, 35, 6863, 38, history.getValues(i++, null)); - assertEntry(8289, 35, 6863, 38, history.getValues(i++, null)); + assertEntry(91686, 159, 18576, 146, history.getValues(i++, null)); + assertEntry(91685, 159, 18574, 146, history.getValues(i++, null)); + assertEntry(8289, 36, 6864, 39, history.getValues(i++, null)); + assertEntry(8289, 34, 6862, 37, history.getValues(i++, null)); assertEntry(113914, 174, 18364, 157, history.getValues(i++, null)); assertEntry(113913, 173, 18364, 157, history.getValues(i++, null)); // Cycle point - assertEntry(11378, 49, 9261, 49, history.getValues(i++, null)); - assertEntry(11377, 48, 9261, 49, history.getValues(i++, null)); - assertEntry(201765, 328, 41808, 291, history.getValues(i++, null)); - assertEntry(201765, 328, 41807, 290, history.getValues(i++, null)); - assertEntry(106106, 218, 39917, 201, history.getValues(i++, null)); - assertEntry(106105, 217, 39917, 201, history.getValues(i++, null)); + assertEntry(11378, 49, 9261, 50, history.getValues(i++, null)); + assertEntry(11377, 48, 9261, 48, history.getValues(i++, null)); + assertEntry(201766, 328, 41808, 291, history.getValues(i++, null)); + assertEntry(201764, 328, 41807, 290, history.getValues(i++, null)); + assertEntry(106106, 219, 39918, 202, history.getValues(i++, null)); + assertEntry(106105, 216, 39916, 200, history.getValues(i++, null)); assertEquals(history.size(), i); // Slice from middle should be augmented @@ -399,8 +399,8 @@ public class NetworkStatsCollectionTest { TIME_B + HOUR_IN_MILLIS); i = 0; assertEntry(2669, 0, 3161, 0, history.getValues(i++, null)); assertEntry(2668, 0, 3160, 0, history.getValues(i++, null)); - assertEntry(91686, 159, 18575, 146, history.getValues(i++, null)); - assertEntry(91685, 159, 18575, 146, history.getValues(i++, null)); + assertEntry(91686, 159, 18576, 146, history.getValues(i++, null)); + assertEntry(91685, 159, 18574, 146, history.getValues(i++, null)); assertEquals(history.size(), i); } @@ -427,34 +427,34 @@ public class NetworkStatsCollectionTest { assertEntry(527798, 761, 78570, 652, history.getValues(i++, null)); assertEntry(527797, 760, 78570, 651, history.getValues(i++, null)); // Cycle point; start data normalization - assertEntry(15015, 0, 23526, 0, history.getValues(i++, null)); - assertEntry(15015, 0, 23526, 0, history.getValues(i++, null)); + assertEntry(15015, 0, 23527, 0, history.getValues(i++, null)); + assertEntry(15015, 0, 23524, 0, history.getValues(i++, null)); assertEntry(124619, 0, 25179, 0, history.getValues(i++, null)); assertEntry(124618, 0, 25177, 0, history.getValues(i++, null)); assertEntry(5338, 0, 6322, 0, history.getValues(i++, null)); assertEntry(5337, 0, 6320, 0, history.getValues(i++, null)); // Anchor point; end data normalization - assertEntry(91686, 159, 18575, 146, history.getValues(i++, null)); - assertEntry(91685, 159, 18575, 146, history.getValues(i++, null)); - assertEntry(8289, 35, 6863, 38, history.getValues(i++, null)); - assertEntry(8289, 35, 6863, 38, history.getValues(i++, null)); + assertEntry(91686, 159, 18576, 146, history.getValues(i++, null)); + assertEntry(91685, 159, 18574, 146, history.getValues(i++, null)); + assertEntry(8289, 36, 6864, 39, history.getValues(i++, null)); + assertEntry(8289, 34, 6862, 37, history.getValues(i++, null)); assertEntry(113914, 174, 18364, 157, history.getValues(i++, null)); assertEntry(113913, 173, 18364, 157, history.getValues(i++, null)); // Cycle point - assertEntry(11378, 49, 9261, 49, history.getValues(i++, null)); - assertEntry(11377, 48, 9261, 49, history.getValues(i++, null)); - assertEntry(201765, 328, 41808, 291, history.getValues(i++, null)); - assertEntry(201765, 328, 41807, 290, history.getValues(i++, null)); - assertEntry(106106, 218, 39917, 201, history.getValues(i++, null)); - assertEntry(106105, 217, 39917, 201, history.getValues(i++, null)); + assertEntry(11378, 49, 9261, 50, history.getValues(i++, null)); + assertEntry(11377, 48, 9261, 48, history.getValues(i++, null)); + assertEntry(201766, 328, 41808, 291, history.getValues(i++, null)); + assertEntry(201764, 328, 41807, 290, history.getValues(i++, null)); + assertEntry(106106, 219, 39918, 202, history.getValues(i++, null)); + assertEntry(106105, 216, 39916, 200, history.getValues(i++, null)); // Slice from middle should be augmented history = getHistory(collection, plan, TIME_B - HOUR_IN_MILLIS, TIME_B + HOUR_IN_MILLIS); i = 0; assertEntry(5338, 0, 6322, 0, history.getValues(i++, null)); assertEntry(5337, 0, 6320, 0, history.getValues(i++, null)); - assertEntry(91686, 159, 18575, 146, history.getValues(i++, null)); - assertEntry(91685, 159, 18575, 146, history.getValues(i++, null)); + assertEntry(91686, 159, 18576, 146, history.getValues(i++, null)); + assertEntry(91685, 159, 18574, 146, history.getValues(i++, null)); assertEquals(history.size(), i); } } diff --git a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java index b346c92391..64c0221c58 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java @@ -42,6 +42,7 @@ import static android.net.NetworkStats.TAG_NONE; import static android.net.NetworkStats.UID_ALL; import static android.net.NetworkStatsHistory.FIELD_ALL; import static android.net.NetworkTemplate.buildTemplateMobileAll; +import static android.net.NetworkTemplate.buildTemplateMobileWithRatType; import static android.net.NetworkTemplate.buildTemplateWifiWildcard; import static android.net.TrafficStats.MB_IN_BYTES; import static android.net.TrafficStats.UID_REMOVED; @@ -60,11 +61,13 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.annotation.NonNull; +import android.annotation.Nullable; import android.app.AlarmManager; import android.app.usage.NetworkStatsManager; import android.content.Context; @@ -92,6 +95,8 @@ import android.os.Message; import android.os.Messenger; import android.os.PowerManager; import android.os.SimpleClock; +import android.telephony.PhoneStateListener; +import android.telephony.ServiceState; import android.telephony.TelephonyManager; import androidx.test.InstrumentationRegistry; @@ -163,11 +168,14 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { private @Mock NetworkStatsSettings mSettings; private @Mock IBinder mBinder; private @Mock AlarmManager mAlarmManager; + private @Mock TelephonyManager mTelephonyManager; private HandlerThread mHandlerThread; private NetworkStatsService mService; private INetworkStatsSession mSession; private INetworkManagementEventObserver mNetworkObserver; + @Nullable + private PhoneStateListener mPhoneStateListener; private final Clock mClock = new SimpleClock(ZoneOffset.UTC) { @Override @@ -195,7 +203,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { mHandlerThread = new HandlerThread("HandlerThread"); final NetworkStatsService.Dependencies deps = makeDependencies(); mService = new NetworkStatsService(mServiceContext, mNetManager, mAlarmManager, wakeLock, - mClock, mServiceContext.getSystemService(TelephonyManager.class), mSettings, + mClock, mTelephonyManager, mSettings, mStatsFactory, new NetworkStatsObservers(), mStatsDir, getBaseDir(mStatsDir), deps); mElapsedRealtime = 0L; @@ -216,6 +224,12 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { ArgumentCaptor.forClass(INetworkManagementEventObserver.class); verify(mNetManager).registerObserver(networkObserver.capture()); mNetworkObserver = networkObserver.getValue(); + + // Capture the phone state listener that created by service. + final ArgumentCaptor phoneStateListenerCaptor = + ArgumentCaptor.forClass(PhoneStateListener.class); + verify(mTelephonyManager).listen(phoneStateListenerCaptor.capture(), anyInt()); + mPhoneStateListener = phoneStateListenerCaptor.getValue(); } @NonNull @@ -534,7 +548,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { } @Test - public void testUid3g4gCombinedByTemplate() throws Exception { + public void testUid3gWimaxCombinedByTemplate() throws Exception { // pretend that network comes online expectDefaultSettings(); NetworkState[] states = new NetworkState[] {buildMobile3gState(IMSI_1)}; @@ -558,10 +572,10 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { assertUidTotal(sTemplateImsi1, UID_RED, 1024L, 8L, 1024L, 8L, 5); - // now switch over to 4g network + // now switch over to wimax network incrementCurrentTime(HOUR_IN_MILLIS); expectDefaultSettings(); - states = new NetworkState[] {buildMobile4gState(TEST_IFACE2)}; + states = new NetworkState[] {buildWimaxState(TEST_IFACE2)}; expectNetworkStatsSummary(buildEmptyStats()); expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) .insertEntry(TEST_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 1024L, 8L, 1024L, 8L, 0L) @@ -588,6 +602,89 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { assertUidTotal(sTemplateImsi1, UID_RED, 1536L, 12L, 1280L, 10L, 10); } + @Test + public void testMobileStatsByRatType() throws Exception { + final NetworkTemplate template3g = + buildTemplateMobileWithRatType(null, TelephonyManager.NETWORK_TYPE_UMTS); + final NetworkTemplate template4g = + buildTemplateMobileWithRatType(null, TelephonyManager.NETWORK_TYPE_LTE); + final NetworkTemplate template5g = + buildTemplateMobileWithRatType(null, TelephonyManager.NETWORK_TYPE_NR); + final NetworkState[] states = new NetworkState[]{buildMobile3gState(IMSI_1)}; + + // 3G network comes online. + expectNetworkStatsSummary(buildEmptyStats()); + expectNetworkStatsUidDetail(buildEmptyStats()); + + setMobileRatTypeAndWaitForIdle(TelephonyManager.NETWORK_TYPE_UMTS); + mService.forceUpdateIfaces(NETWORKS_MOBILE, states, getActiveIface(states), + new VpnInfo[0]); + + // Create some traffic. + incrementCurrentTime(MINUTE_IN_MILLIS); + expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) + .addEntry(new NetworkStats.Entry(TEST_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, + 12L, 18L, 14L, 1L, 0L))); + forcePollAndWaitForIdle(); + + // Verify 3g templates gets stats. + assertUidTotal(sTemplateImsi1, UID_RED, 12L, 18L, 14L, 1L, 0); + assertUidTotal(template3g, UID_RED, 12L, 18L, 14L, 1L, 0); + assertUidTotal(template4g, UID_RED, 0L, 0L, 0L, 0L, 0); + assertUidTotal(template5g, UID_RED, 0L, 0L, 0L, 0L, 0); + + // 4G network comes online. + incrementCurrentTime(MINUTE_IN_MILLIS); + setMobileRatTypeAndWaitForIdle(TelephonyManager.NETWORK_TYPE_LTE); + expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) + // Append more traffic on existing 3g stats entry. + .addEntry(new NetworkStats.Entry(TEST_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, + 16L, 22L, 17L, 2L, 0L)) + // Add entry that is new on 4g. + .addEntry(new NetworkStats.Entry(TEST_IFACE, UID_RED, SET_FOREGROUND, TAG_NONE, + 33L, 27L, 8L, 10L, 1L))); + forcePollAndWaitForIdle(); + + // Verify ALL_MOBILE template gets all. 3g template counters do not increase. + assertUidTotal(sTemplateImsi1, UID_RED, 49L, 49L, 25L, 12L, 1); + assertUidTotal(template3g, UID_RED, 12L, 18L, 14L, 1L, 0); + // Verify 4g template counts appended stats on existing entry and newly created entry. + assertUidTotal(template4g, UID_RED, 4L + 33L, 4L + 27L, 3L + 8L, 1L + 10L, 1); + // Verify 5g template doesn't get anything since no traffic is generated on 5g. + assertUidTotal(template5g, UID_RED, 0L, 0L, 0L, 0L, 0); + + // 5g network comes online. + incrementCurrentTime(MINUTE_IN_MILLIS); + setMobileRatTypeAndWaitForIdle(TelephonyManager.NETWORK_TYPE_NR); + expectNetworkStatsUidDetail(new NetworkStats(getElapsedRealtime(), 1) + // Existing stats remains. + .addEntry(new NetworkStats.Entry(TEST_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, + 16L, 22L, 17L, 2L, 0L)) + .addEntry(new NetworkStats.Entry(TEST_IFACE, UID_RED, SET_FOREGROUND, TAG_NONE, + 33L, 27L, 8L, 10L, 1L)) + // Add some traffic on 5g. + .addEntry(new NetworkStats.Entry(TEST_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, + 5L, 13L, 31L, 9L, 2L))); + forcePollAndWaitForIdle(); + + // Verify ALL_MOBILE template gets all. + assertUidTotal(sTemplateImsi1, UID_RED, 54L, 62L, 56L, 21L, 3); + // 3g/4g template counters do not increase. + assertUidTotal(template3g, UID_RED, 12L, 18L, 14L, 1L, 0); + assertUidTotal(template4g, UID_RED, 4L + 33L, 4L + 27L, 3L + 8L, 1L + 10L, 1); + // Verify 5g template gets the 5g count. + assertUidTotal(template5g, UID_RED, 5L, 13L, 31L, 9L, 2); + } + + // TODO: support per IMSI state + private void setMobileRatTypeAndWaitForIdle(int ratType) { + final ServiceState mockSs = mock(ServiceState.class); + when(mockSs.getDataNetworkType()).thenReturn(ratType); + mPhoneStateListener.onServiceStateChanged(mockSs); + + HandlerUtilsKt.waitForIdle(mHandlerThread, WAIT_TIMEOUT); + } + @Test public void testSummaryForAllUid() throws Exception { // pretend that network comes online @@ -1242,6 +1339,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { final NetworkCapabilities capabilities = new NetworkCapabilities(); capabilities.setCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED, !isMetered); capabilities.setCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING, true); + capabilities.addTransportType(NetworkCapabilities.TRANSPORT_WIFI); return new NetworkState(info, prop, capabilities, WIFI_NETWORK, null, TEST_SSID); } @@ -1259,10 +1357,11 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { final NetworkCapabilities capabilities = new NetworkCapabilities(); capabilities.setCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED, false); capabilities.setCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING, !isRoaming); + capabilities.addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR); return new NetworkState(info, prop, capabilities, MOBILE_NETWORK, subscriberId, null); } - private static NetworkState buildMobile4gState(String iface) { + private static NetworkState buildWimaxState(@NonNull String iface) { final NetworkInfo info = new NetworkInfo(TYPE_WIMAX, 0, null, null); info.setDetailedState(DetailedState.CONNECTED, null, null); final LinkProperties prop = new LinkProperties(); From 7e6e807bec67ddead3c484c3f4d1d94fd8346da9 Mon Sep 17 00:00:00 2001 From: junyulai Date: Thu, 2 Jan 2020 19:35:59 +0800 Subject: [PATCH 2/7] [SM07] Make combine subtype configurable from Settings Note that enabling/disabling would not take effect until device reboot. This will be addressed in follow-up patch. Test: 1. atest NetworkStatsServieTest SettingsBackupTest 2. adb shell settings put global netstats_combine_subtype_enabled 1|0 Bug: 146415925 Change-Id: Ic94da540afa479ed18f1b6fbda4ae3216c37476b Merged-In: Ic94da540afa479ed18f1b6fbda4ae3216c37476b (cherry picked from commit c4f77ac90bf2e48a655ad19b162fe74a23bf3fb0) --- .../net/java/com/android/server/net/NetworkStatsServiceTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java index 64c0221c58..6e6331312e 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java @@ -1294,6 +1294,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { when(mSettings.getPollInterval()).thenReturn(HOUR_IN_MILLIS); when(mSettings.getPollDelay()).thenReturn(0L); when(mSettings.getSampleEnabled()).thenReturn(true); + when(mSettings.getCombineSubtypeEnabled()).thenReturn(false); final Config config = new Config(bucketDuration, deleteAge, deleteAge); when(mSettings.getDevConfig()).thenReturn(config); From 92e995d9cdbb64adadd0fa7abc736804970e8f58 Mon Sep 17 00:00:00 2001 From: junyulai Date: Tue, 25 Feb 2020 21:36:33 +0800 Subject: [PATCH 3/7] [SM08] Add NetworkTemplate unit test for fetching mobile data usage Test: atest NetworkTemplateTest Bug: 129082217 Change-Id: I7eaca623adf93f9b8d53c2e5857ecae90ea572ab Merged-In: I7eaca623adf93f9b8d53c2e5857ecae90ea572ab (cherry picked from commit 4670baace6c8dafe3a30330596c333d6b2389e4d) --- .../java/android/net/NetworkTemplateTest.kt | 155 ++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 tests/net/java/android/net/NetworkTemplateTest.kt diff --git a/tests/net/java/android/net/NetworkTemplateTest.kt b/tests/net/java/android/net/NetworkTemplateTest.kt new file mode 100644 index 0000000000..5dd0fda4da --- /dev/null +++ b/tests/net/java/android/net/NetworkTemplateTest.kt @@ -0,0 +1,155 @@ +/* + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.net + +import android.content.Context +import android.net.ConnectivityManager.TYPE_MOBILE +import android.net.ConnectivityManager.TYPE_WIFI +import android.net.NetworkIdentity.SUBTYPE_COMBINED +import android.net.NetworkIdentity.buildNetworkIdentity +import android.net.NetworkStats.DEFAULT_NETWORK_ALL +import android.net.NetworkStats.METERED_ALL +import android.net.NetworkStats.ROAMING_ALL +import android.net.NetworkTemplate.MATCH_MOBILE +import android.net.NetworkTemplate.MATCH_WIFI +import android.net.NetworkTemplate.NETWORK_TYPE_ALL +import android.net.NetworkTemplate.buildTemplateMobileWithRatType +import android.telephony.TelephonyManager +import com.android.testutils.assertParcelSane +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.mockito.Mockito.doReturn +import org.mockito.Mockito.mock +import org.mockito.MockitoAnnotations +import kotlin.test.assertFalse +import kotlin.test.assertNotEquals +import kotlin.test.assertTrue + +private const val TEST_IMSI1 = "imsi1" +private const val TEST_IMSI2 = "imsi2" +private const val TEST_SSID1 = "ssid1" + +@RunWith(JUnit4::class) +class NetworkTemplateTest { + private val mockContext = mock(Context::class.java) + + private fun buildMobileNetworkState(subscriberId: String): NetworkState = + buildNetworkState(TYPE_MOBILE, subscriberId = subscriberId) + private fun buildWifiNetworkState(ssid: String): NetworkState = + buildNetworkState(TYPE_WIFI, ssid = ssid) + + private fun buildNetworkState( + type: Int, + subscriberId: String? = null, + ssid: String? = null + ): NetworkState { + val info = mock(NetworkInfo::class.java) + doReturn(type).`when`(info).type + doReturn(NetworkInfo.State.CONNECTED).`when`(info).state + val lp = LinkProperties() + val caps = NetworkCapabilities().apply { + setCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED, false) + setCapability(NetworkCapabilities.NET_CAPABILITY_NOT_ROAMING, true) + } + return NetworkState(info, lp, caps, mock(Network::class.java), subscriberId, ssid) + } + + private fun NetworkTemplate.assertMatches(ident: NetworkIdentity) = + assertTrue(matches(ident), "$this does not match $ident") + + private fun NetworkTemplate.assertDoesNotMatch(ident: NetworkIdentity) = + assertFalse(matches(ident), "$this should match $ident") + + @Before + fun setup() { + MockitoAnnotations.initMocks(this) + } + + @Test + fun testRatTypeGroupMatches() { + val stateMobile = buildMobileNetworkState(TEST_IMSI1) + // Build UMTS template that matches mobile identities with RAT in the same + // group with any IMSI. See {@link NetworkTemplate#getCollapsedRatType}. + val templateUmts = buildTemplateMobileWithRatType(null, TelephonyManager.NETWORK_TYPE_UMTS) + // Build normal template that matches mobile identities with any RAT and IMSI. + val templateAll = buildTemplateMobileWithRatType(null, NETWORK_TYPE_ALL) + // Build template with UNKNOWN RAT that matches mobile identities with RAT that + // cannot be determined. + val templateUnknown = + buildTemplateMobileWithRatType(null, TelephonyManager.NETWORK_TYPE_UNKNOWN) + + val identUmts = buildNetworkIdentity( + mockContext, stateMobile, false, TelephonyManager.NETWORK_TYPE_UMTS) + val identHsdpa = buildNetworkIdentity( + mockContext, stateMobile, false, TelephonyManager.NETWORK_TYPE_HSDPA) + val identLte = buildNetworkIdentity( + mockContext, stateMobile, false, TelephonyManager.NETWORK_TYPE_LTE) + val identCombined = buildNetworkIdentity( + mockContext, stateMobile, false, SUBTYPE_COMBINED) + val identImsi2 = buildNetworkIdentity(mockContext, buildMobileNetworkState(TEST_IMSI2), + false, TelephonyManager.NETWORK_TYPE_UMTS) + val identWifi = buildNetworkIdentity( + mockContext, buildWifiNetworkState(TEST_SSID1), true, 0) + + // Assert that identity with the same RAT matches. + templateUmts.assertMatches(identUmts) + templateAll.assertMatches(identUmts) + templateUnknown.assertDoesNotMatch(identUmts) + // Assert that identity with the RAT within the same group matches. + templateUmts.assertMatches(identHsdpa) + templateAll.assertMatches(identHsdpa) + templateUnknown.assertDoesNotMatch(identHsdpa) + // Assert that identity with the RAT out of the same group only matches template with + // NETWORK_TYPE_ALL. + templateUmts.assertDoesNotMatch(identLte) + templateAll.assertMatches(identLte) + templateUnknown.assertDoesNotMatch(identLte) + // Assert that identity with combined RAT only matches with template with NETWORK_TYPE_ALL + // and NETWORK_TYPE_UNKNOWN. + templateUmts.assertDoesNotMatch(identCombined) + templateAll.assertMatches(identCombined) + templateUnknown.assertMatches(identCombined) + // Assert that identity with different IMSI matches. + templateUmts.assertMatches(identImsi2) + templateAll.assertMatches(identImsi2) + templateUnknown.assertDoesNotMatch(identImsi2) + // Assert that wifi identity does not match. + templateUmts.assertDoesNotMatch(identWifi) + templateAll.assertDoesNotMatch(identWifi) + templateUnknown.assertDoesNotMatch(identWifi) + } + + @Test + fun testParcelUnparcel() { + val templateMobile = NetworkTemplate(MATCH_MOBILE, TEST_IMSI1, null, null, METERED_ALL, + ROAMING_ALL, DEFAULT_NETWORK_ALL, TelephonyManager.NETWORK_TYPE_LTE) + val templateWifi = NetworkTemplate(MATCH_WIFI, null, null, TEST_SSID1, METERED_ALL, + ROAMING_ALL, DEFAULT_NETWORK_ALL, 0) + assertParcelSane(templateMobile, 8) + assertParcelSane(templateWifi, 8) + } + + // Verify NETWORK_TYPE_ALL does not conflict with TelephonyManager#NETWORK_TYPE_* constants. + @Test + fun testNetworkTypeAll() { + for (ratType in TelephonyManager.getAllNetworkTypes()) { + assertNotEquals(NETWORK_TYPE_ALL, ratType) + } + } +} From 5637fa73e388fa60bf77aa2d7179edd1f750ec03 Mon Sep 17 00:00:00 2001 From: Automerger Merge Worker Date: Thu, 19 Mar 2020 02:48:22 +0000 Subject: [PATCH 4/7] Kill ConnectivityManager.CONNECTIVITY_ACTION_SUPL CONNECTIVITY_ACTION_SUPL is marked as a "temporary hack" and has never been public. Remove this intent definition since no one is receiving this intent and should use network callback to know the connection change. Bug: 109636544 Test: atest FrameworksNetTests Change-Id: Ie9e5127742beba04f1c191e894e8a29fe1e704bb Merged-In: Ie9e5127742beba04f1c191e894e8a29fe1e704bb (cherry picked from aosp/1224697) --- .../java/android/net/ConnectivityManager.java | 97 +------------------ .../android/server/ConnectivityService.java | 11 +-- .../server/ConnectivityServiceTest.java | 46 ++------- 3 files changed, 14 insertions(+), 140 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 81735ac8f6..99813c0988 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -144,16 +144,6 @@ public class ConnectivityManager { @Deprecated public static final String CONNECTIVITY_ACTION = "android.net.conn.CONNECTIVITY_CHANGE"; - /** - * A temporary hack until SUPL system can get off the legacy APIS. - * They do too many network requests and the long list of apps listening - * and waking due to the CONNECTIVITY_ACTION broadcast makes it expensive. - * Use this broadcast intent instead for SUPL requests. - * @hide - */ - public static final String CONNECTIVITY_ACTION_SUPL = - "android.net.conn.CONNECTIVITY_CHANGE_SUPL"; - /** * The device has connected to a network that has presented a captive * portal, which is blocking Internet connectivity. The user was presented @@ -1517,84 +1507,6 @@ public class ConnectivityManager { return null; } - /** - * Guess what the network request was trying to say so that the resulting - * network is accessible via the legacy (deprecated) API such as - * requestRouteToHost. - * - * This means we should try to be fairly precise about transport and - * capability but ignore things such as networkSpecifier. - * If the request has more than one transport or capability it doesn't - * match the old legacy requests (they selected only single transport/capability) - * so this function cannot map the request to a single legacy type and - * the resulting network will not be available to the legacy APIs. - * - * This code is only called from the requestNetwork API (L and above). - * - * Setting a legacy type causes CONNECTIVITY_ACTION broadcasts, which are expensive - * because they wake up lots of apps - see http://b/23350688 . So we currently only - * do this for SUPL requests, which are the only ones that we know need it. If - * omitting these broadcasts causes unacceptable app breakage, then for backwards - * compatibility we can send them: - * - * if (targetSdkVersion < Build.VERSION_CODES.M) && // legacy API unsupported >= M - * targetSdkVersion >= Build.VERSION_CODES.LOLLIPOP)) // requestNetwork not present < L - * - * TODO - This should be removed when the legacy APIs are removed. - */ - private int inferLegacyTypeForNetworkCapabilities(NetworkCapabilities netCap) { - if (netCap == null) { - return TYPE_NONE; - } - - if (!netCap.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)) { - return TYPE_NONE; - } - - // Do this only for SUPL, until GnssLocationProvider is fixed. http://b/25876485 . - if (!netCap.hasCapability(NetworkCapabilities.NET_CAPABILITY_SUPL)) { - // NOTE: if this causes app breakage, we should not just comment out this early return; - // instead, we should make this early return conditional on the requesting app's target - // SDK version, as described in the comment above. - return TYPE_NONE; - } - - String type = null; - int result = TYPE_NONE; - - if (netCap.hasCapability(NetworkCapabilities.NET_CAPABILITY_CBS)) { - type = "enableCBS"; - result = TYPE_MOBILE_CBS; - } else if (netCap.hasCapability(NetworkCapabilities.NET_CAPABILITY_IMS)) { - type = "enableIMS"; - result = TYPE_MOBILE_IMS; - } else if (netCap.hasCapability(NetworkCapabilities.NET_CAPABILITY_FOTA)) { - type = "enableFOTA"; - result = TYPE_MOBILE_FOTA; - } else if (netCap.hasCapability(NetworkCapabilities.NET_CAPABILITY_DUN)) { - type = "enableDUN"; - result = TYPE_MOBILE_DUN; - } else if (netCap.hasCapability(NetworkCapabilities.NET_CAPABILITY_SUPL)) { - type = "enableSUPL"; - result = TYPE_MOBILE_SUPL; - // back out this hack for mms as they no longer need this and it's causing - // device slowdowns - b/23350688 (note, supl still needs this) - //} else if (netCap.hasCapability(NetworkCapabilities.NET_CAPABILITY_MMS)) { - // type = "enableMMS"; - // result = TYPE_MOBILE_MMS; - } else if (netCap.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)) { - type = "enableHIPRI"; - result = TYPE_MOBILE_HIPRI; - } - if (type != null) { - NetworkCapabilities testCap = networkCapabilitiesForFeature(TYPE_MOBILE, type); - if (testCap.equalsNetCapabilities(netCap) && testCap.equalsTransportTypes(netCap)) { - return result; - } - } - return TYPE_NONE; - } - private int legacyTypeForNetworkCapabilities(NetworkCapabilities netCap) { if (netCap == null) return TYPE_NONE; if (netCap.hasCapability(NetworkCapabilities.NET_CAPABILITY_CBS)) { @@ -3893,9 +3805,8 @@ public class ConnectivityManager { */ public void requestNetwork(@NonNull NetworkRequest request, @NonNull NetworkCallback networkCallback, @NonNull Handler handler) { - int legacyType = inferLegacyTypeForNetworkCapabilities(request.networkCapabilities); CallbackHandler cbHandler = new CallbackHandler(handler); - requestNetwork(request, networkCallback, 0, legacyType, cbHandler); + requestNetwork(request, networkCallback, 0, TYPE_NONE, cbHandler); } /** @@ -3928,8 +3839,7 @@ public class ConnectivityManager { public void requestNetwork(@NonNull NetworkRequest request, @NonNull NetworkCallback networkCallback, int timeoutMs) { checkTimeout(timeoutMs); - int legacyType = inferLegacyTypeForNetworkCapabilities(request.networkCapabilities); - requestNetwork(request, networkCallback, timeoutMs, legacyType, getDefaultHandler()); + requestNetwork(request, networkCallback, timeoutMs, TYPE_NONE, getDefaultHandler()); } /** @@ -3954,9 +3864,8 @@ public class ConnectivityManager { public void requestNetwork(@NonNull NetworkRequest request, @NonNull NetworkCallback networkCallback, @NonNull Handler handler, int timeoutMs) { checkTimeout(timeoutMs); - int legacyType = inferLegacyTypeForNetworkCapabilities(request.networkCapabilities); CallbackHandler cbHandler = new CallbackHandler(handler); - requestNetwork(request, networkCallback, timeoutMs, legacyType, cbHandler); + requestNetwork(request, networkCallback, timeoutMs, TYPE_NONE, cbHandler); } /** diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 4309b849d4..91dd1d59c6 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2240,14 +2240,9 @@ public class ConnectivityService extends IConnectivityManager.Stub if (ConnectivityManager.CONNECTIVITY_ACTION.equals(intent.getAction())) { final NetworkInfo ni = intent.getParcelableExtra( ConnectivityManager.EXTRA_NETWORK_INFO); - if (ni.getType() == ConnectivityManager.TYPE_MOBILE_SUPL) { - intent.setAction(ConnectivityManager.CONNECTIVITY_ACTION_SUPL); - intent.addFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY); - } else { - BroadcastOptions opts = BroadcastOptions.makeBasic(); - opts.setMaxManifestReceiverApiLevel(Build.VERSION_CODES.M); - options = opts.toBundle(); - } + final BroadcastOptions opts = BroadcastOptions.makeBasic(); + opts.setMaxManifestReceiverApiLevel(Build.VERSION_CODES.M); + options = opts.toBundle(); final IBatteryStats bs = mDeps.getBatteryStatsService(); try { bs.noteConnectivityChanged(intent.getIntExtra( diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index f7ad09ca36..4bfb51bca2 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -25,7 +25,6 @@ import static android.content.pm.PackageManager.PERMISSION_DENIED; import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.net.ConnectivityManager.ACTION_CAPTIVE_PORTAL_SIGN_IN; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; -import static android.net.ConnectivityManager.CONNECTIVITY_ACTION_SUPL; import static android.net.ConnectivityManager.EXTRA_NETWORK_INFO; import static android.net.ConnectivityManager.EXTRA_NETWORK_TYPE; import static android.net.ConnectivityManager.NETID_UNSET; @@ -1374,7 +1373,6 @@ public class ConnectivityServiceTest { @NonNull final Predicate filter) { final ConditionVariable cv = new ConditionVariable(); final IntentFilter intentFilter = new IntentFilter(CONNECTIVITY_ACTION); - intentFilter.addAction(CONNECTIVITY_ACTION_SUPL); final BroadcastReceiver receiver = new BroadcastReceiver() { private int remaining = count; public void onReceive(Context context, Intent intent) { @@ -1422,56 +1420,28 @@ public class ConnectivityServiceTest { final NetworkRequest legacyRequest = new NetworkRequest(legacyCaps, TYPE_MOBILE_SUPL, ConnectivityManager.REQUEST_ID_UNSET, NetworkRequest.Type.REQUEST); - // Send request and check that the legacy broadcast for SUPL is sent correctly. + // File request, withdraw it and make sure no broadcast is sent + final ConditionVariable cv2 = registerConnectivityBroadcast(1); final TestNetworkCallback callback = new TestNetworkCallback(); - final ConditionVariable cv2 = registerConnectivityBroadcastThat(1, - intent -> intent.getIntExtra(EXTRA_NETWORK_TYPE, -1) == TYPE_MOBILE_SUPL); mCm.requestNetwork(legacyRequest, callback); callback.expectCallback(CallbackEntry.AVAILABLE, mCellNetworkAgent); - waitFor(cv2); - - // File another request, withdraw it and make sure no broadcast is sent - final ConditionVariable cv3 = registerConnectivityBroadcast(1); - final TestNetworkCallback callback2 = new TestNetworkCallback(); - mCm.requestNetwork(legacyRequest, callback2); - callback2.expectCallback(CallbackEntry.AVAILABLE, mCellNetworkAgent); - mCm.unregisterNetworkCallback(callback2); - assertFalse(cv3.block(800)); // 800ms long enough to at least flake if this is sent + mCm.unregisterNetworkCallback(callback); + assertFalse(cv2.block(800)); // 800ms long enough to at least flake if this is sent // As the broadcast did not fire, the receiver was not unregistered. Do this now. mServiceContext.clearRegisteredReceivers(); - // Withdraw the request and check that the broadcast for disconnection is sent. - final ConditionVariable cv4 = registerConnectivityBroadcastThat(1, intent -> - !((NetworkInfo) intent.getExtra(EXTRA_NETWORK_INFO, -1)).isConnected() - && intent.getIntExtra(EXTRA_NETWORK_TYPE, -1) == TYPE_MOBILE_SUPL); - mCm.unregisterNetworkCallback(callback); - waitFor(cv4); - - // Re-file the request and expect the connected broadcast again - final ConditionVariable cv5 = registerConnectivityBroadcastThat(1, - intent -> intent.getIntExtra(EXTRA_NETWORK_TYPE, -1) == TYPE_MOBILE_SUPL); - final TestNetworkCallback callback3 = new TestNetworkCallback(); - mCm.requestNetwork(legacyRequest, callback3); - callback3.expectCallback(CallbackEntry.AVAILABLE, mCellNetworkAgent); - waitFor(cv5); - - // Disconnect the network and expect two disconnected broadcasts, one for SUPL and one - // for mobile. Use a small hack to check that both have been sent, but the order is - // not contractual. + // Disconnect the network and expect mobile disconnected broadcast. Use a small hack to + // check that has been sent. final AtomicBoolean vanillaAction = new AtomicBoolean(false); - final AtomicBoolean suplAction = new AtomicBoolean(false); - final ConditionVariable cv6 = registerConnectivityBroadcastThat(2, intent -> { + final ConditionVariable cv3 = registerConnectivityBroadcastThat(1, intent -> { if (intent.getAction().equals(CONNECTIVITY_ACTION)) { vanillaAction.set(true); - } else if (intent.getAction().equals(CONNECTIVITY_ACTION_SUPL)) { - suplAction.set(true); } return !((NetworkInfo) intent.getExtra(EXTRA_NETWORK_INFO, -1)).isConnected(); }); mCellNetworkAgent.disconnect(); - waitFor(cv6); + waitFor(cv3); assertTrue(vanillaAction.get()); - assertTrue(suplAction.get()); } @Test From 4129ed1f13eadd12e0a66d4f31773ac1061add9f Mon Sep 17 00:00:00 2001 From: markchien Date: Thu, 19 Mar 2020 13:37:43 +0800 Subject: [PATCH 5/7] TetheringManager API clean up Per API review: - @IntDef defined on the type integer parameter - have getters on each parameter that is set in the TetheringRequest.Builder - new added API should not be deprecated Below APIs is moved from system-current to module-lib-current that only plafrom code(e.g. ConnectivityManager and Settings) can use them. TetheringRequest. onTetherableInterfaceRegexpsChanged, TetheringInterfaceRegexps: Only platform code can use them because interfaces by regular expressions are a mechanism which is planning to be deprecated. Also rename some constants for easier to understand. Bug: 149858697 Bug: 151243337 Test: m doc-comment-check-docs atest TetheringTests Change-Id: Idd041f0fbeca411ea23e49786a50dd7feb77ef45 --- .../java/android/net/ConnectivityManager.java | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 81735ac8f6..7850d9ed6f 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -2575,13 +2575,13 @@ public class ConnectivityManager { } @Override - public void onTetheringFailed(final int resultCode) { + public void onTetheringFailed(final int error) { callback.onTetheringFailed(); } }; final TetheringRequest request = new TetheringRequest.Builder(type) - .setSilentProvisioning(!showProvisioningUi).build(); + .setShouldShowEntitlementUi(showProvisioningUi).build(); mTetheringManager.startTethering(request, executor, tetheringCallback); } @@ -2801,11 +2801,12 @@ public class ConnectivityManager { public static final int TETHER_ERROR_UNAVAIL_IFACE = TetheringManager.TETHER_ERROR_UNAVAIL_IFACE; /** - * @deprecated Use {@link TetheringManager#TETHER_ERROR_MASTER_ERROR}. + * @deprecated Use {@link TetheringManager#TETHER_ERROR_INTERNAL_ERROR}. * {@hide} */ @Deprecated - public static final int TETHER_ERROR_MASTER_ERROR = TetheringManager.TETHER_ERROR_MASTER_ERROR; + public static final int TETHER_ERROR_MASTER_ERROR = + TetheringManager.TETHER_ERROR_INTERNAL_ERROR; /** * @deprecated Use {@link TetheringManager#TETHER_ERROR_TETHER_IFACE_ERROR}. * {@hide} @@ -2821,19 +2822,19 @@ public class ConnectivityManager { public static final int TETHER_ERROR_UNTETHER_IFACE_ERROR = TetheringManager.TETHER_ERROR_UNTETHER_IFACE_ERROR; /** - * @deprecated Use {@link TetheringManager#TETHER_ERROR_ENABLE_NAT_ERROR}. + * @deprecated Use {@link TetheringManager#TETHER_ERROR_ENABLE_FORWARDING_ERROR}. * {@hide} */ @Deprecated public static final int TETHER_ERROR_ENABLE_NAT_ERROR = - TetheringManager.TETHER_ERROR_ENABLE_NAT_ERROR; + TetheringManager.TETHER_ERROR_ENABLE_FORWARDING_ERROR; /** - * @deprecated Use {@link TetheringManager#TETHER_ERROR_DISABLE_NAT_ERROR}. + * @deprecated Use {@link TetheringManager#TETHER_ERROR_DISABLE_FORWARDING_ERROR}. * {@hide} */ @Deprecated public static final int TETHER_ERROR_DISABLE_NAT_ERROR = - TetheringManager.TETHER_ERROR_DISABLE_NAT_ERROR; + TetheringManager.TETHER_ERROR_DISABLE_FORWARDING_ERROR; /** * @deprecated Use {@link TetheringManager#TETHER_ERROR_IFACE_CFG_ERROR}. * {@hide} @@ -2842,13 +2843,13 @@ public class ConnectivityManager { public static final int TETHER_ERROR_IFACE_CFG_ERROR = TetheringManager.TETHER_ERROR_IFACE_CFG_ERROR; /** - * @deprecated Use {@link TetheringManager#TETHER_ERROR_PROVISION_FAILED}. + * @deprecated Use {@link TetheringManager#TETHER_ERROR_PROVISIONING_FAILED}. * {@hide} */ @SystemApi @Deprecated public static final int TETHER_ERROR_PROVISION_FAILED = - TetheringManager.TETHER_ERROR_PROVISION_FAILED; + TetheringManager.TETHER_ERROR_PROVISIONING_FAILED; /** * @deprecated Use {@link TetheringManager#TETHER_ERROR_DHCPSERVER_ERROR}. * {@hide} @@ -2880,7 +2881,14 @@ public class ConnectivityManager { @UnsupportedAppUsage @Deprecated public int getLastTetherError(String iface) { - return mTetheringManager.getLastTetherError(iface); + int error = mTetheringManager.getLastTetherError(iface); + if (error == TetheringManager.TETHER_ERROR_UNKNOWN_TYPE) { + // TETHER_ERROR_UNKNOWN_TYPE was introduced with TetheringManager and has never been + // returned by ConnectivityManager. Convert it to the legacy TETHER_ERROR_UNKNOWN_IFACE + // instead. + error = TetheringManager.TETHER_ERROR_UNKNOWN_IFACE; + } + return error; } /** @hide */ From 38ef330e46375578ba7f5d5e1c39c1c76fa2e6d1 Mon Sep 17 00:00:00 2001 From: markchien Date: Wed, 18 Mar 2020 21:16:15 +0800 Subject: [PATCH 6/7] Move NetworkCallback to last parameter for new exposed requestNetwork Bug: 151243698 Test: atest TetheringTests Change-Id: I87ef1d451eefa6998b9793c4eacabae978376d24 --- .../java/android/net/ConnectivityManager.java | 30 +++++++++++-------- .../android/server/ConnectivityService.java | 3 ++ 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 99813c0988..b8f5890b32 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -3685,29 +3685,29 @@ public class ConnectivityManager { /** * Helper function to request a network with a particular legacy type. * - * @deprecated This is temporarily public for tethering to backwards compatibility that uses - * the NetworkRequest API to request networks with legacy type and relies on - * CONNECTIVITY_ACTION broadcasts instead of NetworkCallbacks. New caller should use + * This API is only for use in internal system code that requests networks with legacy type and + * relies on CONNECTIVITY_ACTION broadcasts instead of NetworkCallbacks. New caller should use * {@link #requestNetwork(NetworkRequest, NetworkCallback, Handler)} instead. * - * TODO: update said system code to rely on NetworkCallbacks and make this method private. - * @param request {@link NetworkRequest} describing this request. - * @param networkCallback The {@link NetworkCallback} to be utilized for this request. Note - * the callback must not be shared - it uniquely specifies this request. * @param timeoutMs The time in milliseconds to attempt looking for a suitable network * before {@link NetworkCallback#onUnavailable()} is called. The timeout must * be a positive value (i.e. >0). * @param legacyType to specify the network type(#TYPE_*). * @param handler {@link Handler} to specify the thread upon which the callback will be invoked. + * @param networkCallback The {@link NetworkCallback} to be utilized for this request. Note + * the callback must not be shared - it uniquely specifies this request. * * @hide */ @SystemApi - @Deprecated + @RequiresPermission(NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK) public void requestNetwork(@NonNull NetworkRequest request, - @NonNull NetworkCallback networkCallback, int timeoutMs, int legacyType, - @NonNull Handler handler) { + int timeoutMs, int legacyType, @NonNull Handler handler, + @NonNull NetworkCallback networkCallback) { + if (legacyType == TYPE_NONE) { + throw new IllegalArgumentException("TYPE_NONE is meaningless legacy type"); + } CallbackHandler cbHandler = new CallbackHandler(handler); NetworkCapabilities nc = request.networkCapabilities; sendRequestForNetwork(nc, networkCallback, timeoutMs, REQUEST, legacyType, cbHandler); @@ -3806,7 +3806,8 @@ public class ConnectivityManager { public void requestNetwork(@NonNull NetworkRequest request, @NonNull NetworkCallback networkCallback, @NonNull Handler handler) { CallbackHandler cbHandler = new CallbackHandler(handler); - requestNetwork(request, networkCallback, 0, TYPE_NONE, cbHandler); + NetworkCapabilities nc = request.networkCapabilities; + sendRequestForNetwork(nc, networkCallback, 0, REQUEST, TYPE_NONE, cbHandler); } /** @@ -3839,7 +3840,9 @@ public class ConnectivityManager { public void requestNetwork(@NonNull NetworkRequest request, @NonNull NetworkCallback networkCallback, int timeoutMs) { checkTimeout(timeoutMs); - requestNetwork(request, networkCallback, timeoutMs, TYPE_NONE, getDefaultHandler()); + NetworkCapabilities nc = request.networkCapabilities; + sendRequestForNetwork(nc, networkCallback, timeoutMs, REQUEST, TYPE_NONE, + getDefaultHandler()); } /** @@ -3865,7 +3868,8 @@ public class ConnectivityManager { @NonNull NetworkCallback networkCallback, @NonNull Handler handler, int timeoutMs) { checkTimeout(timeoutMs); CallbackHandler cbHandler = new CallbackHandler(handler); - requestNetwork(request, networkCallback, timeoutMs, TYPE_NONE, cbHandler); + NetworkCapabilities nc = request.networkCapabilities; + sendRequestForNetwork(nc, networkCallback, timeoutMs, REQUEST, TYPE_NONE, cbHandler); } /** diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 91dd1d59c6..75e073ac36 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -5400,6 +5400,9 @@ public class ConnectivityService extends IConnectivityManager.Stub public NetworkRequest requestNetwork(NetworkCapabilities networkCapabilities, Messenger messenger, int timeoutMs, IBinder binder, int legacyType, @NonNull String callingPackageName) { + if (legacyType != TYPE_NONE && !checkNetworkStackPermission()) { + throw new SecurityException("Insufficient permissions to specify legacy type"); + } final int callingUid = Binder.getCallingUid(); final NetworkRequest.Type type = (networkCapabilities == null) ? NetworkRequest.Type.TRACK_DEFAULT From 4574935dca1523ca87094055f0a0fe3bb15eae04 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Thu, 19 Mar 2020 11:46:55 +0000 Subject: [PATCH 7/7] Make Ethernet interfaces more testable. This CL adds a setIncludeTestInterfaces method to EthernetManager that, when called, causes the Ethernet service to recognize and manage test interfaces created by TestNetworkManager. Bug: 150644681 Test: Tested by EthernetTetheringTest in same topic Change-Id: I86eef7a93267f800dbfc8eafd307effa76a344ca Merged-In: I86eef7a93267f800dbfc8eafd307effa76a344ca (cherry picked from commit 3410fb0aa92bbd4f9d7dc031e89f6f528ff34245) --- core/java/android/net/TestNetworkManager.java | 12 ++++++++++++ .../java/com/android/server/TestNetworkService.java | 5 +++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/core/java/android/net/TestNetworkManager.java b/core/java/android/net/TestNetworkManager.java index c3284df397..a0a563b370 100644 --- a/core/java/android/net/TestNetworkManager.java +++ b/core/java/android/net/TestNetworkManager.java @@ -30,6 +30,18 @@ import com.android.internal.util.Preconditions; */ @TestApi public class TestNetworkManager { + /** + * Prefix for tun interfaces created by this class. + * @hide + */ + public static final String TEST_TUN_PREFIX = "testtun"; + + /** + * Prefix for tap interfaces created by this class. + * @hide + */ + public static final String TEST_TAP_PREFIX = "testtap"; + @NonNull private static final String TAG = TestNetworkManager.class.getSimpleName(); @NonNull private final ITestNetworkManager mService; diff --git a/services/core/java/com/android/server/TestNetworkService.java b/services/core/java/com/android/server/TestNetworkService.java index 81a1372eb1..0ea73460e1 100644 --- a/services/core/java/com/android/server/TestNetworkService.java +++ b/services/core/java/com/android/server/TestNetworkService.java @@ -16,6 +16,9 @@ package com.android.server; +import static android.net.TestNetworkManager.TEST_TAP_PREFIX; +import static android.net.TestNetworkManager.TEST_TUN_PREFIX; + import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; @@ -60,8 +63,6 @@ import java.util.concurrent.atomic.AtomicInteger; class TestNetworkService extends ITestNetworkManager.Stub { @NonNull private static final String TAG = TestNetworkService.class.getSimpleName(); @NonNull private static final String TEST_NETWORK_TYPE = "TEST_NETWORK"; - @NonNull private static final String TEST_TUN_PREFIX = "testtun"; - @NonNull private static final String TEST_TAP_PREFIX = "testtap"; @NonNull private static final AtomicInteger sTestTunIndex = new AtomicInteger(); @NonNull private final Context mContext;