From 8f7d6a7ece49169b70100e0f5833d5801323d976 Mon Sep 17 00:00:00 2001 From: Udam Saini Date: Wed, 7 Jun 2017 12:06:28 -0700 Subject: [PATCH 01/27] Adds necessary permissions to system apis adds privileged permission for getCaptivePortalServerUrl adds tether privileged permission for startTethering,isTetheringSupported bug:62348162 Test: make and manual testing Change-Id: I8eb8e3c9dcd7201abe9ea303ee57fe99073d67eb --- core/java/android/net/ConnectivityManager.java | 7 +++++-- core/java/android/net/IConnectivityManager.aidl | 2 +- .../java/com/android/server/ConnectivityService.java | 11 ++++++++--- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index f478071423..c2786e6cbf 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -1098,6 +1098,7 @@ public class ConnectivityManager { * @hide */ @SystemApi + @RequiresPermission(android.Manifest.permission.LOCAL_MAC_ADDRESS) public String getCaptivePortalServerUrl() { try { return mService.getCaptivePortalServerUrl(); @@ -2061,10 +2062,11 @@ public class ConnectivityManager { * {@hide} */ @SystemApi - @RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE) + @RequiresPermission(android.Manifest.permission.TETHER_PRIVILEGED) public boolean isTetheringSupported() { try { - return mService.isTetheringSupported(); + String pkgName = mContext.getOpPackageName(); + return mService.isTetheringSupported(pkgName); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } @@ -2094,6 +2096,7 @@ public class ConnectivityManager { * @hide */ @SystemApi + @RequiresPermission(android.Manifest.permission.TETHER_PRIVILEGED) public void startTethering(int type, boolean showProvisioningUi, final OnStartTetheringCallback callback) { startTethering(type, showProvisioningUi, callback, null); diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl index 27729dcce7..14cee3604d 100644 --- a/core/java/android/net/IConnectivityManager.aidl +++ b/core/java/android/net/IConnectivityManager.aidl @@ -75,7 +75,7 @@ interface IConnectivityManager int getLastTetherError(String iface); - boolean isTetheringSupported(); + boolean isTetheringSupported(String callerPkg); void startTethering(int type, in ResultReceiver receiver, boolean showProvisioningUi, String callerPkg); diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 224845ce68..98389a663f 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2973,12 +2973,16 @@ public class ConnectivityService extends IConnectivityManager.Stub return mTethering.getTetheredDhcpRanges(); } + @Override + public boolean isTetheringSupported(String callerPkg) { + ConnectivityManager.enforceTetherChangePermission(mContext, callerPkg); + return isTetheringSupported(); + } + // if ro.tether.denied = true we default to no tethering // gservices could set the secure setting to 1 though to enable it on a build where it // had previously been turned off. - @Override - public boolean isTetheringSupported() { - enforceTetherAccessPermission(); + private boolean isTetheringSupported() { int defaultVal = encodeBool(!mSystemProperties.get("ro.tether.denied").equals("true")); boolean tetherSupported = toBool(Settings.Global.getInt(mContext.getContentResolver(), Settings.Global.TETHER_SUPPORTED, defaultVal)); @@ -5380,6 +5384,7 @@ public class ConnectivityService extends IConnectivityManager.Stub @Override public String getCaptivePortalServerUrl() { + enforceConnectivityInternalPermission(); return NetworkMonitor.getCaptivePortalServerHttpUrl(mContext); } From c8a0b1b80c5fe90be22c8180e89d2ac4a4cbd176 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Fri, 23 Jun 2017 10:07:08 +0900 Subject: [PATCH 02/27] IpManager: define InitialConfiguration This patch adds a InitialConfiguration class to IpManager for specifying IP information in IpManager ProvisioningConfiguration at IpManager startup. At the moment this InitialConfiguration is not used, but is validated in startProvsiioning if ProvisioningConfiguration includes one. It will be integrated into IpManager IP provisioning logic in follow-up patches. This patch also includes an example of data driven unit tests using a table of test case. The highlights of this methodology are: 1) easy extensibility for new test case, 2) rich and informative error messages, Unfortunately Java support for inlined data structure literals is poor and some companion static methods for data generation are required for enabling this methodology. Bug: 62988545 Test: added new test in FrameworksNetTests, $ runtest frameworks-net $ runtest frameworks-wifi Change-Id: I060b02603af7d73a6407df89344bf0c000574af2 --- core/java/android/net/LinkAddress.java | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/core/java/android/net/LinkAddress.java b/core/java/android/net/LinkAddress.java index 6e74f14bd1..62de9911bc 100644 --- a/core/java/android/net/LinkAddress.java +++ b/core/java/android/net/LinkAddress.java @@ -16,6 +16,15 @@ package android.net; +import static android.system.OsConstants.IFA_F_DADFAILED; +import static android.system.OsConstants.IFA_F_DEPRECATED; +import static android.system.OsConstants.IFA_F_OPTIMISTIC; +import static android.system.OsConstants.IFA_F_TENTATIVE; +import static android.system.OsConstants.RT_SCOPE_HOST; +import static android.system.OsConstants.RT_SCOPE_LINK; +import static android.system.OsConstants.RT_SCOPE_SITE; +import static android.system.OsConstants.RT_SCOPE_UNIVERSE; + import android.os.Parcel; import android.os.Parcelable; import android.util.Pair; @@ -26,15 +35,6 @@ import java.net.InetAddress; import java.net.InterfaceAddress; import java.net.UnknownHostException; -import static android.system.OsConstants.IFA_F_DADFAILED; -import static android.system.OsConstants.IFA_F_DEPRECATED; -import static android.system.OsConstants.IFA_F_OPTIMISTIC; -import static android.system.OsConstants.IFA_F_TENTATIVE; -import static android.system.OsConstants.RT_SCOPE_HOST; -import static android.system.OsConstants.RT_SCOPE_LINK; -import static android.system.OsConstants.RT_SCOPE_SITE; -import static android.system.OsConstants.RT_SCOPE_UNIVERSE; - /** * Identifies an IP address on a network link. * @@ -101,7 +101,7 @@ public class LinkAddress implements Parcelable { * Per RFC 4193 section 8, fc00::/7 identifies these addresses. */ private boolean isIPv6ULA() { - if (address != null && address instanceof Inet6Address) { + if (address instanceof Inet6Address) { byte[] bytes = address.getAddress(); return ((bytes[0] & (byte)0xfe) == (byte)0xfc); } From adebffc1344cbcfe9346bb45daec4315d9a87ab4 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 12 Jul 2017 10:50:42 -0600 Subject: [PATCH 03/27] Move "metered" persistence to WifiConfiguration. For a long time we've had a nasty tangled dependency between Wi-Fi and NPMS, since they both persisted different details for configured networks. As part of preparing for new carrier data plan APIs, move the tracking of meteredness over to WifiConfiguration. This also cleans up how meteredness is communicated through NetworkAgents to rely completely on NET_CAPABILITY_NOT_METERED by removing the metered flag on NetworkInfo, which has caused confusion and staleness. Migrates any existing user-configured metered networks over to WifiConfiguration once the device finishes booting. Remove support for NetworkQuotaInfo, since this information can no longer be made available to apps. Frustratingly, some apps are using it, so keep the object around returning stub values, and shame them in the logs. Bug: 63391323 Test: builds, boots, Wi-Fi policy is upgraded Exempt-From-Owner-Approval: Bug 63673347 Change-Id: I64f865ddeb65cfcd330f8d2a847368abdf960a07 --- .../java/android/net/ConnectivityManager.java | 10 +---- core/java/android/net/NetworkInfo.java | 32 +------------- .../android/server/ConnectivityService.java | 43 ++++++------------- 3 files changed, 17 insertions(+), 68 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 2979cd8a5d..7a1d85c93d 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -1720,14 +1720,8 @@ public class ConnectivityManager { // ignored } - /** - * Return quota status for the current active network, or {@code null} if no - * network is active. Quota status can change rapidly, so these values - * shouldn't be cached. - * - * @hide - */ - @RequiresPermission(android.Manifest.permission.ACCESS_NETWORK_STATE) + /** {@hide} */ + @Deprecated public NetworkQuotaInfo getActiveNetworkQuotaInfo() { try { return mService.getActiveNetworkQuotaInfo(); diff --git a/core/java/android/net/NetworkInfo.java b/core/java/android/net/NetworkInfo.java index 84c32bec8e..818aa21162 100644 --- a/core/java/android/net/NetworkInfo.java +++ b/core/java/android/net/NetworkInfo.java @@ -16,8 +16,8 @@ package android.net; -import android.os.Parcelable; import android.os.Parcel; +import android.os.Parcelable; import com.android.internal.annotations.VisibleForTesting; @@ -121,7 +121,6 @@ public class NetworkInfo implements Parcelable { private boolean mIsFailover; private boolean mIsAvailable; private boolean mIsRoaming; - private boolean mIsMetered; /** * @hide @@ -154,7 +153,6 @@ public class NetworkInfo implements Parcelable { mIsFailover = source.mIsFailover; mIsAvailable = source.mIsAvailable; mIsRoaming = source.mIsRoaming; - mIsMetered = source.mIsMetered; } } } @@ -326,31 +324,6 @@ public class NetworkInfo implements Parcelable { } } - /** - * Returns if this network is metered. A network is classified as metered - * when the user is sensitive to heavy data usage on that connection due to - * monetary costs, data limitations or battery/performance issues. You - * should check this before doing large data transfers, and warn the user or - * delay the operation until another network is available. - * - * @return {@code true} if large transfers should be avoided, otherwise - * {@code false}. - * @hide - */ - public boolean isMetered() { - synchronized (this) { - return mIsMetered; - } - } - - /** {@hide} */ - @VisibleForTesting - public void setMetered(boolean isMetered) { - synchronized (this) { - mIsMetered = isMetered; - } - } - /** * Reports the current coarse-grained state of the network. * @return the coarse-grained state @@ -434,7 +407,6 @@ public class NetworkInfo implements Parcelable { append(", failover: ").append(mIsFailover). append(", available: ").append(mIsAvailable). append(", roaming: ").append(mIsRoaming). - append(", metered: ").append(mIsMetered). append("]"); return builder.toString(); } @@ -457,7 +429,6 @@ public class NetworkInfo implements Parcelable { dest.writeInt(mIsFailover ? 1 : 0); dest.writeInt(mIsAvailable ? 1 : 0); dest.writeInt(mIsRoaming ? 1 : 0); - dest.writeInt(mIsMetered ? 1 : 0); dest.writeString(mReason); dest.writeString(mExtraInfo); } @@ -476,7 +447,6 @@ public class NetworkInfo implements Parcelable { netInfo.mIsFailover = in.readInt() != 0; netInfo.mIsAvailable = in.readInt() != 0; netInfo.mIsRoaming = in.readInt() != 0; - netInfo.mIsMetered = in.readInt() != 0; netInfo.mReason = in.readString(); netInfo.mExtraInfo = in.readString(); return netInfo; diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index a2e74b6a38..8200289a76 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -1036,8 +1036,7 @@ public class ConnectivityService extends IConnectivityManager.Stub /** * Apply any relevant filters to {@link NetworkState} for the given UID. For * example, this may mark the network as {@link DetailedState#BLOCKED} based - * on {@link #isNetworkWithLinkPropertiesBlocked}, or - * {@link NetworkInfo#isMetered()} based on network policies. + * on {@link #isNetworkWithLinkPropertiesBlocked}. */ private void filterNetworkStateForUid(NetworkState state, int uid, boolean ignoreBlocked) { if (state == null || state.networkInfo == null || state.linkProperties == null) return; @@ -1048,15 +1047,6 @@ public class ConnectivityService extends IConnectivityManager.Stub if (mLockdownTracker != null) { mLockdownTracker.augmentNetworkInfo(state.networkInfo); } - - // TODO: apply metered state closer to NetworkAgentInfo - final long token = Binder.clearCallingIdentity(); - try { - state.networkInfo.setMetered(mPolicyManager.isNetworkMetered(state)); - } catch (RemoteException e) { - } finally { - Binder.restoreCallingIdentity(token); - } } /** @@ -1326,30 +1316,24 @@ public class ConnectivityService extends IConnectivityManager.Stub } @Override + @Deprecated public NetworkQuotaInfo getActiveNetworkQuotaInfo() { - enforceAccessPermission(); - final int uid = Binder.getCallingUid(); - final long token = Binder.clearCallingIdentity(); - try { - final NetworkState state = getUnfilteredActiveNetworkState(uid); - if (state.networkInfo != null) { - try { - return mPolicyManager.getNetworkQuotaInfo(state); - } catch (RemoteException e) { - } - } - return null; - } finally { - Binder.restoreCallingIdentity(token); - } + Log.w(TAG, "Shame on UID " + Binder.getCallingUid() + + " for calling the hidden API getNetworkQuotaInfo(). Shame!"); + return new NetworkQuotaInfo(); } @Override public boolean isActiveNetworkMetered() { enforceAccessPermission(); - final NetworkInfo info = getActiveNetworkInfo(); - return (info != null) ? info.isMetered() : false; + final NetworkCapabilities caps = getNetworkCapabilities(getActiveNetwork()); + if (caps != null) { + return !caps.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED); + } else { + // Always return the most conservative value + return true; + } } private INetworkManagementEventObserver mDataActivityObserver = new BaseNetworkObserver() { @@ -2759,7 +2743,8 @@ public class ConnectivityService extends IConnectivityManager.Stub enforceAccessPermission(); NetworkAgentInfo nai = getNetworkAgentInfoForNetwork(network); - if (nai != null && !nai.networkInfo.isMetered()) { + if (nai != null && nai.networkCapabilities + .hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED)) { return ConnectivityManager.MULTIPATH_PREFERENCE_UNMETERED; } From 9369e61e2d6947c053d0b788a7d36c5db6951ede Mon Sep 17 00:00:00 2001 From: Charles He Date: Mon, 15 May 2017 17:07:18 +0100 Subject: [PATCH 04/27] Opt-out for always-on VPN Always-on VPN is a feature introduced in N. Since then, all VPN apps targeting N+ are assumed to support the feature, and the user or the DPC can turn on / off always-on for any such VPN app. However, a few VPN apps are not designed to support the always-on feature. Enabling always-on for these apps will result in undefined behavior and confusing "Always-on VPN disconnected" notification. This feature provides a new manifest meta-data field through which a VPN app can opt out of the always-on feature explicitly. This will stop the always-on feature from being enabled for the app, both by the user and by the DPC, and will clear its existing always-on state. A @hide API is provided to check whether an app supports always-on VPN. Documentation is updated to reflect the behavior change. Bug: 36650087 Test: runtest --path java/com/android/server/connectivity/VpnTest.java Test: cts-tradefed run cts --module CtsDevicePolicyManagerTestCases --test 'com.android.cts.devicepolicy.MixedDeviceOwnerTest#testAlwaysOnVpnUnsupportedPackage' Test: cts-tradefed run cts --module CtsDevicePolicyManagerTestCases --test 'com.android.cts.devicepolicy.MixedDeviceOwnerTest#testAlwaysOnVpnUnsupportedPackageReplaced' Test: cts-tradefed run cts --module CtsDevicePolicyManagerTestCases --test 'com.android.cts.devicepolicy.MixedProfileOwnerTest#testAlwaysOnVpnUnsupportedPackage' Test: cts-tradefed run cts --module CtsDevicePolicyManagerTestCases --test 'com.android.cts.devicepolicy.MixedProfileOwnerTest#testAlwaysOnVpnUnsupportedPackageReplaced' Test: cts-tradefed run cts --module CtsDevicePolicyManagerTestCases --test 'com.android.cts.devicepolicy.MixedManagedProfileOwnerTest#testAlwaysOnVpnUnsupportedPackage' Test: cts-tradefed run cts --module CtsDevicePolicyManagerTestCases --test 'com.android.cts.devicepolicy.MixedManagedProfileOwnerTest#testAlwaysOnVpnUnsupportedPackageReplaced' Change-Id: I477897a29175e3994d4ecf8ec546e26043c90f13 --- .../java/android/net/ConnectivityManager.java | 23 ++++++++ .../android/net/IConnectivityManager.aidl | 1 + .../android/server/ConnectivityService.java | 29 ++++++++-- .../android/server/connectivity/VpnTest.java | 57 +++++++++++++++---- 4 files changed, 95 insertions(+), 15 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 7a1d85c93d..48123fede6 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -834,6 +834,29 @@ public class ConnectivityManager { } } + /** + * Checks if a VPN app supports always-on mode. + * + * In order to support the always-on feature, an app has to + *
    + *
  • target {@link VERSION_CODES#N API 24} or above, and + *
  • not opt out through the {@link VpnService#METADATA_SUPPORTS_ALWAYS_ON} meta-data + * field. + *
+ * + * @param userId The identifier of the user for whom the VPN app is installed. + * @param vpnPackage The canonical package name of the VPN app. + * @return {@code true} if and only if the VPN app exists and supports always-on mode. + * @hide + */ + public boolean isAlwaysOnVpnPackageSupportedForUser(int userId, @Nullable String vpnPackage) { + try { + return mService.isAlwaysOnVpnPackageSupported(userId, vpnPackage); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } + /** * Configures an always-on VPN connection through a specific application. * This connection is automatically granted and persisted after a reboot. diff --git a/core/java/android/net/IConnectivityManager.aidl b/core/java/android/net/IConnectivityManager.aidl index 14cee3604d..a6fe7389bc 100644 --- a/core/java/android/net/IConnectivityManager.aidl +++ b/core/java/android/net/IConnectivityManager.aidl @@ -123,6 +123,7 @@ interface IConnectivityManager VpnInfo[] getAllVpnInfo(); boolean updateLockdownVpn(); + boolean isAlwaysOnVpnPackageSupported(int userId, String packageName); boolean setAlwaysOnVpnPackage(int userId, String packageName, boolean lockdown); String getAlwaysOnVpnPackage(int userId); diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 8200289a76..71c423c58d 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -40,7 +40,6 @@ import android.content.ContentResolver; import android.content.Context; import android.content.Intent; import android.content.IntentFilter; -import android.content.pm.PackageManager; import android.content.res.Configuration; import android.database.ContentObserver; import android.net.ConnectivityManager; @@ -52,10 +51,10 @@ import android.net.INetworkPolicyManager; import android.net.INetworkStatsService; import android.net.LinkProperties; import android.net.LinkProperties.CompareResult; +import android.net.MatchAllNetworkSpecifier; import android.net.Network; import android.net.NetworkAgent; import android.net.NetworkCapabilities; -import android.net.MatchAllNetworkSpecifier; import android.net.NetworkConfig; import android.net.NetworkInfo; import android.net.NetworkInfo.DetailedState; @@ -124,13 +123,12 @@ import com.android.internal.util.IndentingPrintWriter; import com.android.internal.util.MessageUtils; import com.android.internal.util.WakeupMessage; import com.android.internal.util.XmlUtils; -import com.android.server.LocalServices; import com.android.server.am.BatteryStatsService; import com.android.server.connectivity.DataConnectionStats; import com.android.server.connectivity.KeepaliveTracker; +import com.android.server.connectivity.LingerMonitor; import com.android.server.connectivity.MockableSystemProperties; import com.android.server.connectivity.Nat464Xlat; -import com.android.server.connectivity.LingerMonitor; import com.android.server.connectivity.NetworkAgentInfo; import com.android.server.connectivity.NetworkDiagnostics; import com.android.server.connectivity.NetworkMonitor; @@ -139,8 +137,8 @@ import com.android.server.connectivity.NetworkNotificationManager.NotificationTy import com.android.server.connectivity.PacManager; import com.android.server.connectivity.PermissionMonitor; import com.android.server.connectivity.Tethering; -import com.android.server.connectivity.tethering.TetheringDependencies; import com.android.server.connectivity.Vpn; +import com.android.server.connectivity.tethering.TetheringDependencies; import com.android.server.net.BaseNetworkObserver; import com.android.server.net.LockdownVpnTracker; import com.android.server.net.NetworkPolicyManagerInternal; @@ -1494,6 +1492,12 @@ public class ConnectivityService extends IConnectivityManager.Stub ConnectivityManager.enforceChangePermission(mContext); } + private void enforceSettingsPermission() { + mContext.enforceCallingOrSelfPermission( + android.Manifest.permission.NETWORK_SETTINGS, + "ConnectivityService"); + } + private void enforceTetherAccessPermission() { mContext.enforceCallingOrSelfPermission( android.Manifest.permission.ACCESS_NETWORK_STATE, @@ -3624,6 +3628,21 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + @Override + public boolean isAlwaysOnVpnPackageSupported(int userId, String packageName) { + enforceSettingsPermission(); + enforceCrossUserPermission(userId); + + synchronized (mVpns) { + Vpn vpn = mVpns.get(userId); + if (vpn == null) { + Slog.w(TAG, "User " + userId + " has no Vpn configuration"); + return false; + } + return vpn.isAlwaysOnPackageSupported(packageName); + } + } + @Override public boolean setAlwaysOnVpnPackage(int userId, String packageName, boolean lockdown) { enforceConnectivityInternalPermission(); diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index 506d9e5043..f0b3724955 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -27,13 +27,16 @@ import android.annotation.UserIdInt; import android.app.AppOpsManager; import android.app.NotificationManager; import android.content.Context; -import android.content.Intent; import android.content.pm.ApplicationInfo; import android.content.pm.PackageManager; +import android.content.pm.ResolveInfo; +import android.content.pm.ServiceInfo; import android.content.pm.UserInfo; import android.net.NetworkInfo.DetailedState; import android.net.UidRange; -import android.os.Build; +import android.net.VpnService; +import android.os.Build.VERSION_CODES; +import android.os.Bundle; import android.os.INetworkManagementService; import android.os.Looper; import android.os.UserHandle; @@ -45,22 +48,22 @@ import android.util.ArraySet; import com.android.internal.net.VpnConfig; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Map; -import java.util.Set; - import org.mockito.Answers; -import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.Map; +import java.util.Set; + /** * Tests for {@link Vpn}. * * Build, install and run with: - * runtest --path src/com/android/server/connectivity/VpnTest.java + * runtest --path java/com/android/server/connectivity/VpnTest.java */ public class VpnTest extends AndroidTestCase { private static final String TAG = "VpnTest"; @@ -116,7 +119,7 @@ public class VpnTest extends AndroidTestCase { // Used by {@link Notification.Builder} ApplicationInfo applicationInfo = new ApplicationInfo(); - applicationInfo.targetSdkVersion = Build.VERSION_CODES.CUR_DEVELOPMENT; + applicationInfo.targetSdkVersion = VERSION_CODES.CUR_DEVELOPMENT; when(mContext.getApplicationInfo()).thenReturn(applicationInfo); doNothing().when(mNetService).registerObserver(any()); @@ -314,6 +317,40 @@ public class VpnTest extends AndroidTestCase { order.verify(mNetService).setAllowOnlyVpnForUids(eq(true), aryEq(entireUser)); } + @SmallTest + public void testIsAlwaysOnPackageSupported() throws Exception { + final Vpn vpn = createVpn(primaryUser.id); + + ApplicationInfo appInfo = new ApplicationInfo(); + when(mPackageManager.getApplicationInfoAsUser(eq(PKGS[0]), anyInt(), eq(primaryUser.id))) + .thenReturn(appInfo); + + ServiceInfo svcInfo = new ServiceInfo(); + ResolveInfo resInfo = new ResolveInfo(); + resInfo.serviceInfo = svcInfo; + when(mPackageManager.queryIntentServicesAsUser(any(), eq(PackageManager.GET_META_DATA), + eq(primaryUser.id))) + .thenReturn(Collections.singletonList(resInfo)); + + // null package name should return false + assertFalse(vpn.isAlwaysOnPackageSupported(null)); + + // Pre-N apps are not supported + appInfo.targetSdkVersion = VERSION_CODES.M; + assertFalse(vpn.isAlwaysOnPackageSupported(PKGS[0])); + + // N+ apps are supported by default + appInfo.targetSdkVersion = VERSION_CODES.N; + assertTrue(vpn.isAlwaysOnPackageSupported(PKGS[0])); + + // Apps that opt out explicitly are not supported + appInfo.targetSdkVersion = VERSION_CODES.CUR_DEVELOPMENT; + Bundle metaData = new Bundle(); + metaData.putBoolean(VpnService.METADATA_SUPPORTS_ALWAYS_ON, false); + svcInfo.metaData = metaData; + assertFalse(vpn.isAlwaysOnPackageSupported(PKGS[0])); + } + @SmallTest public void testNotificationShownForAlwaysOnApp() { final UserHandle userHandle = UserHandle.of(primaryUser.id); From 43b7474c088f12e2801e179bd55c57b0a2aba2bc Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Tue, 18 Jul 2017 14:28:27 +0900 Subject: [PATCH 05/27] IP connectivity metrics: fix tests after proto update Update to ipconnectivity.proto in commit 6d2f506bfd788a3685292d404dc9d82a27357cfe broke the associated unit tests (Change-Id: I4cf5b95956df721aecd63fddfb026a7266c190b9) Bug: 34901696 Test: runtest frameworks-net Change-Id: I57a6bad8a9836b1c45690c4589b416786ce1dfa0 --- .../IpConnectivityEventBuilderTest.java | 7 +++++++ .../connectivity/IpConnectivityMetricsTest.java | 15 +++++++++++++++ .../NetdEventListenerServiceTest.java | 8 ++++++++ 3 files changed, 30 insertions(+) diff --git a/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java b/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java index d11565abb9..eff04ab5ad 100644 --- a/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java +++ b/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java @@ -201,9 +201,14 @@ public class IpConnectivityEventBuilderTest extends TestCase { " time_ms: 1", " transports: 0", " default_network_event <", + " default_network_duration_ms: 0", + " final_score: 0", + " initial_score: 0", + " ip_support: 0", " network_id <", " network_id: 102", " >", + " no_default_network_duration_ms: 0", " previous_network_id <", " network_id: 101", " >", @@ -442,6 +447,8 @@ public class IpConnectivityEventBuilderTest extends TestCase { " program_updates_all: 7", " program_updates_allowing_multicast: 3", " received_ras: 10", + " total_packet_dropped: 0", + " total_packet_processed: 0", " zero_lifetime_ras: 1", " >", ">", diff --git a/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java b/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java index e01469b182..cc18b7f322 100644 --- a/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java +++ b/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java @@ -256,9 +256,14 @@ public class IpConnectivityMetricsTest { " time_ms: 300", " transports: 0", " default_network_event <", + " default_network_duration_ms: 0", + " final_score: 0", + " initial_score: 0", + " ip_support: 0", " network_id <", " network_id: 102", " >", + " no_default_network_duration_ms: 0", " previous_network_id <", " network_id: 101", " >", @@ -308,6 +313,8 @@ public class IpConnectivityMetricsTest { " program_updates_all: 7", " program_updates_allowing_multicast: 3", " received_ras: 10", + " total_packet_dropped: 0", + " total_packet_processed: 0", " zero_lifetime_ras: 1", " >", ">", @@ -367,6 +374,10 @@ public class IpConnectivityMetricsTest { " event_types: 1", " event_types: 1", " event_types: 2", + " getaddrinfo_error_count: 0", + " getaddrinfo_query_count: 0", + " gethostbyname_error_count: 0", + " gethostbyname_query_count: 0", " latencies_ms: 3456", " latencies_ms: 45", " latencies_ms: 638", @@ -384,6 +395,10 @@ public class IpConnectivityMetricsTest { " dns_lookup_batch <", " event_types: 1", " event_types: 2", + " getaddrinfo_error_count: 0", + " getaddrinfo_query_count: 0", + " gethostbyname_error_count: 0", + " gethostbyname_query_count: 0", " latencies_ms: 56", " latencies_ms: 34", " return_codes: 0", diff --git a/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java b/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java index f98ab3d069..46f395eae2 100644 --- a/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java +++ b/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java @@ -111,6 +111,10 @@ public class NetdEventListenerServiceTest { " event_types: 1", " event_types: 2", " event_types: 2", + " getaddrinfo_error_count: 0", + " getaddrinfo_query_count: 0", + " gethostbyname_error_count: 0", + " gethostbyname_query_count: 0", " latencies_ms: 3456", " latencies_ms: 267", " latencies_ms: 1230", @@ -142,6 +146,10 @@ public class NetdEventListenerServiceTest { " event_types: 2", " event_types: 1", " event_types: 1", + " getaddrinfo_error_count: 0", + " getaddrinfo_query_count: 0", + " gethostbyname_error_count: 0", + " gethostbyname_query_count: 0", " latencies_ms: 56", " latencies_ms: 78", " latencies_ms: 14", From d681363fd16eab6d76aa12b6fe885f30601739cf Mon Sep 17 00:00:00 2001 From: Charles He Date: Tue, 15 Aug 2017 15:30:22 +0100 Subject: [PATCH 06/27] Opt-out for always-on VPN: rename API. Rename the opt-out flag in AndroidManifest to SERVICE_META_DATA_SUPPORTS_ALWAYS_ON as directed by the API Council. Bug: 64331776 Bug: 36650087 Test: runtest --path java/com/android/server/connectivity/VpnTest.java Change-Id: I24326fad7a89083a2409134640bda81ee0359d08 --- core/java/android/net/ConnectivityManager.java | 4 ++-- tests/net/java/com/android/server/connectivity/VpnTest.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 48123fede6..744ee8ed0e 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -840,8 +840,8 @@ public class ConnectivityManager { * In order to support the always-on feature, an app has to *
    *
  • target {@link VERSION_CODES#N API 24} or above, and - *
  • not opt out through the {@link VpnService#METADATA_SUPPORTS_ALWAYS_ON} meta-data - * field. + *
  • not opt out through the {@link VpnService#SERVICE_META_DATA_SUPPORTS_ALWAYS_ON} + * meta-data field. *
* * @param userId The identifier of the user for whom the VPN app is installed. diff --git a/tests/net/java/com/android/server/connectivity/VpnTest.java b/tests/net/java/com/android/server/connectivity/VpnTest.java index f0b3724955..296cb76560 100644 --- a/tests/net/java/com/android/server/connectivity/VpnTest.java +++ b/tests/net/java/com/android/server/connectivity/VpnTest.java @@ -346,7 +346,7 @@ public class VpnTest extends AndroidTestCase { // Apps that opt out explicitly are not supported appInfo.targetSdkVersion = VERSION_CODES.CUR_DEVELOPMENT; Bundle metaData = new Bundle(); - metaData.putBoolean(VpnService.METADATA_SUPPORTS_ALWAYS_ON, false); + metaData.putBoolean(VpnService.SERVICE_META_DATA_SUPPORTS_ALWAYS_ON, false); svcInfo.metaData = metaData; assertFalse(vpn.isAlwaysOnPackageSupported(PKGS[0])); } From 9712eb35b9aa666c4a4ef10a74bba22e2967d71d Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Wed, 16 Aug 2017 13:19:04 +0900 Subject: [PATCH 07/27] Allow NetworkAgent "immutable updates" to NetworkCapabilities This patch loosens the validation checks when a NetworkAgent updates it NetworkCapabilities: instead of checking that capabilities labeled as "immutable" stay identical across updates, it is now accepted to change immutable capabilities in a way that the new NetworkCapabilities satisfies the old NetworkCapabilities. This allows a NetworkAgent to update itself in order to match more requests, but will still catch NetworkAgents that sends degradation updates causing potentially requests to not match anymore. Bug: 64125969 Test: runtest frameworks-net Merged-In: I2a1b3f9c0be6415e40edc989d0c1b03b5631f7b1 Merged-In: I0ab76de59e87c46a6961229399ff7200bce49838 Merged-In: Ied592bf6112574399a1e808da337004e1c35f244 Merged-In: I01e287b4df82a53a522566d33b3166f7801badca Merged-In: I7ee60daa9c4266e9b9179032815dd7267e06377f Merged-In: I31ef741eb83d64c476e5930d5762514b5d4cb16f (cherry picked from commit bae105a5ccd11430bab14721d1325e2303a673da) Change-Id: I9d630d63336f4db69f3eb52faa8483f1b1e35d16 --- core/java/android/net/NetworkCapabilities.java | 1 - .../java/com/android/server/ConnectivityService.java | 10 +++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index 0b9289345d..4bb8844053 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -770,7 +770,6 @@ public final class NetworkCapabilities implements Parcelable { StringJoiner joiner = new StringJoiner(", "); - // TODO: consider only enforcing that capabilities are not removed, allowing addition. // Ignore NOT_METERED being added or removed as it is effectively dynamic. http://b/63326103 // TODO: properly support NOT_METERED as a mutable and requestable capability. final long mask = ~MUTABLE_CAPABILITIES & ~(1 << NET_CAPABILITY_NOT_METERED); diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index ac41079e52..5110929bb9 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -4582,11 +4582,15 @@ public class ConnectivityService extends IConnectivityManager.Stub */ private void updateCapabilities( int oldScore, NetworkAgentInfo nai, NetworkCapabilities networkCapabilities) { - // Sanity check: a NetworkAgent should not change its static capabilities or parameters. - if (nai.everConnected) { + // Once a NetworkAgent is connected, complain if some immutable capabilities are removed. + if (nai.everConnected && + !nai.networkCapabilities.satisfiedByNetworkCapabilities(networkCapabilities)) { + // TODO: consider not complaining when a network agent degrade its capabilities if this + // does not cause any request (that is not a listen) currently matching that agent to + // stop being matched by the updated agent. String diff = nai.networkCapabilities.describeImmutableDifferences(networkCapabilities); if (!TextUtils.isEmpty(diff)) { - Slog.wtf(TAG, "BUG: " + nai + " changed immutable capabilities:" + diff); + Slog.wtf(TAG, "BUG: " + nai + " lost immutable capabilities:" + diff); } } From f2ccd7a219a783d0db8664b065f962a2cc2dc787 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Tue, 8 Aug 2017 13:06:04 +0900 Subject: [PATCH 08/27] Add convenience methods to IpPrefix and LinkAddress Also moving relevant test files into tests/net as part of runtest framworks-net. Also removes testHashCode in LinkAddress() because this test relies on the assumption that hashCode() is stable across releases or jdk versions, which is absolutely not true. This creates maintenance work for little benefit since hashCode is already tested as part of the equality test. For instance this test is now broken because hashing for InetAddress changed. Bug: 62988545 Bug: 62918393 Test: runtest frameworks-net, added coverage in tests Merged-In: I695bc3f0e801bf13bc4fc0706565758f12b775b4 Merged-In: I6d3f3c50eaec44e3a0787e849ab28e89f6f4a72d Merged-In: Iddfec82a08f845e728adadfa6ec58a60a078d6af Merged-In: I8d6dd5efd226a8b1c4b05d1e1102362b58e094a1 Merged-In: Ied0cc53ac34c7c5f5539507b1979cbf9c215262e Merged-In: I3b2b7dcb1a9a194fc08643b27bbb5a0e84e01412 (cherry picked from commit 1dfb6b67555d04973dfb9d1100dfc1c6a5200633) Change-Id: I9a17094bfdc54b9dec671306618e132a4beb59fc --- core/java/android/net/IpPrefix.java | 16 ++++ core/java/android/net/LinkAddress.java | 23 ++++- .../net/java}/android/net/IpPrefixTest.java | 47 ++++++---- .../java}/android/net/LinkAddressTest.java | 89 ++++++++++--------- 4 files changed, 114 insertions(+), 61 deletions(-) rename {core/tests/coretests/src => tests/net/java}/android/net/IpPrefixTest.java (93%) rename {core/tests/coretests/src => tests/net/java}/android/net/LinkAddressTest.java (92%) diff --git a/core/java/android/net/IpPrefix.java b/core/java/android/net/IpPrefix.java index 6b4f2d5acd..6e2654e3ce 100644 --- a/core/java/android/net/IpPrefix.java +++ b/core/java/android/net/IpPrefix.java @@ -20,6 +20,8 @@ import android.os.Parcel; import android.os.Parcelable; import android.util.Pair; +import java.net.Inet4Address; +import java.net.Inet6Address; import java.net.InetAddress; import java.net.UnknownHostException; import java.util.Arrays; @@ -184,6 +186,20 @@ public final class IpPrefix implements Parcelable { return Arrays.equals(this.address, addrBytes); } + /** + * @hide + */ + public boolean isIPv6() { + return getAddress() instanceof Inet6Address; + } + + /** + * @hide + */ + public boolean isIPv4() { + return getAddress() instanceof Inet4Address; + } + /** * Returns a string representation of this {@code IpPrefix}. * diff --git a/core/java/android/net/LinkAddress.java b/core/java/android/net/LinkAddress.java index 62de9911bc..bcfe938823 100644 --- a/core/java/android/net/LinkAddress.java +++ b/core/java/android/net/LinkAddress.java @@ -76,7 +76,7 @@ public class LinkAddress implements Parcelable { * RFC 6724 section 3.2. * @hide */ - static int scopeForUnicastAddress(InetAddress addr) { + private static int scopeForUnicastAddress(InetAddress addr) { if (addr.isAnyLocalAddress()) { return RT_SCOPE_HOST; } @@ -101,13 +101,29 @@ public class LinkAddress implements Parcelable { * Per RFC 4193 section 8, fc00::/7 identifies these addresses. */ private boolean isIPv6ULA() { - if (address instanceof Inet6Address) { + if (isIPv6()) { byte[] bytes = address.getAddress(); return ((bytes[0] & (byte)0xfe) == (byte)0xfc); } return false; } + /** + * @return true if the address is IPv6. + * @hide + */ + public boolean isIPv6() { + return address instanceof Inet6Address; + } + + /** + * @return true if the address is IPv4 or is a mapped IPv4 address. + * @hide + */ + public boolean isIPv4() { + return address instanceof Inet4Address; + } + /** * Utility function for the constructors. */ @@ -115,7 +131,7 @@ public class LinkAddress implements Parcelable { if (address == null || address.isMulticastAddress() || prefixLength < 0 || - ((address instanceof Inet4Address) && prefixLength > 32) || + (address instanceof Inet4Address && prefixLength > 32) || (prefixLength > 128)) { throw new IllegalArgumentException("Bad LinkAddress params " + address + "/" + prefixLength); @@ -184,6 +200,7 @@ public class LinkAddress implements Parcelable { */ public LinkAddress(String address, int flags, int scope) { // This may throw an IllegalArgumentException; catching it is the caller's responsibility. + // TODO: consider rejecting mapped IPv4 addresses such as "::ffff:192.0.2.5/24". Pair ipAndMask = NetworkUtils.parseIpAndMask(address); init(ipAndMask.first, ipAndMask.second, flags, scope); } diff --git a/core/tests/coretests/src/android/net/IpPrefixTest.java b/tests/net/java/android/net/IpPrefixTest.java similarity index 93% rename from core/tests/coretests/src/android/net/IpPrefixTest.java rename to tests/net/java/android/net/IpPrefixTest.java index 4f2387dcf5..b5b2c07cbc 100644 --- a/core/tests/coretests/src/android/net/IpPrefixTest.java +++ b/tests/net/java/android/net/IpPrefixTest.java @@ -16,18 +16,27 @@ package android.net; -import android.net.IpPrefix; -import android.os.Parcel; -import android.test.suitebuilder.annotation.SmallTest; -import java.net.InetAddress; -import java.util.Random; -import junit.framework.TestCase; - -import static android.test.MoreAsserts.assertNotEqual; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; -public class IpPrefixTest extends TestCase { +import android.os.Parcel; +import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; + +import java.net.InetAddress; +import java.util.Random; + +import org.junit.runner.RunWith; +import org.junit.Test; + +@RunWith(AndroidJUnit4.class) +@SmallTest +public class IpPrefixTest { private static InetAddress Address(String addr) { return InetAddress.parseNumericAddress(addr); @@ -42,7 +51,7 @@ public class IpPrefixTest extends TestCase { (byte) 0x00, (byte) 0x00, (byte) 0x00, (byte) 0xa0 }; - @SmallTest + @Test public void testConstructor() { IpPrefix p; try { @@ -103,6 +112,7 @@ public class IpPrefixTest extends TestCase { } catch(IllegalArgumentException expected) {} } + @Test public void testTruncation() { IpPrefix p; @@ -170,7 +180,7 @@ public class IpPrefixTest extends TestCase { assertFalse(o2.equals(o1)); } - @SmallTest + @Test public void testEquals() { IpPrefix p1, p2; @@ -212,7 +222,7 @@ public class IpPrefixTest extends TestCase { assertAreNotEqual(p1, p2); } - @SmallTest + @Test public void testContains() { IpPrefix p = new IpPrefix("2001:db8:f00::ace:d00d/127"); assertTrue(p.contains(Address("2001:db8:f00::ace:d00c"))); @@ -240,7 +250,7 @@ public class IpPrefixTest extends TestCase { assertFalse(ipv4Default.contains(Address("2001:db8::f00"))); } - @SmallTest + @Test public void testHashCode() { IpPrefix p = new IpPrefix(new byte[4], 0); Random random = new Random(); @@ -261,12 +271,12 @@ public class IpPrefixTest extends TestCase { assertEquals(p.hashCode(), oldP.hashCode()); } if (p.hashCode() != oldP.hashCode()) { - assertNotEqual(p, oldP); + assertNotEquals(p, oldP); } } } - @SmallTest + @Test public void testHashCodeIsNotConstant() { IpPrefix[] prefixes = { new IpPrefix("2001:db8:f00::ace:d00d/127"), @@ -276,12 +286,12 @@ public class IpPrefixTest extends TestCase { }; for (int i = 0; i < prefixes.length; i++) { for (int j = i + 1; j < prefixes.length; j++) { - assertNotEqual(prefixes[i].hashCode(), prefixes[j].hashCode()); + assertNotEquals(prefixes[i].hashCode(), prefixes[j].hashCode()); } } } - @SmallTest + @Test public void testMappedAddressesAreBroken() { // 192.0.2.0/24 != ::ffff:c000:0204/120, but because we use InetAddress, // we are unable to comprehend that. @@ -318,13 +328,16 @@ public class IpPrefixTest extends TestCase { assertEquals(p, p2); } + @Test public void testParceling() { IpPrefix p; p = new IpPrefix("2001:4860:db8::/64"); assertParcelingIsLossless(p); + assertTrue(p.isIPv6()); p = new IpPrefix("192.0.2.0/25"); assertParcelingIsLossless(p); + assertTrue(p.isIPv4()); } } diff --git a/core/tests/coretests/src/android/net/LinkAddressTest.java b/tests/net/java/android/net/LinkAddressTest.java similarity index 92% rename from core/tests/coretests/src/android/net/LinkAddressTest.java rename to tests/net/java/android/net/LinkAddressTest.java index adf8d95c0b..c1ad946695 100644 --- a/core/tests/coretests/src/android/net/LinkAddressTest.java +++ b/tests/net/java/android/net/LinkAddressTest.java @@ -16,6 +16,23 @@ package android.net; +import static android.system.OsConstants.IFA_F_DADFAILED; +import static android.system.OsConstants.IFA_F_DEPRECATED; +import static android.system.OsConstants.IFA_F_OPTIMISTIC; +import static android.system.OsConstants.IFA_F_PERMANENT; +import static android.system.OsConstants.IFA_F_TEMPORARY; +import static android.system.OsConstants.IFA_F_TENTATIVE; +import static android.system.OsConstants.RT_SCOPE_HOST; +import static android.system.OsConstants.RT_SCOPE_LINK; +import static android.system.OsConstants.RT_SCOPE_SITE; +import static android.system.OsConstants.RT_SCOPE_UNIVERSE; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + import java.net.Inet4Address; import java.net.Inet6Address; import java.net.InetAddress; @@ -27,44 +44,35 @@ import java.util.Collections; import java.util.Comparator; import java.util.List; -import android.net.LinkAddress; import android.os.Parcel; -import android.test.AndroidTestCase; -import static android.test.MoreAsserts.assertNotEqual; -import android.test.suitebuilder.annotation.SmallTest; +import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; -import static android.system.OsConstants.IFA_F_DADFAILED; -import static android.system.OsConstants.IFA_F_DEPRECATED; -import static android.system.OsConstants.IFA_F_OPTIMISTIC; -import static android.system.OsConstants.IFA_F_PERMANENT; -import static android.system.OsConstants.IFA_F_TEMPORARY; -import static android.system.OsConstants.IFA_F_TENTATIVE; -import static android.system.OsConstants.RT_SCOPE_HOST; -import static android.system.OsConstants.RT_SCOPE_LINK; -import static android.system.OsConstants.RT_SCOPE_SITE; -import static android.system.OsConstants.RT_SCOPE_UNIVERSE; +import org.junit.runner.RunWith; +import org.junit.Test; -/** - * Tests for {@link LinkAddress}. - */ -public class LinkAddressTest extends AndroidTestCase { +@RunWith(AndroidJUnit4.class) +@SmallTest +public class LinkAddressTest { private static final String V4 = "192.0.2.1"; private static final String V6 = "2001:db8::1"; private static final InetAddress V4_ADDRESS = NetworkUtils.numericToInetAddress(V4); private static final InetAddress V6_ADDRESS = NetworkUtils.numericToInetAddress(V6); + @Test public void testConstants() { // RT_SCOPE_UNIVERSE = 0, but all the other constants should be nonzero. - assertNotEqual(0, RT_SCOPE_HOST); - assertNotEqual(0, RT_SCOPE_LINK); - assertNotEqual(0, RT_SCOPE_SITE); + assertNotEquals(0, RT_SCOPE_HOST); + assertNotEquals(0, RT_SCOPE_LINK); + assertNotEquals(0, RT_SCOPE_SITE); - assertNotEqual(0, IFA_F_DEPRECATED); - assertNotEqual(0, IFA_F_PERMANENT); - assertNotEqual(0, IFA_F_TENTATIVE); + assertNotEquals(0, IFA_F_DEPRECATED); + assertNotEquals(0, IFA_F_PERMANENT); + assertNotEquals(0, IFA_F_TENTATIVE); } + @Test public void testConstructors() throws SocketException { LinkAddress address; @@ -74,12 +82,14 @@ public class LinkAddressTest extends AndroidTestCase { assertEquals(25, address.getPrefixLength()); assertEquals(0, address.getFlags()); assertEquals(RT_SCOPE_UNIVERSE, address.getScope()); + assertTrue(address.isIPv4()); address = new LinkAddress(V6_ADDRESS, 127); assertEquals(V6_ADDRESS, address.getAddress()); assertEquals(127, address.getPrefixLength()); assertEquals(0, address.getFlags()); assertEquals(RT_SCOPE_UNIVERSE, address.getScope()); + assertTrue(address.isIPv6()); // Nonsensical flags/scopes or combinations thereof are acceptable. address = new LinkAddress(V6 + "/64", IFA_F_DEPRECATED | IFA_F_PERMANENT, RT_SCOPE_LINK); @@ -87,12 +97,14 @@ public class LinkAddressTest extends AndroidTestCase { assertEquals(64, address.getPrefixLength()); assertEquals(IFA_F_DEPRECATED | IFA_F_PERMANENT, address.getFlags()); assertEquals(RT_SCOPE_LINK, address.getScope()); + assertTrue(address.isIPv6()); address = new LinkAddress(V4 + "/23", 123, 456); assertEquals(V4_ADDRESS, address.getAddress()); assertEquals(23, address.getPrefixLength()); assertEquals(123, address.getFlags()); assertEquals(456, address.getScope()); + assertTrue(address.isIPv4()); // InterfaceAddress doesn't have a constructor. Fetch some from an interface. List addrs = NetworkInterface.getByName("lo").getInterfaceAddresses(); @@ -174,6 +186,7 @@ public class LinkAddressTest extends AndroidTestCase { } catch(IllegalArgumentException expected) {} } + @Test public void testAddressScopes() { assertEquals(RT_SCOPE_HOST, new LinkAddress("::/128").getScope()); assertEquals(RT_SCOPE_HOST, new LinkAddress("0.0.0.0/32").getScope()); @@ -216,6 +229,7 @@ public class LinkAddressTest extends AndroidTestCase { assertFalse(l2 + " unexpectedly equal to " + l1, l2.equals(l1)); } + @Test public void testEqualsAndSameAddressAs() { LinkAddress l1, l2, l3; @@ -293,26 +307,17 @@ public class LinkAddressTest extends AndroidTestCase { assertIsSameAddressAs(l1, l2); } + @Test public void testHashCode() { - LinkAddress l; + LinkAddress l1, l2; - l = new LinkAddress(V4_ADDRESS, 23); - assertEquals(-982787, l.hashCode()); + l1 = new LinkAddress(V4_ADDRESS, 23); + l2 = new LinkAddress(V4_ADDRESS, 23, 0, RT_SCOPE_HOST); + assertNotEquals(l1.hashCode(), l2.hashCode()); - l = new LinkAddress(V4_ADDRESS, 23, 0, RT_SCOPE_HOST); - assertEquals(-971865, l.hashCode()); - - l = new LinkAddress(V4_ADDRESS, 27); - assertEquals(-982743, l.hashCode()); - - l = new LinkAddress(V6_ADDRESS, 64); - assertEquals(1076522926, l.hashCode()); - - l = new LinkAddress(V6_ADDRESS, 128); - assertEquals(1076523630, l.hashCode()); - - l = new LinkAddress(V6_ADDRESS, 128, IFA_F_TENTATIVE, RT_SCOPE_UNIVERSE); - assertEquals(1076524846, l.hashCode()); + l1 = new LinkAddress(V6_ADDRESS, 128); + l2 = new LinkAddress(V6_ADDRESS, 128, IFA_F_TENTATIVE, RT_SCOPE_UNIVERSE); + assertNotEquals(l1.hashCode(), l2.hashCode()); } private LinkAddress passThroughParcel(LinkAddress l) { @@ -334,6 +339,7 @@ public class LinkAddressTest extends AndroidTestCase { assertEquals(l, l2); } + @Test public void testParceling() { LinkAddress l; @@ -352,6 +358,7 @@ public class LinkAddressTest extends AndroidTestCase { assertFalse(msg, l.isGlobalPreferred()); } + @Test public void testIsGlobalPreferred() { LinkAddress l; From b319377255ddde12135fe3ac53858493290adab0 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Fri, 18 Aug 2017 14:41:22 +0900 Subject: [PATCH 09/27] Really allow NetworkAgent immutable updates to NetworkCapabilities This patch completes commit bae105a5ccd11430bab14721d1325e2303a673da to really allow updates of immutable capabilities to NetworkCapabilities of NetworkAgents by using satisfiedByImmutableNetworkCapabilities instead satisfiedByNetworkCapabilities. Bug: 64125969 Test: runtest frameworks-net Merged-In: I9beeb623792f0ee31abcd4ba9d0ba2451304fb2e Merged-In: Ifbdd005576b3f0fbf278ecec81ce3e4308c6276d Merged-In: Id352fdb6da21a2150d3e8d062d7eba11878f6919 Merged-In: If3742ea0e2151b9b710eda2fed280c31f7960393 Merged-In: Icd8e328e3c810a644bfb83798bd42fed8dc70425 (cherry picked from commit a60c6eab42cd904faaf26c23100939d397b80c39) Change-Id: I737494117d7f1e0198f0ad4d3c41019c9295fe09 --- .../core/java/com/android/server/ConnectivityService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 5110929bb9..ec83a03f8a 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -4583,8 +4583,8 @@ public class ConnectivityService extends IConnectivityManager.Stub private void updateCapabilities( int oldScore, NetworkAgentInfo nai, NetworkCapabilities networkCapabilities) { // Once a NetworkAgent is connected, complain if some immutable capabilities are removed. - if (nai.everConnected && - !nai.networkCapabilities.satisfiedByNetworkCapabilities(networkCapabilities)) { + if (nai.everConnected && !nai.networkCapabilities.satisfiedByImmutableNetworkCapabilities( + networkCapabilities)) { // TODO: consider not complaining when a network agent degrade its capabilities if this // does not cause any request (that is not a listen) currently matching that agent to // stop being matched by the updated agent. From 49ab263c0bd8008a2508e59d00feb18b845484cb Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Thu, 17 Aug 2017 19:23:08 +0900 Subject: [PATCH 10/27] Add tether offload traffic to interface stats as well. Currently, we only count add tethering traffic to per-UID stats, but not to total data usage (i.e., dev and XT stats). This is correct for software tethering, because all software forwarded packets are already included in interface counters, but it is incorrect for hardware offload, because such packets do not increment interface counters. To fix this: 1. Add an argument to ITetheringStatsProvider#getTetherStats to indicate whether per-UID stats are requested. For clarity, define integer constants STATS_PER_IFACE and STATS_PER_UID to represent these operations. 2. Make NetdTetheringStatsProvider return stats only if per-UID stats are requested. (Otherwise tethering traffic would be double-counted). 3. Make OffloadController's stats provider return the same stats regardless of whether per-UID stats were requested or not. 4. Make NetworkStatsService add non-per-UID tethering stats to the dev and XT snapshots. The per-UID snapshots were already correctly adding in per-UID stats. Bug: 29337859 Bug: 32163131 Test: runtest frameworks-net Test: runtest frameworks-telephony Change-Id: I7a4d04ab47694d754874136179f8edad71099638 --- .../server/net/NetworkStatsServiceTest.java | 41 ++++++++++++++----- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java index feb46d39b5..fa997958ba 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java @@ -31,6 +31,8 @@ import static android.net.NetworkStats.ROAMING_YES; import static android.net.NetworkStats.SET_ALL; import static android.net.NetworkStats.SET_DEFAULT; import static android.net.NetworkStats.SET_FOREGROUND; +import static android.net.NetworkStats.STATS_PER_IFACE; +import static android.net.NetworkStats.STATS_PER_UID; import static android.net.NetworkStats.TAG_NONE; import static android.net.NetworkStats.UID_ALL; import static android.net.NetworkStatsHistory.FIELD_ALL; @@ -823,17 +825,24 @@ public class NetworkStatsServiceTest { incrementCurrentTime(HOUR_IN_MILLIS); expectCurrentTime(); expectDefaultSettings(); - expectNetworkStatsSummary(new NetworkStats(getElapsedRealtime(), 1) - .addIfaceValues(TEST_IFACE, 2048L, 16L, 512L, 4L)); + // Traffic seen by kernel counters (includes software tethering). + final NetworkStats ifaceStats = new NetworkStats(getElapsedRealtime(), 1) + .addIfaceValues(TEST_IFACE, 1536L, 12L, 384L, 3L); + // Hardware tethering traffic, not seen by kernel counters. + final NetworkStats tetherStatsHardware = new NetworkStats(getElapsedRealtime(), 1) + .addIfaceValues(TEST_IFACE, 512L, 4L, 128L, 1L); + + // Traffic for UID_RED. final NetworkStats uidStats = new NetworkStats(getElapsedRealtime(), 1) .addValues(TEST_IFACE, UID_RED, SET_DEFAULT, TAG_NONE, 128L, 2L, 128L, 2L, 0L); - final String[] tetherIfacePairs = new String[] { TEST_IFACE, "wlan0" }; + // All tethering traffic, both hardware and software. final NetworkStats tetherStats = new NetworkStats(getElapsedRealtime(), 1) .addValues(TEST_IFACE, UID_TETHERING, SET_DEFAULT, TAG_NONE, 1920L, 14L, 384L, 2L, 0L); - expectNetworkStatsUidDetail(uidStats, tetherIfacePairs, tetherStats); + expectNetworkStatsSummary(ifaceStats, tetherStatsHardware); + expectNetworkStatsUidDetail(uidStats, tetherStats); forcePollAndWaitForIdle(); // verify service recorded history @@ -1013,10 +1022,16 @@ public class NetworkStatsServiceTest { } private void expectNetworkStatsSummary(NetworkStats summary) throws Exception { + expectNetworkStatsSummary(summary, new NetworkStats(0L, 0)); + } + + private void expectNetworkStatsSummary(NetworkStats summary, NetworkStats tetherStats) + throws Exception { when(mConnManager.getAllVpnInfo()).thenReturn(new VpnInfo[0]); - expectNetworkStatsSummaryDev(summary); - expectNetworkStatsSummaryXt(summary); + expectNetworkStatsTethering(STATS_PER_IFACE, tetherStats); + expectNetworkStatsSummaryDev(summary.clone()); + expectNetworkStatsSummaryXt(summary.clone()); } private void expectNetworkStatsSummaryDev(NetworkStats summary) throws Exception { @@ -1027,17 +1042,21 @@ public class NetworkStatsServiceTest { when(mNetManager.getNetworkStatsSummaryXt()).thenReturn(summary); } - private void expectNetworkStatsUidDetail(NetworkStats detail) throws Exception { - expectNetworkStatsUidDetail(detail, new String[0], new NetworkStats(0L, 0)); + private void expectNetworkStatsTethering(int how, NetworkStats stats) + throws Exception { + when(mNetManager.getNetworkStatsTethering(how)).thenReturn(stats); } - private void expectNetworkStatsUidDetail( - NetworkStats detail, String[] tetherIfacePairs, NetworkStats tetherStats) + private void expectNetworkStatsUidDetail(NetworkStats detail) throws Exception { + expectNetworkStatsUidDetail(detail, new NetworkStats(0L, 0)); + } + + private void expectNetworkStatsUidDetail(NetworkStats detail, NetworkStats tetherStats) throws Exception { when(mNetManager.getNetworkStatsUidDetail(UID_ALL)).thenReturn(detail); // also include tethering details, since they are folded into UID - when(mNetManager.getNetworkStatsTethering()).thenReturn(tetherStats); + when(mNetManager.getNetworkStatsTethering(STATS_PER_UID)).thenReturn(tetherStats); } private void expectDefaultSettings() throws Exception { From 5a5c1bb116caddf65065007c8f36ca7516882504 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Tue, 27 Jun 2017 15:13:20 +0900 Subject: [PATCH 11/27] Nat464Xlat: clat management cleanup This patch does some minor refactoring of clat starting/stopping code: - remove unused LinkProperties arguments in updateClat - remove unused Context argument in Nat464Xlat ctor - introduce ensureClatIsStarted and ensureClatIsStopped methods and simplify updateClat - add clatd to NetworkAgentInfo toString() method - clarify some comments This changes prepare for moving BaseNetworkObserver callbacks to ConnectivityService. Bug: 62997041 Bug: 64571917 Test: runtest frameworks-net manually connected to IPv6 only network and went to test-ipv6.com Merged-In: Idb204784614cfe700f73255a7a7b78c5e9ee6eca Merged-In: Ic3808a1afe48efac745b1b378fb12cc5678918ec Merged-In: Ia769aef6ef8b258f44f8979003d271c96264f1b5 Merged-In: I1a19e6fbb0cb13262e90b171d861062469078fb6 Merged-In: I06661bd6bd1456ba34a3bbdb52c120ac01da9d61 Merged-In: Ifccff9f3cfccdb2cdddf2f07561f0787a48bc0f8 (cherry picked from commit b577d65825e623a9868664486482ed137b98b504) Change-Id: Ibb02888633df9643030336c4dbea6c569a47554c --- .../android/server/ConnectivityService.java | 37 +++++++++++++------ .../server/connectivity/Nat464Xlat.java | 29 +++++++++------ .../server/connectivity/NetworkAgentInfo.java | 4 +- 3 files changed, 45 insertions(+), 25 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index ec83a03f8a..adf536bbf4 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2269,7 +2269,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } nai.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_DISCONNECTED); mNetworkAgentInfos.remove(msg.replyTo); - updateClat(null, nai.linkProperties, nai); + maybeStopClat(nai); synchronized (mNetworkForNetId) { // Remove the NetworkAgent, but don't mark the netId as // available until we've told netd to delete it below. @@ -4382,7 +4382,8 @@ public class ConnectivityService extends IConnectivityManager.Stub updateRoutes(newLp, oldLp, netId); updateDnses(newLp, oldLp, netId); - updateClat(newLp, oldLp, networkAgent); + // Start or stop clat accordingly to network state. + updateClat(networkAgent); if (isDefaultNetwork(networkAgent)) { handleApplyDefaultProxy(newLp.getHttpProxy()); } else { @@ -4397,18 +4398,32 @@ public class ConnectivityService extends IConnectivityManager.Stub mKeepaliveTracker.handleCheckKeepalivesStillValid(networkAgent); } - private void updateClat(LinkProperties newLp, LinkProperties oldLp, NetworkAgentInfo nai) { - final boolean wasRunningClat = nai.clatd != null && nai.clatd.isStarted(); - final boolean shouldRunClat = Nat464Xlat.requiresClat(nai); - - if (!wasRunningClat && shouldRunClat) { - nai.clatd = new Nat464Xlat(mContext, mNetd, mTrackerHandler, nai); - nai.clatd.start(); - } else if (wasRunningClat && !shouldRunClat) { - nai.clatd.stop(); + private void updateClat(NetworkAgentInfo nai) { + if (Nat464Xlat.requiresClat(nai)) { + maybeStartClat(nai); + } else { + maybeStopClat(nai); } } + /** Ensure clat has started for this network. */ + private void maybeStartClat(NetworkAgentInfo nai) { + if (nai.clatd != null && nai.clatd.isStarted()) { + return; + } + nai.clatd = new Nat464Xlat(mNetd, mTrackerHandler, nai); + nai.clatd.start(); + } + + /** Ensure clat has stopped for this network. */ + private void maybeStopClat(NetworkAgentInfo nai) { + if (nai.clatd == null) { + return; + } + nai.clatd.stop(); + nai.clatd = null; + } + private void wakeupModifyInterface(String iface, NetworkCapabilities caps, boolean add) { // Marks are only available on WiFi interaces. Checking for // marks on unsupported interfaces is harmless. diff --git a/services/core/java/com/android/server/connectivity/Nat464Xlat.java b/services/core/java/com/android/server/connectivity/Nat464Xlat.java index b390884713..27426d7bde 100644 --- a/services/core/java/com/android/server/connectivity/Nat464Xlat.java +++ b/services/core/java/com/android/server/connectivity/Nat464Xlat.java @@ -18,7 +18,6 @@ package com.android.server.connectivity; import java.net.Inet4Address; -import android.content.Context; import android.net.InterfaceConfiguration; import android.net.ConnectivityManager; import android.net.LinkAddress; @@ -35,17 +34,18 @@ import com.android.server.net.BaseNetworkObserver; import com.android.internal.util.ArrayUtils; /** - * @hide - * * Class to manage a 464xlat CLAT daemon. + * + * @hide */ public class Nat464Xlat extends BaseNetworkObserver { - private static final String TAG = "Nat464Xlat"; + private static final String TAG = Nat464Xlat.class.getSimpleName(); // This must match the interface prefix in clatd.c. private static final String CLAT_PREFIX = "v4-"; - // The network types we will start clatd on. + // The network types we will start clatd on, + // allowing clat only on networks for which we can support IPv6-only. private static final int[] NETWORK_TYPES = { ConnectivityManager.TYPE_MOBILE, ConnectivityManager.TYPE_WIFI, @@ -76,9 +76,7 @@ public class Nat464Xlat extends BaseNetworkObserver { private String mIface; private boolean mIsRunning; - public Nat464Xlat( - Context context, INetworkManagementService nmService, - Handler handler, NetworkAgentInfo nai) { + public Nat464Xlat(INetworkManagementService nmService, Handler handler, NetworkAgentInfo nai) { mNMService = nmService; mHandler = handler; mNetwork = nai; @@ -90,13 +88,14 @@ public class Nat464Xlat extends BaseNetworkObserver { * @return true if the network requires clat, false otherwise. */ public static boolean requiresClat(NetworkAgentInfo nai) { + // TODO: migrate to NetworkCapabilities.TRANSPORT_*. final int netType = nai.networkInfo.getType(); + final boolean supported = ArrayUtils.contains(NETWORK_TYPES, nai.networkInfo.getType()); final boolean connected = nai.networkInfo.isConnected(); + // We only run clat on networks that don't have a native IPv4 address. final boolean hasIPv4Address = - (nai.linkProperties != null) ? nai.linkProperties.hasIPv4Address() : false; - // Only support clat on mobile and wifi for now, because these are the only IPv6-only - // networks we can connect to. - return connected && !hasIPv4Address && ArrayUtils.contains(NETWORK_TYPES, netType); + (nai.linkProperties != null) && nai.linkProperties.hasIPv4Address(); + return supported && connected && !hasIPv4Address; } /** @@ -227,6 +226,7 @@ public class Nat464Xlat extends BaseNetworkObserver { } private void maybeSetIpv6NdOffload(String iface, boolean on) { + // TODO: migrate to NetworkCapabilities.TRANSPORT_*. if (mNetwork.networkInfo.getType() != ConnectivityManager.TYPE_WIFI) { return; } @@ -286,4 +286,9 @@ public class Nat464Xlat extends BaseNetworkObserver { } } } + + @Override + public String toString() { + return "mBaseIface: " + mBaseIface + ", mIface: " + mIface + ", mIsRunning: " + mIsRunning; + } } diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 2a618bcc2e..872923a032 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -562,13 +562,13 @@ public class NetworkAgentInfo implements Comparable { "acceptUnvalidated{" + networkMisc.acceptUnvalidated + "} " + "everCaptivePortalDetected{" + everCaptivePortalDetected + "} " + "lastCaptivePortalDetected{" + lastCaptivePortalDetected + "} " + + "clat{" + clatd + "} " + "}"; } public String name() { return "NetworkAgentInfo [" + networkInfo.getTypeName() + " (" + - networkInfo.getSubtypeName() + ") - " + - (network == null ? "null" : network.toString()) + "]"; + networkInfo.getSubtypeName() + ") - " + Objects.toString(network) + "]"; } // Enables sorting in descending order of score. From 9cd15c7f2eb0315b155a69cb3fc0ebf2e3474985 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Thu, 29 Jun 2017 14:04:13 +0900 Subject: [PATCH 12/27] Nat464Xlat: internal state guards cleanup + state enum This patch does some cleanup of Nat464Xlat internal state guards against the Nat464Xlat state Idle | Started | Running, which reduces code nesting. It also replaces introspection of internal state for distinguishing between different stages in 464xlat lifecycle with an enum explicitly introducing these three Idle | Started | Running states. Bug: 62997041 Bug: 64571917 Test: runtest frameworks-net manually connected to ipv6 network and went to test-ipv6.com Merged-In: I6efc9fed2420ca488731a2b9b9c3c025b16eca10 Merged-In: I188ac4c367db11cb33b67fe92df3a120e3c6fbce Merged-In: I7e2c5db8d537fb0ab47cde37158b7f04d7786942 Merged-In: Ic2246a97618c596dbdbf48cda39c2f5b66e3bfb6 Merged-In: Ib04b9a3d56e9daf61b299a30e24a3c8839819a00 Merged-In: Icc1558a0f0e7c299270f550897347458e2bd3188 (cherry pick from commit 4f6f139869ddadf6f9ed50967c106a10a2e8ce3f) Change-Id: Iebc7d153d8cd0b90d074d8d6eed821fbc3f1370d --- .../server/connectivity/Nat464Xlat.java | 191 ++++++++++-------- 1 file changed, 108 insertions(+), 83 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/Nat464Xlat.java b/services/core/java/com/android/server/connectivity/Nat464Xlat.java index 27426d7bde..f8d23d4c8d 100644 --- a/services/core/java/com/android/server/connectivity/Nat464Xlat.java +++ b/services/core/java/com/android/server/connectivity/Nat464Xlat.java @@ -16,8 +16,6 @@ package com.android.server.connectivity; -import java.net.Inet4Address; - import android.net.InterfaceConfiguration; import android.net.ConnectivityManager; import android.net.LinkAddress; @@ -33,6 +31,9 @@ import android.util.Slog; import com.android.server.net.BaseNetworkObserver; import com.android.internal.util.ArrayUtils; +import java.net.Inet4Address; +import java.util.Objects; + /** * Class to manage a 464xlat CLAT daemon. * @@ -60,21 +61,18 @@ public class Nat464Xlat extends BaseNetworkObserver { // The network we're running on, and its type. private final NetworkAgentInfo mNetwork; - // Internal state variables. - // - // The possible states are: - // - Idle: start() not called. Everything is null. - // - Starting: start() called. Interfaces are non-null. isStarted() returns true. - // mIsRunning is false. - // - Running: start() called, and interfaceLinkStateChanged() told us that mIface is up. - // mIsRunning is true. - // + private enum State { + IDLE, // start() not called. Base iface and stacked iface names are null. + STARTING, // start() called. Base iface and stacked iface names are known. + RUNNING; // start() called, and the stacked iface is known to be up. + } + // Once mIface is non-null and isStarted() is true, methods called by ConnectivityService on // its handler thread must not modify any internal state variables; they are only updated by the // interface observers, called on the notification threads. private String mBaseIface; private String mIface; - private boolean mIsRunning; + private volatile State mState = State.IDLE; public Nat464Xlat(INetworkManagementService nmService, Handler handler, NetworkAgentInfo nai) { mNMService = nmService; @@ -99,20 +97,36 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Determines whether clatd is started. Always true, except a) if start has not yet been called, - * or b) if our interface was removed. + * @return true if clatd has been started and has not yet stopped. + * A true result corresponds to internal states STARTING and RUNNING. */ public boolean isStarted() { - return mIface != null; + return mState != State.IDLE; + } + + /** + * @return true if clatd has been started and the stacked interface is up. + */ + public boolean isRunning() { + return mState == State.RUNNING; + } + + /** + * Sets internal state. + */ + private void enterStartingState(String baseIface) { + mIface = CLAT_PREFIX + baseIface; + mBaseIface = baseIface; + mState = State.STARTING; } /** * Clears internal state. Must not be called by ConnectivityService. */ - private void clear() { + private void enterIdleState() { mIface = null; mBaseIface = null; - mIsRunning = false; + mState = State.IDLE; } /** @@ -136,19 +150,19 @@ public class Nat464Xlat extends BaseNetworkObserver { return; } - mBaseIface = mNetwork.linkProperties.getInterfaceName(); - if (mBaseIface == null) { + String baseIface = mNetwork.linkProperties.getInterfaceName(); + if (baseIface == null) { Slog.e(TAG, "startClat: Can't start clat on null interface"); return; } - mIface = CLAT_PREFIX + mBaseIface; - // From now on, isStarted() will return true. + // TODO: should we only do this if mNMService.startClatd() succeeds? + enterStartingState(baseIface); Slog.i(TAG, "Starting clatd on " + mBaseIface); try { mNMService.startClatd(mBaseIface); } catch(RemoteException|IllegalStateException e) { - Slog.e(TAG, "Error starting clatd: " + e); + Slog.e(TAG, "Error starting clatd on " + mBaseIface, e); } } @@ -156,18 +170,19 @@ public class Nat464Xlat extends BaseNetworkObserver { * Stops the clat daemon. Called by ConnectivityService on the handler thread. */ public void stop() { - if (isStarted()) { - Slog.i(TAG, "Stopping clatd"); - try { - mNMService.stopClatd(mBaseIface); - } catch(RemoteException|IllegalStateException e) { - Slog.e(TAG, "Error stopping clatd: " + e); - } - // When clatd stops and its interface is deleted, interfaceRemoved() will notify - // ConnectivityService and call clear(). - } else { - Slog.e(TAG, "clatd: already stopped"); + if (!isStarted()) { + Slog.e(TAG, "stopClat: already stopped or not started"); + return; } + + Slog.i(TAG, "Stopping clatd on " + mBaseIface); + try { + mNMService.stopClatd(mBaseIface); + } catch(RemoteException|IllegalStateException e) { + Slog.e(TAG, "Error stopping clatd on " + mBaseIface, e); + } + // When clatd stops and its interface is deleted, interfaceRemoved() will notify + // ConnectivityService and call enterIdleState(). } private void updateConnectivityService(LinkProperties lp) { @@ -183,16 +198,19 @@ public class Nat464Xlat extends BaseNetworkObserver { * has no idea that 464xlat is running on top of it. */ public void fixupLinkProperties(LinkProperties oldLp) { - if (mNetwork.clatd != null && - mIsRunning && - mNetwork.linkProperties != null && - !mNetwork.linkProperties.getAllInterfaceNames().contains(mIface)) { - Slog.d(TAG, "clatd running, updating NAI for " + mIface); - for (LinkProperties stacked: oldLp.getStackedLinks()) { - if (mIface.equals(stacked.getInterfaceName())) { - mNetwork.linkProperties.addStackedLink(stacked); - break; - } + if (!isRunning()) { + return; + } + LinkProperties lp = mNetwork.linkProperties; + if (lp == null || lp.getAllInterfaceNames().contains(mIface)) { + return; + } + + Slog.d(TAG, "clatd running, updating NAI for " + mIface); + for (LinkProperties stacked: oldLp.getStackedLinks()) { + if (Objects.equals(mIface, stacked.getInterfaceName())) { + lp.addStackedLink(stacked); + return; } } } @@ -238,57 +256,64 @@ public class Nat464Xlat extends BaseNetworkObserver { } } + /** + * Adds stacked link on base link and transitions to Running state + * This is called by the InterfaceObserver on its own thread, so can race with stop(). + */ @Override public void interfaceLinkStateChanged(String iface, boolean up) { - // Called by the InterfaceObserver on its own thread, so can race with stop(). - if (isStarted() && up && mIface.equals(iface)) { - Slog.i(TAG, "interface " + iface + " is up, mIsRunning " + mIsRunning + "->true"); - - if (!mIsRunning) { - LinkAddress clatAddress = getLinkAddress(iface); - if (clatAddress == null) { - return; - } - mIsRunning = true; - maybeSetIpv6NdOffload(mBaseIface, false); - LinkProperties lp = new LinkProperties(mNetwork.linkProperties); - lp.addStackedLink(makeLinkProperties(clatAddress)); - Slog.i(TAG, "Adding stacked link " + mIface + " on top of " + mBaseIface); - updateConnectivityService(lp); - } + if (!isStarted() || !up || !Objects.equals(mIface, iface)) { + return; } + if (isRunning()) { + return; + } + LinkAddress clatAddress = getLinkAddress(iface); + if (clatAddress == null) { + return; + } + mState = State.RUNNING; + Slog.i(TAG, String.format("interface %s is up, adding stacked link %s on top of %s", + mIface, mIface, mBaseIface)); + + maybeSetIpv6NdOffload(mBaseIface, false); + LinkProperties lp = new LinkProperties(mNetwork.linkProperties); + lp.addStackedLink(makeLinkProperties(clatAddress)); + updateConnectivityService(lp); } @Override public void interfaceRemoved(String iface) { - if (isStarted() && mIface.equals(iface)) { - Slog.i(TAG, "interface " + iface + " removed, mIsRunning " + mIsRunning + "->false"); - - if (mIsRunning) { - // The interface going away likely means clatd has crashed. Ask netd to stop it, - // because otherwise when we try to start it again on the same base interface netd - // will complain that it's already started. - // - // Note that this method can be called by the interface observer at the same time - // that ConnectivityService calls stop(). In this case, the second call to - // stopClatd() will just throw IllegalStateException, which we'll ignore. - try { - mNMService.unregisterObserver(this); - mNMService.stopClatd(mBaseIface); - } catch (RemoteException|IllegalStateException e) { - // Well, we tried. - } - maybeSetIpv6NdOffload(mBaseIface, true); - LinkProperties lp = new LinkProperties(mNetwork.linkProperties); - lp.removeStackedLink(mIface); - clear(); - updateConnectivityService(lp); - } + if (!isStarted() || !Objects.equals(mIface, iface)) { + return; } + if (!isRunning()) { + return; + } + + Slog.i(TAG, "interface " + iface + " removed"); + // The interface going away likely means clatd has crashed. Ask netd to stop it, + // because otherwise when we try to start it again on the same base interface netd + // will complain that it's already started. + // + // Note that this method can be called by the interface observer at the same time + // that ConnectivityService calls stop(). In this case, the second call to + // stopClatd() will just throw IllegalStateException, which we'll ignore. + try { + mNMService.unregisterObserver(this); + mNMService.stopClatd(mBaseIface); + } catch (RemoteException|IllegalStateException e) { + // Well, we tried. + } + maybeSetIpv6NdOffload(mBaseIface, true); + LinkProperties lp = new LinkProperties(mNetwork.linkProperties); + lp.removeStackedLink(mIface); + enterIdleState(); + updateConnectivityService(lp); } @Override public String toString() { - return "mBaseIface: " + mBaseIface + ", mIface: " + mIface + ", mIsRunning: " + mIsRunning; + return "mBaseIface: " + mBaseIface + ", mIface: " + mIface + ", mState: " + mState; } } From 39e10e211184a11fcb61e49d33b850597ff943d3 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Wed, 5 Jul 2017 11:08:48 +0900 Subject: [PATCH 13/27] Nat464Xlat: interface notification handler on ConnectivityService This patch adds a layer of asynchonicity to the NetworkBaseObserver callbacks implemented by Nat464Xlat in order to allow these callbacks to run on the main ConnectivityService handler. This allows to run interfaceLinkStateChanged and interfaceRemoved callbacks in the same thread context as other Nat464Xlat methods and solves the following issues: - NPE risk due to race between fixupLinkProperties called on the ConnectivityService thread and interfaceRemoved called as a callback by NetworkManagementService. - stale LinkProperties reads in both callbacks not called on ConnectivityService handler. - removes the race between stop() and interfaceRemoved(). This patch also: - removes/simplifies comments related to the threading model which are no obsolete. - extract clatd management logic from ConnectivityService into NetworkAgentInfo Bug: 62997041 Bug: 64571917 Test: runtest frameworks-net manually connected to ipv6 network and went to test-ipv6.com Merged-In: I889d98e47423ff3d4746d6ed8015b265286e7c52 Merged-In: I2f002cd197e2eeaaadadd747a6b33d264cd34433 Merged-In: Id3ab064cf9f4417c0e8988fff4167b65b3c8c414 Merged-In: Ib224392c9a185f6bd79fd60cd5cb5549f2a7851e Merged-In: I9116a493ca1cbdf6a25664a1b0017aa6c9b38eb4 Merged-In: I12918d208364eef55067ae9d59fbc38477e1f1c6 (cherry picked from commit 771d5c2f0126ba692897c9716f4098ae6e3a870c) Change-Id: I34c4a0c32ce7c9b7bd7acf8f87b932e15c573bd8 --- .../android/server/ConnectivityService.java | 56 ++++-------- .../server/connectivity/Nat464Xlat.java | 90 +++++++++---------- .../server/connectivity/NetworkAgentInfo.java | 44 +++++++-- 3 files changed, 96 insertions(+), 94 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index adf536bbf4..17292b4490 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2011,16 +2011,7 @@ public class ConnectivityService extends IConnectivityManager.Stub break; } case NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED: { - if (VDBG) { - log("Update of LinkProperties for " + nai.name() + - "; created=" + nai.created + - "; everConnected=" + nai.everConnected); - } - LinkProperties oldLp = nai.linkProperties; - synchronized (nai) { - nai.linkProperties = (LinkProperties)msg.obj; - } - if (nai.everConnected) updateLinkProperties(nai, oldLp); + handleUpdateLinkProperties(nai, (LinkProperties) msg.obj); break; } case NetworkAgent.EVENT_NETWORK_INFO_CHANGED: { @@ -2269,7 +2260,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } nai.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_DISCONNECTED); mNetworkAgentInfos.remove(msg.replyTo); - maybeStopClat(nai); + nai.maybeStopClat(); synchronized (mNetworkForNetId) { // Remove the NetworkAgent, but don't mark the netId as // available until we've told netd to delete it below. @@ -4383,7 +4374,7 @@ public class ConnectivityService extends IConnectivityManager.Stub updateDnses(newLp, oldLp, netId); // Start or stop clat accordingly to network state. - updateClat(networkAgent); + networkAgent.updateClat(mNetd); if (isDefaultNetwork(networkAgent)) { handleApplyDefaultProxy(newLp.getHttpProxy()); } else { @@ -4398,32 +4389,6 @@ public class ConnectivityService extends IConnectivityManager.Stub mKeepaliveTracker.handleCheckKeepalivesStillValid(networkAgent); } - private void updateClat(NetworkAgentInfo nai) { - if (Nat464Xlat.requiresClat(nai)) { - maybeStartClat(nai); - } else { - maybeStopClat(nai); - } - } - - /** Ensure clat has started for this network. */ - private void maybeStartClat(NetworkAgentInfo nai) { - if (nai.clatd != null && nai.clatd.isStarted()) { - return; - } - nai.clatd = new Nat464Xlat(mNetd, mTrackerHandler, nai); - nai.clatd.start(); - } - - /** Ensure clat has stopped for this network. */ - private void maybeStopClat(NetworkAgentInfo nai) { - if (nai.clatd == null) { - return; - } - nai.clatd.stop(); - nai.clatd = null; - } - private void wakeupModifyInterface(String iface, NetworkCapabilities caps, boolean add) { // Marks are only available on WiFi interaces. Checking for // marks on unsupported interfaces is harmless. @@ -4658,6 +4623,21 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + public void handleUpdateLinkProperties(NetworkAgentInfo nai, LinkProperties newLp) { + if (VDBG) { + log("Update of LinkProperties for " + nai.name() + + "; created=" + nai.created + + "; everConnected=" + nai.everConnected); + } + LinkProperties oldLp = nai.linkProperties; + synchronized (nai) { + nai.linkProperties = newLp; + } + if (nai.everConnected) { + updateLinkProperties(nai, oldLp); + } + } + private void sendUpdatedScoreToFactories(NetworkAgentInfo nai) { for (int i = 0; i < nai.numNetworkRequests(); i++) { NetworkRequest nr = nai.requestAt(i); diff --git a/services/core/java/com/android/server/connectivity/Nat464Xlat.java b/services/core/java/com/android/server/connectivity/Nat464Xlat.java index f8d23d4c8d..10c8b8b1e0 100644 --- a/services/core/java/com/android/server/connectivity/Nat464Xlat.java +++ b/services/core/java/com/android/server/connectivity/Nat464Xlat.java @@ -20,22 +20,21 @@ import android.net.InterfaceConfiguration; import android.net.ConnectivityManager; import android.net.LinkAddress; import android.net.LinkProperties; -import android.net.NetworkAgent; import android.net.RouteInfo; -import android.os.Handler; -import android.os.Message; import android.os.INetworkManagementService; import android.os.RemoteException; import android.util.Slog; -import com.android.server.net.BaseNetworkObserver; import com.android.internal.util.ArrayUtils; +import com.android.server.net.BaseNetworkObserver; import java.net.Inet4Address; import java.util.Objects; /** - * Class to manage a 464xlat CLAT daemon. + * Class to manage a 464xlat CLAT daemon. Nat464Xlat is not thread safe and should be manipulated + * from a consistent and unique thread context. It is the responsability of ConnectivityService to + * call into this class from its own Handler thread. * * @hide */ @@ -55,9 +54,6 @@ public class Nat464Xlat extends BaseNetworkObserver { private final INetworkManagementService mNMService; - // ConnectivityService Handler for LinkProperties updates. - private final Handler mHandler; - // The network we're running on, and its type. private final NetworkAgentInfo mNetwork; @@ -67,16 +63,12 @@ public class Nat464Xlat extends BaseNetworkObserver { RUNNING; // start() called, and the stacked iface is known to be up. } - // Once mIface is non-null and isStarted() is true, methods called by ConnectivityService on - // its handler thread must not modify any internal state variables; they are only updated by the - // interface observers, called on the notification threads. private String mBaseIface; private String mIface; - private volatile State mState = State.IDLE; + private State mState = State.IDLE; - public Nat464Xlat(INetworkManagementService nmService, Handler handler, NetworkAgentInfo nai) { + public Nat464Xlat(INetworkManagementService nmService, NetworkAgentInfo nai) { mNMService = nmService; - mHandler = handler; mNetwork = nai; } @@ -104,6 +96,13 @@ public class Nat464Xlat extends BaseNetworkObserver { return mState != State.IDLE; } + /** + * @return true if clatd has been started but the stacked interface is not yet up. + */ + public boolean isStarting() { + return mState == State.STARTING; + } + /** * @return true if clatd has been started and the stacked interface is up. */ @@ -121,7 +120,7 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Clears internal state. Must not be called by ConnectivityService. + * Clears internal state. */ private void enterIdleState() { mIface = null; @@ -130,7 +129,7 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Starts the clat daemon. Called by ConnectivityService on the handler thread. + * Starts the clat daemon. */ public void start() { if (isStarted()) { @@ -167,7 +166,7 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Stops the clat daemon. Called by ConnectivityService on the handler thread. + * Stops the clat daemon. */ public void stop() { if (!isStarted()) { @@ -181,15 +180,8 @@ public class Nat464Xlat extends BaseNetworkObserver { } catch(RemoteException|IllegalStateException e) { Slog.e(TAG, "Error stopping clatd on " + mBaseIface, e); } - // When clatd stops and its interface is deleted, interfaceRemoved() will notify - // ConnectivityService and call enterIdleState(). - } - - private void updateConnectivityService(LinkProperties lp) { - Message msg = mHandler.obtainMessage(NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED, lp); - msg.replyTo = mNetwork.messenger; - Slog.i(TAG, "sending message to ConnectivityService: " + msg); - msg.sendToTarget(); + // When clatd stops and its interface is deleted, handleInterfaceRemoved() will trigger + // ConnectivityService#handleUpdateLinkProperties and call enterIdleState(). } /** @@ -257,19 +249,15 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Adds stacked link on base link and transitions to Running state - * This is called by the InterfaceObserver on its own thread, so can race with stop(). + * Adds stacked link on base link and transitions to RUNNING state. */ - @Override - public void interfaceLinkStateChanged(String iface, boolean up) { - if (!isStarted() || !up || !Objects.equals(mIface, iface)) { - return; - } - if (isRunning()) { + private void handleInterfaceLinkStateChanged(String iface, boolean up) { + if (!isStarting() || !up || !Objects.equals(mIface, iface)) { return; } LinkAddress clatAddress = getLinkAddress(iface); if (clatAddress == null) { + Slog.e(TAG, "cladAddress was null for stacked iface " + iface); return; } mState = State.RUNNING; @@ -279,15 +267,14 @@ public class Nat464Xlat extends BaseNetworkObserver { maybeSetIpv6NdOffload(mBaseIface, false); LinkProperties lp = new LinkProperties(mNetwork.linkProperties); lp.addStackedLink(makeLinkProperties(clatAddress)); - updateConnectivityService(lp); + mNetwork.connService.handleUpdateLinkProperties(mNetwork, lp); } - @Override - public void interfaceRemoved(String iface) { - if (!isStarted() || !Objects.equals(mIface, iface)) { - return; - } - if (!isRunning()) { + /** + * Removes stacked link on base link and transitions to IDLE state. + */ + private void handleInterfaceRemoved(String iface) { + if (!isRunning() || !Objects.equals(mIface, iface)) { return; } @@ -295,21 +282,28 @@ public class Nat464Xlat extends BaseNetworkObserver { // The interface going away likely means clatd has crashed. Ask netd to stop it, // because otherwise when we try to start it again on the same base interface netd // will complain that it's already started. - // - // Note that this method can be called by the interface observer at the same time - // that ConnectivityService calls stop(). In this case, the second call to - // stopClatd() will just throw IllegalStateException, which we'll ignore. try { mNMService.unregisterObserver(this); + // TODO: add STOPPING state to avoid calling stopClatd twice. mNMService.stopClatd(mBaseIface); - } catch (RemoteException|IllegalStateException e) { - // Well, we tried. + } catch(RemoteException|IllegalStateException e) { + Slog.e(TAG, "Error stopping clatd on " + mBaseIface, e); } maybeSetIpv6NdOffload(mBaseIface, true); LinkProperties lp = new LinkProperties(mNetwork.linkProperties); lp.removeStackedLink(mIface); enterIdleState(); - updateConnectivityService(lp); + mNetwork.connService.handleUpdateLinkProperties(mNetwork, lp); + } + + @Override + public void interfaceLinkStateChanged(String iface, boolean up) { + mNetwork.handler.post(() -> { handleInterfaceLinkStateChanged(iface, up); }); + } + + @Override + public void interfaceRemoved(String iface) { + mNetwork.handler.post(() -> { handleInterfaceRemoved(iface); }); } @Override diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 872923a032..7c4ef0f0f3 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -27,7 +27,9 @@ import android.net.NetworkMisc; import android.net.NetworkRequest; import android.net.NetworkState; import android.os.Handler; +import android.os.INetworkManagementService; import android.os.Messenger; +import android.os.RemoteException; import android.os.SystemClock; import android.util.Log; import android.util.SparseArray; @@ -247,9 +249,9 @@ public class NetworkAgentInfo implements Comparable { private static final String TAG = ConnectivityService.class.getSimpleName(); private static final boolean VDBG = false; - private final ConnectivityService mConnService; + public final ConnectivityService connService; private final Context mContext; - private final Handler mHandler; + final Handler handler; public NetworkAgentInfo(Messenger messenger, AsyncChannel ac, Network net, NetworkInfo info, LinkProperties lp, NetworkCapabilities nc, int score, Context context, Handler handler, @@ -261,10 +263,10 @@ public class NetworkAgentInfo implements Comparable { linkProperties = lp; networkCapabilities = nc; currentScore = score; - mConnService = connService; + this.connService = connService; mContext = context; - mHandler = handler; - networkMonitor = mConnService.createNetworkMonitor(context, handler, this, defaultRequest); + this.handler = handler; + networkMonitor = connService.createNetworkMonitor(context, handler, this, defaultRequest); networkMisc = misc; } @@ -430,7 +432,7 @@ public class NetworkAgentInfo implements Comparable { private boolean ignoreWifiUnvalidationPenalty() { boolean isWifi = networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) && networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); - boolean avoidBadWifi = mConnService.avoidBadWifi() || avoidUnvalidated; + boolean avoidBadWifi = connService.avoidBadWifi() || avoidUnvalidated; return isWifi && !avoidBadWifi && everValidated; } @@ -514,8 +516,8 @@ public class NetworkAgentInfo implements Comparable { } if (newExpiry > 0) { - mLingerMessage = mConnService.makeWakeupMessage( - mContext, mHandler, + mLingerMessage = connService.makeWakeupMessage( + mContext, handler, "NETWORK_LINGER_COMPLETE." + network.netId, EVENT_NETWORK_LINGER_COMPLETE, this); mLingerMessage.schedule(newExpiry); @@ -551,6 +553,32 @@ public class NetworkAgentInfo implements Comparable { for (LingerTimer timer : mLingerTimers) { pw.println(timer); } } + public void updateClat(INetworkManagementService netd) { + if (Nat464Xlat.requiresClat(this)) { + maybeStartClat(netd); + } else { + maybeStopClat(); + } + } + + /** Ensure clat has started for this network. */ + public void maybeStartClat(INetworkManagementService netd) { + if (clatd != null && clatd.isStarted()) { + return; + } + clatd = new Nat464Xlat(netd, this); + clatd.start(); + } + + /** Ensure clat has stopped for this network. */ + public void maybeStopClat() { + if (clatd == null) { + return; + } + clatd.stop(); + clatd = null; + } + public String toString() { return "NetworkAgentInfo{ ni{" + networkInfo + "} " + "network{" + network + "} nethandle{" + network.getNetworkHandle() + "} " + From 3c8f96ef4a8532c80b16ee7c21281121d3f9c684 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 29 Aug 2017 15:32:13 -0600 Subject: [PATCH 14/27] Augment network stats based on SubscriptionPlan. When a carrier provides an "anchor" of data usage at a specific moment in time, augment the network statistics used by warning/limit thresholds and Settings UI. For example, if the OS measured 500MB of usage, but the carrier says only 400MB has been used, we "squish" down the OS measured usage to match that anchor. Callers using the hidden API will have their data augmented by default, and the public API offers a way to opt-into augmentation. Thorough testing to verify behavior. Test: bit FrameworksNetTests:android.net.,com.android.server.net. Test: cts-tradefed run commandAndExit cts-dev -m CtsUsageStatsTestCases -t android.app.usage.cts.NetworkUsageStatsTest Bug: 64534190 Change-Id: Id3d4d7625bbf04f57643e51dbf376e3fa0ea8eca --- .../android/net/NetworkStatsHistoryTest.java | 15 + .../net/NetworkStatsCollectionTest.java | 282 ++++++++++++++++-- .../server/net/NetworkStatsServiceTest.java | 60 ++-- 3 files changed, 302 insertions(+), 55 deletions(-) diff --git a/tests/net/java/android/net/NetworkStatsHistoryTest.java b/tests/net/java/android/net/NetworkStatsHistoryTest.java index e7b91b568d..1c0c14eac0 100644 --- a/tests/net/java/android/net/NetworkStatsHistoryTest.java +++ b/tests/net/java/android/net/NetworkStatsHistoryTest.java @@ -485,6 +485,21 @@ public class NetworkStatsHistoryTest extends AndroidTestCase { assertTrue(stats.intersects(Long.MIN_VALUE, TEST_START + 1)); } + public void testSetValues() throws Exception { + stats = new NetworkStatsHistory(HOUR_IN_MILLIS); + stats.recordData(TEST_START, TEST_START + 1, + new NetworkStats.Entry(1024L, 10L, 2048L, 20L, 2L)); + + assertEquals(1024L + 2048L, stats.getTotalBytes()); + + final NetworkStatsHistory.Entry entry = stats.getValues(0, null); + entry.rxBytes /= 2; + entry.txBytes *= 2; + stats.setValues(0, entry); + + assertEquals(512L + 4096L, stats.getTotalBytes()); + } + private static void assertIndexBeforeAfter( NetworkStatsHistory stats, int before, int after, long time) { assertEquals("unexpected before", before, stats.getIndexBefore(time)); diff --git a/tests/net/java/com/android/server/net/NetworkStatsCollectionTest.java b/tests/net/java/com/android/server/net/NetworkStatsCollectionTest.java index 2a32b73d56..0c2ad38a11 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsCollectionTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsCollectionTest.java @@ -17,26 +17,35 @@ package com.android.server.net; import static android.net.ConnectivityManager.TYPE_MOBILE; +import static android.net.NetworkStats.SET_ALL; import static android.net.NetworkStats.SET_DEFAULT; import static android.net.NetworkStats.TAG_NONE; import static android.net.NetworkStats.UID_ALL; +import static android.net.NetworkStatsHistory.FIELD_ALL; import static android.net.NetworkTemplate.buildTemplateMobileAll; +import static android.os.Process.myUid; import static android.text.format.DateUtils.HOUR_IN_MILLIS; import static android.text.format.DateUtils.MINUTE_IN_MILLIS; import android.content.res.Resources; import android.net.NetworkIdentity; import android.net.NetworkStats; +import android.net.NetworkStatsHistory; import android.net.NetworkTemplate; import android.os.Process; import android.os.UserHandle; +import android.telephony.SubscriptionPlan; import android.telephony.TelephonyManager; -import android.support.test.filters.SmallTest; import android.test.AndroidTestCase; import android.test.MoreAsserts; +import android.test.suitebuilder.annotation.SmallTest; +import android.text.format.DateUtils; import com.android.frameworks.tests.net.R; +import libcore.io.IoUtils; +import libcore.io.Streams; + import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.DataOutputStream; @@ -44,9 +53,9 @@ import java.io.File; import java.io.FileOutputStream; import java.io.InputStream; import java.io.OutputStream; - -import libcore.io.IoUtils; -import libcore.io.Streams; +import java.time.ZonedDateTime; +import java.util.ArrayList; +import java.util.List; /** * Tests for {@link NetworkStatsCollection}. @@ -57,6 +66,10 @@ public class NetworkStatsCollectionTest extends AndroidTestCase { private static final String TEST_FILE = "test.bin"; private static final String TEST_IMSI = "310260000000000"; + private static final long TIME_A = 1326088800000L; // UTC: Monday 9th January 2012 06:00:00 AM + private static final long TIME_B = 1326110400000L; // UTC: Monday 9th January 2012 12:00:00 PM + private static final long TIME_C = 1326132000000L; // UTC: Monday 9th January 2012 06:00:00 PM + @Override public void setUp() throws Exception { super.setUp(); @@ -198,11 +211,11 @@ public class NetworkStatsCollectionTest extends AndroidTestCase { collection.getRelevantUids(NetworkStatsAccess.Level.DEVICE)); // Verify security check in getHistory. - assertNotNull(collection.getHistory(buildTemplateMobileAll(TEST_IMSI), myUid, SET_DEFAULT, - TAG_NONE, 0, NetworkStatsAccess.Level.DEFAULT)); + assertNotNull(collection.getHistory(buildTemplateMobileAll(TEST_IMSI), null, myUid, SET_DEFAULT, + TAG_NONE, 0, 0L, 0L, NetworkStatsAccess.Level.DEFAULT, myUid)); try { - collection.getHistory(buildTemplateMobileAll(TEST_IMSI), otherUidInSameUser, - SET_DEFAULT, TAG_NONE, 0, NetworkStatsAccess.Level.DEFAULT); + collection.getHistory(buildTemplateMobileAll(TEST_IMSI), null, otherUidInSameUser, + SET_DEFAULT, TAG_NONE, 0, 0L, 0L, NetworkStatsAccess.Level.DEFAULT, myUid); fail("Should have thrown SecurityException for accessing different UID"); } catch (SecurityException e) { // expected @@ -217,6 +230,213 @@ public class NetworkStatsCollectionTest extends AndroidTestCase { 0, NetworkStatsAccess.Level.DEVICE); } + public void testAugmentPlan() throws Exception { + final File testFile = new File(getContext().getFilesDir(), TEST_FILE); + stageFile(R.raw.netstats_v1, testFile); + + final NetworkStatsCollection emptyCollection = new NetworkStatsCollection(30 * MINUTE_IN_MILLIS); + final NetworkStatsCollection collection = new NetworkStatsCollection(30 * MINUTE_IN_MILLIS); + collection.readLegacyNetwork(testFile); + + // Test a bunch of plans that should result in no augmentation + final List plans = new ArrayList<>(); + + // No plan + plans.add(null); + // No usage anchor + plans.add(SubscriptionPlan.Builder + .createRecurringMonthly(ZonedDateTime.parse("2011-01-14T00:00:00.00Z")).build()); + // Usage anchor far in past + plans.add(SubscriptionPlan.Builder + .createRecurringMonthly(ZonedDateTime.parse("2011-01-14T00:00:00.00Z")) + .setDataUsage(1000L, TIME_A - DateUtils.YEAR_IN_MILLIS).build()); + // Usage anchor far in future + plans.add(SubscriptionPlan.Builder + .createRecurringMonthly(ZonedDateTime.parse("2011-01-14T00:00:00.00Z")) + .setDataUsage(1000L, TIME_A + DateUtils.YEAR_IN_MILLIS).build()); + // Usage anchor near but outside cycle + plans.add(SubscriptionPlan.Builder + .createNonrecurring(ZonedDateTime.parse("2012-01-09T09:00:00.00Z"), + ZonedDateTime.parse("2012-01-09T15:00:00.00Z")) + .setDataUsage(1000L, TIME_C).build()); + + for (SubscriptionPlan plan : plans) { + int i; + NetworkStatsHistory history; + + // Empty collection should be untouched + history = getHistory(emptyCollection, plan, TIME_A, TIME_C); + assertEquals(0L, history.getTotalBytes()); + + // Normal collection should be untouched + history = getHistory(collection, plan, TIME_A, TIME_C); i = 0; + assertEntry(100647, 197, 23649, 185, history.getValues(i++, null)); + assertEntry(100647, 196, 23648, 185, history.getValues(i++, null)); + assertEntry(18323, 76, 15032, 76, history.getValues(i++, null)); + assertEntry(18322, 75, 15031, 75, history.getValues(i++, null)); + assertEntry(527798, 761, 78570, 652, history.getValues(i++, null)); + assertEntry(527797, 760, 78570, 651, history.getValues(i++, null)); + assertEntry(10747, 50, 16838, 55, history.getValues(i++, null)); + assertEntry(10747, 49, 16838, 54, history.getValues(i++, null)); + assertEntry(89191, 151, 18021, 140, history.getValues(i++, null)); + assertEntry(89190, 150, 18020, 139, history.getValues(i++, null)); + assertEntry(3821, 22, 4525, 26, history.getValues(i++, null)); + assertEntry(3820, 22, 4524, 26, history.getValues(i++, null)); + assertEntry(91686, 159, 18575, 146, history.getValues(i++, null)); + assertEntry(91685, 159, 18575, 146, history.getValues(i++, null)); + assertEntry(8289, 35, 6863, 38, history.getValues(i++, null)); + assertEntry(8289, 35, 6863, 38, history.getValues(i++, null)); + assertEntry(113914, 174, 18364, 157, history.getValues(i++, null)); + assertEntry(113913, 173, 18364, 157, history.getValues(i++, null)); + assertEntry(11378, 49, 9261, 49, history.getValues(i++, null)); + assertEntry(11377, 48, 9261, 49, history.getValues(i++, null)); + assertEntry(201765, 328, 41808, 291, history.getValues(i++, null)); + assertEntry(201765, 328, 41807, 290, history.getValues(i++, null)); + assertEntry(106106, 218, 39917, 201, history.getValues(i++, null)); + assertEntry(106105, 217, 39917, 201, history.getValues(i++, null)); + assertEquals(history.size(), i); + + // Slice from middle should be untouched + history = getHistory(collection, plan, TIME_B - HOUR_IN_MILLIS, + TIME_B + HOUR_IN_MILLIS); i = 0; + assertEntry(3821, 22, 4525, 26, history.getValues(i++, null)); + assertEntry(3820, 22, 4524, 26, history.getValues(i++, null)); + assertEntry(91686, 159, 18575, 146, history.getValues(i++, null)); + assertEntry(91685, 159, 18575, 146, history.getValues(i++, null)); + assertEquals(history.size(), i); + } + + // Lower anchor in the middle of plan + { + int i; + NetworkStatsHistory history; + + final SubscriptionPlan plan = SubscriptionPlan.Builder + .createNonrecurring(ZonedDateTime.parse("2012-01-09T09:00:00.00Z"), + ZonedDateTime.parse("2012-01-09T15:00:00.00Z")) + .setDataUsage(200000L, TIME_B).build(); + + // Empty collection should be augmented + history = getHistory(emptyCollection, plan, TIME_A, TIME_C); + assertEquals(200000L, history.getTotalBytes()); + + // Normal collection should be augmented + history = getHistory(collection, plan, TIME_A, TIME_C); i = 0; + assertEntry(100647, 197, 23649, 185, history.getValues(i++, null)); + assertEntry(100647, 196, 23648, 185, history.getValues(i++, null)); + assertEntry(18323, 76, 15032, 76, history.getValues(i++, null)); + assertEntry(18322, 75, 15031, 75, history.getValues(i++, null)); + assertEntry(527798, 761, 78570, 652, history.getValues(i++, null)); + assertEntry(527797, 760, 78570, 651, history.getValues(i++, null)); + // Cycle point; start data normalization + assertEntry(7507, 0, 11763, 0, history.getValues(i++, null)); + assertEntry(7507, 0, 11763, 0, history.getValues(i++, null)); + assertEntry(62309, 0, 12589, 0, history.getValues(i++, null)); + assertEntry(62309, 0, 12588, 0, history.getValues(i++, null)); + assertEntry(2669, 0, 3161, 0, history.getValues(i++, null)); + assertEntry(2668, 0, 3160, 0, history.getValues(i++, null)); + // Anchor point; end data normalization + assertEntry(91686, 159, 18575, 146, history.getValues(i++, null)); + assertEntry(91685, 159, 18575, 146, history.getValues(i++, null)); + assertEntry(8289, 35, 6863, 38, history.getValues(i++, null)); + assertEntry(8289, 35, 6863, 38, history.getValues(i++, null)); + assertEntry(113914, 174, 18364, 157, history.getValues(i++, null)); + assertEntry(113913, 173, 18364, 157, history.getValues(i++, null)); + // Cycle point + assertEntry(11378, 49, 9261, 49, history.getValues(i++, null)); + assertEntry(11377, 48, 9261, 49, history.getValues(i++, null)); + assertEntry(201765, 328, 41808, 291, history.getValues(i++, null)); + assertEntry(201765, 328, 41807, 290, history.getValues(i++, null)); + assertEntry(106106, 218, 39917, 201, history.getValues(i++, null)); + assertEntry(106105, 217, 39917, 201, history.getValues(i++, null)); + assertEquals(history.size(), i); + + // Slice from middle should be augmented + history = getHistory(collection, plan, TIME_B - HOUR_IN_MILLIS, + TIME_B + HOUR_IN_MILLIS); i = 0; + assertEntry(2669, 0, 3161, 0, history.getValues(i++, null)); + assertEntry(2668, 0, 3160, 0, history.getValues(i++, null)); + assertEntry(91686, 159, 18575, 146, history.getValues(i++, null)); + assertEntry(91685, 159, 18575, 146, history.getValues(i++, null)); + assertEquals(history.size(), i); + } + + // Higher anchor in the middle of plan + { + int i; + NetworkStatsHistory history; + + final SubscriptionPlan plan = SubscriptionPlan.Builder + .createNonrecurring(ZonedDateTime.parse("2012-01-09T09:00:00.00Z"), + ZonedDateTime.parse("2012-01-09T15:00:00.00Z")) + .setDataUsage(400000L, TIME_B + MINUTE_IN_MILLIS).build(); + + // Empty collection should be augmented + history = getHistory(emptyCollection, plan, TIME_A, TIME_C); + assertEquals(400000L, history.getTotalBytes()); + + // Normal collection should be augmented + history = getHistory(collection, plan, TIME_A, TIME_C); i = 0; + assertEntry(100647, 197, 23649, 185, history.getValues(i++, null)); + assertEntry(100647, 196, 23648, 185, history.getValues(i++, null)); + assertEntry(18323, 76, 15032, 76, history.getValues(i++, null)); + assertEntry(18322, 75, 15031, 75, history.getValues(i++, null)); + assertEntry(527798, 761, 78570, 652, history.getValues(i++, null)); + assertEntry(527797, 760, 78570, 651, history.getValues(i++, null)); + // Cycle point; start data normalization + assertEntry(15015, 0, 23526, 0, history.getValues(i++, null)); + assertEntry(15015, 0, 23526, 0, history.getValues(i++, null)); + assertEntry(124619, 0, 25179, 0, history.getValues(i++, null)); + assertEntry(124618, 0, 25177, 0, history.getValues(i++, null)); + assertEntry(5338, 0, 6322, 0, history.getValues(i++, null)); + assertEntry(5337, 0, 6320, 0, history.getValues(i++, null)); + // Anchor point; end data normalization + assertEntry(91686, 159, 18575, 146, history.getValues(i++, null)); + assertEntry(91685, 159, 18575, 146, history.getValues(i++, null)); + assertEntry(8289, 35, 6863, 38, history.getValues(i++, null)); + assertEntry(8289, 35, 6863, 38, history.getValues(i++, null)); + assertEntry(113914, 174, 18364, 157, history.getValues(i++, null)); + assertEntry(113913, 173, 18364, 157, history.getValues(i++, null)); + // Cycle point + assertEntry(11378, 49, 9261, 49, history.getValues(i++, null)); + assertEntry(11377, 48, 9261, 49, history.getValues(i++, null)); + assertEntry(201765, 328, 41808, 291, history.getValues(i++, null)); + assertEntry(201765, 328, 41807, 290, history.getValues(i++, null)); + assertEntry(106106, 218, 39917, 201, history.getValues(i++, null)); + assertEntry(106105, 217, 39917, 201, history.getValues(i++, null)); + + // Slice from middle should be augmented + history = getHistory(collection, plan, TIME_B - HOUR_IN_MILLIS, + TIME_B + HOUR_IN_MILLIS); i = 0; + assertEntry(5338, 0, 6322, 0, history.getValues(i++, null)); + assertEntry(5337, 0, 6320, 0, history.getValues(i++, null)); + assertEntry(91686, 159, 18575, 146, history.getValues(i++, null)); + assertEntry(91685, 159, 18575, 146, history.getValues(i++, null)); + assertEquals(history.size(), i); + } + } + + public void testRounding() throws Exception { + final NetworkStatsCollection coll = new NetworkStatsCollection(HOUR_IN_MILLIS); + + // Special values should remain unchanged + for (long time : new long[] { + Long.MIN_VALUE, Long.MAX_VALUE, SubscriptionPlan.TIME_UNKNOWN + }) { + assertEquals(time, coll.roundUp(time)); + assertEquals(time, coll.roundDown(time)); + } + + assertEquals(TIME_A, coll.roundUp(TIME_A)); + assertEquals(TIME_A, coll.roundDown(TIME_A)); + + assertEquals(TIME_A + HOUR_IN_MILLIS, coll.roundUp(TIME_A + 1)); + assertEquals(TIME_A, coll.roundDown(TIME_A + 1)); + + assertEquals(TIME_A, coll.roundUp(TIME_A - 1)); + assertEquals(TIME_A - HOUR_IN_MILLIS, coll.roundDown(TIME_A - 1)); + } + /** * Copy a {@link Resources#openRawResource(int)} into {@link File} for * testing purposes. @@ -235,28 +455,50 @@ public class NetworkStatsCollectionTest extends AndroidTestCase { } } + private static NetworkStatsHistory getHistory(NetworkStatsCollection collection, + SubscriptionPlan augmentPlan, long start, long end) { + return collection.getHistory(buildTemplateMobileAll(TEST_IMSI), augmentPlan, UID_ALL, + SET_ALL, TAG_NONE, FIELD_ALL, start, end, NetworkStatsAccess.Level.DEVICE, myUid()); + } + private static void assertSummaryTotal(NetworkStatsCollection collection, NetworkTemplate template, long rxBytes, long rxPackets, long txBytes, long txPackets, @NetworkStatsAccess.Level int accessLevel) { - final NetworkStats.Entry entry = collection.getSummary( - template, Long.MIN_VALUE, Long.MAX_VALUE, accessLevel) + final NetworkStats.Entry actual = collection.getSummary( + template, Long.MIN_VALUE, Long.MAX_VALUE, accessLevel, myUid()) .getTotal(null); - assertEntry(entry, rxBytes, rxPackets, txBytes, txPackets); + assertEntry(rxBytes, rxPackets, txBytes, txPackets, actual); } private static void assertSummaryTotalIncludingTags(NetworkStatsCollection collection, NetworkTemplate template, long rxBytes, long rxPackets, long txBytes, long txPackets) { - final NetworkStats.Entry entry = collection.getSummary( - template, Long.MIN_VALUE, Long.MAX_VALUE, NetworkStatsAccess.Level.DEVICE) + final NetworkStats.Entry actual = collection.getSummary( + template, Long.MIN_VALUE, Long.MAX_VALUE, NetworkStatsAccess.Level.DEVICE, myUid()) .getTotalIncludingTags(null); - assertEntry(entry, rxBytes, rxPackets, txBytes, txPackets); + assertEntry(rxBytes, rxPackets, txBytes, txPackets, actual); } - private static void assertEntry( - NetworkStats.Entry entry, long rxBytes, long rxPackets, long txBytes, long txPackets) { - assertEquals("unexpected rxBytes", rxBytes, entry.rxBytes); - assertEquals("unexpected rxPackets", rxPackets, entry.rxPackets); - assertEquals("unexpected txBytes", txBytes, entry.txBytes); - assertEquals("unexpected txPackets", txPackets, entry.txPackets); + private static void assertEntry(long rxBytes, long rxPackets, long txBytes, long txPackets, + NetworkStats.Entry actual) { + assertEntry(new NetworkStats.Entry(rxBytes, rxPackets, txBytes, txPackets, 0L), actual); + } + + private static void assertEntry(long rxBytes, long rxPackets, long txBytes, long txPackets, + NetworkStatsHistory.Entry actual) { + assertEntry(new NetworkStats.Entry(rxBytes, rxPackets, txBytes, txPackets, 0L), actual); + } + + private static void assertEntry(NetworkStats.Entry expected, + NetworkStatsHistory.Entry actual) { + assertEntry(expected, new NetworkStats.Entry(actual.rxBytes, actual.rxPackets, + actual.txBytes, actual.txPackets, 0L)); + } + + private static void assertEntry(NetworkStats.Entry expected, + NetworkStats.Entry actual) { + assertEquals("unexpected rxBytes", expected.rxBytes, actual.rxBytes); + assertEquals("unexpected rxPackets", expected.rxPackets, actual.rxPackets); + assertEquals("unexpected txBytes", expected.txBytes, actual.txBytes); + assertEquals("unexpected txPackets", expected.txPackets, actual.txPackets); } } diff --git a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java index fa997958ba..814a626633 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsServiceTest.java @@ -46,19 +46,17 @@ import static android.text.format.DateUtils.HOUR_IN_MILLIS; import static android.text.format.DateUtils.MINUTE_IN_MILLIS; import static android.text.format.DateUtils.WEEK_IN_MILLIS; -import static com.android.server.net.NetworkStatsService.ACTION_NETWORK_STATS_POLL; import static com.android.internal.util.TestUtils.waitForIdleHandler; +import static com.android.server.net.NetworkStatsService.ACTION_NETWORK_STATS_POLL; import static org.junit.Assert.assertEquals; 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.Matchers.any; -import static org.mockito.Matchers.anyInt; -import static org.mockito.Matchers.anyLong; -import static org.mockito.Mockito.when; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import android.app.AlarmManager; import android.app.usage.NetworkStatsManager; @@ -79,24 +77,21 @@ import android.net.NetworkTemplate; import android.os.ConditionVariable; import android.os.Handler; import android.os.HandlerThread; -import android.os.INetworkManagementService; import android.os.IBinder; +import android.os.INetworkManagementService; import android.os.Looper; -import android.os.Messenger; -import android.os.MessageQueue; -import android.os.MessageQueue.IdleHandler; import android.os.Message; +import android.os.Messenger; import android.os.PowerManager; import android.support.test.InstrumentationRegistry; -import android.support.test.filters.SmallTest; import android.support.test.runner.AndroidJUnit4; import android.telephony.TelephonyManager; -import android.test.AndroidTestCase; +import android.test.suitebuilder.annotation.SmallTest; +import android.util.Log; import android.util.TrustedTime; import com.android.internal.net.VpnInfo; import com.android.internal.util.test.BroadcastInterceptingContext; -import com.android.server.net.NetworkStatsService; import com.android.server.net.NetworkStatsService.NetworkStatsSettings; import com.android.server.net.NetworkStatsService.NetworkStatsSettings.Config; @@ -112,9 +107,7 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.io.File; -import java.util.ArrayList; import java.util.Objects; -import java.util.List; /** * Tests for {@link NetworkStatsService}. @@ -983,7 +976,7 @@ public class NetworkStatsServiceTest { // verify summary API final NetworkStats stats = mSession.getSummaryForNetwork(template, start, end); - assertValues(stats, IFACE_ALL, UID_ALL, SET_DEFAULT, TAG_NONE, METERED_ALL, ROAMING_NO, + assertValues(stats, IFACE_ALL, UID_ALL, SET_ALL, TAG_NONE, METERED_ALL, ROAMING_ALL, rxBytes, rxPackets, txBytes, txPackets, operations); } @@ -1107,28 +1100,25 @@ public class NetworkStatsServiceTest { int tag, int metered, int roaming, long rxBytes, long rxPackets, long txBytes, long txPackets, int operations) { final NetworkStats.Entry entry = new NetworkStats.Entry(); - List sets = new ArrayList<>(); - if (set == SET_DEFAULT || set == SET_ALL) { - sets.add(SET_DEFAULT); - } - if (set == SET_FOREGROUND || set == SET_ALL) { - sets.add(SET_FOREGROUND); + final int[] sets; + if (set == SET_ALL) { + sets = new int[] { SET_ALL, SET_DEFAULT, SET_FOREGROUND }; + } else { + sets = new int[] { set }; } - List roamings = new ArrayList<>(); - if (roaming == ROAMING_NO || roaming == ROAMING_ALL) { - roamings.add(ROAMING_NO); - } - if (roaming == ROAMING_YES || roaming == ROAMING_ALL) { - roamings.add(ROAMING_YES); + final int[] roamings; + if (roaming == ROAMING_ALL) { + roamings = new int[] { ROAMING_ALL, ROAMING_YES, ROAMING_NO }; + } else { + roamings = new int[] { roaming }; } - List meterings = new ArrayList<>(); - if (metered == METERED_NO || metered == METERED_ALL) { - meterings.add(METERED_NO); - } - if (metered == METERED_YES || metered == METERED_ALL) { - meterings.add(METERED_YES); + final int[] meterings; + if (metered == METERED_ALL) { + meterings = new int[] { METERED_ALL, METERED_YES, METERED_NO }; + } else { + meterings = new int[] { metered }; } for (int s : sets) { From 2984496383de2ddce3108a97f4737dd816b510ab Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Thu, 31 Aug 2017 14:32:54 +0000 Subject: [PATCH 15/27] Revert "Nat464Xlat: interface notification handler on ConnectivityService" This reverts commit 39e10e211184a11fcb61e49d33b850597ff943d3. Bug: 65225023 Change-Id: Id6c21682cafa86d87f66480237dd731b21f917c5 --- .../android/server/ConnectivityService.java | 56 ++++++++---- .../server/connectivity/Nat464Xlat.java | 90 ++++++++++--------- .../server/connectivity/NetworkAgentInfo.java | 44 ++------- 3 files changed, 94 insertions(+), 96 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 17292b4490..adf536bbf4 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2011,7 +2011,16 @@ public class ConnectivityService extends IConnectivityManager.Stub break; } case NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED: { - handleUpdateLinkProperties(nai, (LinkProperties) msg.obj); + if (VDBG) { + log("Update of LinkProperties for " + nai.name() + + "; created=" + nai.created + + "; everConnected=" + nai.everConnected); + } + LinkProperties oldLp = nai.linkProperties; + synchronized (nai) { + nai.linkProperties = (LinkProperties)msg.obj; + } + if (nai.everConnected) updateLinkProperties(nai, oldLp); break; } case NetworkAgent.EVENT_NETWORK_INFO_CHANGED: { @@ -2260,7 +2269,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } nai.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_DISCONNECTED); mNetworkAgentInfos.remove(msg.replyTo); - nai.maybeStopClat(); + maybeStopClat(nai); synchronized (mNetworkForNetId) { // Remove the NetworkAgent, but don't mark the netId as // available until we've told netd to delete it below. @@ -4374,7 +4383,7 @@ public class ConnectivityService extends IConnectivityManager.Stub updateDnses(newLp, oldLp, netId); // Start or stop clat accordingly to network state. - networkAgent.updateClat(mNetd); + updateClat(networkAgent); if (isDefaultNetwork(networkAgent)) { handleApplyDefaultProxy(newLp.getHttpProxy()); } else { @@ -4389,6 +4398,32 @@ public class ConnectivityService extends IConnectivityManager.Stub mKeepaliveTracker.handleCheckKeepalivesStillValid(networkAgent); } + private void updateClat(NetworkAgentInfo nai) { + if (Nat464Xlat.requiresClat(nai)) { + maybeStartClat(nai); + } else { + maybeStopClat(nai); + } + } + + /** Ensure clat has started for this network. */ + private void maybeStartClat(NetworkAgentInfo nai) { + if (nai.clatd != null && nai.clatd.isStarted()) { + return; + } + nai.clatd = new Nat464Xlat(mNetd, mTrackerHandler, nai); + nai.clatd.start(); + } + + /** Ensure clat has stopped for this network. */ + private void maybeStopClat(NetworkAgentInfo nai) { + if (nai.clatd == null) { + return; + } + nai.clatd.stop(); + nai.clatd = null; + } + private void wakeupModifyInterface(String iface, NetworkCapabilities caps, boolean add) { // Marks are only available on WiFi interaces. Checking for // marks on unsupported interfaces is harmless. @@ -4623,21 +4658,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - public void handleUpdateLinkProperties(NetworkAgentInfo nai, LinkProperties newLp) { - if (VDBG) { - log("Update of LinkProperties for " + nai.name() + - "; created=" + nai.created + - "; everConnected=" + nai.everConnected); - } - LinkProperties oldLp = nai.linkProperties; - synchronized (nai) { - nai.linkProperties = newLp; - } - if (nai.everConnected) { - updateLinkProperties(nai, oldLp); - } - } - private void sendUpdatedScoreToFactories(NetworkAgentInfo nai) { for (int i = 0; i < nai.numNetworkRequests(); i++) { NetworkRequest nr = nai.requestAt(i); diff --git a/services/core/java/com/android/server/connectivity/Nat464Xlat.java b/services/core/java/com/android/server/connectivity/Nat464Xlat.java index 10c8b8b1e0..f8d23d4c8d 100644 --- a/services/core/java/com/android/server/connectivity/Nat464Xlat.java +++ b/services/core/java/com/android/server/connectivity/Nat464Xlat.java @@ -20,21 +20,22 @@ import android.net.InterfaceConfiguration; import android.net.ConnectivityManager; import android.net.LinkAddress; import android.net.LinkProperties; +import android.net.NetworkAgent; import android.net.RouteInfo; +import android.os.Handler; +import android.os.Message; import android.os.INetworkManagementService; import android.os.RemoteException; import android.util.Slog; -import com.android.internal.util.ArrayUtils; import com.android.server.net.BaseNetworkObserver; +import com.android.internal.util.ArrayUtils; import java.net.Inet4Address; import java.util.Objects; /** - * Class to manage a 464xlat CLAT daemon. Nat464Xlat is not thread safe and should be manipulated - * from a consistent and unique thread context. It is the responsability of ConnectivityService to - * call into this class from its own Handler thread. + * Class to manage a 464xlat CLAT daemon. * * @hide */ @@ -54,6 +55,9 @@ public class Nat464Xlat extends BaseNetworkObserver { private final INetworkManagementService mNMService; + // ConnectivityService Handler for LinkProperties updates. + private final Handler mHandler; + // The network we're running on, and its type. private final NetworkAgentInfo mNetwork; @@ -63,12 +67,16 @@ public class Nat464Xlat extends BaseNetworkObserver { RUNNING; // start() called, and the stacked iface is known to be up. } + // Once mIface is non-null and isStarted() is true, methods called by ConnectivityService on + // its handler thread must not modify any internal state variables; they are only updated by the + // interface observers, called on the notification threads. private String mBaseIface; private String mIface; - private State mState = State.IDLE; + private volatile State mState = State.IDLE; - public Nat464Xlat(INetworkManagementService nmService, NetworkAgentInfo nai) { + public Nat464Xlat(INetworkManagementService nmService, Handler handler, NetworkAgentInfo nai) { mNMService = nmService; + mHandler = handler; mNetwork = nai; } @@ -96,13 +104,6 @@ public class Nat464Xlat extends BaseNetworkObserver { return mState != State.IDLE; } - /** - * @return true if clatd has been started but the stacked interface is not yet up. - */ - public boolean isStarting() { - return mState == State.STARTING; - } - /** * @return true if clatd has been started and the stacked interface is up. */ @@ -120,7 +121,7 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Clears internal state. + * Clears internal state. Must not be called by ConnectivityService. */ private void enterIdleState() { mIface = null; @@ -129,7 +130,7 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Starts the clat daemon. + * Starts the clat daemon. Called by ConnectivityService on the handler thread. */ public void start() { if (isStarted()) { @@ -166,7 +167,7 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Stops the clat daemon. + * Stops the clat daemon. Called by ConnectivityService on the handler thread. */ public void stop() { if (!isStarted()) { @@ -180,8 +181,15 @@ public class Nat464Xlat extends BaseNetworkObserver { } catch(RemoteException|IllegalStateException e) { Slog.e(TAG, "Error stopping clatd on " + mBaseIface, e); } - // When clatd stops and its interface is deleted, handleInterfaceRemoved() will trigger - // ConnectivityService#handleUpdateLinkProperties and call enterIdleState(). + // When clatd stops and its interface is deleted, interfaceRemoved() will notify + // ConnectivityService and call enterIdleState(). + } + + private void updateConnectivityService(LinkProperties lp) { + Message msg = mHandler.obtainMessage(NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED, lp); + msg.replyTo = mNetwork.messenger; + Slog.i(TAG, "sending message to ConnectivityService: " + msg); + msg.sendToTarget(); } /** @@ -249,15 +257,19 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Adds stacked link on base link and transitions to RUNNING state. + * Adds stacked link on base link and transitions to Running state + * This is called by the InterfaceObserver on its own thread, so can race with stop(). */ - private void handleInterfaceLinkStateChanged(String iface, boolean up) { - if (!isStarting() || !up || !Objects.equals(mIface, iface)) { + @Override + public void interfaceLinkStateChanged(String iface, boolean up) { + if (!isStarted() || !up || !Objects.equals(mIface, iface)) { + return; + } + if (isRunning()) { return; } LinkAddress clatAddress = getLinkAddress(iface); if (clatAddress == null) { - Slog.e(TAG, "cladAddress was null for stacked iface " + iface); return; } mState = State.RUNNING; @@ -267,14 +279,15 @@ public class Nat464Xlat extends BaseNetworkObserver { maybeSetIpv6NdOffload(mBaseIface, false); LinkProperties lp = new LinkProperties(mNetwork.linkProperties); lp.addStackedLink(makeLinkProperties(clatAddress)); - mNetwork.connService.handleUpdateLinkProperties(mNetwork, lp); + updateConnectivityService(lp); } - /** - * Removes stacked link on base link and transitions to IDLE state. - */ - private void handleInterfaceRemoved(String iface) { - if (!isRunning() || !Objects.equals(mIface, iface)) { + @Override + public void interfaceRemoved(String iface) { + if (!isStarted() || !Objects.equals(mIface, iface)) { + return; + } + if (!isRunning()) { return; } @@ -282,28 +295,21 @@ public class Nat464Xlat extends BaseNetworkObserver { // The interface going away likely means clatd has crashed. Ask netd to stop it, // because otherwise when we try to start it again on the same base interface netd // will complain that it's already started. + // + // Note that this method can be called by the interface observer at the same time + // that ConnectivityService calls stop(). In this case, the second call to + // stopClatd() will just throw IllegalStateException, which we'll ignore. try { mNMService.unregisterObserver(this); - // TODO: add STOPPING state to avoid calling stopClatd twice. mNMService.stopClatd(mBaseIface); - } catch(RemoteException|IllegalStateException e) { - Slog.e(TAG, "Error stopping clatd on " + mBaseIface, e); + } catch (RemoteException|IllegalStateException e) { + // Well, we tried. } maybeSetIpv6NdOffload(mBaseIface, true); LinkProperties lp = new LinkProperties(mNetwork.linkProperties); lp.removeStackedLink(mIface); enterIdleState(); - mNetwork.connService.handleUpdateLinkProperties(mNetwork, lp); - } - - @Override - public void interfaceLinkStateChanged(String iface, boolean up) { - mNetwork.handler.post(() -> { handleInterfaceLinkStateChanged(iface, up); }); - } - - @Override - public void interfaceRemoved(String iface) { - mNetwork.handler.post(() -> { handleInterfaceRemoved(iface); }); + updateConnectivityService(lp); } @Override diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 7c4ef0f0f3..872923a032 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -27,9 +27,7 @@ import android.net.NetworkMisc; import android.net.NetworkRequest; import android.net.NetworkState; import android.os.Handler; -import android.os.INetworkManagementService; import android.os.Messenger; -import android.os.RemoteException; import android.os.SystemClock; import android.util.Log; import android.util.SparseArray; @@ -249,9 +247,9 @@ public class NetworkAgentInfo implements Comparable { private static final String TAG = ConnectivityService.class.getSimpleName(); private static final boolean VDBG = false; - public final ConnectivityService connService; + private final ConnectivityService mConnService; private final Context mContext; - final Handler handler; + private final Handler mHandler; public NetworkAgentInfo(Messenger messenger, AsyncChannel ac, Network net, NetworkInfo info, LinkProperties lp, NetworkCapabilities nc, int score, Context context, Handler handler, @@ -263,10 +261,10 @@ public class NetworkAgentInfo implements Comparable { linkProperties = lp; networkCapabilities = nc; currentScore = score; - this.connService = connService; + mConnService = connService; mContext = context; - this.handler = handler; - networkMonitor = connService.createNetworkMonitor(context, handler, this, defaultRequest); + mHandler = handler; + networkMonitor = mConnService.createNetworkMonitor(context, handler, this, defaultRequest); networkMisc = misc; } @@ -432,7 +430,7 @@ public class NetworkAgentInfo implements Comparable { private boolean ignoreWifiUnvalidationPenalty() { boolean isWifi = networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) && networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); - boolean avoidBadWifi = connService.avoidBadWifi() || avoidUnvalidated; + boolean avoidBadWifi = mConnService.avoidBadWifi() || avoidUnvalidated; return isWifi && !avoidBadWifi && everValidated; } @@ -516,8 +514,8 @@ public class NetworkAgentInfo implements Comparable { } if (newExpiry > 0) { - mLingerMessage = connService.makeWakeupMessage( - mContext, handler, + mLingerMessage = mConnService.makeWakeupMessage( + mContext, mHandler, "NETWORK_LINGER_COMPLETE." + network.netId, EVENT_NETWORK_LINGER_COMPLETE, this); mLingerMessage.schedule(newExpiry); @@ -553,32 +551,6 @@ public class NetworkAgentInfo implements Comparable { for (LingerTimer timer : mLingerTimers) { pw.println(timer); } } - public void updateClat(INetworkManagementService netd) { - if (Nat464Xlat.requiresClat(this)) { - maybeStartClat(netd); - } else { - maybeStopClat(); - } - } - - /** Ensure clat has started for this network. */ - public void maybeStartClat(INetworkManagementService netd) { - if (clatd != null && clatd.isStarted()) { - return; - } - clatd = new Nat464Xlat(netd, this); - clatd.start(); - } - - /** Ensure clat has stopped for this network. */ - public void maybeStopClat() { - if (clatd == null) { - return; - } - clatd.stop(); - clatd = null; - } - public String toString() { return "NetworkAgentInfo{ ni{" + networkInfo + "} " + "network{" + network + "} nethandle{" + network.getNetworkHandle() + "} " + From 92313e436a301828f34be928f0385063acee673f Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 1 Sep 2017 12:51:51 -0600 Subject: [PATCH 16/27] Gracefully handle integer overflows. Try sticking with integer-based math as much as possible for speed, but switch to double-based math if we detect that we'd end up causing an overflow. New tests to verify. Test: bit FrameworksNetTests:com.android.server.net.NetworkStatsCollectionTest Bug: 65257769 Change-Id: I1ae35599be134f81850c0a3d86928b057fba1eff --- .../net/NetworkStatsCollectionTest.java | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/net/java/com/android/server/net/NetworkStatsCollectionTest.java b/tests/net/java/com/android/server/net/NetworkStatsCollectionTest.java index 0c2ad38a11..9c10264656 100644 --- a/tests/net/java/com/android/server/net/NetworkStatsCollectionTest.java +++ b/tests/net/java/com/android/server/net/NetworkStatsCollectionTest.java @@ -27,7 +27,10 @@ import static android.os.Process.myUid; import static android.text.format.DateUtils.HOUR_IN_MILLIS; import static android.text.format.DateUtils.MINUTE_IN_MILLIS; +import static com.android.server.net.NetworkStatsCollection.multiplySafe; + import android.content.res.Resources; +import android.net.ConnectivityManager; import android.net.NetworkIdentity; import android.net.NetworkStats; import android.net.NetworkStatsHistory; @@ -40,6 +43,7 @@ import android.test.AndroidTestCase; import android.test.MoreAsserts; import android.test.suitebuilder.annotation.SmallTest; import android.text.format.DateUtils; +import android.util.RecurrenceRule; import com.android.frameworks.tests.net.R; @@ -53,6 +57,9 @@ import java.io.File; import java.io.FileOutputStream; import java.io.InputStream; import java.io.OutputStream; +import java.time.Clock; +import java.time.Instant; +import java.time.ZoneId; import java.time.ZonedDateTime; import java.util.ArrayList; import java.util.List; @@ -70,14 +77,27 @@ public class NetworkStatsCollectionTest extends AndroidTestCase { private static final long TIME_B = 1326110400000L; // UTC: Monday 9th January 2012 12:00:00 PM private static final long TIME_C = 1326132000000L; // UTC: Monday 9th January 2012 06:00:00 PM + private static Clock sOriginalClock; + @Override public void setUp() throws Exception { super.setUp(); + sOriginalClock = RecurrenceRule.sClock; // ignore any device overlay while testing NetworkTemplate.forceAllNetworkTypes(); } + @Override + protected void tearDown() throws Exception { + super.tearDown(); + RecurrenceRule.sClock = sOriginalClock; + } + + private void setClock(Instant instant) { + RecurrenceRule.sClock = Clock.fixed(instant, ZoneId.systemDefault()); + } + public void testReadLegacyNetwork() throws Exception { final File testFile = new File(getContext().getFilesDir(), TEST_FILE); stageFile(R.raw.netstats_v1, testFile); @@ -238,6 +258,9 @@ public class NetworkStatsCollectionTest extends AndroidTestCase { final NetworkStatsCollection collection = new NetworkStatsCollection(30 * MINUTE_IN_MILLIS); collection.readLegacyNetwork(testFile); + // We're in the future, but not that far off + setClock(Instant.parse("2012-06-01T00:00:00.00Z")); + // Test a bunch of plans that should result in no augmentation final List plans = new ArrayList<>(); @@ -416,6 +439,28 @@ public class NetworkStatsCollectionTest extends AndroidTestCase { } } + public void testAugmentPlanGigantic() throws Exception { + // We're in the future, but not that far off + setClock(Instant.parse("2012-06-01T00:00:00.00Z")); + + // Create a simple history with a ton of measured usage + final NetworkStatsCollection large = new NetworkStatsCollection(HOUR_IN_MILLIS); + final NetworkIdentitySet ident = new NetworkIdentitySet(); + ident.add(new NetworkIdentity(ConnectivityManager.TYPE_MOBILE, -1, TEST_IMSI, null, + false, true)); + large.recordData(ident, UID_ALL, SET_ALL, TAG_NONE, TIME_A, TIME_B, + new NetworkStats.Entry(12_730_893_164L, 1, 0, 0, 0)); + + // Verify untouched total + assertEquals(12_730_893_164L, getHistory(large, null, TIME_A, TIME_C).getTotalBytes()); + + // Verify anchor that might cause overflows + final SubscriptionPlan plan = SubscriptionPlan.Builder + .createRecurringMonthly(ZonedDateTime.parse("2012-01-09T00:00:00.00Z")) + .setDataUsage(4_939_212_390L, TIME_B).build(); + assertEquals(4_939_212_386L, getHistory(large, plan, TIME_A, TIME_C).getTotalBytes()); + } + public void testRounding() throws Exception { final NetworkStatsCollection coll = new NetworkStatsCollection(HOUR_IN_MILLIS); @@ -437,6 +482,25 @@ public class NetworkStatsCollectionTest extends AndroidTestCase { assertEquals(TIME_A - HOUR_IN_MILLIS, coll.roundDown(TIME_A - 1)); } + public void testMultiplySafe() { + assertEquals(25, multiplySafe(50, 1, 2)); + assertEquals(100, multiplySafe(50, 2, 1)); + + assertEquals(-10, multiplySafe(30, -1, 3)); + assertEquals(0, multiplySafe(30, 0, 3)); + assertEquals(10, multiplySafe(30, 1, 3)); + assertEquals(20, multiplySafe(30, 2, 3)); + assertEquals(30, multiplySafe(30, 3, 3)); + assertEquals(40, multiplySafe(30, 4, 3)); + + assertEquals(100_000_000_000L, + multiplySafe(300_000_000_000L, 10_000_000_000L, 30_000_000_000L)); + assertEquals(100_000_000_010L, + multiplySafe(300_000_000_000L, 10_000_000_001L, 30_000_000_000L)); + assertEquals(823_202_048L, + multiplySafe(4_939_212_288L, 2_121_815_528L, 12_730_893_165L)); + } + /** * Copy a {@link Resources#openRawResource(int)} into {@link File} for * testing purposes. From 88f49acd03179004d7dc401917b5c7174deef4f8 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Tue, 5 Sep 2017 13:25:07 +0900 Subject: [PATCH 17/27] ConnectivityService: improve wakelock logging This patch adds the following wakelock related counters to connectivity service dumps included in bug reports: - total number of wakelok acquisitions and releases - total cumulative wakelock duration - longest time the lock was held Bug: 65085354 Test: runtest frameworks-net, also manually dumped connectivity service and check new logging Change-Id: I8f67750c2eea73abf3d44f7f6df484427a8ea3f9 --- .../android/server/ConnectivityService.java | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index adf536bbf4..4546bfbfbc 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -456,6 +456,11 @@ public class ConnectivityService extends IConnectivityManager.Stub private static final int MAX_WAKELOCK_LOGS = 20; private final LocalLog mWakelockLogs = new LocalLog(MAX_WAKELOCK_LOGS); + private int mTotalWakelockAcquisitions = 0; + private int mTotalWakelockReleases = 0; + private long mTotalWakelockDurationMs = 0; + private long mMaxWakelockDurationMs = 0; + private long mLastWakeLockAcquireTimestamp = 0; // Array of tracking network validation and results private static final int MAX_VALIDATION_LOGS = 10; @@ -1947,6 +1952,14 @@ public class ConnectivityService extends IConnectivityManager.Stub pw.println(); pw.println("NetTransition WakeLock activity (most recent first):"); pw.increaseIndent(); + pw.println("total acquisitions: " + mTotalWakelockAcquisitions); + pw.println("total releases: " + mTotalWakelockReleases); + pw.println("cumulative duration: " + (mTotalWakelockDurationMs / 1000) + "s"); + pw.println("longest duration: " + (mMaxWakelockDurationMs / 1000) + "s"); + if (mTotalWakelockAcquisitions > mTotalWakelockReleases) { + long duration = SystemClock.elapsedRealtime() - mLastWakeLockAcquireTimestamp; + pw.println("currently holding WakeLock for: " + (duration / 1000) + "s"); + } mWakelockLogs.reverseDump(fd, pw, args); pw.decreaseIndent(); } @@ -3014,6 +3027,8 @@ public class ConnectivityService extends IConnectivityManager.Stub return; } mNetTransitionWakeLock.acquire(); + mLastWakeLockAcquireTimestamp = SystemClock.elapsedRealtime(); + mTotalWakelockAcquisitions++; } mWakelockLogs.log("ACQUIRE for " + forWhom); Message msg = mHandler.obtainMessage(EVENT_EXPIRE_NET_TRANSITION_WAKELOCK); @@ -3046,6 +3061,10 @@ public class ConnectivityService extends IConnectivityManager.Stub return; } mNetTransitionWakeLock.release(); + long lockDuration = SystemClock.elapsedRealtime() - mLastWakeLockAcquireTimestamp; + mTotalWakelockDurationMs += lockDuration; + mMaxWakelockDurationMs = Math.max(mMaxWakelockDurationMs, lockDuration); + mTotalWakelockReleases++; } mWakelockLogs.log(String.format("RELEASE (%s)", event)); } From 45445770ab7b58e3bbbd9f6ec94fd0b13f26e826 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Fri, 1 Sep 2017 01:23:32 +0000 Subject: [PATCH 18/27] Nat464Xlat: correct racefree teardown This patch relays the NetworkBaseObserver notifications about nat 464xlat stacked interfaces onto the ConnectivityService handler. This allows to process interface up and down notifications in the same thread context and eliminates several races: - NPE risk due to race between fixupLinkProperties called on ConnectivityService thread and interfaceRemoved called on NetworkManagementService thread. - stale LinkProperties pointer reads in both NetworkBaseObserver callbacks not called on ConnectivityService handler. - removes the race between stop() and interfaceRemoved(). - removes superfluous LinkProperties notifications when stop() is called before the stacked interface goes up. The teardown procedure logic common to stop() and interfaceRemoved() is put into enterStoppedState() and enterIdleState(). This allows to distinguish and correctly handle the following teardown scenarios: - an IPv4 appears -> ConnectivityService calls Nat464Xlat#stop() -> Nat464Xlat calls stopClatd -> clatd stops -> if the stacked interface was up, it is removed -> Nat464Xlat#interfaceRemoved() is triggered and a LinkProperties update is sent. - network disconnects -> ConnectivityService calls Nat464Xlat#stop() -> Nat464Xlat calls stopClatd -> clatd stops -> if the stacked interface was up, it is removed -> Nat464Xlat#interfaceRemoved() is triggered and a LinkProperties update is sent. - clatd crashes or exit -> Nat464Xlat#interfaceRemoved() is triggered -> Nat464Xlat unregisters itself as a network observer -> ConnectivityService is updated about the stacked interface missing, and restarts Nat464Xlat if needed. Note that the first two scenarios have two cases: stop() can be called before the notification for the stacked interface going up (STARTED), or after (RUNNING). In the first case, Nat464Xlat must unregister immediately as a network observer to avoid leaks. This patch also: - removes/simplifies comments related to the threading model which are no obsolete. - extract clatd management logic from ConnectivityService into NetworkAgentInfo - add new unit tests where there was none before. Bug: 62918393 Bug: 62997041 Bug: 64571917 Bug: 65225023 Test: runtest frameworks-net Merged-In: I27221a8a60fd9760b567ed322cc79228df877e56 Merged-In: I8f07dfbe5ea8259ff9f5793503f534945e67ad74 Merged-In: I8612db5e5050690db8cf41dd04944b4c22da340c Merged-In: Icb2dc8229b5ea45e319233b588f2dbe39ea40d4c Merged-In: Ibafea69224e832a6316c17dbb9b2d62a233088ac (cherry picked from commit ef502887ec58886e9347afb841aa06cb0d13acea) Change-Id: I9d075048873b0e1c5ed45b5674ada3fb303c2bfb --- .../android/server/ConnectivityService.java | 61 ++--- .../server/connectivity/Nat464Xlat.java | 191 ++++++++------- .../server/connectivity/NetworkAgentInfo.java | 36 +++ .../server/connectivity/Nat464XlatTest.java | 226 ++++++++++++++++++ 4 files changed, 396 insertions(+), 118 deletions(-) create mode 100644 tests/net/java/com/android/server/connectivity/Nat464XlatTest.java diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index adf536bbf4..f8c6ba492a 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2011,16 +2011,7 @@ public class ConnectivityService extends IConnectivityManager.Stub break; } case NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED: { - if (VDBG) { - log("Update of LinkProperties for " + nai.name() + - "; created=" + nai.created + - "; everConnected=" + nai.everConnected); - } - LinkProperties oldLp = nai.linkProperties; - synchronized (nai) { - nai.linkProperties = (LinkProperties)msg.obj; - } - if (nai.everConnected) updateLinkProperties(nai, oldLp); + handleUpdateLinkProperties(nai, (LinkProperties) msg.obj); break; } case NetworkAgent.EVENT_NETWORK_INFO_CHANGED: { @@ -2269,7 +2260,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } nai.networkMonitor.sendMessage(NetworkMonitor.CMD_NETWORK_DISCONNECTED); mNetworkAgentInfos.remove(msg.replyTo); - maybeStopClat(nai); + nai.maybeStopClat(); synchronized (mNetworkForNetId) { // Remove the NetworkAgent, but don't mark the netId as // available until we've told netd to delete it below. @@ -4383,7 +4374,7 @@ public class ConnectivityService extends IConnectivityManager.Stub updateDnses(newLp, oldLp, netId); // Start or stop clat accordingly to network state. - updateClat(networkAgent); + networkAgent.updateClat(mNetd); if (isDefaultNetwork(networkAgent)) { handleApplyDefaultProxy(newLp.getHttpProxy()); } else { @@ -4398,32 +4389,6 @@ public class ConnectivityService extends IConnectivityManager.Stub mKeepaliveTracker.handleCheckKeepalivesStillValid(networkAgent); } - private void updateClat(NetworkAgentInfo nai) { - if (Nat464Xlat.requiresClat(nai)) { - maybeStartClat(nai); - } else { - maybeStopClat(nai); - } - } - - /** Ensure clat has started for this network. */ - private void maybeStartClat(NetworkAgentInfo nai) { - if (nai.clatd != null && nai.clatd.isStarted()) { - return; - } - nai.clatd = new Nat464Xlat(mNetd, mTrackerHandler, nai); - nai.clatd.start(); - } - - /** Ensure clat has stopped for this network. */ - private void maybeStopClat(NetworkAgentInfo nai) { - if (nai.clatd == null) { - return; - } - nai.clatd.stop(); - nai.clatd = null; - } - private void wakeupModifyInterface(String iface, NetworkCapabilities caps, boolean add) { // Marks are only available on WiFi interaces. Checking for // marks on unsupported interfaces is harmless. @@ -4658,6 +4623,26 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + public void handleUpdateLinkProperties(NetworkAgentInfo nai, LinkProperties newLp) { + if (mNetworkForNetId.get(nai.network.netId) != nai) { + // Ignore updates for disconnected networks + return; + } + + if (VDBG) { + log("Update of LinkProperties for " + nai.name() + + "; created=" + nai.created + + "; everConnected=" + nai.everConnected); + } + LinkProperties oldLp = nai.linkProperties; + synchronized (nai) { + nai.linkProperties = newLp; + } + if (nai.everConnected) { + updateLinkProperties(nai, oldLp); + } + } + private void sendUpdatedScoreToFactories(NetworkAgentInfo nai) { for (int i = 0; i < nai.numNetworkRequests(); i++) { NetworkRequest nr = nai.requestAt(i); diff --git a/services/core/java/com/android/server/connectivity/Nat464Xlat.java b/services/core/java/com/android/server/connectivity/Nat464Xlat.java index f8d23d4c8d..e6585ad194 100644 --- a/services/core/java/com/android/server/connectivity/Nat464Xlat.java +++ b/services/core/java/com/android/server/connectivity/Nat464Xlat.java @@ -20,22 +20,21 @@ import android.net.InterfaceConfiguration; import android.net.ConnectivityManager; import android.net.LinkAddress; import android.net.LinkProperties; -import android.net.NetworkAgent; import android.net.RouteInfo; -import android.os.Handler; -import android.os.Message; import android.os.INetworkManagementService; import android.os.RemoteException; import android.util.Slog; -import com.android.server.net.BaseNetworkObserver; import com.android.internal.util.ArrayUtils; +import com.android.server.net.BaseNetworkObserver; import java.net.Inet4Address; import java.util.Objects; /** - * Class to manage a 464xlat CLAT daemon. + * Class to manage a 464xlat CLAT daemon. Nat464Xlat is not thread safe and should be manipulated + * from a consistent and unique thread context. It is the responsibility of ConnectivityService to + * call into this class from its own Handler thread. * * @hide */ @@ -55,28 +54,23 @@ public class Nat464Xlat extends BaseNetworkObserver { private final INetworkManagementService mNMService; - // ConnectivityService Handler for LinkProperties updates. - private final Handler mHandler; - // The network we're running on, and its type. private final NetworkAgentInfo mNetwork; private enum State { IDLE, // start() not called. Base iface and stacked iface names are null. STARTING, // start() called. Base iface and stacked iface names are known. - RUNNING; // start() called, and the stacked iface is known to be up. + RUNNING, // start() called, and the stacked iface is known to be up. + STOPPING; // stop() called, this Nat464Xlat is still registered as a network observer for + // the stacked interface. } - // Once mIface is non-null and isStarted() is true, methods called by ConnectivityService on - // its handler thread must not modify any internal state variables; they are only updated by the - // interface observers, called on the notification threads. private String mBaseIface; private String mIface; - private volatile State mState = State.IDLE; + private State mState = State.IDLE; - public Nat464Xlat(INetworkManagementService nmService, Handler handler, NetworkAgentInfo nai) { + public Nat464Xlat(INetworkManagementService nmService, NetworkAgentInfo nai) { mNMService = nmService; - mHandler = handler; mNetwork = nai; } @@ -89,6 +83,8 @@ public class Nat464Xlat extends BaseNetworkObserver { // TODO: migrate to NetworkCapabilities.TRANSPORT_*. final int netType = nai.networkInfo.getType(); final boolean supported = ArrayUtils.contains(NETWORK_TYPES, nai.networkInfo.getType()); + // TODO: this should also consider if the network is in SUSPENDED state to avoid stopping + // clatd in SUSPENDED state. final boolean connected = nai.networkInfo.isConnected(); // We only run clat on networks that don't have a native IPv4 address. final boolean hasIPv4Address = @@ -104,6 +100,13 @@ public class Nat464Xlat extends BaseNetworkObserver { return mState != State.IDLE; } + /** + * @return true if clatd has been started but the stacked interface is not yet up. + */ + public boolean isStarting() { + return mState == State.STARTING; + } + /** * @return true if clatd has been started and the stacked interface is up. */ @@ -112,25 +115,77 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Sets internal state. + * @return true if clatd has been stopped. + */ + public boolean isStopping() { + return mState == State.STOPPING; + } + + /** + * Start clatd, register this Nat464Xlat as a network observer for the stacked interface, + * and set internal state. */ private void enterStartingState(String baseIface) { + try { + mNMService.registerObserver(this); + } catch(RemoteException e) { + Slog.e(TAG, + "startClat: Can't register interface observer for clat on " + mNetwork.name()); + return; + } + try { + mNMService.startClatd(baseIface); + } catch(RemoteException|IllegalStateException e) { + Slog.e(TAG, "Error starting clatd on " + baseIface, e); + } mIface = CLAT_PREFIX + baseIface; mBaseIface = baseIface; mState = State.STARTING; } /** - * Clears internal state. Must not be called by ConnectivityService. + * Enter running state just after getting confirmation that the stacked interface is up, and + * turn ND offload off if on WiFi. + */ + private void enterRunningState() { + maybeSetIpv6NdOffload(mBaseIface, false); + mState = State.RUNNING; + } + + /** + * Stop clatd, and turn ND offload on if it had been turned off. + */ + private void enterStoppingState() { + if (isRunning()) { + maybeSetIpv6NdOffload(mBaseIface, true); + } + + try { + mNMService.stopClatd(mBaseIface); + } catch(RemoteException|IllegalStateException e) { + Slog.e(TAG, "Error stopping clatd on " + mBaseIface, e); + } + + mState = State.STOPPING; + } + + /** + * Unregister as a base observer for the stacked interface, and clear internal state. */ private void enterIdleState() { + try { + mNMService.unregisterObserver(this); + } catch(RemoteException|IllegalStateException e) { + Slog.e(TAG, "Error unregistering clatd observer on " + mBaseIface, e); + } + mIface = null; mBaseIface = null; mState = State.IDLE; } /** - * Starts the clat daemon. Called by ConnectivityService on the handler thread. + * Starts the clat daemon. */ public void start() { if (isStarted()) { @@ -143,53 +198,30 @@ public class Nat464Xlat extends BaseNetworkObserver { return; } - try { - mNMService.registerObserver(this); - } catch(RemoteException e) { - Slog.e(TAG, "startClat: Can't register interface observer for clat on " + mNetwork); - return; - } - String baseIface = mNetwork.linkProperties.getInterfaceName(); if (baseIface == null) { Slog.e(TAG, "startClat: Can't start clat on null interface"); return; } // TODO: should we only do this if mNMService.startClatd() succeeds? + Slog.i(TAG, "Starting clatd on " + baseIface); enterStartingState(baseIface); - - Slog.i(TAG, "Starting clatd on " + mBaseIface); - try { - mNMService.startClatd(mBaseIface); - } catch(RemoteException|IllegalStateException e) { - Slog.e(TAG, "Error starting clatd on " + mBaseIface, e); - } } /** - * Stops the clat daemon. Called by ConnectivityService on the handler thread. + * Stops the clat daemon. */ public void stop() { if (!isStarted()) { - Slog.e(TAG, "stopClat: already stopped or not started"); return; } - Slog.i(TAG, "Stopping clatd on " + mBaseIface); - try { - mNMService.stopClatd(mBaseIface); - } catch(RemoteException|IllegalStateException e) { - Slog.e(TAG, "Error stopping clatd on " + mBaseIface, e); - } - // When clatd stops and its interface is deleted, interfaceRemoved() will notify - // ConnectivityService and call enterIdleState(). - } - private void updateConnectivityService(LinkProperties lp) { - Message msg = mHandler.obtainMessage(NetworkAgent.EVENT_NETWORK_PROPERTIES_CHANGED, lp); - msg.replyTo = mNetwork.messenger; - Slog.i(TAG, "sending message to ConnectivityService: " + msg); - msg.sendToTarget(); + boolean wasStarting = isStarting(); + enterStoppingState(); + if (wasStarting) { + enterIdleState(); + } } /** @@ -257,59 +289,58 @@ public class Nat464Xlat extends BaseNetworkObserver { } /** - * Adds stacked link on base link and transitions to Running state - * This is called by the InterfaceObserver on its own thread, so can race with stop(). + * Adds stacked link on base link and transitions to RUNNING state. */ - @Override - public void interfaceLinkStateChanged(String iface, boolean up) { - if (!isStarted() || !up || !Objects.equals(mIface, iface)) { - return; - } - if (isRunning()) { + private void handleInterfaceLinkStateChanged(String iface, boolean up) { + if (!isStarting() || !up || !Objects.equals(mIface, iface)) { return; } + LinkAddress clatAddress = getLinkAddress(iface); if (clatAddress == null) { + Slog.e(TAG, "clatAddress was null for stacked iface " + iface); return; } - mState = State.RUNNING; + Slog.i(TAG, String.format("interface %s is up, adding stacked link %s on top of %s", mIface, mIface, mBaseIface)); - - maybeSetIpv6NdOffload(mBaseIface, false); + enterRunningState(); LinkProperties lp = new LinkProperties(mNetwork.linkProperties); lp.addStackedLink(makeLinkProperties(clatAddress)); - updateConnectivityService(lp); + mNetwork.connService().handleUpdateLinkProperties(mNetwork, lp); } - @Override - public void interfaceRemoved(String iface) { - if (!isStarted() || !Objects.equals(mIface, iface)) { + /** + * Removes stacked link on base link and transitions to IDLE state. + */ + private void handleInterfaceRemoved(String iface) { + if (!Objects.equals(mIface, iface)) { return; } - if (!isRunning()) { + if (!isRunning() && !isStopping()) { return; } Slog.i(TAG, "interface " + iface + " removed"); - // The interface going away likely means clatd has crashed. Ask netd to stop it, - // because otherwise when we try to start it again on the same base interface netd - // will complain that it's already started. - // - // Note that this method can be called by the interface observer at the same time - // that ConnectivityService calls stop(). In this case, the second call to - // stopClatd() will just throw IllegalStateException, which we'll ignore. - try { - mNMService.unregisterObserver(this); - mNMService.stopClatd(mBaseIface); - } catch (RemoteException|IllegalStateException e) { - // Well, we tried. + if (!isStopping()) { + // Ensure clatd is stopped if stop() has not been called: this likely means that clatd + // has crashed. + enterStoppingState(); } - maybeSetIpv6NdOffload(mBaseIface, true); - LinkProperties lp = new LinkProperties(mNetwork.linkProperties); - lp.removeStackedLink(mIface); enterIdleState(); - updateConnectivityService(lp); + LinkProperties lp = new LinkProperties(mNetwork.linkProperties); + lp.removeStackedLink(iface); + mNetwork.connService().handleUpdateLinkProperties(mNetwork, lp); + } + + @Override + public void interfaceLinkStateChanged(String iface, boolean up) { + mNetwork.handler().post(() -> { handleInterfaceLinkStateChanged(iface, up); }); + } + + @Override + public void interfaceRemoved(String iface) { + mNetwork.handler().post(() -> { handleInterfaceRemoved(iface); }); } @Override diff --git a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java index 872923a032..e96f4b0383 100644 --- a/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java +++ b/services/core/java/com/android/server/connectivity/NetworkAgentInfo.java @@ -27,7 +27,9 @@ import android.net.NetworkMisc; import android.net.NetworkRequest; import android.net.NetworkState; import android.os.Handler; +import android.os.INetworkManagementService; import android.os.Messenger; +import android.os.RemoteException; import android.os.SystemClock; import android.util.Log; import android.util.SparseArray; @@ -268,6 +270,14 @@ public class NetworkAgentInfo implements Comparable { networkMisc = misc; } + public ConnectivityService connService() { + return mConnService; + } + + public Handler handler() { + return mHandler; + } + // Functions for manipulating the requests satisfied by this network. // // These functions must only called on ConnectivityService's main thread. @@ -551,6 +561,32 @@ public class NetworkAgentInfo implements Comparable { for (LingerTimer timer : mLingerTimers) { pw.println(timer); } } + public void updateClat(INetworkManagementService netd) { + if (Nat464Xlat.requiresClat(this)) { + maybeStartClat(netd); + } else { + maybeStopClat(); + } + } + + /** Ensure clat has started for this network. */ + public void maybeStartClat(INetworkManagementService netd) { + if (clatd != null && clatd.isStarted()) { + return; + } + clatd = new Nat464Xlat(netd, this); + clatd.start(); + } + + /** Ensure clat has stopped for this network. */ + public void maybeStopClat() { + if (clatd == null) { + return; + } + clatd.stop(); + clatd = null; + } + public String toString() { return "NetworkAgentInfo{ ni{" + networkInfo + "} " + "network{" + network + "} nethandle{" + network.getNetworkHandle() + "} " + diff --git a/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java b/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java new file mode 100644 index 0000000000..e3f46a40e2 --- /dev/null +++ b/tests/net/java/com/android/server/connectivity/Nat464XlatTest.java @@ -0,0 +1,226 @@ +/* + * Copyright (C) 2017 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.server.connectivity; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +import android.net.ConnectivityManager; +import android.net.InterfaceConfiguration; +import android.net.LinkAddress; +import android.net.LinkProperties; +import android.net.NetworkInfo; +import android.os.Handler; +import android.os.INetworkManagementService; +import android.os.test.TestLooper; +import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; + +import com.android.server.ConnectivityService; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; + +@RunWith(AndroidJUnit4.class) +@SmallTest +public class Nat464XlatTest { + + static final String BASE_IFACE = "test0"; + static final String STACKED_IFACE = "v4-test0"; + static final LinkAddress ADDR = new LinkAddress("192.0.2.5/29"); + + @Mock ConnectivityService mConnectivity; + @Mock INetworkManagementService mNms; + @Mock InterfaceConfiguration mConfig; + @Mock NetworkAgentInfo mNai; + + TestLooper mLooper; + Handler mHandler; + + Nat464Xlat makeNat464Xlat() { + return new Nat464Xlat(mNms, mNai); + } + + @Before + public void setUp() throws Exception { + mLooper = new TestLooper(); + mHandler = new Handler(mLooper.getLooper()); + + MockitoAnnotations.initMocks(this); + + mNai.linkProperties = new LinkProperties(); + mNai.linkProperties.setInterfaceName(BASE_IFACE); + mNai.networkInfo = new NetworkInfo(null); + mNai.networkInfo.setType(ConnectivityManager.TYPE_WIFI); + when(mNai.connService()).thenReturn(mConnectivity); + when(mNai.handler()).thenReturn(mHandler); + + when(mNms.getInterfaceConfig(eq(STACKED_IFACE))).thenReturn(mConfig); + when(mConfig.getLinkAddress()).thenReturn(ADDR); + } + + @Test + public void testNormalStartAndStop() throws Exception { + Nat464Xlat nat = makeNat464Xlat(); + ArgumentCaptor c = ArgumentCaptor.forClass(LinkProperties.class); + + // ConnectivityService starts clat. + nat.start(); + + verify(mNms).registerObserver(eq(nat)); + verify(mNms).startClatd(eq(BASE_IFACE)); + + // Stacked interface up notification arrives. + nat.interfaceLinkStateChanged(STACKED_IFACE, true); + mLooper.dispatchNext(); + + verify(mNms).getInterfaceConfig(eq(STACKED_IFACE)); + verify(mNms).setInterfaceIpv6NdOffload(eq(BASE_IFACE), eq(false)); + verify(mConnectivity).handleUpdateLinkProperties(eq(mNai), c.capture()); + assertFalse(c.getValue().getStackedLinks().isEmpty()); + assertTrue(c.getValue().getAllInterfaceNames().contains(STACKED_IFACE)); + assertRunning(nat); + + // ConnectivityService stops clat (Network disconnects, IPv4 addr appears, ...). + nat.stop(); + + verify(mNms).stopClatd(eq(BASE_IFACE)); + verify(mNms).setInterfaceIpv6NdOffload(eq(BASE_IFACE), eq(true)); + + // Stacked interface removed notification arrives. + nat.interfaceRemoved(STACKED_IFACE); + mLooper.dispatchNext(); + + verify(mNms).unregisterObserver(eq(nat)); + verify(mConnectivity, times(2)).handleUpdateLinkProperties(eq(mNai), c.capture()); + assertTrue(c.getValue().getStackedLinks().isEmpty()); + assertFalse(c.getValue().getAllInterfaceNames().contains(STACKED_IFACE)); + assertIdle(nat); + + verifyNoMoreInteractions(mNms, mConnectivity); + } + + @Test + public void testClatdCrashWhileRunning() throws Exception { + Nat464Xlat nat = makeNat464Xlat(); + ArgumentCaptor c = ArgumentCaptor.forClass(LinkProperties.class); + + // ConnectivityService starts clat. + nat.start(); + + verify(mNms).registerObserver(eq(nat)); + verify(mNms).startClatd(eq(BASE_IFACE)); + + // Stacked interface up notification arrives. + nat.interfaceLinkStateChanged(STACKED_IFACE, true); + mLooper.dispatchNext(); + + verify(mNms).getInterfaceConfig(eq(STACKED_IFACE)); + verify(mNms).setInterfaceIpv6NdOffload(eq(BASE_IFACE), eq(false)); + verify(mConnectivity, times(1)).handleUpdateLinkProperties(eq(mNai), c.capture()); + assertFalse(c.getValue().getStackedLinks().isEmpty()); + assertTrue(c.getValue().getAllInterfaceNames().contains(STACKED_IFACE)); + assertRunning(nat); + + // Stacked interface removed notification arrives (clatd crashed, ...). + nat.interfaceRemoved(STACKED_IFACE); + mLooper.dispatchNext(); + + verify(mNms).unregisterObserver(eq(nat)); + verify(mNms).stopClatd(eq(BASE_IFACE)); + verify(mNms).setInterfaceIpv6NdOffload(eq(BASE_IFACE), eq(true)); + verify(mConnectivity, times(2)).handleUpdateLinkProperties(eq(mNai), c.capture()); + assertTrue(c.getValue().getStackedLinks().isEmpty()); + assertFalse(c.getValue().getAllInterfaceNames().contains(STACKED_IFACE)); + assertIdle(nat); + + // ConnectivityService stops clat: no-op. + nat.stop(); + + verifyNoMoreInteractions(mNms, mConnectivity); + } + + @Test + public void testStopBeforeClatdStarts() throws Exception { + Nat464Xlat nat = makeNat464Xlat(); + + // ConnectivityService starts clat. + nat.start(); + + verify(mNms).registerObserver(eq(nat)); + verify(mNms).startClatd(eq(BASE_IFACE)); + + // ConnectivityService immediately stops clat (Network disconnects, IPv4 addr appears, ...) + nat.stop(); + + verify(mNms).unregisterObserver(eq(nat)); + verify(mNms).stopClatd(eq(BASE_IFACE)); + assertIdle(nat); + + // In-flight interface up notification arrives: no-op + nat.interfaceLinkStateChanged(STACKED_IFACE, true); + mLooper.dispatchNext(); + + + // Interface removed notification arrives after stopClatd() takes effect: no-op. + nat.interfaceRemoved(STACKED_IFACE); + mLooper.dispatchNext(); + + assertIdle(nat); + + verifyNoMoreInteractions(mNms, mConnectivity); + } + + @Test + public void testStopAndClatdNeverStarts() throws Exception { + Nat464Xlat nat = makeNat464Xlat(); + + // ConnectivityService starts clat. + nat.start(); + + verify(mNms).registerObserver(eq(nat)); + verify(mNms).startClatd(eq(BASE_IFACE)); + + // ConnectivityService immediately stops clat (Network disconnects, IPv4 addr appears, ...) + nat.stop(); + + verify(mNms).unregisterObserver(eq(nat)); + verify(mNms).stopClatd(eq(BASE_IFACE)); + assertIdle(nat); + + verifyNoMoreInteractions(mNms, mConnectivity); + } + + static void assertIdle(Nat464Xlat nat) { + assertTrue("Nat464Xlat was not IDLE", !nat.isStarted()); + } + + static void assertRunning(Nat464Xlat nat) { + assertTrue("Nat464Xlat was not RUNNING", nat.isRunning()); + } +} From 70b92088717fef8e3b268eab03429163f4e25d77 Mon Sep 17 00:00:00 2001 From: Rubin Xu Date: Tue, 5 Sep 2017 18:40:49 +0100 Subject: [PATCH 19/27] Always add local subnet routes to the interface's routing table For some networks such as mobile data connections, its LinkProperties does not contain routes for the local subnet so no such route is added to the interface's routing table. This can be problematic especially if the device is in VPN lockdown mode where there exists high-priority PROHIBIT routing rule which in turn blocks the network's default gateway route from being added (next hop address hitting the prohibit rule). We fix this by patching LinkProperties to always include direct connected routes when they are received by ConnectivityService. This has the added advantage that when apps get LinkProperties, they see the directly connected routes as well. Bug: 63662962 Test: runtest frameworks-core -c android.net.LinkPropertiesTest Test: runtest frameworks-services -c com.android.server.ConnectivityServiceTest Test: Start with device with mobile data, set up ics-OpenVPN in always-on lockdown mode. Turn off mobile data then turn it back on, observe mobile data connectivity is restored and VPN successfully reconnects. (cherry picked from commit 1bb5c0818f0c4fb426e13b65a3ba3db7f36c3d88) Change-Id: Ia14f88bcf49d37286519c26dff6b7180303e2cbe --- core/java/android/net/LinkProperties.java | 18 ++++- .../src/android/net/LinkPropertiesTest.java | 79 ++++++++++++++++++- .../android/server/ConnectivityService.java | 8 +- .../server/ConnectivityServiceTest.java | 73 ++++++++++++++++- 4 files changed, 170 insertions(+), 8 deletions(-) diff --git a/core/java/android/net/LinkProperties.java b/core/java/android/net/LinkProperties.java index 1bb0fbb74a..f527f77ddd 100644 --- a/core/java/android/net/LinkProperties.java +++ b/core/java/android/net/LinkProperties.java @@ -18,14 +18,13 @@ package android.net; import android.annotation.NonNull; import android.annotation.Nullable; -import android.net.ProxyInfo; -import android.os.Parcelable; import android.os.Parcel; +import android.os.Parcelable; import android.text.TextUtils; -import java.net.InetAddress; import java.net.Inet4Address; import java.net.Inet6Address; +import java.net.InetAddress; import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Collection; @@ -503,12 +502,23 @@ public final class LinkProperties implements Parcelable { return Collections.unmodifiableList(mRoutes); } + /** + * Make sure this LinkProperties instance contains routes that cover the local subnet + * of its link addresses. Add any route that is missing. + * @hide + */ + public void ensureDirectlyConnectedRoutes() { + for (LinkAddress addr: mLinkAddresses) { + addRoute(new RouteInfo(addr, null, mIfaceName)); + } + } + /** * Returns all the routes on this link and all the links stacked above it. * @hide */ public List getAllRoutes() { - List routes = new ArrayList(); + List routes = new ArrayList<>(); routes.addAll(mRoutes); for (LinkProperties stacked: mStackedLinks.values()) { routes.addAll(stacked.getAllRoutes()); diff --git a/core/tests/coretests/src/android/net/LinkPropertiesTest.java b/core/tests/coretests/src/android/net/LinkPropertiesTest.java index d5f632190d..9686dd9e57 100644 --- a/core/tests/coretests/src/android/net/LinkPropertiesTest.java +++ b/core/tests/coretests/src/android/net/LinkPropertiesTest.java @@ -24,10 +24,15 @@ import android.net.RouteInfo; import android.system.OsConstants; import android.test.suitebuilder.annotation.SmallTest; import android.test.suitebuilder.annotation.Suppress; +import android.util.ArraySet; + import junit.framework.TestCase; import java.net.InetAddress; -import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Set; public class LinkPropertiesTest extends TestCase { @@ -678,4 +683,76 @@ public class LinkPropertiesTest extends TestCase { stacked.addRoute(new RouteInfo((IpPrefix) null, stackedAddress)); assertTrue(v6lp.isReachable(DNS1)); } + + @SmallTest + public void testLinkPropertiesEnsureDirectlyConnectedRoutes() { + // IPv4 case: no route added initially + LinkProperties rmnet0 = new LinkProperties(); + rmnet0.setInterfaceName("rmnet0"); + rmnet0.addLinkAddress(new LinkAddress("10.0.0.2/8")); + RouteInfo directRoute0 = new RouteInfo(new IpPrefix("10.0.0.0/8"), null, + rmnet0.getInterfaceName()); + + // Since no routes is added explicitly, getAllRoutes() should return empty. + assertTrue(rmnet0.getAllRoutes().isEmpty()); + rmnet0.ensureDirectlyConnectedRoutes(); + // ensureDirectlyConnectedRoutes() should have added the missing local route. + assertEqualRoutes(Collections.singletonList(directRoute0), rmnet0.getAllRoutes()); + + // IPv4 case: both direct and default routes added initially + LinkProperties rmnet1 = new LinkProperties(); + rmnet1.setInterfaceName("rmnet1"); + rmnet1.addLinkAddress(new LinkAddress("10.0.0.3/8")); + RouteInfo defaultRoute1 = new RouteInfo((IpPrefix) null, + NetworkUtils.numericToInetAddress("10.0.0.1"), rmnet1.getInterfaceName()); + RouteInfo directRoute1 = new RouteInfo(new IpPrefix("10.0.0.0/8"), null, + rmnet1.getInterfaceName()); + rmnet1.addRoute(defaultRoute1); + rmnet1.addRoute(directRoute1); + + // Check added routes + assertEqualRoutes(Arrays.asList(defaultRoute1, directRoute1), rmnet1.getAllRoutes()); + // ensureDirectlyConnectedRoutes() shouldn't change the routes since direct connected + // route is already part of the configuration. + rmnet1.ensureDirectlyConnectedRoutes(); + assertEqualRoutes(Arrays.asList(defaultRoute1, directRoute1), rmnet1.getAllRoutes()); + + // IPv6 case: only default routes added initially + LinkProperties rmnet2 = new LinkProperties(); + rmnet2.setInterfaceName("rmnet2"); + rmnet2.addLinkAddress(new LinkAddress("fe80::cafe/64")); + rmnet2.addLinkAddress(new LinkAddress("2001:db8::2/64")); + RouteInfo defaultRoute2 = new RouteInfo((IpPrefix) null, + NetworkUtils.numericToInetAddress("2001:db8::1"), rmnet2.getInterfaceName()); + RouteInfo directRoute2 = new RouteInfo(new IpPrefix("2001:db8::/64"), null, + rmnet2.getInterfaceName()); + RouteInfo linkLocalRoute2 = new RouteInfo(new IpPrefix("fe80::/64"), null, + rmnet2.getInterfaceName()); + rmnet2.addRoute(defaultRoute2); + + assertEqualRoutes(Arrays.asList(defaultRoute2), rmnet2.getAllRoutes()); + rmnet2.ensureDirectlyConnectedRoutes(); + assertEqualRoutes(Arrays.asList(defaultRoute2, directRoute2, linkLocalRoute2), + rmnet2.getAllRoutes()); + + // Corner case: no interface name + LinkProperties rmnet3 = new LinkProperties(); + rmnet3.addLinkAddress(new LinkAddress("192.168.0.2/24")); + RouteInfo directRoute3 = new RouteInfo(new IpPrefix("192.168.0.0/24"), null, + rmnet3.getInterfaceName()); + + assertTrue(rmnet3.getAllRoutes().isEmpty()); + rmnet3.ensureDirectlyConnectedRoutes(); + assertEqualRoutes(Collections.singletonList(directRoute3), rmnet3.getAllRoutes()); + + } + + private void assertEqualRoutes(Collection expected, Collection actual) { + Set expectedSet = new ArraySet<>(expected); + Set actualSet = new ArraySet<>(actual); + // Duplicated entries in actual routes are considered failures + assertEquals(actual.size(), actualSet.size()); + + assertEquals(expectedSet, actualSet); + } } diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 04b2112fae..f1ea85374d 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -4343,11 +4343,13 @@ public class ConnectivityService extends IConnectivityManager.Stub int currentScore, NetworkMisc networkMisc) { enforceConnectivityInternalPermission(); + LinkProperties lp = new LinkProperties(linkProperties); + lp.ensureDirectlyConnectedRoutes(); // TODO: Instead of passing mDefaultRequest, provide an API to determine whether a Network // satisfies mDefaultRequest. final NetworkAgentInfo nai = new NetworkAgentInfo(messenger, new AsyncChannel(), - new Network(reserveNetId()), new NetworkInfo(networkInfo), new LinkProperties( - linkProperties), new NetworkCapabilities(networkCapabilities), currentScore, + new Network(reserveNetId()), new NetworkInfo(networkInfo), lp, + new NetworkCapabilities(networkCapabilities), currentScore, mContext, mTrackerHandler, new NetworkMisc(networkMisc), mDefaultRequest, this); synchronized (this) { nai.networkMonitor.systemReady = mSystemReady; @@ -4657,6 +4659,8 @@ public class ConnectivityService extends IConnectivityManager.Stub synchronized (nai) { nai.linkProperties = newLp; } + // msg.obj is already a defensive copy. + nai.linkProperties.ensureDirectlyConnectedRoutes(); if (nai.everConnected) { updateLinkProperties(nai, oldLp); } diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index f6481cf591..8816d43ef8 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -64,6 +64,7 @@ import android.net.NetworkInfo.DetailedState; import android.net.NetworkMisc; import android.net.NetworkRequest; import android.net.NetworkSpecifier; +import android.net.NetworkUtils; import android.net.RouteInfo; import android.net.StringNetworkSpecifier; import android.net.metrics.IpConnectivityLog; @@ -88,6 +89,7 @@ import android.test.AndroidTestCase; import android.test.mock.MockContentResolver; import android.test.suitebuilder.annotation.SmallTest; import android.text.TextUtils; +import android.util.ArraySet; import android.util.Log; import android.util.LogPrinter; @@ -109,7 +111,10 @@ import org.mockito.Spy; import java.net.InetAddress; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; +import java.util.List; import java.util.Objects; +import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; @@ -304,6 +309,10 @@ public class ConnectivityServiceTest extends AndroidTestCase { private String mRedirectUrl; MockNetworkAgent(int transport) { + this(transport, new LinkProperties()); + } + + MockNetworkAgent(int transport, LinkProperties linkProperties) { final int type = transportToLegacyType(transport); final String typeName = ConnectivityManager.getNetworkTypeName(type); mNetworkInfo = new NetworkInfo(type, 0, typeName, "Mock"); @@ -329,7 +338,7 @@ public class ConnectivityServiceTest extends AndroidTestCase { mHandlerThread.start(); mNetworkAgent = new NetworkAgent(mHandlerThread.getLooper(), mServiceContext, "Mock-" + typeName, mNetworkInfo, mNetworkCapabilities, - new LinkProperties(), mScore, new NetworkMisc()) { + linkProperties, mScore, new NetworkMisc()) { @Override public void unwanted() { mDisconnected.open(); } @@ -3338,6 +3347,68 @@ public class ConnectivityServiceTest extends AndroidTestCase { assertException(() -> { mCm.requestRouteToHostAddress(TYPE_NONE, null); }, unsupported); } + @SmallTest + public void testLinkPropertiesEnsuresDirectlyConnectedRoutes() { + final NetworkRequest networkRequest = new NetworkRequest.Builder() + .addTransportType(TRANSPORT_WIFI).build(); + final TestNetworkCallback networkCallback = new TestNetworkCallback(); + mCm.registerNetworkCallback(networkRequest, networkCallback); + + LinkProperties lp = new LinkProperties(); + lp.setInterfaceName("wlan0"); + LinkAddress myIpv4Address = new LinkAddress("192.168.12.3/24"); + RouteInfo myIpv4DefaultRoute = new RouteInfo((IpPrefix) null, + NetworkUtils.numericToInetAddress("192.168.12.1"), lp.getInterfaceName()); + lp.addLinkAddress(myIpv4Address); + lp.addRoute(myIpv4DefaultRoute); + + // Verify direct routes are added when network agent is first registered in + // ConnectivityService. + MockNetworkAgent networkAgent = new MockNetworkAgent(TRANSPORT_WIFI, lp); + networkAgent.connect(true); + networkCallback.expectCallback(CallbackState.AVAILABLE, networkAgent); + networkCallback.expectCallback(CallbackState.NETWORK_CAPABILITIES, networkAgent); + CallbackInfo cbi = networkCallback.expectCallback(CallbackState.LINK_PROPERTIES, + networkAgent); + networkCallback.expectCapabilitiesWith(NET_CAPABILITY_VALIDATED, networkAgent); + networkCallback.assertNoCallback(); + checkDirectlyConnectedRoutes(cbi.arg, Arrays.asList(myIpv4Address), + Arrays.asList(myIpv4DefaultRoute)); + checkDirectlyConnectedRoutes(mCm.getLinkProperties(networkAgent.getNetwork()), + Arrays.asList(myIpv4Address), Arrays.asList(myIpv4DefaultRoute)); + + // Verify direct routes are added during subsequent link properties updates. + LinkProperties newLp = new LinkProperties(lp); + LinkAddress myIpv6Address1 = new LinkAddress("fe80::cafe/64"); + LinkAddress myIpv6Address2 = new LinkAddress("2001:db8::2/64"); + newLp.addLinkAddress(myIpv6Address1); + newLp.addLinkAddress(myIpv6Address2); + networkAgent.sendLinkProperties(newLp); + cbi = networkCallback.expectCallback(CallbackState.LINK_PROPERTIES, networkAgent); + networkCallback.assertNoCallback(); + checkDirectlyConnectedRoutes(cbi.arg, + Arrays.asList(myIpv4Address, myIpv6Address1, myIpv6Address2), + Arrays.asList(myIpv4DefaultRoute)); + mCm.unregisterNetworkCallback(networkCallback); + } + + private void checkDirectlyConnectedRoutes(Object callbackObj, + Collection linkAddresses, Collection otherRoutes) { + assertTrue(callbackObj instanceof LinkProperties); + LinkProperties lp = (LinkProperties) callbackObj; + + Set expectedRoutes = new ArraySet<>(); + expectedRoutes.addAll(otherRoutes); + for (LinkAddress address : linkAddresses) { + RouteInfo localRoute = new RouteInfo(address, null, lp.getInterfaceName()); + // Duplicates in linkAddresses are considered failures + assertTrue(expectedRoutes.add(localRoute)); + } + List observedRoutes = lp.getRoutes(); + assertEquals(expectedRoutes.size(), observedRoutes.size()); + assertTrue(observedRoutes.containsAll(expectedRoutes)); + } + private static void assertEmpty(T[] ts) { int length = ts.length; assertEquals("expected empty array, but length was " + length, 0, length); From 599db833bb984f7e2f246207a7de010fe2f5772e Mon Sep 17 00:00:00 2001 From: Rubin Xu Date: Mon, 11 Sep 2017 15:21:10 +0100 Subject: [PATCH 20/27] Patch incoming LinkProperties before it's visible to the outside Otherwise we risk a race condition when we are fixing the LinkProperties routes, other parts of ConnectivityService is reading the field at the same time. Test: runtest frameworks-net -c com.android.server.ConnectivityServiceTest Test: runtest frameworks-core -c android.net.LinkPropertiesTest Bug: 65529483 Bug: 35995111 (cherry picked from commit 6c1f6fd78927514df73ddace81edac4899c4cda4) Change-Id: I539578703570a901e0a5dff0155422ca78c52401 --- .../core/java/com/android/server/ConnectivityService.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index f1ea85374d..e70a294d46 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -4649,7 +4649,8 @@ public class ConnectivityService extends IConnectivityManager.Stub // Ignore updates for disconnected networks return; } - + // newLp is already a defensive copy. + newLp.ensureDirectlyConnectedRoutes(); if (VDBG) { log("Update of LinkProperties for " + nai.name() + "; created=" + nai.created + @@ -4659,8 +4660,6 @@ public class ConnectivityService extends IConnectivityManager.Stub synchronized (nai) { nai.linkProperties = newLp; } - // msg.obj is already a defensive copy. - nai.linkProperties.ensureDirectlyConnectedRoutes(); if (nai.everConnected) { updateLinkProperties(nai, oldLp); } From 8b3f87160b2016870e4acea8cbedcd555836c63b Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Mon, 4 Sep 2017 13:24:43 +0900 Subject: [PATCH 21/27] Connectivity metrics: collect NFLOG wakeup events This patch stores NFLOG packet wakeup events sent by Netd to the system server into a ring buffer inside NetdEventListenerService. The content of this buffer is accessible by $ dumpsys connmetrics or $ dumpsys connmetrics list, and is added to bug reports. The wakeup event buffer stores currently uid and timestamps. Bug: 34901696 Bug: 62179647 Test: runtest frameworks-net, new unit tests Merged-In: Ie8db6f8572b1a929a20398d8dc03e189bc488382 (cherry picked from commit f562ac34a51da55e4d15e34f0cd1cb597e7d926c) Change-Id: Ibe706872a80dfd06abd9779a2116ca7e4bc0fb77 --- .../NetdEventListenerServiceTest.java | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java b/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java index 46f395eae2..3a93b9432c 100644 --- a/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java +++ b/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java @@ -19,6 +19,7 @@ package com.android.server.connectivity; import static android.net.metrics.INetdEventListener.EVENT_GETADDRINFO; import static android.net.metrics.INetdEventListener.EVENT_GETHOSTBYNAME; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; @@ -74,6 +75,52 @@ public class NetdEventListenerServiceTest { mNetdEventListenerService = new NetdEventListenerService(mCm); } + @Test + public void testWakeupEventLogging() throws Exception { + final int BUFFER_LENGTH = NetdEventListenerService.WAKEUP_EVENT_BUFFER_LENGTH; + + // Assert no events + String[] events1 = listNetdEvent(); + assertEquals(new String[]{""}, events1); + + long now = System.currentTimeMillis(); + String prefix = "iface:wlan0"; + int[] uids = { 10001, 10002, 10004, 1000, 10052, 10023, 10002, 10123, 10004 }; + for (int uid : uids) { + mNetdEventListenerService.onWakeupEvent(prefix, uid, uid, now); + } + + String[] events2 = listNetdEvent(); + assertEquals(uids.length, events2.length); + for (int i = 0; i < uids.length; i++) { + String got = events2[i]; + assertContains(got, "wlan0"); + assertContains(got, "uid: " + uids[i]); + } + + int uid = 20000; + for (int i = 0; i < BUFFER_LENGTH * 2; i++) { + long ts = now + 10; + mNetdEventListenerService.onWakeupEvent(prefix, uid, uid, ts); + } + + // Assert there are BUFFER_LENGTH events all with uid 20000 + String[] events3 = listNetdEvent(); + assertEquals(BUFFER_LENGTH, events3.length); + for (String got : events3) { + assertContains(got, "wlan0"); + assertContains(got, "uid: " + uid); + } + + uid = 45678; + mNetdEventListenerService.onWakeupEvent(prefix, uid, uid, now); + + String[] events4 = listNetdEvent(); + String lastEvent = events4[events4.length - 1]; + assertContains(lastEvent, "wlan0"); + assertContains(lastEvent, "uid: " + uid); + } + @Test public void testDnsLogging() throws Exception { asyncDump(100); @@ -329,4 +376,15 @@ public class NetdEventListenerServiceTest { } return log.toString(); } + + String[] listNetdEvent() throws Exception { + StringWriter buffer = new StringWriter(); + PrintWriter writer = new PrintWriter(buffer); + mNetdEventListenerService.list(writer); + return buffer.toString().split("\\n"); + } + + static void assertContains(String got, String want) { + assertTrue(got + " did not contain \"" + want + "\"", got.contains(want)); + } } From 611f62c6b99b3dfe9e95793ddef3edd13913a32b Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Tue, 5 Sep 2017 13:34:48 +0900 Subject: [PATCH 22/27] Connectivity metrics: add WakeupStats events This patch defines a new WakeupStats event in ipconnectivity.proto and populates these events from the NFLOG wakeup events stored in NetdEventListenerService. There is one WakeupStats object per known interface on which ingress packets arrive and may wake the system up. Example from $ adb shell dumpsys connmetrics list: ... WakeupStats(wlan0, total: 58, root: 0, system: 3, apps: 38, non-apps: 0, unrouted: 17, 6111s) WakeupEvent(13:36:31.686, iface wlan0, uid -1) WakeupEvent(13:38:50.846, iface wlan0, uid -1) WakeupEvent(13:39:16.676, iface wlan0, uid 10065) WakeupEvent(13:40:32.144, iface wlan0, uid 1000) WakeupEvent(13:40:35.827, iface wlan0, uid 1000) WakeupEvent(13:40:47.913, iface wlan0, uid 10004) WakeupEvent(13:40:52.622, iface wlan0, uid 10014) WakeupEvent(13:41:06.036, iface wlan0, uid 10004) ... Bug: 34901696 Bug: 62179647 Test: runtest frameworks-net Merged-In: Ie2676b20bfb411a1902f4942643df0c20e268d99 (cherry pick from commit 60c9f63b66926745603978e1bd6372b3a44561d1) Change-Id: I3087f446fc998fc1ca895d975b80c4a1dd029bf3 --- .../IpConnectivityEventBuilderTest.java | 84 +++++++++++++++---- .../IpConnectivityMetricsTest.java | 46 ++++++++++ .../NetdEventListenerServiceTest.java | 84 +++++++++++++++++-- 3 files changed, 193 insertions(+), 21 deletions(-) diff --git a/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java b/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java index eff04ab5ad..f72a1c638e 100644 --- a/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java +++ b/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java @@ -16,6 +16,8 @@ package com.android.server.connectivity; +import static android.net.metrics.INetdEventListener.EVENT_GETADDRINFO; +import static android.net.metrics.INetdEventListener.EVENT_GETHOSTBYNAME; import static com.android.server.connectivity.MetricsTestUtil.aBool; import static com.android.server.connectivity.MetricsTestUtil.aByteArray; import static com.android.server.connectivity.MetricsTestUtil.aLong; @@ -31,29 +33,41 @@ import static com.android.server.connectivity.metrics.nano.IpConnectivityLogClas import static com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.ETHERNET; import static com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.MULTIPLE; import static com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.WIFI; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import android.net.ConnectivityMetricsEvent; import android.net.metrics.ApfProgramEvent; import android.net.metrics.ApfStats; +import android.net.metrics.ConnectStats; import android.net.metrics.DefaultNetworkEvent; import android.net.metrics.DhcpClientEvent; import android.net.metrics.DhcpErrorEvent; import android.net.metrics.DnsEvent; +import android.net.metrics.DnsEvent; import android.net.metrics.IpManagerEvent; import android.net.metrics.IpReachabilityEvent; import android.net.metrics.NetworkEvent; import android.net.metrics.RaEvent; import android.net.metrics.ValidationProbeEvent; +import android.net.metrics.WakeupStats; +import android.support.test.runner.AndroidJUnit4; import android.test.suitebuilder.annotation.SmallTest; + import com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.IpConnectivityEvent; + import java.util.Arrays; import java.util.List; -import junit.framework.TestCase; + +import org.junit.runner.RunWith; +import org.junit.Test; // TODO: instead of comparing textpb to textpb, parse textpb and compare proto to proto. -public class IpConnectivityEventBuilderTest extends TestCase { +@RunWith(AndroidJUnit4.class) +@SmallTest +public class IpConnectivityEventBuilderTest { - @SmallTest + @Test public void testLinkLayerInferrence() { ConnectivityMetricsEvent ev = describeIpEvent( aType(IpReachabilityEvent.class), @@ -182,7 +196,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { verifySerialization(want, ev); } - @SmallTest + @Test public void testDefaultNetworkEventSerialization() { ConnectivityMetricsEvent ev = describeIpEvent( aType(DefaultNetworkEvent.class), @@ -223,7 +237,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { verifySerialization(want, ev); } - @SmallTest + @Test public void testDhcpClientEventSerialization() { ConnectivityMetricsEvent ev = describeIpEvent( aType(DhcpClientEvent.class), @@ -249,7 +263,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { verifySerialization(want, ev); } - @SmallTest + @Test public void testDhcpErrorEventSerialization() { ConnectivityMetricsEvent ev = describeIpEvent( aType(DhcpErrorEvent.class), @@ -274,7 +288,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { verifySerialization(want, ev); } - @SmallTest + @Test public void testIpManagerEventSerialization() { ConnectivityMetricsEvent ev = describeIpEvent( aType(IpManagerEvent.class), @@ -300,7 +314,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { verifySerialization(want, ev); } - @SmallTest + @Test public void testIpReachabilityEventSerialization() { ConnectivityMetricsEvent ev = describeIpEvent( aType(IpReachabilityEvent.class), @@ -324,7 +338,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { verifySerialization(want, ev); } - @SmallTest + @Test public void testNetworkEventSerialization() { ConnectivityMetricsEvent ev = describeIpEvent( aType(NetworkEvent.class), @@ -353,7 +367,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { verifySerialization(want, ev); } - @SmallTest + @Test public void testValidationProbeEventSerialization() { ConnectivityMetricsEvent ev = describeIpEvent( aType(ValidationProbeEvent.class), @@ -380,7 +394,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { verifySerialization(want, ev); } - @SmallTest + @Test public void testApfProgramEventSerialization() { ConnectivityMetricsEvent ev = describeIpEvent( aType(ApfProgramEvent.class), @@ -414,7 +428,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { verifySerialization(want, ev); } - @SmallTest + @Test public void testApfStatsSerialization() { ConnectivityMetricsEvent ev = describeIpEvent( aType(ApfStats.class), @@ -457,7 +471,7 @@ public class IpConnectivityEventBuilderTest extends TestCase { verifySerialization(want, ev); } - @SmallTest + @Test public void testRaEventSerialization() { ConnectivityMetricsEvent ev = describeIpEvent( aType(RaEvent.class), @@ -490,11 +504,49 @@ public class IpConnectivityEventBuilderTest extends TestCase { verifySerialization(want, ev); } + @Test + public void testWakeupStatsSerialization() { + WakeupStats stats = new WakeupStats("wlan0"); + stats.totalWakeups = 14; + stats.applicationWakeups = 5; + stats.nonApplicationWakeups = 1; + stats.rootWakeups = 2; + stats.systemWakeups = 3; + stats.unroutedWakeups = 3; + + IpConnectivityEvent got = IpConnectivityEventBuilder.toProto(stats); + String want = String.join("\n", + "dropped_events: 0", + "events <", + " if_name: \"\"", + " link_layer: 4", + " network_id: 0", + " time_ms: 0", + " transports: 0", + " wakeup_stats <", + " application_wakeups: 5", + " duration_sec: 0", + " non_application_wakeups: 1", + " root_wakeups: 2", + " system_wakeups: 3", + " total_wakeups: 14", + " unrouted_wakeups: 3", + " >", + ">", + "version: 2\n"); + + verifySerialization(want, got); + } + static void verifySerialization(String want, ConnectivityMetricsEvent... input) { + List protoInput = + IpConnectivityEventBuilder.toProto(Arrays.asList(input)); + verifySerialization(want, protoInput.toArray(new IpConnectivityEvent[0])); + } + + static void verifySerialization(String want, IpConnectivityEvent... input) { try { - List proto = - IpConnectivityEventBuilder.toProto(Arrays.asList(input)); - byte[] got = IpConnectivityEventBuilder.serialize(0, proto); + byte[] got = IpConnectivityEventBuilder.serialize(0, Arrays.asList(input)); IpConnectivityLog log = IpConnectivityLog.parseFrom(got); assertEquals(want, log.toString()); } catch (Exception e) { diff --git a/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java b/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java index cc18b7f322..ede5988cdc 100644 --- a/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java +++ b/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java @@ -224,6 +224,15 @@ public class IpConnectivityMetricsTest { dnsEvent(101, EVENT_GETADDRINFO, 0, 56); dnsEvent(101, EVENT_GETHOSTBYNAME, 0, 34); + // iface, uid + wakeupEvent("wlan0", 1000); + wakeupEvent("rmnet0", 10123); + wakeupEvent("wlan0", 1000); + wakeupEvent("rmnet0", 10008); + wakeupEvent("wlan0", -1); + wakeupEvent("wlan0", 10008); + wakeupEvent("rmnet0", 1000); + String want = String.join("\n", "dropped_events: 0", "events <", @@ -405,6 +414,38 @@ public class IpConnectivityMetricsTest { " return_codes: 0", " >", ">", + "events <", + " if_name: \"\"", + " link_layer: 2", + " network_id: 0", + " time_ms: 0", + " transports: 0", + " wakeup_stats <", + " application_wakeups: 2", + " duration_sec: 0", + " non_application_wakeups: 0", + " root_wakeups: 0", + " system_wakeups: 1", + " total_wakeups: 3", + " unrouted_wakeups: 0", + " >", + ">", + "events <", + " if_name: \"\"", + " link_layer: 4", + " network_id: 0", + " time_ms: 0", + " transports: 0", + " wakeup_stats <", + " application_wakeups: 1", + " duration_sec: 0", + " non_application_wakeups: 0", + " root_wakeups: 0", + " system_wakeups: 2", + " total_wakeups: 4", + " unrouted_wakeups: 1", + " >", + ">", "version: 2\n"); verifySerialization(want, getdump("flush")); @@ -425,6 +466,11 @@ public class IpConnectivityMetricsTest { mNetdListener.onDnsEvent(netId, type, result, latency, "", null, 0, 0); } + void wakeupEvent(String iface, int uid) throws Exception { + String prefix = NetdEventListenerService.WAKEUP_EVENT_IFACE_PREFIX + iface; + mNetdListener.onWakeupEvent(prefix, uid, uid, 0); + } + List verifyEvents(int n, int timeoutMs) throws Exception { ArgumentCaptor captor = ArgumentCaptor.forClass(ConnectivityMetricsEvent.class); diff --git a/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java b/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java index 3a93b9432c..2b105e5c92 100644 --- a/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java +++ b/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java @@ -38,9 +38,11 @@ import android.support.test.runner.AndroidJUnit4; import android.system.OsConstants; import android.test.suitebuilder.annotation.SmallTest; import android.util.Base64; + import com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.DNSLookupBatch; import com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.IpConnectivityEvent; import com.android.server.connectivity.metrics.nano.IpConnectivityLogClass.IpConnectivityLog; + import java.io.FileOutputStream; import java.io.PrintWriter; import java.io.StringWriter; @@ -48,6 +50,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Comparator; import java.util.List; + import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -91,9 +94,13 @@ public class NetdEventListenerServiceTest { } String[] events2 = listNetdEvent(); - assertEquals(uids.length, events2.length); + int expectedLength2 = uids.length + 1; // +1 for the WakeupStats line + assertEquals(expectedLength2, events2.length); + assertContains(events2[0], "WakeupStats"); + assertContains(events2[0], "wlan0"); for (int i = 0; i < uids.length; i++) { - String got = events2[i]; + String got = events2[i+1]; + assertContains(got, "WakeupEvent"); assertContains(got, "wlan0"); assertContains(got, "uid: " + uids[i]); } @@ -104,10 +111,14 @@ public class NetdEventListenerServiceTest { mNetdEventListenerService.onWakeupEvent(prefix, uid, uid, ts); } - // Assert there are BUFFER_LENGTH events all with uid 20000 String[] events3 = listNetdEvent(); - assertEquals(BUFFER_LENGTH, events3.length); - for (String got : events3) { + int expectedLength3 = BUFFER_LENGTH + 1; // +1 for the WakeupStats line + assertEquals(expectedLength3, events3.length); + assertContains(events2[0], "WakeupStats"); + assertContains(events2[0], "wlan0"); + for (int i = 1; i < expectedLength3; i++) { + String got = events3[i]; + assertContains(got, "WakeupEvent"); assertContains(got, "wlan0"); assertContains(got, "uid: " + uid); } @@ -117,10 +128,68 @@ public class NetdEventListenerServiceTest { String[] events4 = listNetdEvent(); String lastEvent = events4[events4.length - 1]; + assertContains(lastEvent, "WakeupEvent"); assertContains(lastEvent, "wlan0"); assertContains(lastEvent, "uid: " + uid); } + @Test + public void testWakeupStatsLogging() throws Exception { + wakeupEvent("wlan0", 1000); + wakeupEvent("rmnet0", 10123); + wakeupEvent("wlan0", 1000); + wakeupEvent("rmnet0", 10008); + wakeupEvent("wlan0", -1); + wakeupEvent("wlan0", 10008); + wakeupEvent("rmnet0", 1000); + wakeupEvent("wlan0", 10004); + wakeupEvent("wlan0", 1000); + wakeupEvent("wlan0", 0); + wakeupEvent("wlan0", -1); + wakeupEvent("rmnet0", 10052); + wakeupEvent("wlan0", 0); + wakeupEvent("rmnet0", 1000); + wakeupEvent("wlan0", 1010); + + String got = flushStatistics(); + String want = String.join("\n", + "dropped_events: 0", + "events <", + " if_name: \"\"", + " link_layer: 2", + " network_id: 0", + " time_ms: 0", + " transports: 0", + " wakeup_stats <", + " application_wakeups: 3", + " duration_sec: 0", + " non_application_wakeups: 0", + " root_wakeups: 0", + " system_wakeups: 2", + " total_wakeups: 5", + " unrouted_wakeups: 0", + " >", + ">", + "events <", + " if_name: \"\"", + " link_layer: 4", + " network_id: 0", + " time_ms: 0", + " transports: 0", + " wakeup_stats <", + " application_wakeups: 2", + " duration_sec: 0", + " non_application_wakeups: 1", + " root_wakeups: 2", + " system_wakeups: 3", + " total_wakeups: 10", + " unrouted_wakeups: 2", + " >", + ">", + "version: 2\n"); + assertEquals(want, got); + } + @Test public void testDnsLogging() throws Exception { asyncDump(100); @@ -344,6 +413,11 @@ public class NetdEventListenerServiceTest { mNetdEventListenerService.onDnsEvent(netId, type, result, latency, "", null, 0, 0); } + void wakeupEvent(String iface, int uid) throws Exception { + String prefix = NetdEventListenerService.WAKEUP_EVENT_IFACE_PREFIX + iface; + mNetdEventListenerService.onWakeupEvent(prefix, uid, uid, 0); + } + void asyncDump(long durationMs) throws Exception { final long stop = System.currentTimeMillis() + durationMs; final PrintWriter pw = new PrintWriter(new FileOutputStream("/dev/null")); From 6c35b8082c6c33d71e2ecc525e82a1efffe84a10 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Tue, 19 Sep 2017 13:15:26 +0900 Subject: [PATCH 23/27] Wakeup packet events: addressing a few comments This patch addresses a few post-submit comment for commits f562ac34a51dc and 60c9f63b66921. Bug: 34901696 Bug: 62179647 Test: runtest frameworks-net Merged-In: I4abec57e0c6bc869dc57b5eb54582dd977b64c30 (cherry picked from commit 175b574e27daa0d8832b8cc9615a15fce998309a) Change-Id: Ied9d0cec98685e5a91ed2ca2c81ad88d7ae8d751 --- .../server/connectivity/IpConnectivityEventBuilderTest.java | 4 ++-- .../server/connectivity/IpConnectivityMetricsTest.java | 4 ++-- .../server/connectivity/NetdEventListenerServiceTest.java | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java b/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java index f72a1c638e..262417620c 100644 --- a/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java +++ b/tests/net/java/com/android/server/connectivity/IpConnectivityEventBuilderTest.java @@ -512,7 +512,7 @@ public class IpConnectivityEventBuilderTest { stats.nonApplicationWakeups = 1; stats.rootWakeups = 2; stats.systemWakeups = 3; - stats.unroutedWakeups = 3; + stats.noUidWakeups = 3; IpConnectivityEvent got = IpConnectivityEventBuilder.toProto(stats); String want = String.join("\n", @@ -526,11 +526,11 @@ public class IpConnectivityEventBuilderTest { " wakeup_stats <", " application_wakeups: 5", " duration_sec: 0", + " no_uid_wakeups: 3", " non_application_wakeups: 1", " root_wakeups: 2", " system_wakeups: 3", " total_wakeups: 14", - " unrouted_wakeups: 3", " >", ">", "version: 2\n"); diff --git a/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java b/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java index ede5988cdc..a395c480f5 100644 --- a/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java +++ b/tests/net/java/com/android/server/connectivity/IpConnectivityMetricsTest.java @@ -423,11 +423,11 @@ public class IpConnectivityMetricsTest { " wakeup_stats <", " application_wakeups: 2", " duration_sec: 0", + " no_uid_wakeups: 0", " non_application_wakeups: 0", " root_wakeups: 0", " system_wakeups: 1", " total_wakeups: 3", - " unrouted_wakeups: 0", " >", ">", "events <", @@ -439,11 +439,11 @@ public class IpConnectivityMetricsTest { " wakeup_stats <", " application_wakeups: 1", " duration_sec: 0", + " no_uid_wakeups: 1", " non_application_wakeups: 0", " root_wakeups: 0", " system_wakeups: 2", " total_wakeups: 4", - " unrouted_wakeups: 1", " >", ">", "version: 2\n"); diff --git a/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java b/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java index 2b105e5c92..6723601fc5 100644 --- a/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java +++ b/tests/net/java/com/android/server/connectivity/NetdEventListenerServiceTest.java @@ -163,11 +163,11 @@ public class NetdEventListenerServiceTest { " wakeup_stats <", " application_wakeups: 3", " duration_sec: 0", + " no_uid_wakeups: 0", " non_application_wakeups: 0", " root_wakeups: 0", " system_wakeups: 2", " total_wakeups: 5", - " unrouted_wakeups: 0", " >", ">", "events <", @@ -179,11 +179,11 @@ public class NetdEventListenerServiceTest { " wakeup_stats <", " application_wakeups: 2", " duration_sec: 0", + " no_uid_wakeups: 2", " non_application_wakeups: 1", " root_wakeups: 2", " system_wakeups: 3", " total_wakeups: 10", - " unrouted_wakeups: 2", " >", ">", "version: 2\n"); From f15fc752c5f397c6a46d63fea3b42bf58cd9eb5a Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Wed, 20 Sep 2017 11:20:14 +0900 Subject: [PATCH 24/27] Implement ConnectivityService TODO and fix many race conditions This patch implements an outstanding TODO in ConnectivityService to add synchronization over the map of network request ids to network agent info objects. This structure is accessed from multiple threads: - Binder thread on public aidl methods, most notably via getDefaultNetwork(). - Internal handler. This leads to many race conditions that can crash the system server and reboot the phone if getDefaultNetwork() is called on a Binder thread to service a public ConnectivityManager api while the default network state is being updated on the internal handler after losing the default network. Bug: 65911184 Test: runtest frameworks-net Merged-In: I86c830ebd559e31d4576a7606705a056afb064ac Merged-In: I2011e23c9f894c079ab66cd7cc5c14af572a956d Merged-In: Ic70901a6aa22a03e97f494e793920ab07a0fd612 Merged-In: I4a7658e1fa6946063ab86a251269413903841ee8 Merged-In: Ia59d45f4e95a536d7982f61ac9c9a1bfc5e8ebb8 (cherry picked from commit cd95278e55bd0e4935f42214d964f8c6aa52b4ea) Change-Id: I2e26bef9eddd342f51c02b991632c7ea04fe7e66 --- .../android/server/ConnectivityService.java | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index adf536bbf4..12cdc74b37 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2201,7 +2201,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // A network factory has connected. Send it all current NetworkRequests. for (NetworkRequestInfo nri : mNetworkRequests.values()) { if (nri.request.isListen()) continue; - NetworkAgentInfo nai = mNetworkForRequestId.get(nri.request.requestId); + NetworkAgentInfo nai = getNetworkForRequest(nri.request.requestId); ac.sendMessage(android.net.NetworkFactory.CMD_REQUEST_NETWORK, (nai != null ? nai.getCurrentScore() : 0), 0, nri.request); } @@ -2278,9 +2278,9 @@ public class ConnectivityService extends IConnectivityManager.Stub // Remove all previously satisfied requests. for (int i = 0; i < nai.numNetworkRequests(); i++) { NetworkRequest request = nai.requestAt(i); - NetworkAgentInfo currentNetwork = mNetworkForRequestId.get(request.requestId); + NetworkAgentInfo currentNetwork = getNetworkForRequest(request.requestId); if (currentNetwork != null && currentNetwork.network.netId == nai.network.netId) { - mNetworkForRequestId.remove(request.requestId); + clearNetworkForRequest(request.requestId); sendUpdatedScoreToFactories(request, 0); } } @@ -2356,7 +2356,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } } rematchAllNetworksAndRequests(null, 0); - if (nri.request.isRequest() && mNetworkForRequestId.get(nri.request.requestId) == null) { + if (nri.request.isRequest() && getNetworkForRequest(nri.request.requestId) == null) { sendUpdatedScoreToFactories(nri.request, 0); } } @@ -2411,7 +2411,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // 2. Unvalidated WiFi will not be reaped when validated cellular // is currently satisfying the request. This is desirable when // WiFi ends up validating and out scoring cellular. - mNetworkForRequestId.get(nri.request.requestId).getCurrentScore() < + getNetworkForRequest(nri.request.requestId).getCurrentScore() < nai.getCurrentScoreAsValidated())) { return false; } @@ -2438,7 +2438,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (mNetworkRequests.get(nri.request) == null) { return; } - if (mNetworkForRequestId.get(nri.request.requestId) != null) { + if (getNetworkForRequest(nri.request.requestId) != null) { return; } if (VDBG || (DBG && nri.request.isRequest())) { @@ -2478,7 +2478,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mNetworkRequestInfoLogs.log("RELEASE " + nri); if (nri.request.isRequest()) { boolean wasKept = false; - NetworkAgentInfo nai = mNetworkForRequestId.get(nri.request.requestId); + NetworkAgentInfo nai = getNetworkForRequest(nri.request.requestId); if (nai != null) { boolean wasBackgroundNetwork = nai.isBackgroundNetwork(); nai.removeRequest(nri.request.requestId); @@ -2495,7 +2495,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } else { wasKept = true; } - mNetworkForRequestId.remove(nri.request.requestId); + clearNetworkForRequest(nri.request.requestId); if (!wasBackgroundNetwork && nai.isBackgroundNetwork()) { // Went from foreground to background. updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities); @@ -4286,7 +4286,8 @@ public class ConnectivityService extends IConnectivityManager.Stub * and the are the highest scored network available. * the are keyed off the Requests requestId. */ - // TODO: Yikes, this is accessed on multiple threads: add synchronization. + // NOTE: Accessed on multiple threads, must be synchronized on itself. + @GuardedBy("mNetworkForRequestId") private final SparseArray mNetworkForRequestId = new SparseArray(); @@ -4316,8 +4317,26 @@ public class ConnectivityService extends IConnectivityManager.Stub // priority networks like Wi-Fi are active. private final NetworkRequest mDefaultMobileDataRequest; + private NetworkAgentInfo getNetworkForRequest(int requestId) { + synchronized (mNetworkForRequestId) { + return mNetworkForRequestId.get(requestId); + } + } + + private void clearNetworkForRequest(int requestId) { + synchronized (mNetworkForRequestId) { + mNetworkForRequestId.remove(requestId); + } + } + + private void setNetworkForRequest(int requestId, NetworkAgentInfo nai) { + synchronized (mNetworkForRequestId) { + mNetworkForRequestId.put(requestId, nai); + } + } + private NetworkAgentInfo getDefaultNetwork() { - return mNetworkForRequestId.get(mDefaultRequest.requestId); + return getNetworkForRequest(mDefaultRequest.requestId); } private boolean isDefaultNetwork(NetworkAgentInfo nai) { @@ -4879,7 +4898,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // requests or not, and doesn't affect the network's score. if (nri.request.isListen()) continue; - final NetworkAgentInfo currentNetwork = mNetworkForRequestId.get(nri.request.requestId); + final NetworkAgentInfo currentNetwork = getNetworkForRequest(nri.request.requestId); final boolean satisfies = newNetwork.satisfies(nri.request); if (newNetwork == currentNetwork && satisfies) { if (VDBG) { @@ -4911,7 +4930,7 @@ public class ConnectivityService extends IConnectivityManager.Stub if (VDBG) log(" accepting network in place of null"); } newNetwork.unlingerRequest(nri.request); - mNetworkForRequestId.put(nri.request.requestId, newNetwork); + setNetworkForRequest(nri.request.requestId, newNetwork); if (!newNetwork.addRequest(nri.request)) { Slog.wtf(TAG, "BUG: " + newNetwork.name() + " already has " + nri.request); } @@ -4945,7 +4964,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } newNetwork.removeRequest(nri.request.requestId); if (currentNetwork == newNetwork) { - mNetworkForRequestId.remove(nri.request.requestId); + clearNetworkForRequest(nri.request.requestId); sendUpdatedScoreToFactories(nri.request, 0); } else { Slog.wtf(TAG, "BUG: Removing request " + nri.request.requestId + " from " + From ffacaef81322fcdcaba54ec3c6b6026c23c6ff3c Mon Sep 17 00:00:00 2001 From: Chalard Jean Date: Tue, 26 Sep 2017 15:45:18 +0900 Subject: [PATCH 25/27] Do not throw on call to isTetheringSupported w/o permission ...just return false instead. This will change in P. Test: Made an app to test this. Made sure it doesn't have Test: the required permission. Checked it crashes with Test: SecurityException without this change. Checked it Test: doesn't with it. Bug: 65404184 Change-Id: Id20d3c240ec5d70d085e0366b92ab3a514f3e7c8 --- core/java/android/net/ConnectivityManager.java | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 744ee8ed0e..d7ecc81ffd 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -2078,16 +2078,30 @@ public class ConnectivityManager { * {@code ro.tether.denied} system property, Settings.TETHER_SUPPORTED or * due to device configuration. * + *

If this app does not have permission to use this API, it will always + * return false rather than throw an exception.

+ * + *

If the device has a hotspot provisioning app, the caller is required to hold the + * {@link android.Manifest.permission.TETHER_PRIVILEGED} permission.

+ * + *

Otherwise, this method requires the caller to hold the ability to modify system + * settings as determined by {@link android.provider.Settings.System#canWrite}.

+ * * @return a boolean - {@code true} indicating Tethering is supported. * * {@hide} */ @SystemApi - @RequiresPermission(android.Manifest.permission.TETHER_PRIVILEGED) + @RequiresPermission(anyOf = {android.Manifest.permission.TETHER_PRIVILEGED, + android.Manifest.permission.WRITE_SETTINGS}) public boolean isTetheringSupported() { + String pkgName = mContext.getOpPackageName(); try { - String pkgName = mContext.getOpPackageName(); return mService.isTetheringSupported(pkgName); + } catch (SecurityException e) { + // This API is not available to this caller, but for backward-compatibility + // this will just return false instead of throwing. + return false; } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } From 2c9dd64d940e361c547f24d48dd4672382a65983 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Thu, 24 Aug 2017 22:35:10 +0900 Subject: [PATCH 26/27] Declare support for Ethernet if the service is running. On some devices, support for TYPE_ETHERNET is not specified in the networkAttributes config resource, even though the device is capable of supporting Ethernet (e.g., via USB host adapters). This leads to Ethernet working but various connectivity APIs behaving as if it was not - for example, no CONNECTIVITY_ACTION broadcasts will be issues when it connects or disconnects. Ensure that ConnectivityService always treats Ethernet as available if the service is running. Currently the service is started if the device supports FEATURE_ETHERNET or FEATURE_USB_HOST. (cherry picked from commit 7bbe3eee52c08ee92a81b7bed395ca5499554cc4) Bug: 37359230 Test: bullhead builds, boots Test: ConnectivityServiceTest passes Test: Ethernet is available even if removed from networkAttributes resource Test: ConnectivityManagerTest CTS test passes Change-Id: I9b6db4edeaf966ee6715011dd92770b9d25dd938 Merged-In: I9b6db4edeaf966ee6715011dd92770b9d25dd938 --- .../com/android/server/ConnectivityService.java | 14 ++++++++++++++ .../android/server/ConnectivityServiceTest.java | 16 ++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 2b4f0f35fe..9afa825a7d 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -19,6 +19,7 @@ package com.android.server; import static android.Manifest.permission.RECEIVE_DATA_ACTIVITY_CHANGE; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; import static android.net.ConnectivityManager.NETID_UNSET; +import static android.net.ConnectivityManager.TYPE_ETHERNET; import static android.net.ConnectivityManager.TYPE_NONE; import static android.net.ConnectivityManager.TYPE_VPN; import static android.net.ConnectivityManager.getNetworkTypeName; @@ -90,6 +91,7 @@ import android.os.PowerManager; import android.os.Process; import android.os.RemoteException; import android.os.ResultReceiver; +import android.os.ServiceManager; import android.os.ServiceSpecificException; import android.os.SystemClock; import android.os.UserHandle; @@ -781,6 +783,13 @@ public class ConnectivityService extends IConnectivityManager.Stub mNetworksDefined++; // used only in the log() statement below. } + // Do the same for Ethernet, since it's often not specified in the configs, although many + // devices can use it via USB host adapters. + if (mNetConfigs[TYPE_ETHERNET] == null && hasService(Context.ETHERNET_SERVICE)) { + mLegacyTypeTracker.addSupportedType(TYPE_ETHERNET); + mNetworksDefined++; + } + if (VDBG) log("mNetworksDefined=" + mNetworksDefined); mProtectedNetworks = new ArrayList(); @@ -5546,6 +5555,11 @@ public class ConnectivityService extends IConnectivityManager.Stub return new WakeupMessage(c, h, s, cmd, 0, 0, obj); } + @VisibleForTesting + public boolean hasService(String name) { + return ServiceManager.checkService(name) != null; + } + private void logDefaultNetworkEvent(NetworkAgentInfo newNai, NetworkAgentInfo prevNai) { int newNetid = NETID_UNSET; int prevNetid = NETID_UNSET; diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 8816d43ef8..6674f20317 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -19,6 +19,8 @@ package com.android.server; import static android.net.ConnectivityManager.CONNECTIVITY_ACTION; import static android.net.ConnectivityManager.TYPE_ETHERNET; import static android.net.ConnectivityManager.TYPE_MOBILE; +import static android.net.ConnectivityManager.TYPE_MOBILE_FOTA; +import static android.net.ConnectivityManager.TYPE_MOBILE_MMS; import static android.net.ConnectivityManager.TYPE_NONE; import static android.net.ConnectivityManager.TYPE_WIFI; import static android.net.ConnectivityManager.getNetworkTypeName; @@ -781,6 +783,13 @@ public class ConnectivityServiceTest extends AndroidTestCase { return new FakeWakeupMessage(context, handler, cmdName, cmd, 0, 0, obj); } + @Override + public boolean hasService(String name) { + // Currenty, the only relevant service that ConnectivityService checks for is + // ETHERNET_SERVICE. + return Context.ETHERNET_SERVICE.equals(name); + } + public WrappedNetworkMonitor getLastCreatedWrappedNetworkMonitor() { return mLastCreatedNetworkMonitor; } @@ -928,6 +937,13 @@ public class ConnectivityServiceTest extends AndroidTestCase { // will fail. Failing here is much easier to debug. assertTrue(mCm.isNetworkSupported(TYPE_WIFI)); assertTrue(mCm.isNetworkSupported(TYPE_MOBILE)); + assertTrue(mCm.isNetworkSupported(TYPE_MOBILE_MMS)); + assertFalse(mCm.isNetworkSupported(TYPE_MOBILE_FOTA)); + + // Check that TYPE_ETHERNET is supported. Unlike the asserts above, which only validate our + // mocks, this assert exercises the ConnectivityService code path that ensures that + // TYPE_ETHERNET is supported if the ethernet service is running. + assertTrue(mCm.isNetworkSupported(TYPE_ETHERNET)); } @SmallTest From 7f9a22313498719899457c41b12ad56ef5eeb8d8 Mon Sep 17 00:00:00 2001 From: Hugo Benichi Date: Sat, 30 Sep 2017 22:17:07 +0900 Subject: [PATCH 27/27] DO NOT MERGE Ignore DUN in describeImmutableDifferences This patch changes describeImmutableDifferences in NetworkCapabilities to ignore differences in NET_CAPABILITY_DUN, so that updateCapabilities in ConnectivityService to not report wtf errors when a NetworkAgent degrades its NetworkCapabilities object by removing NET_CAPABILITY_DUN. Bug: 65257223 Test: runtest frameworks-net Change-Id: I115ed1b366da01a3f8c3c6e97e0db8ce995fd377 --- core/java/android/net/NetworkCapabilities.java | 4 +++- tests/net/java/android/net/NetworkCapabilitiesTest.java | 9 +++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/core/java/android/net/NetworkCapabilities.java b/core/java/android/net/NetworkCapabilities.java index 4bb8844053..f038c24013 100644 --- a/core/java/android/net/NetworkCapabilities.java +++ b/core/java/android/net/NetworkCapabilities.java @@ -772,7 +772,9 @@ public final class NetworkCapabilities implements Parcelable { // Ignore NOT_METERED being added or removed as it is effectively dynamic. http://b/63326103 // TODO: properly support NOT_METERED as a mutable and requestable capability. - final long mask = ~MUTABLE_CAPABILITIES & ~(1 << NET_CAPABILITY_NOT_METERED); + // Ignore DUN being added or removed. http://b/65257223. + final long mask = ~MUTABLE_CAPABILITIES + & ~(1 << NET_CAPABILITY_NOT_METERED) & ~(1 << NET_CAPABILITY_DUN); long oldImmutableCapabilities = this.mNetworkCapabilities & mask; long newImmutableCapabilities = that.mNetworkCapabilities & mask; if (oldImmutableCapabilities != newImmutableCapabilities) { diff --git a/tests/net/java/android/net/NetworkCapabilitiesTest.java b/tests/net/java/android/net/NetworkCapabilitiesTest.java index 7346f9f950..743344f589 100644 --- a/tests/net/java/android/net/NetworkCapabilitiesTest.java +++ b/tests/net/java/android/net/NetworkCapabilitiesTest.java @@ -17,6 +17,7 @@ package android.net; import static android.net.NetworkCapabilities.NET_CAPABILITY_CBS; +import static android.net.NetworkCapabilities.NET_CAPABILITY_DUN; import static android.net.NetworkCapabilities.NET_CAPABILITY_EIMS; import static android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET; import static android.net.NetworkCapabilities.NET_CAPABILITY_NOT_METERED; @@ -143,6 +144,14 @@ public class NetworkCapabilitiesTest { assertEquals("", nc1.describeImmutableDifferences(nc2)); assertEquals("", nc1.describeImmutableDifferences(nc1)); + // DUN changing (http://b/65257223) + nc1 = new NetworkCapabilities() + .addCapability(NET_CAPABILITY_DUN) + .addCapability(NET_CAPABILITY_INTERNET); + nc2 = new NetworkCapabilities().addCapability(NET_CAPABILITY_INTERNET); + assertEquals("", nc1.describeImmutableDifferences(nc2)); + assertEquals("", nc1.describeImmutableDifferences(nc1)); + // Immutable capability changing nc1 = new NetworkCapabilities() .addCapability(NET_CAPABILITY_INTERNET)