From 0d72b183492de986030bb2f923b7e437068c4730 Mon Sep 17 00:00:00 2001 From: junyulai Date: Wed, 3 Mar 2021 21:28:08 +0800 Subject: [PATCH 1/2] [FUI24] Use getAllNetworkStateSnapshot in NetworkPolicyManagerService Currently, in NPMS, getAllNetworkState is used to get all information of all connected networks. However, this will not be available after ConnectivityService goes to mainline module. Thus, replace the usage with new API. Test: atest NetworkPolicyManagerServiceTest Bug: 174123988 Change-Id: I99bca71cb3b98f74eab036fac4517ee926eb539c --- .../net/NetworkPolicyManagerService.java | 57 ++++++++----------- .../net/NetworkPolicyManagerServiceTest.java | 47 +++++++-------- 2 files changed, 49 insertions(+), 55 deletions(-) diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java index 2f80470354008..1d68b7cd2caf1 100644 --- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java +++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java @@ -170,7 +170,7 @@ import android.net.NetworkPolicyManager.UidState; import android.net.NetworkRequest; import android.net.NetworkSpecifier; import android.net.NetworkStack; -import android.net.NetworkState; +import android.net.NetworkStateSnapshot; import android.net.NetworkStats; import android.net.NetworkTemplate; import android.net.TelephonyNetworkSpecifier; @@ -1887,14 +1887,14 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { } /** - * Collect all ifaces from a {@link NetworkState} into the given set. + * Collect all ifaces from a {@link NetworkStateSnapshot} into the given set. */ - private static void collectIfaces(ArraySet ifaces, NetworkState state) { - final String baseIface = state.linkProperties.getInterfaceName(); + private static void collectIfaces(ArraySet ifaces, NetworkStateSnapshot snapshot) { + final String baseIface = snapshot.linkProperties.getInterfaceName(); if (baseIface != null) { ifaces.add(baseIface); } - for (LinkProperties stackedLink : state.linkProperties.getStackedLinks()) { + for (LinkProperties stackedLink : snapshot.linkProperties.getStackedLinks()) { final String stackedIface = stackedLink.getInterfaceName(); if (stackedIface != null) { ifaces.add(stackedIface); @@ -1964,7 +1964,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { } /** - * Examine all connected {@link NetworkState}, looking for + * Examine all connected {@link NetworkStateSnapshot}, looking for * {@link NetworkPolicy} that need to be enforced. When matches found, set * remaining quota based on usage cycle and historical stats. */ @@ -1973,29 +1973,27 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { if (LOGV) Slog.v(TAG, "updateNetworkRulesNL()"); Trace.traceBegin(TRACE_TAG_NETWORK, "updateNetworkRulesNL"); - final NetworkState[] states; + final List snapshots; try { - states = defeatNullable(mConnManager.getAllNetworkState()); + snapshots = mConnManager.getAllNetworkStateSnapshot(); } catch (RemoteException e) { - // ignored; service lives in system_server + // Cannot happen, service lives in system_server. return; } // First, generate identities of all connected networks so we can // quickly compare them against all defined policies below. mNetIdToSubId.clear(); - final ArrayMap identified = new ArrayMap<>(); - for (NetworkState state : states) { - if (state.network != null) { - mNetIdToSubId.put(state.network.netId, parseSubId(state)); - } + final ArrayMap identified = new ArrayMap<>(); + for (final NetworkStateSnapshot snapshot : snapshots) { + mNetIdToSubId.put(snapshot.network.netId, parseSubId(snapshot)); // Policies matched by NPMS only match by subscriber ID or by ssid. Thus subtype // in the object created here is never used and its value doesn't matter, so use // NETWORK_TYPE_UNKNOWN. - final NetworkIdentity ident = NetworkIdentity.buildNetworkIdentity(mContext, state, + final NetworkIdentity ident = NetworkIdentity.buildNetworkIdentity(mContext, snapshot, true, TelephonyManager.NETWORK_TYPE_UNKNOWN /* subType */); - identified.put(state, ident); + identified.put(snapshot, ident); } final ArraySet newMeteredIfaces = new ArraySet<>(); @@ -2069,10 +2067,10 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { // One final pass to catch any metered ifaces that don't have explicitly // defined policies; typically Wi-Fi networks. - for (NetworkState state : states) { - if (!state.networkCapabilities.hasCapability(NET_CAPABILITY_NOT_METERED)) { + for (final NetworkStateSnapshot snapshot : snapshots) { + if (!snapshot.networkCapabilities.hasCapability(NET_CAPABILITY_NOT_METERED)) { matchingIfaces.clear(); - collectIfaces(matchingIfaces, state); + collectIfaces(matchingIfaces, snapshot); for (int j = matchingIfaces.size() - 1; j >= 0; j--) { final String iface = matchingIfaces.valueAt(j); if (!newMeteredIfaces.contains(iface)) { @@ -2104,16 +2102,16 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { // Finally, calculate our opportunistic quotas mSubscriptionOpportunisticQuota.clear(); - for (NetworkState state : states) { + for (final NetworkStateSnapshot snapshot : snapshots) { if (!quotaEnabled) continue; - if (state.network == null) continue; - final int subId = getSubIdLocked(state.network); + if (snapshot.network == null) continue; + final int subId = getSubIdLocked(snapshot.network); final SubscriptionPlan plan = getPrimarySubscriptionPlanLocked(subId); if (plan == null) continue; final long quotaBytes; final long limitBytes = plan.getDataLimitBytes(); - if (!state.networkCapabilities.hasCapability(NET_CAPABILITY_NOT_ROAMING)) { + if (!snapshot.networkCapabilities.hasCapability(NET_CAPABILITY_NOT_ROAMING)) { // Clamp to 0 when roaming quotaBytes = 0; } else if (limitBytes == SubscriptionPlan.BYTES_UNKNOWN) { @@ -2131,7 +2129,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { .truncatedTo(ChronoUnit.DAYS) .toInstant().toEpochMilli(); final long totalBytes = getTotalBytes( - NetworkTemplate.buildTemplateMobileAll(state.subscriberId), + NetworkTemplate.buildTemplateMobileAll(snapshot.subscriberId), start, startOfDay); final long remainingBytes = limitBytes - totalBytes; // Number of remaining days including current day @@ -5628,11 +5626,10 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { } } - private int parseSubId(NetworkState state) { + private int parseSubId(@NonNull NetworkStateSnapshot snapshot) { int subId = INVALID_SUBSCRIPTION_ID; - if (state != null && state.networkCapabilities != null - && state.networkCapabilities.hasTransport(TRANSPORT_CELLULAR)) { - NetworkSpecifier spec = state.networkCapabilities.getNetworkSpecifier(); + if (snapshot.networkCapabilities.hasTransport(TRANSPORT_CELLULAR)) { + NetworkSpecifier spec = snapshot.networkCapabilities.getNetworkSpecifier(); if (spec instanceof TelephonyNetworkSpecifier) { subId = ((TelephonyNetworkSpecifier) spec).getSubscriptionId(); } @@ -5709,10 +5706,6 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { return (uidRules & rule) != 0; } - private static @NonNull NetworkState[] defeatNullable(@Nullable NetworkState[] val) { - return (val != null) ? val : new NetworkState[0]; - } - private static boolean getBooleanDefeatingNullable(@Nullable PersistableBundle bundle, String key, boolean defaultValue) { return (bundle != null) ? bundle.getBoolean(key, defaultValue) : defaultValue; diff --git a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java index 13c3919cefc5e..163c7014169ba 100644 --- a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java @@ -115,7 +115,7 @@ import android.net.LinkProperties; import android.net.Network; import android.net.NetworkCapabilities; import android.net.NetworkPolicy; -import android.net.NetworkState; +import android.net.NetworkStateSnapshot; import android.net.NetworkStats; import android.net.NetworkStatsHistory; import android.net.NetworkTemplate; @@ -1072,7 +1072,7 @@ public class NetworkPolicyManagerServiceTest { @FlakyTest @Test public void testNetworkPolicyAppliedCycleLastMonth() throws Exception { - NetworkState[] state = null; + List snapshots = null; NetworkStats stats = null; final int CYCLE_DAY = 15; @@ -1084,8 +1084,8 @@ public class NetworkPolicyManagerServiceTest { // first, pretend that wifi network comes online. no policy active, // which means we shouldn't push limit to interface. - state = new NetworkState[] { buildWifi() }; - when(mConnManager.getAllNetworkState()).thenReturn(state); + snapshots = List.of(buildWifi()); + when(mConnManager.getAllNetworkStateSnapshot()).thenReturn(snapshots); mPolicyListener.expect().onMeteredIfacesChanged(any()); mServiceContext.sendBroadcast(new Intent(CONNECTIVITY_ACTION)); @@ -1093,7 +1093,7 @@ public class NetworkPolicyManagerServiceTest { // now change cycle to be on 15th, and test in early march, to verify we // pick cycle day in previous month. - when(mConnManager.getAllNetworkState()).thenReturn(state); + when(mConnManager.getAllNetworkStateSnapshot()).thenReturn(snapshots); // pretend that 512 bytes total have happened stats = new NetworkStats(getElapsedRealtime(), 1) @@ -1339,7 +1339,7 @@ public class NetworkPolicyManagerServiceTest { @Test public void testMeteredNetworkWithoutLimit() throws Exception { - NetworkState[] state = null; + List snapshots = null; NetworkStats stats = null; final long TIME_FEB_15 = 1171497600000L; @@ -1349,12 +1349,12 @@ public class NetworkPolicyManagerServiceTest { setCurrentTimeMillis(TIME_MAR_10); // bring up wifi network with metered policy - state = new NetworkState[] { buildWifi() }; + snapshots = List.of(buildWifi()); stats = new NetworkStats(getElapsedRealtime(), 1) .insertEntry(TEST_IFACE, 0L, 0L, 0L, 0L); { - when(mConnManager.getAllNetworkState()).thenReturn(state); + when(mConnManager.getAllNetworkStateSnapshot()).thenReturn(snapshots); when(mStatsService.getNetworkTotalBytes(sTemplateWifi, TIME_FEB_15, currentTimeMillis())).thenReturn(stats.getTotalBytes()); @@ -1477,7 +1477,8 @@ public class NetworkPolicyManagerServiceTest { } private PersistableBundle setupUpdateMobilePolicyCycleTests() throws RemoteException { - when(mConnManager.getAllNetworkState()).thenReturn(new NetworkState[0]); + when(mConnManager.getAllNetworkStateSnapshot()) + .thenReturn(new ArrayList()); setupTelephonySubscriptionManagers(FAKE_SUB_ID, FAKE_SUBSCRIBER_ID); @@ -1489,7 +1490,8 @@ public class NetworkPolicyManagerServiceTest { @Test public void testUpdateMobilePolicyCycleWithNullConfig() throws RemoteException { - when(mConnManager.getAllNetworkState()).thenReturn(new NetworkState[0]); + when(mConnManager.getAllNetworkStateSnapshot()) + .thenReturn(new ArrayList()); setupTelephonySubscriptionManagers(FAKE_SUB_ID, FAKE_SUBSCRIBER_ID); @@ -1720,7 +1722,7 @@ public class NetworkPolicyManagerServiceTest { reset(mTelephonyManager, mNetworkManager, mNotifManager); expectMobileDefaults(); - expectNetworkState(true /* roaming */); + expectNetworkStateSnapshot(true /* roaming */); mService.updateNetworks(); @@ -1749,7 +1751,7 @@ public class NetworkPolicyManagerServiceTest { // Capabilities change to roaming final ConnectivityManager.NetworkCallback callback = mNetworkCallbackCaptor.getValue(); assertNotNull(callback); - expectNetworkState(true /* roaming */); + expectNetworkStateSnapshot(true /* roaming */); callback.onCapabilitiesChanged( new Network(TEST_NET_ID), buildNetworkCapabilities(TEST_SUB_ID, true /* roaming */)); @@ -2035,14 +2037,14 @@ public class NetworkPolicyManagerServiceTest { mService.setNetworkPolicies(policies); } - private static NetworkState buildWifi() { + private static NetworkStateSnapshot buildWifi() { final LinkProperties prop = new LinkProperties(); prop.setInterfaceName(TEST_IFACE); final NetworkCapabilities networkCapabilities = new NetworkCapabilities(); networkCapabilities.addTransportType(TRANSPORT_WIFI); networkCapabilities.setSSID(TEST_SSID); - return new NetworkState(TYPE_WIFI, prop, networkCapabilities, new Network(TEST_NET_ID), - null); + return new NetworkStateSnapshot(new Network(TEST_NET_ID), networkCapabilities, prop, + null /*subscriberId*/, TYPE_WIFI); } private void expectHasInternetPermission(int uid, boolean hasIt) throws Exception { @@ -2059,15 +2061,14 @@ public class NetworkPolicyManagerServiceTest { PackageManager.PERMISSION_DENIED); } - private void expectNetworkState(boolean roaming) throws Exception { + private void expectNetworkStateSnapshot(boolean roaming) throws Exception { when(mCarrierConfigManager.getConfigForSubId(eq(TEST_SUB_ID))) .thenReturn(mCarrierConfig); - when(mConnManager.getAllNetworkState()).thenReturn(new NetworkState[] { - new NetworkState(TYPE_MOBILE, - buildLinkProperties(TEST_IFACE), - buildNetworkCapabilities(TEST_SUB_ID, roaming), - new Network(TEST_NET_ID), TEST_IMSI) - }); + List snapshots = List.of(new NetworkStateSnapshot( + new Network(TEST_NET_ID), + buildNetworkCapabilities(TEST_SUB_ID, roaming), + buildLinkProperties(TEST_IFACE), TEST_IMSI, TYPE_MOBILE)); + when(mConnManager.getAllNetworkStateSnapshot()).thenReturn(snapshots); } private void expectDefaultCarrierConfig() throws Exception { @@ -2078,7 +2079,7 @@ public class NetworkPolicyManagerServiceTest { private TelephonyManager expectMobileDefaults() throws Exception { TelephonyManager tmSub = setupTelephonySubscriptionManagers(TEST_SUB_ID, TEST_IMSI); doNothing().when(tmSub).setPolicyDataEnabled(anyBoolean()); - expectNetworkState(false /* roaming */); + expectNetworkStateSnapshot(false /* roaming */); return tmSub; } From a76fc32242f9e02a4e50e875570e918e5eb52e25 Mon Sep 17 00:00:00 2001 From: junyulai Date: Wed, 3 Mar 2021 22:42:21 +0800 Subject: [PATCH 2/2] [FUI25] Migrate NetworkPolicyManagerService to use ConnectivityManager Test: atest NetworkPolicyManagerServiceTest Bug: 174123988 Change-Id: I6512d3f704f35f2cc3555dcc40b46c7294d3b455 --- .../server/net/NetworkPolicyManagerService.java | 17 ++++++----------- .../java/com/android/server/SystemServer.java | 7 +------ .../net/NetworkPolicyManagerServiceTest.java | 10 ++++------ 3 files changed, 11 insertions(+), 23 deletions(-) diff --git a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java index 1d68b7cd2caf1..aee0947f39f9a 100644 --- a/services/core/java/com/android/server/net/NetworkPolicyManagerService.java +++ b/services/core/java/com/android/server/net/NetworkPolicyManagerService.java @@ -431,7 +431,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { private final CarrierConfigManager mCarrierConfigManager; private final MultipathPolicyTracker mMultipathPolicyTracker; - private IConnectivityManager mConnManager; + private ConnectivityManager mConnManager; private PowerManagerInternal mPowerManagerInternal; private PowerWhitelistManager mPowerWhitelistManager; @@ -711,8 +711,9 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { new NetworkPolicyManagerInternalImpl()); } - public void bindConnectivityManager(IConnectivityManager connManager) { - mConnManager = Objects.requireNonNull(connManager, "missing IConnectivityManager"); + public void bindConnectivityManager() { + mConnManager = Objects.requireNonNull(mContext.getSystemService(ConnectivityManager.class), + "missing ConnectivityManager"); } @GuardedBy("mUidRulesFirstLock") @@ -943,7 +944,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { mContext.registerReceiver(mCarrierConfigReceiver, carrierConfigFilter, null, mHandler); // listen for meteredness changes - mContext.getSystemService(ConnectivityManager.class).registerNetworkCallback( + mConnManager.registerNetworkCallback( new NetworkRequest.Builder().build(), mNetworkCallback); mAppStandby.addListener(new NetPolicyAppIdleStateChangeListener()); @@ -1973,13 +1974,7 @@ public class NetworkPolicyManagerService extends INetworkPolicyManager.Stub { if (LOGV) Slog.v(TAG, "updateNetworkRulesNL()"); Trace.traceBegin(TRACE_TAG_NETWORK, "updateNetworkRulesNL"); - final List snapshots; - try { - snapshots = mConnManager.getAllNetworkStateSnapshot(); - } catch (RemoteException e) { - // Cannot happen, service lives in system_server. - return; - } + final List snapshots = mConnManager.getAllNetworkStateSnapshot(); // First, generate identities of all connected networks so we can // quickly compare them against all defined policies below. diff --git a/services/java/com/android/server/SystemServer.java b/services/java/com/android/server/SystemServer.java index 18c7e128b04e0..a6a99f232ef45 100644 --- a/services/java/com/android/server/SystemServer.java +++ b/services/java/com/android/server/SystemServer.java @@ -51,7 +51,6 @@ import android.graphics.GraphicsStatsService; import android.hardware.display.DisplayManagerInternal; import android.net.ConnectivityManager; import android.net.ConnectivityModuleConnector; -import android.net.IConnectivityManager; import android.net.NetworkStackClient; import android.os.BaseBundle; import android.os.Binder; @@ -1107,7 +1106,6 @@ public final class SystemServer { VcnManagementService vcnManagement = null; NetworkStatsService networkStats = null; NetworkPolicyManagerService networkPolicy = null; - IConnectivityManager connectivity = null; NsdService serviceDiscovery = null; WindowManagerService wm = null; SerialService serial = null; @@ -1631,10 +1629,7 @@ public final class SystemServer { // services to initialize. mSystemServiceManager.startServiceFromJar(CONNECTIVITY_SERVICE_INITIALIZER_CLASS, CONNECTIVITY_SERVICE_APEX_PATH); - connectivity = IConnectivityManager.Stub.asInterface( - ServiceManager.getService(Context.CONNECTIVITY_SERVICE)); - // TODO: Use ConnectivityManager instead of ConnectivityService. - networkPolicy.bindConnectivityManager(connectivity); + networkPolicy.bindConnectivityManager(); t.traceEnd(); t.traceBegin("StartVpnManagerService"); diff --git a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java index 163c7014169ba..fb01ff6e16c64 100644 --- a/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/net/NetworkPolicyManagerServiceTest.java @@ -108,7 +108,6 @@ import android.content.pm.PackageManager; import android.content.pm.Signature; import android.content.pm.UserInfo; import android.net.ConnectivityManager; -import android.net.IConnectivityManager; import android.net.INetworkManagementEventObserver; import android.net.INetworkPolicyListener; import android.net.LinkProperties; @@ -242,8 +241,7 @@ public class NetworkPolicyManagerServiceTest { private @Mock IActivityManager mActivityManager; private @Mock INetworkManagementService mNetworkManager; - private @Mock IConnectivityManager mConnManager; - private @Mock ConnectivityManager mConnectivityManager; + private @Mock ConnectivityManager mConnManager; private @Mock NotificationManager mNotifManager; private @Mock PackageManager mPackageManager; private @Mock IPackageManager mIpm; @@ -361,7 +359,7 @@ public class NetworkPolicyManagerServiceTest { case Context.NOTIFICATION_SERVICE: return mNotifManager; case Context.CONNECTIVITY_SERVICE: - return mConnectivityManager; + return mConnManager; case Context.USER_SERVICE: return mUserManager; default: @@ -390,7 +388,7 @@ public class NetworkPolicyManagerServiceTest { mFutureIntent = newRestrictBackgroundChangedFuture(); mService = new NetworkPolicyManagerService(mServiceContext, mActivityManager, mNetworkManager, mIpm, mClock, mPolicyDir, true); - mService.bindConnectivityManager(mConnManager); + mService.bindConnectivityManager(); mPolicyListener = new NetworkPolicyListenerAnswer(mService); // Sets some common expectations. @@ -429,7 +427,7 @@ public class NetworkPolicyManagerServiceTest { when(mUserManager.getUsers()).thenReturn(buildUserInfoList()); when(mNetworkManager.isBandwidthControlEnabled()).thenReturn(true); when(mNetworkManager.setDataSaverModeEnabled(anyBoolean())).thenReturn(true); - doNothing().when(mConnectivityManager) + doNothing().when(mConnManager) .registerNetworkCallback(any(), mNetworkCallbackCaptor.capture()); // Create the expected carrier config