From b37958eec98824b1ac73feca9df98911821db498 Mon Sep 17 00:00:00 2001 From: Cody Kesting Date: Fri, 15 May 2020 10:36:01 -0700 Subject: [PATCH] Forward unknown Data Stall types to Connectivity Diagnostics. This CL forwards suspected Data Stall events detected with unknown detection methods to ConnectivityDiagnostics. Currently, ConnectivityService drops any data stall events with unknown detection methods, which leads to false negatives for Connectivity Diagnostics registrants. This change ensures that registrants will still be notified as NetworkStack is updated to use new detection methods. The documentation for ConnectivityDiagnosticsManager#DataStallReport is also updated to reflect that the detection methods included in the report are a bit mask of detection methods used. Implicitly, this means that data stalls detected via unknown methods will have an empty bit mask (0x00). Bug: 156294356 Test: atest ConnectivityDiagnosticsManager Change-Id: I62d0bf91fcc17c7921afd519c72551399906bd6b --- .../net/ConnectivityDiagnosticsManager.java | 6 +-- .../android/server/ConnectivityService.java | 51 +++++++++++-------- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/core/java/android/net/ConnectivityDiagnosticsManager.java b/core/java/android/net/ConnectivityDiagnosticsManager.java index 9086d49231..275e38c744 100644 --- a/core/java/android/net/ConnectivityDiagnosticsManager.java +++ b/core/java/android/net/ConnectivityDiagnosticsManager.java @@ -437,7 +437,7 @@ public class ConnectivityDiagnosticsManager { */ private long mReportTimestamp; - /** The detection method used to identify the suspected data stall */ + /** A bitmask of the detection methods used to identify the suspected data stall */ @DetectionMethod private final int mDetectionMethod; /** LinkProperties available on the Network at the reported timestamp */ @@ -499,9 +499,9 @@ public class ConnectivityDiagnosticsManager { } /** - * Returns the detection method used to identify this suspected data stall. + * Returns the bitmask of detection methods used to identify this suspected data stall. * - * @return The detection method used to identify the suspected data stall + * @return The bitmask of detection methods used to identify the suspected data stall */ public int getDetectionMethod() { return mDetectionMethod; diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index efa1f9af67..6e026ab7fe 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -3094,30 +3094,24 @@ public class ConnectivityService extends IConnectivityManager.Stub } private void notifyDataStallSuspected(DataStallReportParcelable p, int netId) { + log("Data stall detected with methods: " + p.detectionMethod); + final PersistableBundle extras = new PersistableBundle(); - switch (p.detectionMethod) { - case DETECTION_METHOD_DNS_EVENTS: - extras.putInt(KEY_DNS_CONSECUTIVE_TIMEOUTS, p.dnsConsecutiveTimeouts); - break; - case DETECTION_METHOD_TCP_METRICS: - extras.putInt(KEY_TCP_PACKET_FAIL_RATE, p.tcpPacketFailRate); - extras.putInt(KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS, - p.tcpMetricsCollectionPeriodMillis); - break; - default: - // TODO(b/156294356): update for new data stall detection methods - log("Unknown data stall detection method, ignoring: " + p.detectionMethod); - return; + int detectionMethod = 0; + if (hasDataStallDetectionMethod(p, DETECTION_METHOD_DNS_EVENTS)) { + extras.putInt(KEY_DNS_CONSECUTIVE_TIMEOUTS, p.dnsConsecutiveTimeouts); + detectionMethod |= DETECTION_METHOD_DNS_EVENTS; + } + if (hasDataStallDetectionMethod(p, DETECTION_METHOD_TCP_METRICS)) { + extras.putInt(KEY_TCP_PACKET_FAIL_RATE, p.tcpPacketFailRate); + extras.putInt(KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS, + p.tcpMetricsCollectionPeriodMillis); + detectionMethod |= DETECTION_METHOD_TCP_METRICS; } - notifyDataStallSuspected(p.detectionMethod, netId, p.timestampMillis, extras); - } - - private void notifyDataStallSuspected(int detectionMethod, int netId, long timestampMillis, - @NonNull PersistableBundle extras) { final Message msg = mConnectivityDiagnosticsHandler.obtainMessage( ConnectivityDiagnosticsHandler.EVENT_DATA_STALL_SUSPECTED, detectionMethod, netId, - timestampMillis); + p.timestampMillis); msg.setData(new Bundle(extras)); // NetworkStateTrackerHandler currently doesn't take any actions based on data @@ -3126,6 +3120,10 @@ public class ConnectivityService extends IConnectivityManager.Stub mConnectivityDiagnosticsHandler.sendMessage(msg); } + private boolean hasDataStallDetectionMethod(DataStallReportParcelable p, int detectionMethod) { + return (p.detectionMethod & detectionMethod) != 0; + } + private boolean networkRequiresPrivateDnsValidation(NetworkAgentInfo nai) { return isPrivateDnsValidationRequired(nai.networkCapabilities); } @@ -8185,6 +8183,19 @@ public class ConnectivityService extends IConnectivityManager.Stub + "creators"); } - notifyDataStallSuspected(detectionMethod, network.netId, timestampMillis, extras); + final DataStallReportParcelable p = new DataStallReportParcelable(); + p.timestampMillis = timestampMillis; + p.detectionMethod = detectionMethod; + + if (hasDataStallDetectionMethod(p, DETECTION_METHOD_DNS_EVENTS)) { + p.dnsConsecutiveTimeouts = extras.getInt(KEY_DNS_CONSECUTIVE_TIMEOUTS); + } + if (hasDataStallDetectionMethod(p, DETECTION_METHOD_TCP_METRICS)) { + p.tcpPacketFailRate = extras.getInt(KEY_TCP_PACKET_FAIL_RATE); + p.tcpMetricsCollectionPeriodMillis = extras.getInt( + KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS); + } + + notifyDataStallSuspected(p, network.netId); } }