Merge changes I289d935f,I0121a4ac,Ie73ba058,I4393afcb,Ifebd946e, ... into rvc-dev

* changes:
  NetworkStats: apply464xlatAdjustments - don't remove CLAT_UID
  NetworkStats: apply464xlatAdjustments - remove useBpfStats parameter.
  fix com.android.server.net.NetworkStatsFactoryTest#testDoubleClatAccounting100MBDownload
  fix com.android.server.net.NetworkStatsFactoryTest#testDoubleClatAccounting
  fix com.android.server.net.NetworkStatsFactoryTest#testDoubleClatAccountingSimple
  fix android.net.NetworkStatsTest#testApply464xlatAdjustments
  NetworkStats - no need for xt_qtaguid clat bw fixups

8 files changed, 48 insertions, 98 deletions

diff --git a/core/java/android/net/NetworkStats.java b/core/java/android/net/NetworkStats.java
index b7fb280efdc4..34e48eb44f32 100644
--- a/core/java/android/net/NetworkStats.java
+++ b/core/java/android/net/NetworkStats.java
@@ -16,8 +16,6 @@
 
 package android.net;
 
-import static android.os.Process.CLAT_UID;
-
 import android.annotation.IntDef;
 import android.annotation.NonNull;
 import android.annotation.Nullable;
@@ -1047,73 +1045,54 @@ public final class NetworkStats implements Parcelable {
     }
 
     /**
-     * Calculate and apply adjustments to captured statistics for 464xlat traffic counted twice.
+     * Calculate and apply adjustments to captured statistics for 464xlat traffic.
      *
-     * <p>This mutates both base and stacked traffic stats, to account respectively for
-     * double-counted traffic and IPv4/IPv6 header size difference.
+     * <p>This mutates stacked traffic stats, to account for IPv4/IPv6 header size difference.
      *
-     * <p>For 464xlat traffic, xt_qtaguid sees every IPv4 packet twice, once as a native IPv4
-     * packet on the stacked interface, and once as translated to an IPv6 packet on the
-     * base interface. For correct stats accounting on the base interface, if using xt_qtaguid,
-     * every rx 464xlat packet needs to be subtracted from the root UID on the base interface
-     * (http://b/12249687, http:/b/33681750), and every tx 464xlat packet which was counted onto
-     * clat uid should be ignored.
+     * <p>UID stats, which are only accounted on the stacked interface, need to be increased
+     * by 20 bytes/packet to account for translation overhead.
      *
-     * As for eBPF, the per uid stats is collected by different hook, the rx packets on base
-     * interface will not be counted. Thus, the adjustment on root uid is not needed. However, the
-     * tx traffic counted in the same way xt_qtaguid does, so the traffic on clat uid still
-     * needs to be ignored.
+     * <p>The potential additional overhead of 8 bytes/packet for ip fragments is ignored.
+     *
+     * <p>Interface stats need to sum traffic on both stacked and base interface because:
+     *   - eBPF offloaded packets appear only on the stacked interface
+     *   - Non-offloaded ingress packets appear only on the stacked interface
+     *     (due to iptables raw PREROUTING drop rules)
+     *   - Non-offloaded egress packets appear only on the stacked interface
+     *     (due to ignoring traffic from clat daemon by uid match)
+     * (and of course the 20 bytes/packet overhead needs to be applied to stacked interface stats)
      *
      * <p>This method will behave fine if {@code stackedIfaces} is an non-synchronized but add-only
      * {@code ConcurrentHashMap}
      * @param baseTraffic Traffic on the base interfaces. Will be mutated.
      * @param stackedTraffic Stats with traffic stacked on top of our ifaces. Will also be mutated.
      * @param stackedIfaces Mapping ipv6if -> ipv4if interface where traffic is counted on both.
-     * @param useBpfStats True if eBPF is in use.
      * @hide
      */
     public static void apply464xlatAdjustments(NetworkStats baseTraffic,
-            NetworkStats stackedTraffic, Map<String, String> stackedIfaces, boolean useBpfStats) {
-        // Total 464xlat traffic to subtract from uid 0 on all base interfaces.
-        // stackedIfaces may grow afterwards, but NetworkStats will just be resized automatically.
-        final NetworkStats adjustments = new NetworkStats(0, stackedIfaces.size());
-
+            NetworkStats stackedTraffic, Map<String, String> stackedIfaces) {
         // For recycling
         Entry entry = null;
-        Entry adjust = new NetworkStats.Entry(IFACE_ALL, 0, 0, 0, 0, 0, 0, 0L, 0L, 0L, 0L, 0L);
-
         for (int i = 0; i < stackedTraffic.size; i++) {
             entry = stackedTraffic.getValues(i, entry);
-            if (entry.iface == null || !entry.iface.startsWith(CLATD_INTERFACE_PREFIX)) {
-                continue;
-            }
-            final String baseIface = stackedIfaces.get(entry.iface);
-            if (baseIface == null) {
-                continue;
-            }
-            // Subtract xt_qtaguid 464lat rx traffic seen for the root UID on the current base
-            // interface. As for eBPF, the per uid stats is collected by different hook, the rx
-            // packets on base interface will not be counted.
-            adjust.iface = baseIface;
-            if (!useBpfStats) {
-                adjust.rxBytes = -(entry.rxBytes + entry.rxPackets * IPV4V6_HEADER_DELTA);
-                adjust.rxPackets = -entry.rxPackets;
-            }
-            adjustments.combineValues(adjust);
+            if (entry == null) continue;
+            if (entry.iface == null) continue;
+            if (!entry.iface.startsWith(CLATD_INTERFACE_PREFIX)) continue;
 
             // For 464xlat traffic, per uid stats only counts the bytes of the native IPv4 packet
             // sent on the stacked interface with prefix "v4-" and drops the IPv6 header size after
             // unwrapping. To account correctly for on-the-wire traffic, add the 20 additional bytes
             // difference for all packets (http://b/12249687, http:/b/33681750).
+            //
+            // Note: this doesn't account for LRO/GRO/GSO/TSO (ie. >mtu) traffic correctly, nor
+            // does it correctly account for the 8 extra bytes in the IPv6 fragmentation header.
+            //
+            // While the ebpf code path does try to simulate proper post segmentation packet
+            // counts, we have nothing of the sort of xt_qtaguid stats.
             entry.rxBytes += entry.rxPackets * IPV4V6_HEADER_DELTA;
             entry.txBytes += entry.txPackets * IPV4V6_HEADER_DELTA;
             stackedTraffic.setValues(i, entry);
         }
-
-        // Traffic on clat uid is v6 tx traffic that is already counted with app uid on the stacked
-        // v4 interface, so it needs to be removed to avoid double-counting.
-        baseTraffic.removeUids(new int[] {CLAT_UID});
-        baseTraffic.combineAllValues(adjustments);
     }
 
     /**
@@ -1125,8 +1104,8 @@ public final class NetworkStats implements Parcelable {
      * @param stackedIfaces Mapping ipv6if -> ipv4if interface where traffic is counted on both.
      * @hide
      */
