Merge "add TCP data stall metrics" into rvc-dev

4 files changed, 138 insertions, 31 deletions

diff --git a/src/android/net/util/DataStallUtils.java b/src/android/net/util/DataStallUtils.java
index 5787879..3391a71 100644
--- a/src/android/net/util/DataStallUtils.java
+++ b/src/android/net/util/DataStallUtils.java
@@ -16,15 +16,30 @@
 
 package android.net.util;
 
+import android.annotation.IntDef;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
 /**
  * Collection of utilities for data stall.
  */
 public class DataStallUtils {
+    public static final int DATA_STALL_EVALUATION_TYPE_NONE = 0;
     /** Detect data stall using dns timeout counts. */
     public static final int DATA_STALL_EVALUATION_TYPE_DNS = 1 << 0;
     /** Detect data stall using tcp connection fail rate. */
     public static final int DATA_STALL_EVALUATION_TYPE_TCP = 1 << 1;
 
+    @IntDef(prefix = { "DATA_STALL_EVALUATION_TYPE_" }, value = {
+        DATA_STALL_EVALUATION_TYPE_NONE,
+        DATA_STALL_EVALUATION_TYPE_DNS,
+        DATA_STALL_EVALUATION_TYPE_TCP,
+    })
+    @Retention(RetentionPolicy.SOURCE)
+    public @interface EvaluationType {
+    }
+
     // Default configuration values for data stall detection.
     public static final int DEFAULT_CONSECUTIVE_DNS_TIMEOUT_THRESHOLD = 5;
     public static final int DEFAULT_DATA_STALL_MIN_EVALUATE_TIME_MS = 60 * 1000;
diff --git a/src/com/android/networkstack/metrics/DataStallDetectionStats.java b/src/com/android/networkstack/metrics/DataStallDetectionStats.java
index 7a1f9ac..d294e04 100644
--- a/src/com/android/networkstack/metrics/DataStallDetectionStats.java
+++ b/src/com/android/networkstack/metrics/DataStallDetectionStats.java
@@ -19,8 +19,10 @@ package com.android.networkstack.metrics;
 import android.net.util.NetworkStackUtils;
 import android.net.wifi.WifiInfo;
 
+import androidx.annotation.IntRange;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import androidx.annotation.VisibleForTesting;
 
 import com.android.internal.util.HexDump;
 import com.android.server.connectivity.nano.CellularData;
@@ -41,18 +43,35 @@ import java.util.Objects;
  * @hide
  */
 public final class DataStallDetectionStats {
-    private static final int UNKNOWN_SIGNAL_STRENGTH = -1;
+    public static final int UNKNOWN_SIGNAL_STRENGTH = -1;
+    // Default value of TCP signals.
+    @VisibleForTesting
+    public static final int UNSPECIFIED_TCP_FAIL_RATE = -1;
+    @VisibleForTesting
+    public static final int UNSPECIFIED_TCP_PACKETS_COUNT = -1;
+    @VisibleForTesting
     @NonNull
-    final byte[] mCellularInfo;
+    public final byte[] mCellularInfo;
+    @VisibleForTesting
     @NonNull
-    final byte[] mWifiInfo;
+    public final byte[] mWifiInfo;
     @NonNull
-    final byte[] mDns;
-    final int mEvaluationType;
-    final int mNetworkType;
+    public final byte[] mDns;
+    @VisibleForTesting
+    public final int mEvaluationType;
+    @VisibleForTesting
+    public final int mNetworkType;
+    // The TCP packets fail rate percentage from the latest tcp polling. -1 means the TCP signal is
+    // not known or not supported on the SDK version of this device.
+    @VisibleForTesting @IntRange(from = -1, to = 100)
+    public final int mTcpFailRate;
+    // Number of packets sent since the last received packet.
+    @VisibleForTesting
+    public final int mTcpSentSinceLastRecv;
 
     public DataStallDetectionStats(@Nullable byte[] cell, @Nullable byte[] wifi,
-                @NonNull int[] returnCode, @NonNull long[] dnsTime, int evalType, int netType) {
+                @NonNull int[] returnCode, @NonNull long[] dnsTime, int evalType, int netType,
+                int failRate, int sentSinceLastRecv) {
         mCellularInfo = emptyCellDataIfNull(cell);
         mWifiInfo = emptyWifiInfoIfNull(wifi);
 
@@ -62,9 +81,20 @@ public final class DataStallDetectionStats {
         mDns = MessageNano.toByteArray(dns);
         mEvaluationType = evalType;
         mNetworkType = netType;
+        mTcpFailRate = failRate;
+        mTcpSentSinceLastRecv = sentSinceLastRecv;
     }
 
-    private byte[] emptyCellDataIfNull(@Nullable byte[] cell) {
+    /**
+     * Because metrics data must contain data for each field even if it's not supported or not
+     * available, generate a byte array representing an empty {@link CellularData} if the
+     * {@link CellularData} is unavailable.
+     *
+     * @param cell a byte array representing current {@link CellularData} of {@code this}
+     * @return a byte array of a {@link CellularData}.
+     */
+    @VisibleForTesting
+    public static byte[] emptyCellDataIfNull(@Nullable byte[] cell) {
         if (cell != null) return cell;
 
         CellularData data  = new CellularData();
@@ -75,7 +105,16 @@ public final class DataStallDetectionStats {
         return MessageNano.toByteArray(data);
     }
 
-    private byte[] emptyWifiInfoIfNull(@Nullable byte[] wifi) {
+    /**
+     * Because metrics data must contain data for each field even if it's not supported or not
+     * available, generate a byte array representing an empty {@link WifiData} if the
+     * {@link WiFiData} is unavailable.
+     *
+     * @param wifi a byte array representing current {@link WiFiData} of {@code this}.
+     * @return a byte array of a {@link WiFiData}.
+     */
+    @VisibleForTesting
+    public static byte[] emptyWifiInfoIfNull(@Nullable byte[] wifi) {
         if (wifi != null) return wifi;
 
         WifiData data = new WifiData();
@@ -95,7 +134,11 @@ public final class DataStallDetectionStats {
           .append(", cell info: ")
           .append(HexDump.toHexString(mCellularInfo))
           .append(", dns: ")
-          .append(HexDump.toHexString(mDns));
+          .append(HexDump.toHexString(mDns))
+          .append(", tcp fail rate: ")
+          .append(mTcpFailRate)
+          .append(", tcp received: ")
+          .append(mTcpSentSinceLastRecv);
         return sb.toString();
     }
 
@@ -107,12 +150,15 @@ public final class DataStallDetectionStats {
             && (mEvaluationType == other.mEvaluationType)
             && Arrays.equals(mWifiInfo, other.mWifiInfo)
             && Arrays.equals(mCellularInfo, other.mCellularInfo)
-            && Arrays.equals(mDns, other.mDns);
+            && Arrays.equals(mDns, other.mDns)
+            && (mTcpFailRate == other.mTcpFailRate)
+            && (mTcpSentSinceLastRecv == other.mTcpSentSinceLastRecv);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(mNetworkType, mEvaluationType, mWifiInfo, mCellularInfo, mDns);
+        return Objects.hash(mNetworkType, mEvaluationType, mWifiInfo, mCellularInfo, mDns,
+                mTcpFailRate, mTcpSentSinceLastRecv);
     }
 
     /**
@@ -131,6 +177,8 @@ public final class DataStallDetectionStats {
         private final List<Long> mDnsTimeStamp = new ArrayList<Long>();
         private int mEvaluationType;
         private int mNetworkType;
+        private int mTcpFailRate = UNSPECIFIED_TCP_FAIL_RATE;
+        private int mTcpSentSinceLastRecv = UNSPECIFIED_TCP_PACKETS_COUNT;
 
         /**
          * Add a dns event into Builder.
@@ -168,6 +216,34 @@ public final class DataStallDetectionStats {
         }
 
         /**
+         * Set the TCP packet fail rate into Builder. The data is included since android R.
+         *
+         * @param rate the TCP packet fail rate of the logged network. The default value is
+         *               {@code UNSPECIFIED_TCP_FAIL_RATE}, which means the TCP signal is not known
+         *               or not supported on the SDK version of this device.
+         * @return {@code this} {@link Builder} instance.
+         */
+        public Builder setTcpFailRate(@IntRange(from = -1, to = 100) int rate) {
+            mTcpFailRate = rate;
+            return this;
+        }
+
+        /**
+         * Set the number of TCP packets sent since the last received packet into Builder. The data
+         * starts to be included since android R.
+         *
+         * @param count the number of packets sent since the last received packet of the logged
+         *              network. Keep it unset as default value or set to
+         *              {@code UNSPECIFIED_TCP_PACKETS_COUNT} if the tcp signal is unsupported with
+         *              current device android sdk version or the packets count is unknown.
+         * @return {@code this} {@link Builder} instance.
+         */
+        public Builder setTcpSentSinceLastRecv(int count) {
+            mTcpSentSinceLastRecv = count;
+            return this;
+        }
+
+        /**
          * Set the wifi data into Builder.
          *
          * @param info a {@link WifiInfo} of the connected wifi network.
@@ -223,7 +299,7 @@ public final class DataStallDetectionStats {
             return new DataStallDetectionStats(mCellularInfo, mWifiInfo,
                     NetworkStackUtils.convertToIntArray(mDnsReturnCode),
                     NetworkStackUtils.convertToLongArray(mDnsTimeStamp),
-                    mEvaluationType, mNetworkType);
+                    mEvaluationType, mNetworkType, mTcpFailRate, mTcpSentSinceLastRecv);
         }
     }
 }
diff --git a/src/com/android/networkstack/metrics/DataStallStatsUtils.java b/src/com/android/networkstack/metrics/DataStallStatsUtils.java
index e7a6c3d..c0719c0 100644
--- a/src/com/android/networkstack/metrics/DataStallStatsUtils.java
+++ b/src/com/android/networkstack/metrics/DataStallStatsUtils.java
@@ -74,6 +74,8 @@ public class DataStallStatsUtils {
                 stats.mNetworkType,
                 stats.mWifiInfo,
                 stats.mCellularInfo,
-                stats.mDns);
+                stats.mDns,
+                stats.mTcpFailRate,
+                stats.mTcpSentSinceLastRecv);
     }
 }
diff --git a/src/com/android/server/connectivity/NetworkMonitor.java b/src/com/android/server/connectivity/NetworkMonitor.java
index 7fb6761..093118a 100755
--- a/src/com/android/server/connectivity/NetworkMonitor.java
+++ b/src/com/android/server/connectivity/NetworkMonitor.java
@@ -48,6 +48,7 @@ import static android.net.util.DataStallUtils.CONFIG_DATA_STALL_MIN_EVALUATE_INT
 import static android.net.util.DataStallUtils.CONFIG_DATA_STALL_TCP_POLLING_INTERVAL;
 import static android.net.util.DataStallUtils.CONFIG_DATA_STALL_VALID_DNS_TIME_THRESHOLD;
 import static android.net.util.DataStallUtils.DATA_STALL_EVALUATION_TYPE_DNS;
+import static android.net.util.DataStallUtils.DATA_STALL_EVALUATION_TYPE_NONE;
 import static android.net.util.DataStallUtils.DATA_STALL_EVALUATION_TYPE_TCP;
 import static android.net.util.DataStallUtils.DEFAULT_CONSECUTIVE_DNS_TIMEOUT_THRESHOLD;
 import static android.net.util.DataStallUtils.DEFAULT_DATA_STALL_EVALUATION_TYPES;
@@ -112,6 +113,7 @@ import android.net.metrics.NetworkEvent;
 import android.net.metrics.ValidationProbeEvent;
 import android.net.shared.NetworkMonitorUtils;
 import android.net.shared.PrivateDnsConfig;
+import android.net.util.DataStallUtils.EvaluationType;
 import android.net.util.NetworkStackUtils;
 import android.net.util.SharedLog;
 import android.net.util.Stopwatch;
@@ -489,8 +491,8 @@ public class NetworkMonitor extends StateMachine {
     @Nullable
     private final DnsStallDetector mDnsStallDetector;
     private long mLastProbeTime;
-    // Set to true if data stall is suspected and reset to false after metrics are sent to statsd.
-    private boolean mCollectDataStallMetrics;
+    // The signal causing a data stall to be suspected. Reset to 0 after metrics are sent to statsd.
+    private @EvaluationType int mDataStallTypeToCollect;
     private boolean mAcceptPartialConnectivity = false;
     private final EvaluationState mEvaluationState = new EvaluationState();
 
@@ -977,8 +979,6 @@ public class NetworkMonitor extends StateMachine {
 
         boolean evaluateDataStall() {
             if (isDataStall()) {
-                // TODO: Add tcp info into metrics.
-                mCollectDataStallMetrics = true;
                 validationLog("Suspecting data stall, reevaluate");
                 return true;
             }
@@ -999,7 +999,8 @@ public class NetworkMonitor extends StateMachine {
         }
     }
 
-    private void writeDataStallStats(@NonNull final CaptivePortalProbeResult result) {
+    private void maybeWriteDataStallStats(@NonNull final CaptivePortalProbeResult result) {
+        if (mDataStallTypeToCollect == DATA_STALL_EVALUATION_TYPE_NONE) return;
         /*
          * Collect data stall detection level information for each transport type. Collect type
          * specific information for cellular and wifi only currently. Generate
@@ -1007,19 +1008,22 @@ public class NetworkMonitor extends StateMachine {
          * TRANSPORT_WIFI and TRANSPORT_VPN, two DataStallDetectionStats will be generated.
          */
         final int[] transports = mNetworkCapabilities.getTransportTypes();
-
         for (int i = 0; i < transports.length; i++) {
-            final DataStallDetectionStats stats = buildDataStallDetectionStats(transports[i]);
+            final DataStallDetectionStats stats =
+                    buildDataStallDetectionStats(transports[i], mDataStallTypeToCollect);
             mDependencies.writeDataStallDetectionStats(stats, result);
         }
-        mCollectDataStallMetrics = false;
+        mDataStallTypeToCollect = DATA_STALL_EVALUATION_TYPE_NONE;
     }
 
     @VisibleForTesting
-    protected DataStallDetectionStats buildDataStallDetectionStats(int transport) {
+    protected DataStallDetectionStats buildDataStallDetectionStats(int transport,
+            @EvaluationType int evaluationType) {
         final DataStallDetectionStats.Builder stats = new DataStallDetectionStats.Builder();
-        if (VDBG_STALL) log("collectDataStallMetrics: type=" + transport);
-        stats.setEvaluationType(DATA_STALL_EVALUATION_TYPE_DNS);
+        if (VDBG_STALL) {
+            log("collectDataStallMetrics: type=" + transport + ", evaluation=" + evaluationType);
+        }
+        stats.setEvaluationType(evaluationType);
         stats.setNetworkType(transport);
         switch (transport) {
             case NetworkCapabilities.TRANSPORT_WIFI:
@@ -1044,11 +1048,21 @@ public class NetworkMonitor extends StateMachine {
                 // No transport type specific information for the other types.
                 break;
         }
+
         addDnsEvents(stats);
+        addTcpStats(stats);
 
         return stats.build();
     }
 
+    private void addTcpStats(@NonNull final DataStallDetectionStats.Builder stats) {
+        final TcpSocketTracker tst = getTcpSocketTracker();
+        if (tst == null) return;
+
+        stats.setTcpSentSinceLastRecv(tst.getSentSinceLastRecv());
+        stats.setTcpFailRate(tst.getLatestPacketFailPercentage());
+    }
+
     @VisibleForTesting
     protected void addDnsEvents(@NonNull final DataStallDetectionStats.Builder stats) {
         final DnsStallDetector dsd = getDnsStallDetector();
@@ -1438,9 +1452,7 @@ public class NetworkMonitor extends StateMachine {
                             (CaptivePortalProbeResult) message.obj;
                     mLastProbeTime = SystemClock.elapsedRealtime();
 
-                    if (mCollectDataStallMetrics) {
-                        writeDataStallStats(probeResult);
-                    }
+                    maybeWriteDataStallStats(probeResult);
 
                     if (probeResult.isSuccessful()) {
                         // Transit EvaluatingPrivateDnsState to get to Validated
@@ -1919,7 +1931,8 @@ public class NetworkMonitor extends StateMachine {
                 DEFAULT_DATA_STALL_VALID_DNS_TIME_THRESHOLD_MS);
     }
 
-    private int getDataStallEvaluationType() {
+    @VisibleForTesting
+    int getDataStallEvaluationType() {
         return mDependencies.getDeviceConfigPropertyInt(NAMESPACE_CONNECTIVITY,
                 CONFIG_DATA_STALL_EVALUATION_TYPE,
                 DEFAULT_DATA_STALL_EVALUATION_TYPES);
@@ -3140,9 +3153,8 @@ public class NetworkMonitor extends StateMachine {
         return mDnsStallDetector;
     }
 
-    @VisibleForTesting
     @Nullable
-    protected TcpSocketTracker getTcpSocketTracker() {
+    private TcpSocketTracker getTcpSocketTracker() {
         return mTcpTracker;
     }
 
@@ -3179,6 +3191,7 @@ public class NetworkMonitor extends StateMachine {
                 result = false;
             } else if (tst.isDataStallSuspected()) {
                 result = true;
+                mDataStallTypeToCollect = DATA_STALL_EVALUATION_TYPE_TCP;
 
                 final DataStallReportParcelable p = new DataStallReportParcelable();
                 p.detectionMethod = DETECTION_METHOD_TCP_METRICS;
@@ -3203,6 +3216,7 @@ public class NetworkMonitor extends StateMachine {
             if (dsd.isDataStallSuspected(mConsecutiveDnsTimeoutThreshold,
                     mDataStallValidDnsTimeThreshold)) {
                 result = true;
+                mDataStallTypeToCollect = DATA_STALL_EVALUATION_TYPE_DNS;
                 logNetworkEvent(NetworkEvent.NETWORK_CONSECUTIVE_DNS_TIMEOUT_FOUND);
 
                 final DataStallReportParcelable p = new DataStallReportParcelable();