Merge changes I6a48d4db,I6741c41c,Ifec6bde5,Icd0717c5 am: 02bd168520 am: de83629fc1 am: 9672f71a14

Original change: https://android-review.googlesource.com/c/platform/frameworks/base/+/1490617

Change-Id: Ie76caf460ac86251cd0ceca981bdf2c7e4bcfc44

4 files changed, 88 insertions, 112 deletions

diff --git a/core/java/android/net/TrafficStats.java b/core/java/android/net/TrafficStats.java
index a985e934ed3c..737683050528 100644
--- a/core/java/android/net/TrafficStats.java
+++ b/core/java/android/net/TrafficStats.java
@@ -976,11 +976,17 @@ public class TrafficStats {
         }
     }
 
-    // NOTE: keep these in sync with android_net_TrafficStats.cpp
-    private static final int TYPE_RX_BYTES = 0;
-    private static final int TYPE_RX_PACKETS = 1;
-    private static final int TYPE_TX_BYTES = 2;
-    private static final int TYPE_TX_PACKETS = 3;
-    private static final int TYPE_TCP_RX_PACKETS = 4;
-    private static final int TYPE_TCP_TX_PACKETS = 5;
+    // NOTE: keep these in sync with {@code com_android_server_net_NetworkStatsService.cpp}.
+    /** {@hide} */
+    public static final int TYPE_RX_BYTES = 0;
+    /** {@hide} */
+    public static final int TYPE_RX_PACKETS = 1;
+    /** {@hide} */
+    public static final int TYPE_TX_BYTES = 2;
+    /** {@hide} */
+    public static final int TYPE_TX_PACKETS = 3;
+    /** {@hide} */
+    public static final int TYPE_TCP_RX_PACKETS = 4;
+    /** {@hide} */
+    public static final int TYPE_TCP_TX_PACKETS = 5;
 }
diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java
index 12c24d418611..81a6641de8a4 100644
--- a/services/core/java/com/android/server/net/NetworkStatsService.java
+++ b/services/core/java/com/android/server/net/NetworkStatsService.java
@@ -1084,12 +1084,10 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
             return nativeIfaceStats;
         } else {
             // When tethering offload is in use, nativeIfaceStats does not contain usage from
-            // offload, add it back here.
-            // When tethering offload is not in use, nativeIfaceStats contains tethering usage.
-            // this does not cause double-counting of tethering traffic, because
-            // NetdTetheringStatsProvider returns zero NetworkStats
-            // when called with STATS_PER_IFACE.
-            return nativeIfaceStats + getTetherStats(iface, type);
+            // offload, add it back here. Note that the included statistics might be stale
+            // since polling newest stats from hardware might impact system health and not
+            // suitable for TrafficStats API use cases.
+            return nativeIfaceStats + getProviderIfaceStats(iface, type);
         }
     }
 
@@ -1100,39 +1098,28 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
             return nativeTotalStats;
         } else {
             // Refer to comment in getIfaceStats
-            return nativeTotalStats + getTetherStats(IFACE_ALL, type);
+            return nativeTotalStats + getProviderIfaceStats(IFACE_ALL, type);
         }
     }
 
