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

Commit b955ac2f authored by Remi NGUYEN VAN's avatar Remi NGUYEN VAN
Browse files

Address comments on NetworkStack AIDL v6 

Address issues found during AIDL review:
 - Rename clientAddr to singleClientAddr
 - Do not use a ParcelableBundle for notifyNetworkTested or
   notifyDataStallSuspected; instead use AIDL parcelables for stronger
   backwards compatibility guarantees.

Test: atest NetworkMonitorTest ConnectivityServiceTest
      ConnectivityServiceIntegrationTest, manual
Bug: 153500847
Change-Id: Id9b71784e5f6294d203230e57737979e063ff0f8
parent 50665529

packages/Tethering/src/android/net/dhcp/DhcpServingParamsParcelExt.java

+1 −1
Original line number Original line Diff line number Diff line
@@ -169,7 +169,7 @@ public class DhcpServingParamsParcelExt extends DhcpServingParamsParcel { 
     * <p>If not set, the default value is null.

     * <p>If not set, the default value is null.

     */

     */

    public DhcpServingParamsParcelExt setSingleClientAddr(@Nullable Inet4Address clientAddr) {

    public DhcpServingParamsParcelExt setSingleClientAddr(@Nullable Inet4Address clientAddr) {

        this.clientAddr = clientAddr == null ? 0 : inet4AddressToIntHTH(clientAddr);

        this.singleClientAddr = clientAddr == null ? 0 : inet4AddressToIntHTH(clientAddr);

        return this;

        return this;

    }

    }

packages/Tethering/tests/unit/src/android/net/dhcp/DhcpServingParamsParcelExtTest.java

+1 −1
Original line number Original line Diff line number Diff line
@@ -110,7 +110,7 @@ public class DhcpServingParamsParcelExtTest { 
    @Test

    @Test

    public void testSetClientAddr() {

    public void testSetClientAddr() {

        mParcel.setSingleClientAddr(TEST_CLIENT_ADDRESS);

        mParcel.setSingleClientAddr(TEST_CLIENT_ADDRESS);

        assertEquals(TEST_CLIENT_ADDRESS_PARCELED, mParcel.clientAddr);

        assertEquals(TEST_CLIENT_ADDRESS_PARCELED, mParcel.singleClientAddr);

    }

    }





    private static Inet4Address inet4Addr(String addr) {

    private static Inet4Address inet4Addr(String addr) {

packages/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java

+1 −1
Original line number Original line Diff line number Diff line
@@ -1689,7 +1689,7 @@ public class TetheringTest { 
        final DhcpServingParamsParcel params = dhcpParamsCaptor.getValue();

        final DhcpServingParamsParcel params = dhcpParamsCaptor.getValue();

        assertEquals(serverAddr, intToInet4AddressHTH(params.serverAddr).getHostAddress());

        assertEquals(serverAddr, intToInet4AddressHTH(params.serverAddr).getHostAddress());

        assertEquals(24, params.serverAddrPrefixLength);

        assertEquals(24, params.serverAddrPrefixLength);

        assertEquals(clientAddrParceled, params.clientAddr);

        assertEquals(clientAddrParceled, params.singleClientAddr);

    }

    }





    @Test

    @Test

services/core/java/com/android/server/ConnectivityService.java

+52 −27
Original line number Original line Diff line number Diff line
@@ -18,6 +18,14 @@ package com.android.server; 




import static android.Manifest.permission.RECEIVE_DATA_ACTIVITY_CHANGE;

import static android.Manifest.permission.RECEIVE_DATA_ACTIVITY_CHANGE;

import static android.content.pm.PackageManager.PERMISSION_GRANTED;

import static android.content.pm.PackageManager.PERMISSION_GRANTED;

import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport.KEY_NETWORK_PROBES_ATTEMPTED_BITMASK;

import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport.KEY_NETWORK_PROBES_SUCCEEDED_BITMASK;

import static android.net.ConnectivityDiagnosticsManager.ConnectivityReport.KEY_NETWORK_VALIDATION_RESULT;

import static android.net.ConnectivityDiagnosticsManager.DataStallReport.DETECTION_METHOD_DNS_EVENTS;

import static android.net.ConnectivityDiagnosticsManager.DataStallReport.DETECTION_METHOD_TCP_METRICS;

import static android.net.ConnectivityDiagnosticsManager.DataStallReport.KEY_DNS_CONSECUTIVE_TIMEOUTS;

import static android.net.ConnectivityDiagnosticsManager.DataStallReport.KEY_TCP_METRICS_COLLECTION_PERIOD_MILLIS;

import static android.net.ConnectivityDiagnosticsManager.DataStallReport.KEY_TCP_PACKET_FAIL_RATE;

import static android.net.ConnectivityManager.CONNECTIVITY_ACTION;

import static android.net.ConnectivityManager.CONNECTIVITY_ACTION;

import static android.net.ConnectivityManager.NETID_UNSET;

import static android.net.ConnectivityManager.NETID_UNSET;

import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OPPORTUNISTIC;

import static android.net.ConnectivityManager.PRIVATE_DNS_MODE_OPPORTUNISTIC;
@@ -72,6 +80,7 @@ import android.net.ConnectionInfo; 
import android.net.ConnectivityDiagnosticsManager.ConnectivityReport;

import android.net.ConnectivityDiagnosticsManager.ConnectivityReport;

import android.net.ConnectivityDiagnosticsManager.DataStallReport;

import android.net.ConnectivityDiagnosticsManager.DataStallReport;

import android.net.ConnectivityManager;

import android.net.ConnectivityManager;

import android.net.DataStallReportParcelable;

import android.net.ICaptivePortal;

import android.net.ICaptivePortal;

import android.net.IConnectivityDiagnosticsCallback;

import android.net.IConnectivityDiagnosticsCallback;

import android.net.IConnectivityManager;

import android.net.IConnectivityManager;
@@ -108,6 +117,7 @@ import android.net.NetworkSpecifier; 
import android.net.NetworkStack;

import android.net.NetworkStack;

import android.net.NetworkStackClient;

import android.net.NetworkStackClient;

import android.net.NetworkState;

import android.net.NetworkState;

import android.net.NetworkTestResultParcelable;

import android.net.NetworkUtils;

import android.net.NetworkUtils;

import android.net.NetworkWatchlistManager;

import android.net.NetworkWatchlistManager;

import android.net.PrivateDnsConfigParcel;

import android.net.PrivateDnsConfigParcel;
@@ -2811,14 +2821,6 @@ public class ConnectivityService extends IConnectivityManager.Stub 




                    handleNetworkTested(nai, results.mTestResult,

                    handleNetworkTested(nai, results.mTestResult,

                            (results.mRedirectUrl == null) ? "" : results.mRedirectUrl);

                            (results.mRedirectUrl == null) ? "" : results.mRedirectUrl);



                    // Invoke ConnectivityReport generation for this Network test event.

                    final Message m =

                            mConnectivityDiagnosticsHandler.obtainMessage(

                                    ConnectivityDiagnosticsHandler.EVENT_NETWORK_TESTED,

                                    new ConnectivityReportEvent(results.mTimestampMillis, nai));

                    m.setData(msg.getData());

                    mConnectivityDiagnosticsHandler.sendMessage(m);

                    break;

                    break;

                }

                }

                case EVENT_PROVISIONING_NOTIFICATION: {

                case EVENT_PROVISIONING_NOTIFICATION: {
@@ -2997,23 +2999,33 @@ public class ConnectivityService extends IConnectivityManager.Stub 




        @Override

        @Override

        public void notifyNetworkTested(int testResult, @Nullable String redirectUrl) {

        public void notifyNetworkTested(int testResult, @Nullable String redirectUrl) {

            notifyNetworkTestedWithExtras(testResult, redirectUrl, SystemClock.elapsedRealtime(),

            // Legacy version of notifyNetworkTestedWithExtras.

                    PersistableBundle.EMPTY);

            // Would only be called if the system has a NetworkStack module older than the

            // framework, which does not happen in practice.

        }

        }





        @Override

        @Override

        public void notifyNetworkTestedWithExtras(

        public void notifyNetworkTestedWithExtras(NetworkTestResultParcelable p) {

                int testResult,

            final Message msg = mTrackerHandler.obtainMessage(

                @Nullable String redirectUrl,

                long timestampMillis,

                @NonNull PersistableBundle extras) {

            final Message msg =

                    mTrackerHandler.obtainMessage(

                    EVENT_NETWORK_TESTED,

                    EVENT_NETWORK_TESTED,

                    new NetworkTestedResults(

                    new NetworkTestedResults(

                                    mNetId, testResult, timestampMillis, redirectUrl));

                            mNetId, p.result, p.timestampMillis, p.redirectUrl));

            msg.setData(new Bundle(extras));

            mTrackerHandler.sendMessage(msg);

            mTrackerHandler.sendMessage(msg);



            // Invoke ConnectivityReport generation for this Network test event.

            final NetworkAgentInfo nai = getNetworkAgentInfoForNetId(mNetId);

            if (nai == null) return;

            final Message m = mConnectivityDiagnosticsHandler.obtainMessage(

                    ConnectivityDiagnosticsHandler.EVENT_NETWORK_TESTED,

                    new ConnectivityReportEvent(p.timestampMillis, nai));



            final PersistableBundle extras = new PersistableBundle();

            extras.putInt(KEY_NETWORK_VALIDATION_RESULT, p.result);

            extras.putInt(KEY_NETWORK_PROBES_SUCCEEDED_BITMASK, p.probesSucceeded);

            extras.putInt(KEY_NETWORK_PROBES_ATTEMPTED_BITMASK, p.probesAttempted);



            m.setData(new Bundle(extras));

            mConnectivityDiagnosticsHandler.sendMessage(m);

        }

        }





        @Override

        @Override
@@ -3062,12 +3074,25 @@ public class ConnectivityService extends IConnectivityManager.Stub 
        }

        }





        @Override

        @Override

        public void notifyDataStallSuspected(

        public void notifyDataStallSuspected(DataStallReportParcelable p) {

                long timestampMillis, int detectionMethod, PersistableBundle extras) {

            final Message msg = mConnectivityDiagnosticsHandler.obtainMessage(

            final Message msg =

                    mConnectivityDiagnosticsHandler.obtainMessage(

                    ConnectivityDiagnosticsHandler.EVENT_DATA_STALL_SUSPECTED,

                    ConnectivityDiagnosticsHandler.EVENT_DATA_STALL_SUSPECTED,

                            detectionMethod, mNetId, timestampMillis);

                    p.detectionMethod, mNetId, p.timestampMillis);



            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:

                    log("Unknown data stall detection method, ignoring: " + p.detectionMethod);

                    return;

            }

            msg.setData(new Bundle(extras));

            msg.setData(new Bundle(extras));





            // NetworkStateTrackerHandler currently doesn't take any actions based on data

            // NetworkStateTrackerHandler currently doesn't take any actions based on data

tests/net/java/com/android/server/ConnectivityServiceTest.java

+28 −27
Original line number Original line Diff line number Diff line
@@ -144,6 +144,7 @@ import android.net.ConnectivityManager.PacketKeepalive; 
import android.net.ConnectivityManager.PacketKeepaliveCallback;

import android.net.ConnectivityManager.PacketKeepaliveCallback;

import android.net.ConnectivityManager.TooManyRequestsException;

import android.net.ConnectivityManager.TooManyRequestsException;

import android.net.ConnectivityThread;

import android.net.ConnectivityThread;

import android.net.DataStallReportParcelable;

import android.net.IConnectivityDiagnosticsCallback;

import android.net.IConnectivityDiagnosticsCallback;

import android.net.IDnsResolver;

import android.net.IDnsResolver;

import android.net.IIpConnectivityMetrics;

import android.net.IIpConnectivityMetrics;
@@ -170,6 +171,7 @@ import android.net.NetworkSpecifier; 
import android.net.NetworkStack;

import android.net.NetworkStack;

import android.net.NetworkStackClient;

import android.net.NetworkStackClient;

import android.net.NetworkState;

import android.net.NetworkState;

import android.net.NetworkTestResultParcelable;

import android.net.NetworkUtils;

import android.net.NetworkUtils;

import android.net.ProxyInfo;

import android.net.ProxyInfo;

import android.net.ResolverParamsParcel;

import android.net.ResolverParamsParcel;
@@ -196,7 +198,6 @@ import android.os.Looper; 
import android.os.Parcel;

import android.os.Parcel;

import android.os.ParcelFileDescriptor;

import android.os.ParcelFileDescriptor;

import android.os.Parcelable;

import android.os.Parcelable;

import android.os.PersistableBundle;

import android.os.Process;

import android.os.Process;

import android.os.RemoteException;

import android.os.RemoteException;

import android.os.SystemClock;

import android.os.SystemClock;
@@ -580,14 +581,6 @@ public class ConnectivityServiceTest { 
    }

    }





    private class TestNetworkAgentWrapper extends NetworkAgentWrapper {

    private class TestNetworkAgentWrapper extends NetworkAgentWrapper {

        private static final int VALIDATION_RESULT_BASE = NETWORK_VALIDATION_PROBE_DNS

                | NETWORK_VALIDATION_PROBE_HTTP

                | NETWORK_VALIDATION_PROBE_HTTPS;

        private static final int VALIDATION_RESULT_VALID = VALIDATION_RESULT_BASE

                | NETWORK_VALIDATION_RESULT_VALID;

        private static final int VALIDATION_RESULT_PARTIAL = VALIDATION_RESULT_BASE

                | NETWORK_VALIDATION_PROBE_FALLBACK

                | NETWORK_VALIDATION_RESULT_PARTIAL;

        private static final int VALIDATION_RESULT_INVALID = 0;

        private static final int VALIDATION_RESULT_INVALID = 0;





        private static final long DATA_STALL_TIMESTAMP = 10L;

        private static final long DATA_STALL_TIMESTAMP = 10L;
@@ -595,12 +588,10 @@ public class ConnectivityServiceTest { 




        private INetworkMonitor mNetworkMonitor;

        private INetworkMonitor mNetworkMonitor;

        private INetworkMonitorCallbacks mNmCallbacks;

        private INetworkMonitorCallbacks mNmCallbacks;

        private int mNmValidationResult = VALIDATION_RESULT_BASE;

        private int mNmValidationResult = VALIDATION_RESULT_INVALID;

        private int mProbesCompleted;

        private int mProbesCompleted;

        private int mProbesSucceeded;

        private int mProbesSucceeded;

        private String mNmValidationRedirectUrl = null;

        private String mNmValidationRedirectUrl = null;

        private PersistableBundle mValidationExtras = PersistableBundle.EMPTY;

        private PersistableBundle mDataStallExtras = PersistableBundle.EMPTY;

        private boolean mNmProvNotificationRequested = false;

        private boolean mNmProvNotificationRequested = false;





        private final ConditionVariable mNetworkStatusReceived = new ConditionVariable();

        private final ConditionVariable mNetworkStatusReceived = new ConditionVariable();
@@ -668,8 +659,13 @@ public class ConnectivityServiceTest { 
            }

            }





            mNmCallbacks.notifyProbeStatusChanged(mProbesCompleted, mProbesSucceeded);

            mNmCallbacks.notifyProbeStatusChanged(mProbesCompleted, mProbesSucceeded);

            mNmCallbacks.notifyNetworkTestedWithExtras(

            final NetworkTestResultParcelable p = new NetworkTestResultParcelable();

                    mNmValidationResult, mNmValidationRedirectUrl, TIMESTAMP, mValidationExtras);

            p.result = mNmValidationResult;

            p.probesAttempted = mProbesCompleted;

            p.probesSucceeded = mProbesSucceeded;

            p.redirectUrl = mNmValidationRedirectUrl;

            p.timestampMillis = TIMESTAMP;

            mNmCallbacks.notifyNetworkTestedWithExtras(p);





            if (mNmValidationRedirectUrl != null) {

            if (mNmValidationRedirectUrl != null) {

                mNmCallbacks.showProvisioningNotification(

                mNmCallbacks.showProvisioningNotification(
@@ -751,9 +747,9 @@ public class ConnectivityServiceTest { 
        }

        }





        void setNetworkValid(boolean isStrictMode) {

        void setNetworkValid(boolean isStrictMode) {

            mNmValidationResult = VALIDATION_RESULT_VALID;

            mNmValidationResult = NETWORK_VALIDATION_RESULT_VALID;

            mNmValidationRedirectUrl = null;

            mNmValidationRedirectUrl = null;

            int probesSucceeded = VALIDATION_RESULT_BASE & ~NETWORK_VALIDATION_PROBE_HTTP;

            int probesSucceeded = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS;

            if (isStrictMode) {

            if (isStrictMode) {

                probesSucceeded |= NETWORK_VALIDATION_PROBE_PRIVDNS;

                probesSucceeded |= NETWORK_VALIDATION_PROBE_PRIVDNS;

            }

            }
@@ -765,8 +761,9 @@ public class ConnectivityServiceTest { 
        void setNetworkInvalid(boolean isStrictMode) {

        void setNetworkInvalid(boolean isStrictMode) {

            mNmValidationResult = VALIDATION_RESULT_INVALID;

            mNmValidationResult = VALIDATION_RESULT_INVALID;

            mNmValidationRedirectUrl = null;

            mNmValidationRedirectUrl = null;

            int probesCompleted = VALIDATION_RESULT_BASE;

            int probesCompleted = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS

            int probesSucceeded = VALIDATION_RESULT_INVALID;

                    | NETWORK_VALIDATION_PROBE_HTTP;

            int probesSucceeded = 0;

            // If the isStrictMode is true, it means the network is invalid when NetworkMonitor

            // If the isStrictMode is true, it means the network is invalid when NetworkMonitor

            // tried to validate the private DNS but failed.

            // tried to validate the private DNS but failed.

            if (isStrictMode) {

            if (isStrictMode) {
@@ -782,7 +779,7 @@ public class ConnectivityServiceTest { 
            mNmValidationRedirectUrl = redirectUrl;

            mNmValidationRedirectUrl = redirectUrl;

            // Suppose the portal is found when NetworkMonitor probes NETWORK_VALIDATION_PROBE_HTTP

            // Suppose the portal is found when NetworkMonitor probes NETWORK_VALIDATION_PROBE_HTTP

            // in the beginning, so the NETWORK_VALIDATION_PROBE_HTTPS hasn't probed yet.

            // in the beginning, so the NETWORK_VALIDATION_PROBE_HTTPS hasn't probed yet.

            int probesCompleted = VALIDATION_RESULT_BASE & ~NETWORK_VALIDATION_PROBE_HTTPS;

            int probesCompleted = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP;

            int probesSucceeded = VALIDATION_RESULT_INVALID;

            int probesSucceeded = VALIDATION_RESULT_INVALID;

            if (isStrictMode) {

            if (isStrictMode) {

                probesCompleted |= NETWORK_VALIDATION_PROBE_PRIVDNS;

                probesCompleted |= NETWORK_VALIDATION_PROBE_PRIVDNS;
@@ -791,18 +788,20 @@ public class ConnectivityServiceTest { 
        }

        }





        void setNetworkPartial() {

        void setNetworkPartial() {

            mNmValidationResult = VALIDATION_RESULT_PARTIAL;

            mNmValidationResult = NETWORK_VALIDATION_RESULT_PARTIAL;

            mNmValidationRedirectUrl = null;

            mNmValidationRedirectUrl = null;

            int probesCompleted = VALIDATION_RESULT_BASE;

            int probesCompleted = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS

            int probesSucceeded = VALIDATION_RESULT_BASE & ~NETWORK_VALIDATION_PROBE_HTTPS;

                    | NETWORK_VALIDATION_PROBE_FALLBACK;

            int probesSucceeded = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_FALLBACK;

            setProbesStatus(probesCompleted, probesSucceeded);

            setProbesStatus(probesCompleted, probesSucceeded);

        }

        }





        void setNetworkPartialValid(boolean isStrictMode) {

        void setNetworkPartialValid(boolean isStrictMode) {

            setNetworkPartial();

            setNetworkPartial();

            mNmValidationResult |= VALIDATION_RESULT_VALID;

            mNmValidationResult |= NETWORK_VALIDATION_RESULT_VALID;

            int probesCompleted = VALIDATION_RESULT_BASE;

            int probesCompleted = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS

            int probesSucceeded = VALIDATION_RESULT_BASE & ~NETWORK_VALIDATION_PROBE_HTTPS;

                    | NETWORK_VALIDATION_PROBE_HTTP;

            int probesSucceeded = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP;

            // Suppose the partial network cannot pass the private DNS validation as well, so only

            // Suppose the partial network cannot pass the private DNS validation as well, so only

            // add NETWORK_VALIDATION_PROBE_DNS in probesCompleted but not probesSucceeded.

            // add NETWORK_VALIDATION_PROBE_DNS in probesCompleted but not probesSucceeded.

            if (isStrictMode) {

            if (isStrictMode) {
@@ -838,8 +837,10 @@ public class ConnectivityServiceTest { 
        }

        }





        void notifyDataStallSuspected() throws Exception {

        void notifyDataStallSuspected() throws Exception {

            mNmCallbacks.notifyDataStallSuspected(

            final DataStallReportParcelable p = new DataStallReportParcelable();

                    DATA_STALL_TIMESTAMP, DATA_STALL_DETECTION_METHOD, mDataStallExtras);

            p.detectionMethod = DATA_STALL_DETECTION_METHOD;

            p.timestampMillis = DATA_STALL_TIMESTAMP;

            mNmCallbacks.notifyDataStallSuspected(p);

        }

        }

    }

    }