Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 782cbfa3 authored by Cody Kesting's avatar Cody Kesting
Browse files

Define callback for suspected data stall. 

Currently, NetworkStack does not notify ConnectivityService explicitly
when it detects a potential data stall. This adds a new method
notifyDataStallSuspected() to INetworkMonitorCallbacks for notifying
ConnectivityService.

ConstantsShim is defined for using constants from the Android platform
that are introduced after the NetworkStack mainline module was defined.

Bug: 143187964
Test: compiles
Test: atest NetworkStackTests
Change-Id: Ie3b5b4e71e4a4b15a5a84f761eaead3acf6fc807
Merged-In: Ie3b5b4e71e4a4b15a5a84f761eaead3acf6fc807
parent 33f2e3bc

apishim/29/com/android/networkstack/apishim/api29/ConstantsShim.java

0 → 100644
+26 −0
Original line number Diff line number Diff line 
/*

 * 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 com.android.networkstack.apishim.api29;



/**

 * Utility class for defining and importing constants from the Android platform.

 */

public class ConstantsShim {

    // Constants defined in android.net.ConnectivityDiagnosticsManager.DataStallReport.

    public static final int DETECTION_METHOD_DNS_EVENTS = 1;

    public static final int DETECTION_METHOD_TCP_METRICS = 2;

}

apishim/30/com/android/networkstack/apishim/ConstantsShim.java

0 → 100644
+29 −0
Original line number Diff line number Diff line 
/*

 * 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 com.android.networkstack.apishim;



import static android.net.ConnectivityDiagnosticsManager.DataStallReport;



/**

 * Utility class for defining and importing constants from the Android platform.

 */

public class ConstantsShim extends com.android.networkstack.apishim.api29.ConstantsShim {

    public static final int DETECTION_METHOD_DNS_EVENTS =

            DataStallReport.DETECTION_METHOD_DNS_EVENTS;

    public static final int DETECTION_METHOD_TCP_METRICS =

            DataStallReport.DETECTION_METHOD_TCP_METRICS;

}

common/networkstackclient/src/android/net/INetworkMonitorCallbacks.aidl

+2 −0
Original line number Diff line number Diff line
@@ -31,4 +31,6 @@ oneway interface INetworkMonitorCallbacks { 
    void notifyProbeStatusChanged(int probesCompleted, int probesSucceeded);

    void notifyNetworkTestedWithExtras(int testResult, @nullable String redirectUrl,

            long timestampMillis, in PersistableBundle extras);

    void notifyDataStallSuspected(long timestampMillis, int detectionMethod,

            in PersistableBundle extras);

}
 
 No newline at end of file

src/com/android/server/connectivity/NetworkMonitor.java

+18 −0
Original line number Diff line number Diff line
@@ -68,6 +68,8 @@ import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_USE_HTTPS; 
import static android.net.util.NetworkStackUtils.isEmpty;

import static android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY;



import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_DNS_EVENTS;

import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_TCP_METRICS;

import static com.android.networkstack.util.DnsUtils.PRIVATE_DNS_PROBE_HOST_SUFFIX;

import static com.android.networkstack.util.DnsUtils.TYPE_ADDRCONFIG;
@@ -103,6 +105,7 @@ import android.net.wifi.WifiManager; 
import android.os.Build;

import android.os.Bundle;

import android.os.Message;

import android.os.PersistableBundle;

import android.os.Process;

import android.os.RemoteException;

