From e953a50e6b586b1974233115e9e8a7b98b7cf6a5 Mon Sep 17 00:00:00 2001 From: Automerger Merge Worker Date: Thu, 12 Mar 2020 09:46:43 +0000 Subject: [PATCH 1/7] Add permission check functions to NetworkStatsService This is shameless copied from ConnectivityService and plan to be used in follow-up change. For long term perspective we should have a permission util that contains those util in one place. Test: m -j Change-Id: If70e6e831c0386b604d0151cc0650f68457b6e5f Merged-In: If70e6e831c0386b604d0151cc0650f68457b6e5f Merged-In: I837545b54f36c8f21c878cbb1fd028ff23be6cb8 Bug: 149652079 (cherry picked from commit 4da7af7e7e4a0d3360cd41e98e4d6fa731d21515) --- .../server/net/NetworkStatsService.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index 4af31b0369..cb3da918f4 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -23,6 +23,7 @@ import static android.content.Intent.ACTION_SHUTDOWN; import static android.content.Intent.ACTION_UID_REMOVED; import static android.content.Intent.ACTION_USER_REMOVED; import static android.content.Intent.EXTRA_UID; +import static android.content.pm.PackageManager.PERMISSION_GRANTED; import static android.net.ConnectivityManager.ACTION_TETHER_STATE_CHANGED; import static android.net.ConnectivityManager.isNetworkTypeMobile; import static android.net.NetworkStack.checkNetworkStackPermission; @@ -1793,6 +1794,24 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } } + // TODO: It is copied from ConnectivitySerivce, consider refactor these check permission + // functions to a proper util. + private boolean checkAnyPermissionOf(String... permissions) { + for (String permission : permissions) { + if (mContext.checkCallingOrSelfPermission(permission) == PERMISSION_GRANTED) { + return true; + } + } + return false; + } + + private void enforceAnyPermissionOf(String... permissions) { + if (!checkAnyPermissionOf(permissions)) { + throw new SecurityException("Requires one of the following permissions: " + + String.join(", ", permissions) + "."); + } + } + /** * Registers a custom provider of {@link android.net.NetworkStats} to combine the network * statistics that cannot be seen by the kernel to system. To unregister, invoke the From 050c7de0935a8db2ae8b3b595b71022d1dd34913 Mon Sep 17 00:00:00 2001 From: junyulai Date: Mon, 16 Mar 2020 15:00:24 +0800 Subject: [PATCH 2/7] Fix typo in aosp/1242602 Test: m -j Bug: 149652079 Change-Id: Ie023c8371785fc239c54eacbf056141c2e9e3444 Merged-In: Ie023c8371785fc239c54eacbf056141c2e9e3444 (cherry picked from aosp/1259461) --- .../core/java/com/android/server/net/NetworkStatsService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index cb3da918f4..45047048cc 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -1794,7 +1794,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } } - // TODO: It is copied from ConnectivitySerivce, consider refactor these check permission + // TODO: It is copied from ConnectivityService, consider refactor these check permission // functions to a proper util. private boolean checkAnyPermissionOf(String... permissions) { for (String permission : permissions) { From f62907d9ca02350c508c588b6a2fec7f13188398 Mon Sep 17 00:00:00 2001 From: Automerger Merge Worker Date: Thu, 12 Mar 2020 09:46:48 +0000 Subject: [PATCH 3/7] [SP20] Check signature permission when accessing network stats provider Currently, registerNetworkStatsProvider requires the UPDATE_DEVICE_STATS permission. This is a privileged permission so it can be granted to preinstalled apps. Thus, apps like GmsCore, or preinstalled apps will be able to update network stats. This change checks for a new permission that would only allow signature apps to declare that. Also check MAINLINE_NETWORK_STACK permission to allow NetworkStack process to use it. Test: adb shell dumpsys netstats Test: atest FrameworksNetTests Bug: 149652079 Change-Id: Iaecbf10a7610461bd52e315659006c7332c416e6 Merged-In: Iaecbf10a7610461bd52e315659006c7332c416e6 Merged-In: Idfebd0a1988c3dcfd812d87e30f6a2034d6fbf6b (cherry picked from commit e9e8d8f9ff096c1723eba1d74b17bb7cade7f23b) --- core/java/android/app/usage/NetworkStatsManager.java | 5 ++++- .../java/com/android/server/net/NetworkStatsService.java | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/core/java/android/app/usage/NetworkStatsManager.java b/core/java/android/app/usage/NetworkStatsManager.java index 5b98188300..7d13f050a7 100644 --- a/core/java/android/app/usage/NetworkStatsManager.java +++ b/core/java/android/app/usage/NetworkStatsManager.java @@ -29,6 +29,7 @@ import android.net.ConnectivityManager; import android.net.DataUsageRequest; import android.net.INetworkStatsService; import android.net.NetworkIdentity; +import android.net.NetworkStack; import android.net.NetworkTemplate; import android.net.netstats.provider.AbstractNetworkStatsProvider; import android.net.netstats.provider.NetworkStatsProviderCallback; @@ -540,7 +541,9 @@ public class NetworkStatsManager { * @hide */ @SystemApi - @RequiresPermission(android.Manifest.permission.UPDATE_DEVICE_STATS) + @RequiresPermission(anyOf = { + android.Manifest.permission.NETWORK_STATS_PROVIDER, + NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK}) @NonNull public NetworkStatsProviderCallback registerNetworkStatsProvider( @NonNull String tag, @NonNull AbstractNetworkStatsProvider provider) { diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index 45047048cc..a6c2c827fa 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -17,6 +17,7 @@ package com.android.server.net; import static android.Manifest.permission.ACCESS_NETWORK_STATE; +import static android.Manifest.permission.NETWORK_STATS_PROVIDER; import static android.Manifest.permission.READ_NETWORK_USAGE_HISTORY; import static android.Manifest.permission.UPDATE_DEVICE_STATS; import static android.content.Intent.ACTION_SHUTDOWN; @@ -1828,7 +1829,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { */ public @NonNull INetworkStatsProviderCallback registerNetworkStatsProvider( @NonNull String tag, @NonNull INetworkStatsProvider provider) { - mContext.enforceCallingOrSelfPermission(UPDATE_DEVICE_STATS, TAG); + enforceAnyPermissionOf(NETWORK_STATS_PROVIDER, + NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK); Objects.requireNonNull(provider, "provider is null"); Objects.requireNonNull(tag, "tag is null"); try { From 27d58bfd962ec47e181f1d967193b14cadaff7e5 Mon Sep 17 00:00:00 2001 From: junyulai Date: Fri, 6 Mar 2020 14:50:48 +0800 Subject: [PATCH 4/7] [SP21] Address comments for API council review about aosp/1172143 Test: atest FrameworksNetTests ImsPhoneCallTrackerTest Test: atest TetheringTests NetworkStackTests Test: m doc-comment-check-docs Fix: 148552904 Change-Id: I141393f229e772d2eb9f7c156849e379bd71b845 Merged-In: I141393f229e772d2eb9f7c156849e379bd71b845 (cherry picked from aosp/1253717) --- .../app/usage/NetworkStatsManager.java | 48 +++-- .../provider/INetworkStatsProvider.aidl | 6 +- .../INetworkStatsProviderCallback.aidl | 6 +- .../provider/NetworkStatsProvider.java | 195 ++++++++++++++++++ .../server/net/NetworkStatsService.java | 23 +-- 5 files changed, 245 insertions(+), 33 deletions(-) create mode 100644 core/java/android/net/netstats/provider/NetworkStatsProvider.java diff --git a/core/java/android/app/usage/NetworkStatsManager.java b/core/java/android/app/usage/NetworkStatsManager.java index 7d13f050a7..d6e77624a9 100644 --- a/core/java/android/app/usage/NetworkStatsManager.java +++ b/core/java/android/app/usage/NetworkStatsManager.java @@ -31,9 +31,8 @@ import android.net.INetworkStatsService; import android.net.NetworkIdentity; import android.net.NetworkStack; import android.net.NetworkTemplate; -import android.net.netstats.provider.AbstractNetworkStatsProvider; -import android.net.netstats.provider.NetworkStatsProviderCallback; -import android.net.netstats.provider.NetworkStatsProviderWrapper; +import android.net.netstats.provider.INetworkStatsProviderCallback; +import android.net.netstats.provider.NetworkStatsProvider; import android.os.Binder; import android.os.Handler; import android.os.Looper; @@ -528,34 +527,53 @@ public class NetworkStatsManager { /** * Registers a custom provider of {@link android.net.NetworkStats} to provide network statistics - * to the system. To unregister, invoke {@link NetworkStatsProviderCallback#unregister()}. + * to the system. To unregister, invoke {@link #unregisterNetworkStatsProvider}. * Note that no de-duplication of statistics between providers is performed, so each provider - * must only report network traffic that is not being reported by any other provider. + * must only report network traffic that is not being reported by any other provider. Also note + * that the provider cannot be re-registered after unregistering. * * @param tag a human readable identifier of the custom network stats provider. This is only * used for debugging. - * @param provider the subclass of {@link AbstractNetworkStatsProvider} that needs to be + * @param provider the subclass of {@link NetworkStatsProvider} that needs to be * registered to the system. - * @return a {@link NetworkStatsProviderCallback}, which can be used to report events to the - * system or unregister the provider. * @hide */ @SystemApi @RequiresPermission(anyOf = { android.Manifest.permission.NETWORK_STATS_PROVIDER, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK}) - @NonNull public NetworkStatsProviderCallback registerNetworkStatsProvider( + @NonNull public void registerNetworkStatsProvider( @NonNull String tag, - @NonNull AbstractNetworkStatsProvider provider) { + @NonNull NetworkStatsProvider provider) { try { - final NetworkStatsProviderWrapper wrapper = new NetworkStatsProviderWrapper(provider); - return new NetworkStatsProviderCallback( - mService.registerNetworkStatsProvider(tag, wrapper)); + if (provider.getProviderCallbackBinder() != null) { + throw new IllegalArgumentException("provider is already registered"); + } + final INetworkStatsProviderCallback cbBinder = + mService.registerNetworkStatsProvider(tag, provider.getProviderBinder()); + provider.setProviderCallbackBinder(cbBinder); + } catch (RemoteException e) { + e.rethrowAsRuntimeException(); + } + } + + /** + * Unregisters an instance of {@link NetworkStatsProvider}. + * + * @param provider the subclass of {@link NetworkStatsProvider} that needs to be + * unregistered to the system. + * @hide + */ + @SystemApi + @RequiresPermission(anyOf = { + android.Manifest.permission.NETWORK_STATS_PROVIDER, + NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK}) + @NonNull public void unregisterNetworkStatsProvider(@NonNull NetworkStatsProvider provider) { + try { + provider.getProviderCallbackBinderOrThrow().unregister(); } catch (RemoteException e) { e.rethrowAsRuntimeException(); } - // Unreachable code, but compiler doesn't know about it. - return null; } private static NetworkTemplate createTemplate(int networkType, String subscriberId) { diff --git a/core/java/android/net/netstats/provider/INetworkStatsProvider.aidl b/core/java/android/net/netstats/provider/INetworkStatsProvider.aidl index 55b3d4edb1..4078b24921 100644 --- a/core/java/android/net/netstats/provider/INetworkStatsProvider.aidl +++ b/core/java/android/net/netstats/provider/INetworkStatsProvider.aidl @@ -22,7 +22,7 @@ package android.net.netstats.provider; * @hide */ oneway interface INetworkStatsProvider { - void requestStatsUpdate(int token); - void setLimit(String iface, long quotaBytes); - void setAlert(long quotaBytes); + void onRequestStatsUpdate(int token); + void onSetLimit(String iface, long quotaBytes); + void onSetAlert(long quotaBytes); } diff --git a/core/java/android/net/netstats/provider/INetworkStatsProviderCallback.aidl b/core/java/android/net/netstats/provider/INetworkStatsProviderCallback.aidl index 3ea9318f10..bd336dd348 100644 --- a/core/java/android/net/netstats/provider/INetworkStatsProviderCallback.aidl +++ b/core/java/android/net/netstats/provider/INetworkStatsProviderCallback.aidl @@ -24,8 +24,8 @@ import android.net.NetworkStats; * @hide */ oneway interface INetworkStatsProviderCallback { - void onStatsUpdated(int token, in NetworkStats ifaceStats, in NetworkStats uidStats); - void onAlertReached(); - void onLimitReached(); + void notifyStatsUpdated(int token, in NetworkStats ifaceStats, in NetworkStats uidStats); + void notifyAlertReached(); + void notifyLimitReached(); void unregister(); } diff --git a/core/java/android/net/netstats/provider/NetworkStatsProvider.java b/core/java/android/net/netstats/provider/NetworkStatsProvider.java new file mode 100644 index 0000000000..7639d2244c --- /dev/null +++ b/core/java/android/net/netstats/provider/NetworkStatsProvider.java @@ -0,0 +1,195 @@ +/* + * 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.netstats.provider; + +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.annotation.SystemApi; +import android.net.NetworkStats; +import android.os.RemoteException; + +/** + * A base class that allows external modules to implement a custom network statistics provider. + * @hide + */ +@SystemApi +public abstract class NetworkStatsProvider { + /** + * A value used by {@link #onSetLimit} and {@link #onSetAlert} indicates there is no limit. + */ + public static final int QUOTA_UNLIMITED = -1; + + @NonNull private final INetworkStatsProvider mProviderBinder = + new INetworkStatsProvider.Stub() { + + @Override + public void onRequestStatsUpdate(int token) { + NetworkStatsProvider.this.onRequestStatsUpdate(token); + } + + @Override + public void onSetLimit(String iface, long quotaBytes) { + NetworkStatsProvider.this.onSetLimit(iface, quotaBytes); + } + + @Override + public void onSetAlert(long quotaBytes) { + NetworkStatsProvider.this.onSetAlert(quotaBytes); + } + }; + + // The binder given by the service when successfully registering. Only null before registering, + // never null once non-null. + @Nullable + private INetworkStatsProviderCallback mProviderCbBinder; + + /** + * Return the binder invoked by the service and redirect function calls to the overridden + * methods. + * @hide + */ + @NonNull + public INetworkStatsProvider getProviderBinder() { + return mProviderBinder; + } + + /** + * Store the binder that was returned by the service when successfully registering. Note that + * the provider cannot be re-registered. Hence this method can only be called once per provider. + * + * @hide + */ + public void setProviderCallbackBinder(@NonNull INetworkStatsProviderCallback binder) { + if (mProviderCbBinder != null) { + throw new IllegalArgumentException("provider is already registered"); + } + mProviderCbBinder = binder; + } + + /** + * Get the binder that was returned by the service when successfully registering. Or null if the + * provider was never registered. + * + * @hide + */ + @Nullable + public INetworkStatsProviderCallback getProviderCallbackBinder() { + return mProviderCbBinder; + } + + /** + * Get the binder that was returned by the service when successfully registering. Throw an + * {@link IllegalStateException} if the provider is not registered. + * + * @hide + */ + @NonNull + public INetworkStatsProviderCallback getProviderCallbackBinderOrThrow() { + if (mProviderCbBinder == null) { + throw new IllegalStateException("the provider is not registered"); + } + return mProviderCbBinder; + } + + /** + * Notify the system of new network statistics. + * + * Send the network statistics recorded since the last call to {@link #notifyStatsUpdated}. Must + * be called as soon as possible after {@link NetworkStatsProvider#onRequestStatsUpdate(int)} + * being called. Responding later increases the probability stats will be dropped. The + * provider can also call this whenever it wants to reports new stats for any reason. + * Note that the system will not necessarily immediately propagate the statistics to + * reflect the update. + * + * @param token the token under which these stats were gathered. Providers can call this method + * with the current token as often as they want, until the token changes. + * {@see NetworkStatsProvider#onRequestStatsUpdate()} + * @param ifaceStats the {@link NetworkStats} per interface to be reported. + * The provider should not include any traffic that is already counted by + * kernel interface counters. + * @param uidStats the same stats as above, but counts {@link NetworkStats} + * per uid. + */ + public void notifyStatsUpdated(int token, @NonNull NetworkStats ifaceStats, + @NonNull NetworkStats uidStats) { + try { + getProviderCallbackBinderOrThrow().notifyStatsUpdated(token, ifaceStats, uidStats); + } catch (RemoteException e) { + e.rethrowAsRuntimeException(); + } + } + + /** + * Notify system that the quota set by {@code onSetAlert} has been reached. + */ + public void notifyAlertReached() { + try { + getProviderCallbackBinderOrThrow().notifyAlertReached(); + } catch (RemoteException e) { + e.rethrowAsRuntimeException(); + } + } + + /** + * Notify system that the quota set by {@code onSetLimit} has been reached. + */ + public void notifyLimitReached() { + try { + getProviderCallbackBinderOrThrow().notifyLimitReached(); + } catch (RemoteException e) { + e.rethrowAsRuntimeException(); + } + } + + /** + * Called by {@code NetworkStatsService} when it requires to know updated stats. + * The provider MUST respond by calling {@link #notifyStatsUpdated} as soon as possible. + * Responding later increases the probability stats will be dropped. Memory allowing, the + * system will try to take stats into account up to one minute after calling + * {@link #onRequestStatsUpdate}. + * + * @param token a positive number identifying the new state of the system under which + * {@link NetworkStats} have to be gathered from now on. When this is called, + * custom implementations of providers MUST tally and report the latest stats with + * the previous token, under which stats were being gathered so far. + */ + public abstract void onRequestStatsUpdate(int token); + + /** + * Called by {@code NetworkStatsService} when setting the interface quota for the specified + * upstream interface. When this is called, the custom implementation should block all egress + * packets on the {@code iface} associated with the provider when {@code quotaBytes} bytes have + * been reached, and MUST respond to it by calling + * {@link NetworkStatsProvider#notifyLimitReached()}. + * + * @param iface the interface requiring the operation. + * @param quotaBytes the quota defined as the number of bytes, starting from zero and counting + * from now. A value of {@link #QUOTA_UNLIMITED} indicates there is no limit. + */ + public abstract void onSetLimit(@NonNull String iface, long quotaBytes); + + /** + * Called by {@code NetworkStatsService} when setting the alert bytes. Custom implementations + * MUST call {@link NetworkStatsProvider#notifyAlertReached()} when {@code quotaBytes} bytes + * have been reached. Unlike {@link #onSetLimit(String, long)}, the custom implementation should + * not block all egress packets. + * + * @param quotaBytes the quota defined as the number of bytes, starting from zero and counting + * from now. A value of {@link #QUOTA_UNLIMITED} indicates there is no alert. + */ + public abstract void onSetAlert(long quotaBytes); +} diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index a6c2c827fa..46e865c7e8 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -103,7 +103,7 @@ import android.net.NetworkTemplate; import android.net.TrafficStats; import android.net.netstats.provider.INetworkStatsProvider; import android.net.netstats.provider.INetworkStatsProviderCallback; -import android.net.netstats.provider.NetworkStatsProviderCallback; +import android.net.netstats.provider.NetworkStatsProvider; import android.os.BestClock; import android.os.Binder; import android.os.DropBoxManager; @@ -558,7 +558,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } catch (RemoteException e) { // ignored; service lives in system_server } - invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.setAlert(mGlobalAlertBytes)); + invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.onSetAlert(mGlobalAlertBytes)); } @Override @@ -1376,7 +1376,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { Trace.traceBegin(TRACE_TAG_NETWORK, "provider.requestStatsUpdate"); final int registeredCallbackCount = mStatsProviderCbList.getRegisteredCallbackCount(); mStatsProviderSem.drainPermits(); - invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.requestStatsUpdate(0 /* unused */)); + invokeForAllStatsProviderCallbacks( + (cb) -> cb.mProvider.onRequestStatsUpdate(0 /* unused */)); try { mStatsProviderSem.tryAcquire(registeredCallbackCount, MAX_STATS_PROVIDER_POLL_WAIT_TIME_MS, TimeUnit.MILLISECONDS); @@ -1551,7 +1552,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { @Override public void setStatsProviderLimitAsync(@NonNull String iface, long quota) { Slog.v(TAG, "setStatsProviderLimitAsync(" + iface + "," + quota + ")"); - invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.setLimit(iface, quota)); + invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.onSetLimit(iface, quota)); } } @@ -1820,12 +1821,10 @@ public class NetworkStatsService extends INetworkStatsService.Stub { * * @param tag a human readable identifier of the custom network stats provider. * @param provider the {@link INetworkStatsProvider} binder corresponding to the - * {@link android.net.netstats.provider.AbstractNetworkStatsProvider} to be - * registered. + * {@link NetworkStatsProvider} to be registered. * - * @return a binder interface of - * {@link android.net.netstats.provider.NetworkStatsProviderCallback}, which can be - * used to report events to the system. + * @return a {@link INetworkStatsProviderCallback} binder + * interface, which can be used to report events to the system. */ public @NonNull INetworkStatsProviderCallback registerNetworkStatsProvider( @NonNull String tag, @NonNull INetworkStatsProvider provider) { @@ -1931,7 +1930,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } @Override - public void onStatsUpdated(int token, @Nullable NetworkStats ifaceStats, + public void notifyStatsUpdated(int token, @Nullable NetworkStats ifaceStats, @Nullable NetworkStats uidStats) { // TODO: 1. Use token to map ifaces to correct NetworkIdentity. // 2. Store the difference and store it directly to the recorder. @@ -1943,12 +1942,12 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } @Override - public void onAlertReached() throws RemoteException { + public void notifyAlertReached() throws RemoteException { mAlertObserver.limitReached(LIMIT_GLOBAL_ALERT, null /* unused */); } @Override - public void onLimitReached() { + public void notifyLimitReached() { Log.d(TAG, mTag + ": onLimitReached"); LocalServices.getService(NetworkPolicyManagerInternal.class) .onStatsProviderLimitReached(mTag); From ca952de92d7982bb9fcf1ad04477130c28475c75 Mon Sep 17 00:00:00 2001 From: junyulai Date: Fri, 13 Mar 2020 16:12:35 +0800 Subject: [PATCH 5/7] [SP23] Address misc API review comment about NetworkStats This change add several javadoc for NetworkStats and its Entry class. Also remove IFACE_ALL constant accordlingly. Test: atest FrameworkNetTests Test: m doc-comment-check-docs Bug: 148895143 Change-Id: Id1b8f41be79f847ad73197ff0e18258b6ee8a730 Merged-In: Id1b8f41be79f847ad73197ff0e18258b6ee8a730 (cherry picked from aosp/1256351) --- core/java/android/net/NetworkStats.java | 45 ++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/core/java/android/net/NetworkStats.java b/core/java/android/net/NetworkStats.java index 2f536ffd8c..10f7dbfc57 100644 --- a/core/java/android/net/NetworkStats.java +++ b/core/java/android/net/NetworkStats.java @@ -21,7 +21,6 @@ import static android.os.Process.CLAT_UID; import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.Nullable; -import android.annotation.SuppressLint; import android.annotation.SystemApi; import android.compat.annotation.UnsupportedAppUsage; import android.os.Parcel; @@ -58,9 +57,12 @@ import java.util.function.Predicate; public final class NetworkStats implements Parcelable { private static final String TAG = "NetworkStats"; - /** {@link #iface} value when interface details unavailable. */ - @SuppressLint("CompileTimeConstant") + /** + * {@link #iface} value when interface details unavailable. + * @hide + */ @Nullable public static final String IFACE_ALL = null; + /** * Virtual network interface for video telephony. This is for VT data usage counting * purpose. @@ -248,7 +250,13 @@ public final class NetworkStats implements Parcelable { @UnsupportedAppUsage private long[] operations; - /** @hide */ + /** + * Basic element of network statistics. Contains the number of packets and number of bytes + * transferred on both directions in a given set of conditions. See + * {@link Entry#Entry(String, int, int, int, int, int, int, long, long, long, long, long)}. + * + * @hide + */ @SystemApi public static class Entry { /** @hide */ @@ -319,6 +327,35 @@ public final class NetworkStats implements Parcelable { rxBytes, rxPackets, txBytes, txPackets, operations); } + /** + * Construct a {@link Entry} object by giving statistics of packet and byte transferred on + * both direction, and associated with a set of given conditions. + * + * @param iface interface name of this {@link Entry}. Or null if not specified. + * @param uid uid of this {@link Entry}. {@link #UID_TETHERING} if this {@link Entry} is + * for tethering. Or {@link #UID_ALL} if this {@link NetworkStats} is only + * counting iface stats. + * @param set usage state of this {@link Entry}. Should be one of the following + * values: {@link #SET_DEFAULT}, {@link #SET_FOREGROUND}. + * @param tag tag of this {@link Entry}. + * @param metered metered state of this {@link Entry}. Should be one of the following + * values: {link #METERED_YES}, {link #METERED_NO}. + * @param roaming roaming state of this {@link Entry}. Should be one of the following + * values: {link #ROAMING_YES}, {link #ROAMING_NO}. + * @param defaultNetwork default network status of this {@link Entry}. Should be one + * of the following values: {link #DEFAULT_NETWORK_YES}, + * {link #DEFAULT_NETWORK_NO}. + * @param rxBytes Number of bytes received for this {@link Entry}. Statistics should + * represent the contents of IP packets, including IP headers. + * @param rxPackets Number of packets received for this {@link Entry}. Statistics should + * represent the contents of IP packets, including IP headers. + * @param txBytes Number of bytes transmitted for this {@link Entry}. Statistics should + * represent the contents of IP packets, including IP headers. + * @param txPackets Number of bytes transmitted for this {@link Entry}. Statistics should + * represent the contents of IP packets, including IP headers. + * @param operations count of network operations performed for this {@link Entry}. This can + * be used to derive bytes-per-operation. + */ public Entry(@Nullable String iface, int uid, @State int set, int tag, @Meteredness int metered, @Roaming int roaming, @DefaultNetwork int defaultNetwork, long rxBytes, long rxPackets, long txBytes, long txPackets, long operations) { From 02a6340ca0cc5838c1e5469e5391b9273284a5e6 Mon Sep 17 00:00:00 2001 From: junyulai Date: Fri, 13 Mar 2020 19:04:17 +0800 Subject: [PATCH 6/7] [SP24] Rename functions that add Entry unconditionally Currently, in NetworkStats, there are many methods to manipulate the records. However, some methods are similar and ambiguous, such as addEntry, addValues, setValues, addIfaceValues, combineValues and combineAllValues. Thus, properly grouping and renaming methods are necessary. In this change, for methods that add one record unconditionally, name them insertEntry. setIfaceValues -> insertEntry addEntry -> insertEntry Test: atest FrameworksNetTests ImsPhoneCallTrackerTest TetheringTests Test: m doc-comment-check-docs Bug: 148895143 Change-Id: I801ddc49e283a254b9586700c9b597c0adb0d459 Merged-In: I801ddc49e283a254b9586700c9b597c0adb0d459 (cherry picked from aosp/1256352) --- core/java/android/net/NetworkStats.java | 24 +++++++++---------- .../server/net/NetworkStatsFactory.java | 6 ++--- .../server/net/NetworkStatsService.java | 2 +- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/core/java/android/net/NetworkStats.java b/core/java/android/net/NetworkStats.java index 10f7dbfc57..0fc74b0894 100644 --- a/core/java/android/net/NetworkStats.java +++ b/core/java/android/net/NetworkStats.java @@ -503,7 +503,7 @@ public final class NetworkStats implements Parcelable { NetworkStats.Entry entry = null; for (int i = 0; i < size; i++) { entry = getValues(i, entry); - clone.addEntry(entry); + clone.insertEntry(entry); } return clone; } @@ -530,26 +530,26 @@ public final class NetworkStats implements Parcelable { /** @hide */ @VisibleForTesting - public NetworkStats addIfaceValues( + public NetworkStats insertEntry( String iface, long rxBytes, long rxPackets, long txBytes, long txPackets) { - return addEntry( + return insertEntry( iface, UID_ALL, SET_DEFAULT, TAG_NONE, rxBytes, rxPackets, txBytes, txPackets, 0L); } /** @hide */ @VisibleForTesting - public NetworkStats addEntry(String iface, int uid, int set, int tag, long rxBytes, + public NetworkStats insertEntry(String iface, int uid, int set, int tag, long rxBytes, long rxPackets, long txBytes, long txPackets, long operations) { - return addEntry(new Entry( + return insertEntry(new Entry( iface, uid, set, tag, rxBytes, rxPackets, txBytes, txPackets, operations)); } /** @hide */ @VisibleForTesting - public NetworkStats addEntry(String iface, int uid, int set, int tag, int metered, int roaming, - int defaultNetwork, long rxBytes, long rxPackets, long txBytes, long txPackets, - long operations) { - return addEntry(new Entry( + public NetworkStats insertEntry(String iface, int uid, int set, int tag, int metered, + int roaming, int defaultNetwork, long rxBytes, long rxPackets, long txBytes, + long txPackets, long operations) { + return insertEntry(new Entry( iface, uid, set, tag, metered, roaming, defaultNetwork, rxBytes, rxPackets, txBytes, txPackets, operations)); } @@ -559,7 +559,7 @@ public final class NetworkStats implements Parcelable { * object can be recycled across multiple calls. * @hide */ - public NetworkStats addEntry(Entry entry) { + public NetworkStats insertEntry(Entry entry) { if (size >= capacity) { final int newLength = Math.max(size, 10) * 3 / 2; iface = Arrays.copyOf(iface, newLength); @@ -702,7 +702,7 @@ public final class NetworkStats implements Parcelable { entry.roaming, entry.defaultNetwork); if (i == -1) { // only create new entry when positive contribution - addEntry(entry); + insertEntry(entry); } else { rxBytes[i] += entry.rxBytes; rxPackets[i] += entry.rxPackets; @@ -1040,7 +1040,7 @@ public final class NetworkStats implements Parcelable { entry.operations = Math.max(entry.operations, 0); } - result.addEntry(entry); + result.insertEntry(entry); } return result; diff --git a/services/core/java/com/android/server/net/NetworkStatsFactory.java b/services/core/java/com/android/server/net/NetworkStatsFactory.java index 22b01bee6c..75ffe35674 100644 --- a/services/core/java/com/android/server/net/NetworkStatsFactory.java +++ b/services/core/java/com/android/server/net/NetworkStatsFactory.java @@ -229,7 +229,7 @@ public class NetworkStatsFactory { entry.txPackets += reader.nextLong(); } - stats.addEntry(entry); + stats.insertEntry(entry); reader.finishLine(); } } catch (NullPointerException|NumberFormatException e) { @@ -279,7 +279,7 @@ public class NetworkStatsFactory { entry.txBytes = reader.nextLong(); entry.txPackets = reader.nextLong(); - stats.addEntry(entry); + stats.insertEntry(entry); reader.finishLine(); } } catch (NullPointerException|NumberFormatException e) { @@ -439,7 +439,7 @@ public class NetworkStatsFactory { if ((limitIfaces == null || ArrayUtils.contains(limitIfaces, entry.iface)) && (limitUid == UID_ALL || limitUid == entry.uid) && (limitTag == TAG_ALL || limitTag == entry.tag)) { - stats.addEntry(entry); + stats.insertEntry(entry); } reader.finishLine(); diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index 46e865c7e8..10136b3b8b 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -759,7 +759,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { final NetworkStatsHistory.Entry entry = history.getValues(start, end, now, null); final NetworkStats stats = new NetworkStats(end - start, 1); - stats.addEntry(new NetworkStats.Entry(IFACE_ALL, UID_ALL, SET_ALL, TAG_NONE, + stats.insertEntry(new NetworkStats.Entry(IFACE_ALL, UID_ALL, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL, DEFAULT_NETWORK_ALL, entry.rxBytes, entry.rxPackets, entry.txBytes, entry.txPackets, entry.operations)); return stats; From 544bc807a4641b540e9f432986ce209cb3250954 Mon Sep 17 00:00:00 2001 From: junyulai Date: Mon, 16 Mar 2020 13:27:28 +0800 Subject: [PATCH 7/7] [SP25] Rename functions that add Entry conditionally Currently, in NetworkStats, there are many methods to manipulate the records. However, some methods are similar and ambiguous, such as addEntry, addValues, setValues, addIfaceValues, combineValues and combineAllValues. Thus, properly grouping and renaming methods are necessary. In this change, for methods that add one record conditionally, name them addEntry. addValues -> addEntry Test: atest FrameworksNetTests ImsPhoneCallTrackerTest TetheringTests Fix: 148895143 Change-Id: I9495a198cf247e6c79100f7ac1edcea370b071de --- core/java/android/net/NetworkStats.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/java/android/net/NetworkStats.java b/core/java/android/net/NetworkStats.java index 0fc74b0894..9c1fb41ecc 100644 --- a/core/java/android/net/NetworkStats.java +++ b/core/java/android/net/NetworkStats.java @@ -721,7 +721,7 @@ public final class NetworkStats implements Parcelable { * @param entry the {@link Entry} to add. * @return a new constructed {@link NetworkStats} object that contains the result. */ - public @NonNull NetworkStats addValues(@NonNull Entry entry) { + public @NonNull NetworkStats addEntry(@NonNull Entry entry) { return this.clone().combineValues(entry); }