LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Address comments on NetworkStack AIDL v6

Browse Source 
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
This commit is contained in:
Remi NGUYEN VAN
2020-04-15 18:39:28 +09:00
parent c44ca579ad
commit 9b647ca69a
2 changed files with 80 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

79
services/core/java/com/android/server/ConnectivityService.java
View File

@@ -18,6 +18,14 @@ package com.android.server;
import static android.Manifest.permission.RECEIVE_DATA_ACTIVITY_CHANGE;
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.NETID_UNSET;
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.DataStallReport;
import android.net.ConnectivityManager;
import android.net.DataStallReportParcelable;
import android.net.ICaptivePortal;
import android.net.IConnectivityDiagnosticsCallback;
import android.net.IConnectivityManager;
@@ -108,6 +117,7 @@ import android.net.NetworkSpecifier;
import android.net.NetworkStack;
import android.net.NetworkStackClient;
import android.net.NetworkState;
import android.net.NetworkTestResultParcelable;
import android.net.NetworkUtils;
import android.net.NetworkWatchlistManager;
import android.net.PrivateDnsConfigParcel;
@@ -2811,14 +2821,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
handleNetworkTested(nai, results.mTestResult,
(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;
}
case EVENT_PROVISIONING_NOTIFICATION: {
@@ -2997,23 +2999,33 @@ public class ConnectivityService extends IConnectivityManager.Stub
@Override
public void notifyNetworkTested(int testResult, @Nullable String redirectUrl) {
notifyNetworkTestedWithExtras(testResult, redirectUrl, SystemClock.elapsedRealtime(),
PersistableBundle.EMPTY);
// Legacy version of notifyNetworkTestedWithExtras.
// Would only be called if the system has a NetworkStack module older than the
// framework, which does not happen in practice.
}
@Override
public void notifyNetworkTestedWithExtras(
int testResult,
@Nullable String redirectUrl,
long timestampMillis,
@NonNull PersistableBundle extras) {
final Message msg =
mTrackerHandler.obtainMessage(
EVENT_NETWORK_TESTED,
new NetworkTestedResults(
mNetId, testResult, timestampMillis, redirectUrl));
msg.setData(new Bundle(extras));
public void notifyNetworkTestedWithExtras(NetworkTestResultParcelable p) {
final Message msg = mTrackerHandler.obtainMessage(
EVENT_NETWORK_TESTED,
new NetworkTestedResults(
mNetId, p.result, p.timestampMillis, p.redirectUrl));
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
@@ -3062,12 +3074,25 @@ public class ConnectivityService extends IConnectivityManager.Stub
}
@Override
public void notifyDataStallSuspected(
long timestampMillis, int detectionMethod, PersistableBundle extras) {
final Message msg =
mConnectivityDiagnosticsHandler.obtainMessage(
ConnectivityDiagnosticsHandler.EVENT_DATA_STALL_SUSPECTED,
detectionMethod, mNetId, timestampMillis);
public void notifyDataStallSuspected(DataStallReportParcelable p) {
final Message msg = mConnectivityDiagnosticsHandler.obtainMessage(
ConnectivityDiagnosticsHandler.EVENT_DATA_STALL_SUSPECTED,
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));
// NetworkStateTrackerHandler currently doesn't take any actions based on data

55
tests/net/java/com/android/server/ConnectivityServiceTest.java
View File

@@ -144,6 +144,7 @@ import android.net.ConnectivityManager.PacketKeepalive;
import android.net.ConnectivityManager.PacketKeepaliveCallback;
import android.net.ConnectivityManager.TooManyRequestsException;
import android.net.ConnectivityThread;
import android.net.DataStallReportParcelable;
import android.net.IConnectivityDiagnosticsCallback;
import android.net.IDnsResolver;
import android.net.IIpConnectivityMetrics;
@@ -170,6 +171,7 @@ import android.net.NetworkSpecifier;
import android.net.NetworkStack;
import android.net.NetworkStackClient;
import android.net.NetworkState;
import android.net.NetworkTestResultParcelable;
import android.net.NetworkUtils;
import android.net.ProxyInfo;
import android.net.ResolverParamsParcel;
@@ -196,7 +198,6 @@ import android.os.Looper;
import android.os.Parcel;
import android.os.ParcelFileDescriptor;
import android.os.Parcelable;
import android.os.PersistableBundle;
import android.os.Process;
import android.os.RemoteException;
import android.os.SystemClock;
@@ -580,14 +581,6 @@ public class ConnectivityServiceTest {
}
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 long DATA_STALL_TIMESTAMP = 10L;
@@ -595,12 +588,10 @@ public class ConnectivityServiceTest {
private INetworkMonitor mNetworkMonitor;
private INetworkMonitorCallbacks mNmCallbacks;
private int mNmValidationResult = VALIDATION_RESULT_BASE;
private int mNmValidationResult = VALIDATION_RESULT_INVALID;
private int mProbesCompleted;
private int mProbesSucceeded;
private String mNmValidationRedirectUrl = null;
private PersistableBundle mValidationExtras = PersistableBundle.EMPTY;
private PersistableBundle mDataStallExtras = PersistableBundle.EMPTY;
private boolean mNmProvNotificationRequested = false;
private final ConditionVariable mNetworkStatusReceived = new ConditionVariable();
@@ -668,8 +659,13 @@ public class ConnectivityServiceTest {
}
mNmCallbacks.notifyProbeStatusChanged(mProbesCompleted, mProbesSucceeded);
mNmCallbacks.notifyNetworkTestedWithExtras(
mNmValidationResult, mNmValidationRedirectUrl, TIMESTAMP, mValidationExtras);
final NetworkTestResultParcelable p = new NetworkTestResultParcelable();
p.result = mNmValidationResult;
p.probesAttempted = mProbesCompleted;
p.probesSucceeded = mProbesSucceeded;
p.redirectUrl = mNmValidationRedirectUrl;
p.timestampMillis = TIMESTAMP;
mNmCallbacks.notifyNetworkTestedWithExtras(p);
if (mNmValidationRedirectUrl != null) {
mNmCallbacks.showProvisioningNotification(
@@ -751,9 +747,9 @@ public class ConnectivityServiceTest {
}
void setNetworkValid(boolean isStrictMode) {
mNmValidationResult = VALIDATION_RESULT_VALID;
mNmValidationResult = NETWORK_VALIDATION_RESULT_VALID;
mNmValidationRedirectUrl = null;
int probesSucceeded = VALIDATION_RESULT_BASE & ~NETWORK_VALIDATION_PROBE_HTTP;
int probesSucceeded = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS;
if (isStrictMode) {
probesSucceeded |= NETWORK_VALIDATION_PROBE_PRIVDNS;
}
@@ -765,8 +761,9 @@ public class ConnectivityServiceTest {
void setNetworkInvalid(boolean isStrictMode) {
mNmValidationResult = VALIDATION_RESULT_INVALID;
mNmValidationRedirectUrl = null;
int probesCompleted = VALIDATION_RESULT_BASE;
int probesSucceeded = VALIDATION_RESULT_INVALID;
int probesCompleted = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS
| NETWORK_VALIDATION_PROBE_HTTP;
int probesSucceeded = 0;
// If the isStrictMode is true, it means the network is invalid when NetworkMonitor
// tried to validate the private DNS but failed.
if (isStrictMode) {
@@ -782,7 +779,7 @@ public class ConnectivityServiceTest {
mNmValidationRedirectUrl = redirectUrl;
// 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.
int probesCompleted = VALIDATION_RESULT_BASE & ~NETWORK_VALIDATION_PROBE_HTTPS;
int probesCompleted = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTP;
int probesSucceeded = VALIDATION_RESULT_INVALID;
if (isStrictMode) {
probesCompleted |= NETWORK_VALIDATION_PROBE_PRIVDNS;
@@ -791,18 +788,20 @@ public class ConnectivityServiceTest {
}
void setNetworkPartial() {
mNmValidationResult = VALIDATION_RESULT_PARTIAL;
mNmValidationResult = NETWORK_VALIDATION_RESULT_PARTIAL;
mNmValidationRedirectUrl = null;
int probesCompleted = VALIDATION_RESULT_BASE;
int probesSucceeded = VALIDATION_RESULT_BASE & ~NETWORK_VALIDATION_PROBE_HTTPS;
int probesCompleted = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_HTTPS
| NETWORK_VALIDATION_PROBE_FALLBACK;
int probesSucceeded = NETWORK_VALIDATION_PROBE_DNS | NETWORK_VALIDATION_PROBE_FALLBACK;
setProbesStatus(probesCompleted, probesSucceeded);
}
void setNetworkPartialValid(boolean isStrictMode) {
setNetworkPartial();
mNmValidationResult |= VALIDATION_RESULT_VALID;
int probesCompleted = VALIDATION_RESULT_BASE;
int probesSucceeded = VALIDATION_RESULT_BASE & ~NETWORK_VALIDATION_PROBE_HTTPS;
mNmValidationResult |= NETWORK_VALIDATION_RESULT_VALID;
int probesCompleted = NETWORK_VALIDATION_PROBE_DNS | 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
// add NETWORK_VALIDATION_PROBE_DNS in probesCompleted but not probesSucceeded.
if (isStrictMode) {
@@ -838,8 +837,10 @@ public class ConnectivityServiceTest {
}
void notifyDataStallSuspected() throws Exception {
mNmCallbacks.notifyDataStallSuspected(
DATA_STALL_TIMESTAMP, DATA_STALL_DETECTION_METHOD, mDataStallExtras);
final DataStallReportParcelable p = new DataStallReportParcelable();
p.detectionMethod = DATA_STALL_DETECTION_METHOD;
p.timestampMillis = DATA_STALL_TIMESTAMP;
mNmCallbacks.notifyDataStallSuspected(p);
}
}