import android.os.SystemClock;
@@ -613,6 +616,15 @@ public class NetworkMonitor extends StateMachine { 
        }

    }



    private void notifyDataStallSuspected(int detectionMethod, PersistableBundle extras) {

        try {

            mCallback.notifyDataStallSuspected(

                    SystemClock.elapsedRealtime(), detectionMethod, extras);

        } catch (RemoteException e) {

            Log.e(TAG, "Error sending notification for suspected data stall", e);

        }

    }



    // DefaultState is the parent of all States.  It exists only to handle CMD_* messages but

    // does not entail any real state (hence no enter() or exit() routines).

    private class DefaultState extends State {
@@ -2275,6 +2287,9 @@ public class NetworkMonitor extends StateMachine { 
                result = false;

            } else if (tst.isDataStallSuspected()) {

                result = true;



                // TODO(b/147249364): add metrics to PersistableBundle once keys are defined

                notifyDataStallSuspected(DETECTION_METHOD_TCP_METRICS, PersistableBundle.EMPTY);

            }

            if (DBG || VDBG_STALL) {

                msg.add("tcp packets received=" + tst.getLatestReceivedCount())
@@ -2292,6 +2307,9 @@ public class NetworkMonitor extends StateMachine { 
                    mDataStallValidDnsTimeThreshold)) {

                result = true;

                logNetworkEvent(NetworkEvent.NETWORK_CONSECUTIVE_DNS_TIMEOUT_FOUND);



                // TODO(b/147249364): add metrics to PersistableBundle once keys are defined

                notifyDataStallSuspected(DETECTION_METHOD_DNS_EVENTS, PersistableBundle.EMPTY);

            }

            if (DBG || VDBG_STALL) {

                msg.add("consecutive dns timeout count=" + dsd.getConsecutiveTimeoutCount());

tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java

+21 −5
Original line number Diff line number Diff line
@@ -38,6 +38,8 @@ import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_FALLBACK_PROBE_S 
import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_OTHER_FALLBACK_URLS;

import static android.net.util.NetworkStackUtils.CAPTIVE_PORTAL_USE_HTTPS;



import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_DNS_EVENTS;

import static com.android.networkstack.apishim.ConstantsShim.DETECTION_METHOD_TCP_METRICS;

import static com.android.networkstack.util.DnsUtils.PRIVATE_DNS_PROBE_HOST_SUFFIX;



import static junit.framework.Assert.assertEquals;
@@ -48,6 +50,7 @@ import static org.junit.Assert.assertNotNull; 
import static org.junit.Assert.assertNull;

import static org.junit.Assert.assertTrue;

import static org.junit.Assert.fail;

import static org.mockito.ArgumentMatchers.anyLong;

import static org.mockito.ArgumentMatchers.eq;

import static org.mockito.Mockito.any;

import static org.mockito.Mockito.anyInt;
@@ -88,6 +91,7 @@ import android.os.Bundle; 
import android.os.ConditionVariable;

import android.os.Handler;

import android.os.Looper;

import android.os.PersistableBundle;

import android.os.Process;

import android.os.RemoteException;

import android.os.SystemClock;
@@ -771,15 +775,17 @@ public class NetworkMonitorTest { 
    }



    @Test

    public void testIsDataStall_EvaluationDnsOnNotMeteredNetwork() {

    public void testIsDataStall_EvaluationDnsOnNotMeteredNetwork() throws Exception {

        WrappedNetworkMonitor wrappedMonitor = makeNotMeteredNetworkMonitor();

        wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 100);

        makeDnsTimeoutEvent(wrappedMonitor, DEFAULT_DNS_TIMEOUT_THRESHOLD);

        assertTrue(wrappedMonitor.isDataStall());

        verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_DNS_EVENTS),

                any(PersistableBundle.class));

    }



    @Test

    public void testIsDataStall_EvaluationDnsOnMeteredNetwork() {

    public void testIsDataStall_EvaluationDnsOnMeteredNetwork() throws Exception {

        WrappedNetworkMonitor wrappedMonitor = makeMeteredNetworkMonitor();

        wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 100);

        assertFalse(wrappedMonitor.isDataStall());
@@ -787,10 +793,12 @@ public class NetworkMonitorTest { 
        wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 1000);

        makeDnsTimeoutEvent(wrappedMonitor, DEFAULT_DNS_TIMEOUT_THRESHOLD);

        assertTrue(wrappedMonitor.isDataStall());

        verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_DNS_EVENTS),

                any(PersistableBundle.class));

    }



    @Test

    public void testIsDataStall_EvaluationDnsWithDnsTimeoutCount() {

    public void testIsDataStall_EvaluationDnsWithDnsTimeoutCount() throws Exception {

        WrappedNetworkMonitor wrappedMonitor = makeMeteredNetworkMonitor();

        wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 1000);

        makeDnsTimeoutEvent(wrappedMonitor, 3);
@@ -802,6 +810,8 @@ public class NetworkMonitorTest { 


        makeDnsTimeoutEvent(wrappedMonitor, 3);

        assertTrue(wrappedMonitor.isDataStall());

        verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_DNS_EVENTS),

                any(PersistableBundle.class));



        // Set the value to larger than the default dns log size.

        setConsecutiveDnsTimeoutThreshold(51);
@@ -812,6 +822,8 @@ public class NetworkMonitorTest { 


        makeDnsTimeoutEvent(wrappedMonitor, 1);

        assertTrue(wrappedMonitor.isDataStall());

        verify(mCallbacks, times(2)).notifyDataStallSuspected(anyLong(),

                eq(DETECTION_METHOD_DNS_EVENTS), any(PersistableBundle.class));

    }



    @Test
@@ -828,7 +840,7 @@ public class NetworkMonitorTest { 
    }



    @Test

    public void testIsDataStall_EvaluationDnsWithDnsTimeThreshold() {

    public void testIsDataStall_EvaluationDnsWithDnsTimeThreshold() throws Exception {

        // Test dns events happened in valid dns time threshold.

        WrappedNetworkMonitor wrappedMonitor = makeMeteredNetworkMonitor();

        wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 100);
@@ -836,6 +848,8 @@ public class NetworkMonitorTest { 
        assertFalse(wrappedMonitor.isDataStall());

        wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 1000);

        assertTrue(wrappedMonitor.isDataStall());

        verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_DNS_EVENTS),

                any(PersistableBundle.class));



        // Test dns events happened before valid dns time threshold.

        setValidDataStallDnsTimeThreshold(0);
@@ -848,7 +862,7 @@ public class NetworkMonitorTest { 
    }



    @Test

    public void testIsDataStall_EvaluationTcp() {

    public void testIsDataStall_EvaluationTcp() throws Exception {

        // Evaluate TCP only. Expect ignoring DNS signal.

        setDataStallEvaluationType(DATA_STALL_EVALUATION_TYPE_TCP);

        WrappedNetworkMonitor wrappedMonitor = makeMonitor(METERED_CAPABILITIES);
@@ -869,6 +883,8 @@ public class NetworkMonitorTest { 
        wrappedMonitor.sendTcpPollingEvent();

        HandlerUtilsKt.waitForIdle(wrappedMonitor.getHandler(), HANDLER_TIMEOUT_MS);

        assertTrue(wrappedMonitor.isDataStall());

        verify(mCallbacks).notifyDataStallSuspected(anyLong(), eq(DETECTION_METHOD_TCP_METRICS),

                any(PersistableBundle.class));

    }



    @Test