-    public void apply464xlatAdjustments(Map<String, String> stackedIfaces, boolean useBpfStats) {
-        apply464xlatAdjustments(this, this, stackedIfaces, useBpfStats);
+    public void apply464xlatAdjustments(Map<String, String> stackedIfaces) {
+        apply464xlatAdjustments(this, this, stackedIfaces);
     }
 
     /**
diff --git a/services/core/java/com/android/server/net/NetworkStatsFactory.java b/services/core/java/com/android/server/net/NetworkStatsFactory.java
index 3dac106418c5..86ad0b308676 100644
--- a/services/core/java/com/android/server/net/NetworkStatsFactory.java
+++ b/services/core/java/com/android/server/net/NetworkStatsFactory.java
@@ -152,12 +152,10 @@ public class NetworkStatsFactory {
 
     /**
      * Applies 464xlat adjustments with ifaces noted with {@link #noteStackedIface(String, String)}.
-     * @see NetworkStats#apply464xlatAdjustments(NetworkStats, NetworkStats, Map, boolean)
+     * @see NetworkStats#apply464xlatAdjustments(NetworkStats, NetworkStats, Map)
      */
-    public void apply464xlatAdjustments(NetworkStats baseTraffic,
-            NetworkStats stackedTraffic, boolean useBpfStats) {
-        NetworkStats.apply464xlatAdjustments(baseTraffic, stackedTraffic, mStackedIfaces,
-                useBpfStats);
+    public void apply464xlatAdjustments(NetworkStats baseTraffic, NetworkStats stackedTraffic) {
+        NetworkStats.apply464xlatAdjustments(baseTraffic, stackedTraffic, mStackedIfaces);
     }
 
     public NetworkStatsFactory() {
@@ -380,7 +378,7 @@ public class NetworkStatsFactory {
         // network, the overhead is their fault.
         // No locking here: apply464xlatAdjustments behaves fine with an add-only
         // ConcurrentHashMap.
-        delta.apply464xlatAdjustments(mStackedIfaces, mUseBpfStats);
+        delta.apply464xlatAdjustments(mStackedIfaces);
 
         // Migrate data usage over a VPN to the TUN network.
         for (VpnInfo info : vpnArray) {
diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java
index cf5a41232123..ba9f486092f7 100644
--- a/services/core/java/com/android/server/net/NetworkStatsService.java
+++ b/services/core/java/com/android/server/net/NetworkStatsService.java
@@ -1882,14 +1882,13 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
         // fold tethering stats and operations into uid snapshot
         final NetworkStats tetherSnapshot = getNetworkStatsTethering(STATS_PER_UID);
         tetherSnapshot.filter(UID_ALL, ifaces, TAG_ALL);
-        mStatsFactory.apply464xlatAdjustments(uidSnapshot, tetherSnapshot,
-                mUseBpfTrafficStats);
+        mStatsFactory.apply464xlatAdjustments(uidSnapshot, tetherSnapshot);
         uidSnapshot.combineAllValues(tetherSnapshot);
 
         // get a stale copy of uid stats snapshot provided by providers.
         final NetworkStats providerStats = getNetworkStatsFromProviders(STATS_PER_UID);
         providerStats.filter(UID_ALL, ifaces, TAG_ALL);
-        mStatsFactory.apply464xlatAdjustments(uidSnapshot, providerStats, mUseBpfTrafficStats);
+        mStatsFactory.apply464xlatAdjustments(uidSnapshot, providerStats);
         uidSnapshot.combineAllValues(providerStats);
 
         uidSnapshot.combineAllValues(mUidOperations);
diff --git a/tests/net/java/android/net/NetworkStatsTest.java b/tests/net/java/android/net/NetworkStatsTest.java
index 98f705f45e98..735fa7cf3751 100644
--- a/tests/net/java/android/net/NetworkStatsTest.java
+++ b/tests/net/java/android/net/NetworkStatsTest.java
@@ -909,8 +909,8 @@ public class NetworkStatsTest {
                 13805 /* txPackets */,
                 0 /* operations */);
 
-        // Traffic measured for the root uid on the base interface if eBPF is in use.
-        final NetworkStats.Entry ebpfRootUidEntry = new NetworkStats.Entry(
+        // Traffic measured for the root uid on the base interface.
+        final NetworkStats.Entry rootUidEntry = new NetworkStats.Entry(
                 baseIface, rootUid, SET_DEFAULT, TAG_NONE,
                 163577 /* rxBytes */,
                 187 /* rxPackets */,
@@ -918,17 +918,6 @@ public class NetworkStatsTest {
                 97 /* txPackets */,
                 0 /* operations */);
 
-        // Traffic measured for the root uid on the base interface if xt_qtaguid is in use.
-        // Incorrectly includes appEntry's bytes and packets, plus IPv4-IPv6 translation
-        // overhead (20 bytes per packet), in rx direction.
-        final NetworkStats.Entry xtRootUidEntry = new NetworkStats.Entry(
-                baseIface, rootUid, SET_DEFAULT, TAG_NONE,
-                31113087 /* rxBytes */,
-                22588 /* rxPackets */,
-                17607 /* txBytes */,
-                97 /* txPackets */,
-                0 /* operations */);
-
         final NetworkStats.Entry otherEntry = new NetworkStats.Entry(
                 otherIface, appUid, SET_DEFAULT, TAG_NONE,
                 2600  /* rxBytes */,
@@ -937,21 +926,14 @@ public class NetworkStatsTest {
                 3 /* txPackets */,
                 0 /* operations */);
 
-        final NetworkStats statsXt = new NetworkStats(TEST_START, 3)
-                .insertEntry(appEntry)
-                .insertEntry(xtRootUidEntry)
-                .insertEntry(otherEntry);
-
-        final NetworkStats statsEbpf = new NetworkStats(TEST_START, 3)
+        final NetworkStats stats = new NetworkStats(TEST_START, 3)
                 .insertEntry(appEntry)
-                .insertEntry(ebpfRootUidEntry)
+                .insertEntry(rootUidEntry)
                 .insertEntry(otherEntry);
 
-        statsXt.apply464xlatAdjustments(stackedIface, false);
-        statsEbpf.apply464xlatAdjustments(stackedIface, true);
+        stats.apply464xlatAdjustments(stackedIface);
 
-        assertEquals(3, statsXt.size());
-        assertEquals(3, statsEbpf.size());
+        assertEquals(3, stats.size());
         final NetworkStats.Entry expectedAppUid = new NetworkStats.Entry(
                 v4Iface, appUid, SET_DEFAULT, TAG_NONE,
                 30949510,
@@ -966,12 +948,9 @@ public class NetworkStatsTest {
                 17607,
                 97,
                 0);
-        assertEquals(expectedAppUid, statsXt.getValues(0, null));
-        assertEquals(expectedRootUid, statsXt.getValues(1, null));
-        assertEquals(otherEntry, statsXt.getValues(2, null));
-        assertEquals(expectedAppUid, statsEbpf.getValues(0, null));
-        assertEquals(expectedRootUid, statsEbpf.getValues(1, null));
-        assertEquals(otherEntry, statsEbpf.getValues(2, null));
+        assertEquals(expectedAppUid, stats.getValues(0, null));
+        assertEquals(expectedRootUid, stats.getValues(1, null));
+        assertEquals(otherEntry, stats.getValues(2, null));
     }
 
     @Test
@@ -996,7 +975,7 @@ public class NetworkStatsTest {
                 .insertEntry(secondEntry);
 
         // Empty map: no adjustment
-        stats.apply464xlatAdjustments(new ArrayMap<>(), false);
+        stats.apply464xlatAdjustments(new ArrayMap<>());
 
         assertEquals(2, stats.size());
         assertEquals(firstEntry, stats.getValues(0, null));
diff --git a/tests/net/java/com/android/server/net/NetworkStatsFactoryTest.java b/tests/net/java/com/android/server/net/NetworkStatsFactoryTest.java
index 4473492d7972..e4996d981fac 100644
--- a/tests/net/java/com/android/server/net/NetworkStatsFactoryTest.java
+++ b/tests/net/java/com/android/server/net/NetworkStatsFactoryTest.java
@@ -446,7 +446,7 @@ public class NetworkStatsFactoryTest extends NetworkStatsBaseTest {
         assertStatsEntry(stats, "v4-wlan0", 1000, SET_DEFAULT, 0x0, 30812L, 2310L);
         assertStatsEntry(stats, "v4-wlan0", 10102, SET_DEFAULT, 0x0, 10022L, 3330L);
         assertStatsEntry(stats, "v4-wlan0", 10060, SET_DEFAULT, 0x0, 9532772L, 254112L);
-        assertStatsEntry(stats, "wlan0", 0, SET_DEFAULT, 0x0, 15229L, 0L);
+        assertStatsEntry(stats, "wlan0", 0, SET_DEFAULT, 0x0, 0L, 0L);
         assertStatsEntry(stats, "wlan0", 1000, SET_DEFAULT, 0x0, 6126L, 2013L);
         assertStatsEntry(stats, "wlan0", 10013, SET_DEFAULT, 0x0, 0L, 144L);
         assertStatsEntry(stats, "wlan0", 10018, SET_DEFAULT, 0x0, 5980263L, 167667L);
@@ -468,9 +468,7 @@ public class NetworkStatsFactoryTest extends NetworkStatsBaseTest {
         long appRxBytesAfter = 439237478L;
         assertEquals("App traffic should be ~100MB", 110553449, appRxBytesAfter - appRxBytesBefore);
 
-        long rootRxBytesBefore = 1394011L;
-        long rootRxBytesAfter = 1398634L;
-        assertEquals("UID 0 traffic should be ~0", 4623, rootRxBytesAfter - rootRxBytesBefore);
+        long rootRxBytes = 330187296L;
 
         mFactory.noteStackedIface("v4-wlan0", "wlan0");
         NetworkStats stats;
@@ -478,12 +476,12 @@ public class NetworkStatsFactoryTest extends NetworkStatsBaseTest {
         // Stats snapshot before the download
         stats = parseDetailedStats(R.raw.xt_qtaguid_with_clat_100mb_download_before);
         assertStatsEntry(stats, "v4-wlan0", 10106, SET_FOREGROUND, 0x0, appRxBytesBefore, 5199872L);
-        assertStatsEntry(stats, "wlan0", 0, SET_DEFAULT, 0x0, rootRxBytesBefore, 0L);
+        assertStatsEntry(stats, "wlan0", 0, SET_DEFAULT, 0x0, rootRxBytes, 0L);
 
         // Stats snapshot after the download
         stats = parseDetailedStats(R.raw.xt_qtaguid_with_clat_100mb_download_after);
         assertStatsEntry(stats, "v4-wlan0", 10106, SET_FOREGROUND, 0x0, appRxBytesAfter, 7867488L);
-        assertStatsEntry(stats, "wlan0", 0, SET_DEFAULT, 0x0, rootRxBytesAfter, 0L);
+        assertStatsEntry(stats, "wlan0", 0, SET_DEFAULT, 0x0, rootRxBytes, 0L);
     }
 
     /**
diff --git a/tests/net/res/raw/xt_qtaguid_with_clat b/tests/net/res/raw/xt_qtaguid_with_clat
index 6cd7499545be..f04b32f08332 100644
--- a/tests/net/res/raw/xt_qtaguid_with_clat
+++ b/tests/net/res/raw/xt_qtaguid_with_clat
@@ -7,7 +7,7 @@ idx iface acct_tag_hex uid_tag_int cnt_set rx_bytes rx_packets tx_bytes tx_packe
 7 v4-wlan0 0x0 10060 1 1448660 1041 31192 753 1448660 1041 0 0 0 0 31192 753 0 0 0 0
 8 v4-wlan0 0x0 10102 0 9702 16 2870 23 9702 16 0 0 0 0 2870 23 0 0 0 0
 9 v4-wlan0 0x0 10102 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
-10 wlan0 0x0 0 0 11058671 7892 0 0 11043898 7811 13117 61 1656 20 0 0 0 0 0 0
+10 wlan0 0x0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
 11 wlan0 0x0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
 12 wlan0 0x0 1000 0 6126 13 2013 16 5934 11 192 2 0 0 1821 14 192 2 0 0
 13 wlan0 0x0 1000 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
@@ -41,5 +41,3 @@ idx iface acct_tag_hex uid_tag_int cnt_set rx_bytes rx_packets tx_bytes tx_packe
 41 dummy0 0x0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
 42 lo 0x0 0 0 1288 16 1288 16 0 0 532 8 756 8 0 0 532 8 756 8
 43 lo 0x0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
-44 wlan0 0x0 1029 0 0 0 312046 5113 0 0 0 0 0 0 306544 5046 3230 38 2272 29
-45 wlan0 0x0 1029 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
\ No newline at end of file
diff --git a/tests/net/res/raw/xt_qtaguid_with_clat_100mb_download_after b/tests/net/res/raw/xt_qtaguid_with_clat_100mb_download_after
index 9f86153a33cf..12d98ca29f57 100644
--- a/tests/net/res/raw/xt_qtaguid_with_clat_100mb_download_after
+++ b/tests/net/res/raw/xt_qtaguid_with_clat_100mb_download_after
@@ -9,7 +9,7 @@ idx iface acct_tag_hex uid_tag_int cnt_set rx_bytes rx_packets tx_bytes tx_packe
 9 v4-wlan0 0x0 10057 1 728 7 392 7 0 0 728 7 0 0 0 0 392 7 0 0
 10 v4-wlan0 0x0 10106 0 2232 18 2232 18 0 0 2232 18 0 0 0 0 2232 18 0 0
 11 v4-wlan0 0x0 10106 1 432952718 314238 5442288 121260 432950238 314218 2480 20 0 0 5433900 121029 8388 231 0 0
-12 wlan0 0x0 0 0 440746376 329772 0 0 439660007 315369 232001 1276 854368 13127 0 0 0 0 0 0
+12 wlan0 0x0 0 0 330187296 250652 0 0 329106990 236273 226202 1255 854104 13124 0 0 0 0 0 0
 13 wlan0 0x0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
 14 wlan0 0x0 1000 0 77113 272 56151 575 77113 272 0 0 0 0 19191 190 36960 385 0 0
 15 wlan0 0x0 1000 1 20227 80 8356 72 18539 74 1688 6 0 0 7562 66 794 6 0 0
diff --git a/tests/net/res/raw/xt_qtaguid_with_clat_simple b/tests/net/res/raw/xt_qtaguid_with_clat_simple
index b37fae6d2a3d..a1d6d411bad8 100644
--- a/tests/net/res/raw/xt_qtaguid_with_clat_simple
+++ b/tests/net/res/raw/xt_qtaguid_with_clat_simple
@@ -1,5 +1,4 @@
 idx iface acct_tag_hex uid_tag_int cnt_set rx_bytes rx_packets tx_bytes tx_packets rx_tcp_bytes rx_tcp_packets rx_udp_bytes rx_udp_packets rx_other_bytes rx_other_packets tx_tcp_bytes tx_tcp_packets tx_udp_bytes tx_udp_packets tx_other_bytes tx_other_packets
 2 v4-wlan0 0x0 10060 0 42600 213 4100 41 42600 213 0 0 0 0 4100 41 0 0 0 0
 3 v4-wlan0 0x0 10060 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
-4 wlan0 0x0 0 0 46860 213 0 0 46860 213 0 0 0 0 0 0 0 0 0 0
-5 wlan0 0x0 1029 0 0 0 4920 41 0 0 0 0 0 0 4920 41 0 0 0 0
+4 wlan0 0x0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0