From 3ae5bdab6eab95febb4886ca51ef260f9783d006 Mon Sep 17 00:00:00 2001 From: Aaron Huang Date: Mon, 26 Oct 2020 17:34:50 +0800 Subject: [PATCH 1/2] Add a new API in BatteryStatsManager for connectivity service Connectivity service(CS) is going to be a mainline module and currently it uses IBatteryStats to communicate with battery stats service directly. CS cannot use this way after becomes a mainline module so add new system APIs to communicate with battery stats service through BatteryStatsManger. The new API reportNetworkInterfaceForTransports requires system-only permission NETWORK_STACK or MAINLINE_NETWORK_STACK so that given the only caller was in ConnectivityService. This change only add new surface, the usage in CS will be updated in follow-up CL. Bug: 171686421 Test: builds, boot, mobile and wifi work. Change-Id: Ifbd4a9784ed7664751e69d530f8204e292fa6b39 Merged-In: Ifbd4a9784ed7664751e69d530f8204e292fa6b39 --- core/api/module-lib-current.txt | 4 +++ core/java/android/os/BatteryStatsManager.java | 26 +++++++++++++++++-- .../server/am/BatteryStatsService.java | 3 ++- 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/core/api/module-lib-current.txt b/core/api/module-lib-current.txt index 3529858a0c959..b3562e5f496d2 100644 --- a/core/api/module-lib-current.txt +++ b/core/api/module-lib-current.txt @@ -56,6 +56,10 @@ package android.net { package android.os { + public final class BatteryStatsManager { + method @RequiresPermission(anyOf={android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, android.Manifest.permission.NETWORK_STACK}) public void reportNetworkInterfaceForTransports(@NonNull String, @NonNull int[]) throws java.lang.RuntimeException; + } + public class Binder implements android.os.IBinder { method public final void markVintfStability(); } diff --git a/core/java/android/os/BatteryStatsManager.java b/core/java/android/os/BatteryStatsManager.java index 258e58d7d0198..3f4a21878687f 100644 --- a/core/java/android/os/BatteryStatsManager.java +++ b/core/java/android/os/BatteryStatsManager.java @@ -24,6 +24,7 @@ import android.annotation.RequiresPermission; import android.annotation.SystemApi; import android.annotation.SystemService; import android.content.Context; +import android.net.NetworkStack; import android.os.connectivity.CellularBatteryStats; import android.os.connectivity.WifiBatteryStats; import android.telephony.DataConnectionRealTimeInfo; @@ -416,10 +417,31 @@ public final class BatteryStatsManager { } } + /** + * Notifies the battery stats of a new interface, and the transport types of the network that + * includes that interface. + * + * @param iface The interface of the network. + * @param transportTypes The transport type of the network {@link Transport}. + * @hide + */ + @SystemApi(client = SystemApi.Client.MODULE_LIBRARIES) + @RequiresPermission(anyOf = { + NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, + android.Manifest.permission.NETWORK_STACK}) + public void reportNetworkInterfaceForTransports(@NonNull String iface, + @NonNull int[] transportTypes) throws RuntimeException { + try { + mBatteryStats.noteNetworkInterfaceForTransports(iface, transportTypes); + } catch (RemoteException e) { + e.rethrowFromSystemServer(); + } + } + private static int getDataConnectionPowerState(boolean isActive) { // TODO: DataConnectionRealTimeInfo is under telephony package but the constants are used - // for both Wifi and mobile. It would make more sense to separate the constants to a generic - // class or move it to generic package. + // for both Wifi and mobile. It would make more sense to separate the constants to a + // generic class or move it to generic package. return isActive ? DataConnectionRealTimeInfo.DC_POWER_STATE_HIGH : DataConnectionRealTimeInfo.DC_POWER_STATE_LOW; } diff --git a/services/core/java/com/android/server/am/BatteryStatsService.java b/services/core/java/com/android/server/am/BatteryStatsService.java index 146d301dba65d..85d7f1cec83fe 100644 --- a/services/core/java/com/android/server/am/BatteryStatsService.java +++ b/services/core/java/com/android/server/am/BatteryStatsService.java @@ -73,6 +73,7 @@ import com.android.internal.util.DumpUtils; import com.android.internal.util.FrameworkStatsLog; import com.android.internal.util.ParseUtils; import com.android.net.module.util.NetworkCapabilitiesUtils; +import com.android.net.module.util.PermissionUtils; import com.android.server.LocalServices; import com.android.server.connectivity.DataConnectionStats; import com.android.server.net.BaseNetworkObserver; @@ -1103,7 +1104,7 @@ public final class BatteryStatsService extends IBatteryStats.Stub @Override public void noteNetworkInterfaceForTransports(final String iface, int[] transportTypes) { - enforceCallingPermission(); + PermissionUtils.enforceNetworkStackPermission(mContext); mStats.noteNetworkInterfaceForTransports(iface, transportTypes); } From d394db6b4fb06e595fa3d34635b39fbc8f5f089a Mon Sep 17 00:00:00 2001 From: Aaron Huang Date: Tue, 27 Oct 2020 03:36:19 +0800 Subject: [PATCH 2/2] Communicate with BatteryStatsService through BatteryStatsManager Mainline connectivity service is only allowed to use formal APIs. Use new system API in BatteryStatsManager instead of calling BatteryStatsService directly. Bug: 171686421 Test: FrameworksNetTests Change-Id: I1d1f0d2d456003e842ad77519667c0532995610d Merged-In: I1d1f0d2d456003e842ad77519667c0532995610d --- .../android/server/ConnectivityService.java | 14 +++++++++++-- .../server/ConnectivityServiceTest.java | 21 ++++++++++++------- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 0b7dbdebce32f..3adb17779cb44 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1038,6 +1038,16 @@ public class ConnectivityService extends IConnectivityManager.Stub public IBatteryStats getBatteryStatsService() { return BatteryStatsService.getService(); } + + /** + * @see BatteryStatsManager + */ + public void reportNetworkInterfaceForTransports(Context context, String iface, + int[] transportTypes) { + final BatteryStatsManager batteryStats = + context.getSystemService(BatteryStatsManager.class); + batteryStats.reportNetworkInterfaceForTransports(iface, transportTypes); + } } public ConnectivityService(Context context) { @@ -6275,13 +6285,13 @@ public class ConnectivityService extends IConnectivityManager.Stub oldLp != null ? oldLp.getAllInterfaceNames() : null, newLp != null ? newLp.getAllInterfaceNames() : null); if (!interfaceDiff.added.isEmpty()) { - final IBatteryStats bs = mDeps.getBatteryStatsService(); for (final String iface : interfaceDiff.added) { try { if (DBG) log("Adding iface " + iface + " to network " + netId); mNetd.networkAddInterface(netId, iface); wakeupModifyInterface(iface, caps, true); - bs.noteNetworkInterfaceForTransports(iface, caps.getTransportTypes()); + mDeps.reportNetworkInterfaceForTransports(mContext, iface, + caps.getTransportTypes()); } catch (Exception e) { loge("Exception adding interface: " + e); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index d5580e08d4daf..86adf5debfc13 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -7803,7 +7803,8 @@ public class ConnectivityServiceTest { mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, cellLp); mCellNetworkAgent.connect(true); waitForIdle(); - verify(mBatteryStatsService).noteNetworkInterfaceForTransports(cellLp.getInterfaceName(), + verify(mDeps).reportNetworkInterfaceForTransports(mServiceContext, + cellLp.getInterfaceName(), new int[] { TRANSPORT_CELLULAR }); reset(mBatteryStatsService); @@ -7812,7 +7813,8 @@ public class ConnectivityServiceTest { mWiFiNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI, wifiLp); mWiFiNetworkAgent.connect(true); waitForIdle(); - verify(mBatteryStatsService).noteNetworkInterfaceForTransports(wifiLp.getInterfaceName(), + verify(mDeps).reportNetworkInterfaceForTransports(mServiceContext, + wifiLp.getInterfaceName(), new int[] { TRANSPORT_WIFI }); reset(mBatteryStatsService); @@ -7823,7 +7825,8 @@ public class ConnectivityServiceTest { mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, cellLp); mCellNetworkAgent.connect(true); waitForIdle(); - verify(mBatteryStatsService).noteNetworkInterfaceForTransports(cellLp.getInterfaceName(), + verify(mDeps).reportNetworkInterfaceForTransports(mServiceContext, + cellLp.getInterfaceName(), new int[] { TRANSPORT_CELLULAR }); mCellNetworkAgent.disconnect(); } @@ -7907,7 +7910,8 @@ public class ConnectivityServiceTest { assertRoutesAdded(cellNetId, ipv6Subnet, defaultRoute); verify(mMockDnsResolver, times(1)).createNetworkCache(eq(cellNetId)); verify(mMockNetd, times(1)).networkAddInterface(cellNetId, MOBILE_IFNAME); - verify(mBatteryStatsService).noteNetworkInterfaceForTransports(cellLp.getInterfaceName(), + verify(mDeps).reportNetworkInterfaceForTransports(mServiceContext, + cellLp.getInterfaceName(), new int[] { TRANSPORT_CELLULAR }); networkCallback.expectAvailableThenValidatedCallbacks(mCellNetworkAgent); @@ -7928,8 +7932,8 @@ public class ConnectivityServiceTest { // Make sure BatteryStats was not told about any v4- interfaces, as none should have // come online yet. waitForIdle(); - verify(mBatteryStatsService, never()).noteNetworkInterfaceForTransports(startsWith("v4-"), - any()); + verify(mDeps, never()) + .reportNetworkInterfaceForTransports(eq(mServiceContext), startsWith("v4-"), any()); verifyNoMoreInteractions(mMockNetd); verifyNoMoreInteractions(mMockDnsResolver); @@ -7981,8 +7985,9 @@ public class ConnectivityServiceTest { assertTrue(ArrayUtils.contains(resolvrParams.servers, "8.8.8.8")); for (final LinkProperties stackedLp : stackedLpsAfterChange) { - verify(mBatteryStatsService).noteNetworkInterfaceForTransports( - stackedLp.getInterfaceName(), new int[] { TRANSPORT_CELLULAR }); + verify(mDeps).reportNetworkInterfaceForTransports( + mServiceContext, stackedLp.getInterfaceName(), + new int[] { TRANSPORT_CELLULAR }); } reset(mMockNetd); when(mMockNetd.interfaceGetCfg(CLAT_PREFIX + MOBILE_IFNAME))