-    private long getTetherStats(String iface, int type) {
-        final NetworkStats tetherSnapshot;
-        final long token = Binder.clearCallingIdentity();
-        try {
-            tetherSnapshot = getNetworkStatsTethering(STATS_PER_IFACE);
-        } catch (RemoteException e) {
-            Slog.w(TAG, "Error get TetherStats: " + e);
-            return 0;
-        } finally {
-            Binder.restoreCallingIdentity(token);
-        }
-        HashSet<String> limitIfaces;
+    private long getProviderIfaceStats(@Nullable String iface, int type) {
+        final NetworkStats providerSnapshot = getNetworkStatsFromProviders(STATS_PER_IFACE);
+        final HashSet<String> limitIfaces;
         if (iface == IFACE_ALL) {
             limitIfaces = null;
         } else {
-            limitIfaces = new HashSet<String>();
+            limitIfaces = new HashSet<>();
             limitIfaces.add(iface);
         }
-        NetworkStats.Entry entry = tetherSnapshot.getTotal(null, limitIfaces);
-        if (LOGD) Slog.d(TAG, "TetherStats: iface=" + iface + " type=" + type +
-                " entry=" + entry);
+        final NetworkStats.Entry entry = providerSnapshot.getTotal(null, limitIfaces);
         switch (type) {
-            case 0: // TYPE_RX_BYTES
+            case TrafficStats.TYPE_RX_BYTES:
                 return entry.rxBytes;
-            case 1: // TYPE_RX_PACKETS
+            case TrafficStats.TYPE_RX_PACKETS:
                 return entry.rxPackets;
-            case 2: // TYPE_TX_BYTES
+            case TrafficStats.TYPE_TX_BYTES:
                 return entry.txBytes;
-            case 3: // TYPE_TX_PACKETS
+            case TrafficStats.TYPE_TX_PACKETS:
                 return entry.txPackets;
             default:
                 return 0;
@@ -1429,14 +1416,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         final NetworkStats devSnapshot = readNetworkStatsSummaryDev();
         Trace.traceEnd(TRACE_TAG_NETWORK);
 
-        // Tethering snapshot for dev and xt stats. Counts per-interface data from tethering stats
-        // providers that isn't already counted by dev and XT stats.
-        Trace.traceBegin(TRACE_TAG_NETWORK, "snapshotTether");
-        final NetworkStats tetherSnapshot = getNetworkStatsTethering(STATS_PER_IFACE);
-        Trace.traceEnd(TRACE_TAG_NETWORK);
-        xtSnapshot.combineAllValues(tetherSnapshot);
-        devSnapshot.combineAllValues(tetherSnapshot);
-
         // Snapshot for dev/xt stats from all custom stats providers. Counts per-interface data
         // from stats providers that isn't already counted by dev and XT stats.
         Trace.traceBegin(TRACE_TAG_NETWORK, "snapshotStatsProvider");
@@ -1511,29 +1490,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         final boolean persistUid = (flags & FLAG_PERSIST_UID) != 0;
         final boolean persistForce = (flags & FLAG_PERSIST_FORCE) != 0;
 
-        // Request asynchronous stats update from all providers for next poll. And wait a bit of
-        // time to allow providers report-in given that normally binder call should be fast. Note
-        // that size of list might be changed because addition/removing at the same time. For
-        // addition, the stats of the missed provider can only be collected in next poll;
-        // for removal, wait might take up to MAX_STATS_PROVIDER_POLL_WAIT_TIME_MS
-        // once that happened.
-        // TODO: request with a valid token.
-        Trace.traceBegin(TRACE_TAG_NETWORK, "provider.requestStatsUpdate");
-        final int registeredCallbackCount = mStatsProviderCbList.size();
-        mStatsProviderSem.drainPermits();
-        invokeForAllStatsProviderCallbacks(
-                (cb) -> cb.mProvider.onRequestStatsUpdate(0 /* unused */));
-        try {
-            mStatsProviderSem.tryAcquire(registeredCallbackCount,
-                    MAX_STATS_PROVIDER_POLL_WAIT_TIME_MS, TimeUnit.MILLISECONDS);
-        } catch (InterruptedException e) {
-            // Strictly speaking it's possible a provider happened to deliver between the timeout
-            // and the log, and that doesn't matter too much as this is just a debug log.
-            Log.d(TAG, "requestStatsUpdate - providers responded "
-                    + mStatsProviderSem.availablePermits()
-                    + "/" + registeredCallbackCount + " : " + e);
-        }
-        Trace.traceEnd(TRACE_TAG_NETWORK);
+        performPollFromProvidersLocked();
 
         // TODO: consider marking "untrusted" times in historical stats
         final long currentTime = mClock.millis();
@@ -1578,6 +1535,33 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         Trace.traceEnd(TRACE_TAG_NETWORK);
     }
 
+    @GuardedBy("mStatsLock")
+    private void performPollFromProvidersLocked() {
+        // Request asynchronous stats update from all providers for next poll. And wait a bit of
+        // time to allow providers report-in given that normally binder call should be fast. Note
+        // that size of list might be changed because addition/removing at the same time. For
+        // addition, the stats of the missed provider can only be collected in next poll;
+        // for removal, wait might take up to MAX_STATS_PROVIDER_POLL_WAIT_TIME_MS
+        // once that happened.
+        // TODO: request with a valid token.
+        Trace.traceBegin(TRACE_TAG_NETWORK, "provider.requestStatsUpdate");
+        final int registeredCallbackCount = mStatsProviderCbList.size();
+        mStatsProviderSem.drainPermits();
+        invokeForAllStatsProviderCallbacks(
+                (cb) -> cb.mProvider.onRequestStatsUpdate(0 /* unused */));
+        try {
+            mStatsProviderSem.tryAcquire(registeredCallbackCount,
+                    MAX_STATS_PROVIDER_POLL_WAIT_TIME_MS, TimeUnit.MILLISECONDS);
+        } catch (InterruptedException e) {
+            // Strictly speaking it's possible a provider happened to deliver between the timeout
+            // and the log, and that doesn't matter too much as this is just a debug log.
+            Log.d(TAG, "requestStatsUpdate - providers responded "
+                    + mStatsProviderSem.availablePermits()
+                    + "/" + registeredCallbackCount + " : " + e);
+        }
+        Trace.traceEnd(TRACE_TAG_NETWORK);
+    }
+
     /**
      * Sample recent statistics summary into {@link EventLog}.
      */
@@ -1931,9 +1915,13 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
     }
 
     /**
-     * Return snapshot of current tethering statistics. Will return empty
-     * {@link NetworkStats} if any problems are encountered.
+     * Return snapshot of current non-offloaded tethering statistics. Will return empty
+     * {@link NetworkStats} if any problems are encountered, or queried by {@code STATS_PER_IFACE}
+     * since it is already included by {@link #nativeGetIfaceStat}.
+     * See {@code OffloadTetheringStatsProvider} for offloaded tethering stats.
      */
+    // TODO: Remove this by implementing {@link NetworkStatsProvider} for non-offloaded
+    //  tethering stats.
     private NetworkStats getNetworkStatsTethering(int how) throws RemoteException {
         try {
             return mNetworkManager.getNetworkStatsTethering(how);
@@ -2226,13 +2214,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         }
     }
 
-    private static int TYPE_RX_BYTES;
-    private static int TYPE_RX_PACKETS;
-    private static int TYPE_TX_BYTES;
-    private static int TYPE_TX_PACKETS;
-    private static int TYPE_TCP_RX_PACKETS;
-    private static int TYPE_TCP_TX_PACKETS;
-
     private static native long nativeGetTotalStat(int type, boolean useBpfStats);
     private static native long nativeGetIfaceStat(String iface, int type, boolean useBpfStats);
     private static native long nativeGetUidStat(int uid, int type, boolean useBpfStats);
diff --git a/services/core/jni/com_android_server_net_NetworkStatsService.cpp b/services/core/jni/com_android_server_net_NetworkStatsService.cpp
index 0275f3ea32f7..10b248a70e7e 100644
--- a/services/core/jni/com_android_server_net_NetworkStatsService.cpp
+++ b/services/core/jni/com_android_server_net_NetworkStatsService.cpp
@@ -215,21 +215,6 @@ static const JNINativeMethod gMethods[] = {
 };
 
 int register_android_server_net_NetworkStatsService(JNIEnv* env) {
-    jclass netStatsService = env->FindClass("com/android/server/net/NetworkStatsService");
-    jfieldID rxBytesId = env->GetStaticFieldID(netStatsService, "TYPE_RX_BYTES", "I");
-    jfieldID rxPacketsId = env->GetStaticFieldID(netStatsService, "TYPE_RX_PACKETS", "I");
-    jfieldID txBytesId = env->GetStaticFieldID(netStatsService, "TYPE_TX_BYTES", "I");
-    jfieldID txPacketsId = env->GetStaticFieldID(netStatsService, "TYPE_TX_PACKETS", "I");
-    jfieldID tcpRxPacketsId = env->GetStaticFieldID(netStatsService, "TYPE_TCP_RX_PACKETS", "I");
-    jfieldID tcpTxPacketsId = env->GetStaticFieldID(netStatsService, "TYPE_TCP_TX_PACKETS", "I");
-
-    env->SetStaticIntField(netStatsService, rxBytesId, RX_BYTES);
-    env->SetStaticIntField(netStatsService, rxPacketsId, RX_PACKETS);
-    env->SetStaticIntField(netStatsService, txBytesId, TX_BYTES);
-    env->SetStaticIntField(netStatsService, txPacketsId, TX_PACKETS);
-    env->SetStaticIntField(netStatsService, tcpRxPacketsId, TCP_RX_PACKETS);
-    env->SetStaticIntField(netStatsService, tcpTxPacketsId, TCP_TX_PACKETS);
-
     return jniRegisterNativeMethods(env, "com/android/server/net/NetworkStatsService", gMethods,
                                     NELEM(gMethods));
 }
diff --git a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java
index 7abe1893dd9e..cd9406cf3481 100644
--- a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java
+++ b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java
@@ -35,7 +35,6 @@ import static android.net.NetworkStats.ROAMING_YES;
 import static android.net.NetworkStats.SET_ALL;
 import static android.net.NetworkStats.SET_DEFAULT;
 import static android.net.NetworkStats.SET_FOREGROUND;
-import static android.net.NetworkStats.STATS_PER_IFACE;
 import static android.net.NetworkStats.STATS_PER_UID;
 import static android.net.NetworkStats.TAG_ALL;
 import static android.net.NetworkStats.TAG_NONE;
@@ -994,7 +993,7 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
     public void testTethering() throws Exception {
         // pretend first mobile network comes online
         expectDefaultSettings();
-        NetworkState[] states = new NetworkState[] {buildMobile3gState(IMSI_1)};
+        final NetworkState[] states = new NetworkState[]{buildMobile3gState(IMSI_1)};
         expectNetworkStatsSummary(buildEmptyStats());
         expectNetworkStatsUidDetail(buildEmptyStats());
 
@@ -1004,23 +1003,39 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
         incrementCurrentTime(HOUR_IN_MILLIS);
         expectDefaultSettings();
 
+        // Register custom provider and retrieve callback.
+        final TestableNetworkStatsProviderBinder provider =
+                new TestableNetworkStatsProviderBinder();
+        final INetworkStatsProviderCallback cb =
+                mService.registerNetworkStatsProvider("TEST-TETHERING-OFFLOAD", provider);
+        assertNotNull(cb);
+        final long now = getElapsedRealtime();
+
         // Traffic seen by kernel counters (includes software tethering).
-        final NetworkStats ifaceStats = new NetworkStats(getElapsedRealtime(), 1)
+        final NetworkStats swIfaceStats = new NetworkStats(now, 1)
                 .insertEntry(TEST_IFACE, 1536L, 12L, 384L, 3L);
         // Hardware tethering traffic, not seen by kernel counters.
-        final NetworkStats tetherStatsHardware = new NetworkStats(getElapsedRealtime(), 1)
-                .insertEntry(TEST_IFACE, 512L, 4L, 128L, 1L);
+        final NetworkStats tetherHwIfaceStats = new NetworkStats(now, 1)
+                .insertEntry(new NetworkStats.Entry(TEST_IFACE, UID_ALL, SET_DEFAULT,
+                        TAG_NONE, METERED_YES, ROAMING_NO, DEFAULT_NETWORK_YES,
+                        512L, 4L, 128L, 1L, 0L));
+        final NetworkStats tetherHwUidStats = new NetworkStats(now, 1)
+                .insertEntry(new NetworkStats.Entry(TEST_IFACE, UID_TETHERING, SET_DEFAULT,
+                        TAG_NONE, METERED_YES, ROAMING_NO, DEFAULT_NETWORK_YES,
+                        512L, 4L, 128L, 1L, 0L));
+        cb.notifyStatsUpdated(0 /* unused */, tetherHwIfaceStats, tetherHwUidStats);
 
-        // Traffic for UID_RED.
-        final NetworkStats uidStats = new NetworkStats(getElapsedRealtime(), 1)
+        // Fake some traffic done by apps on the device (as opposed to tethering), and record it
+        // into UID stats (as opposed to iface stats).
+        final NetworkStats localUidStats = new NetworkStats(now, 1)
                 .insertEntry(TEST_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 128L, 2L, 128L, 2L, 0L);
-        // All tethering traffic, both hardware and software.
-        final NetworkStats tetherStats = new NetworkStats(getElapsedRealtime(), 1)
-                .insertEntry(TEST_IFACE, UID_TETHERING, SET_DEFAULT, TAG_NONE, 1920L, 14L, 384L, 2L,
+        // Software per-uid tethering traffic.
+        final NetworkStats tetherSwUidStats = new NetworkStats(now, 1)
+                .insertEntry(TEST_IFACE, UID_TETHERING, SET_DEFAULT, TAG_NONE, 1408L, 10L, 256L, 1L,
                         0L);
 
-        expectNetworkStatsSummary(ifaceStats, tetherStatsHardware);
-        expectNetworkStatsUidDetail(uidStats, tetherStats);
+        expectNetworkStatsSummary(swIfaceStats);
+        expectNetworkStatsUidDetail(localUidStats, tetherSwUidStats);
         forcePollAndWaitForIdle();
 
         // verify service recorded history
@@ -1362,12 +1377,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
     }
 
     private void expectNetworkStatsSummary(NetworkStats summary) throws Exception {
-        expectNetworkStatsSummary(summary, new NetworkStats(0L, 0));
-    }
-
-    private void expectNetworkStatsSummary(NetworkStats summary, NetworkStats tetherStats)
-            throws Exception {
-        expectNetworkStatsTethering(STATS_PER_IFACE, tetherStats);
         expectNetworkStatsSummaryDev(summary.clone());
         expectNetworkStatsSummaryXt(summary.clone());
     }
@@ -1380,11 +1389,6 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest {
         when(mStatsFactory.readNetworkStatsSummaryXt()).thenReturn(summary);
     }
 
-    private void expectNetworkStatsTethering(int how, NetworkStats stats)
-            throws Exception {
-        when(mNetManager.getNetworkStatsTethering(how)).thenReturn(stats);
-    }
-
     private void expectNetworkStatsUidDetail(NetworkStats detail) throws Exception {
         expectNetworkStatsUidDetail(detail, new NetworkStats(0L, 0));
     }