Merge "Add null check for tupleOrig in ConntrackMessage"
diff --git a/framework/src/android/net/NetworkCapabilities.java b/framework/src/android/net/NetworkCapabilities.java
index 3cc9c65..92e9599 100644
--- a/framework/src/android/net/NetworkCapabilities.java
+++ b/framework/src/android/net/NetworkCapabilities.java
@@ -1348,6 +1348,18 @@
}
/**
+ * Gets the transports as an int. Internal callers only.
+ *
+ * Prefer getTransportTypes/hasTransportType if not immediately collapsing back into a scalar.
+ *
+ * @return a long integer representing the transport types.
+ * @hide
+ */
+ public long getTransportTypesInternal() {
+ return mTransportTypes;
+ }
+
+ /**
* Sets all the transports set on this {@code NetworkCapability} instance.
* This overwrites any existing transports.
*
diff --git a/service/jni/com_android_server_BpfNetMaps.cpp b/service/jni/com_android_server_BpfNetMaps.cpp
index 77cffda..9ced44e 100644
--- a/service/jni/com_android_server_BpfNetMaps.cpp
+++ b/service/jni/com_android_server_BpfNetMaps.cpp
@@ -54,6 +54,10 @@
if (!isOk(status)) {
uid_t uid = getuid();
ALOGE("BpfNetMaps jni init failure as uid=%d", uid);
+ // We probably only ever get called from system_server (ie. AID_SYSTEM)
+ // or from tests, and never from network_stack (ie. AID_NETWORK_STACK).
+ // However, if we ever do add calls from production network_stack code
+ // we do want to make sure this initializes correctly.
// TODO: Fix tests to not use this jni lib, so we can unconditionally abort()
if (uid == AID_SYSTEM || uid == AID_NETWORK_STACK) abort();
}
diff --git a/service/jni/onload.cpp b/service/jni/onload.cpp
index 4bbaae6..ed74430 100644
--- a/service/jni/onload.cpp
+++ b/service/jni/onload.cpp
@@ -38,11 +38,11 @@
return JNI_ERR;
}
- if (register_com_android_server_BpfNetMaps(env) < 0) {
- return JNI_ERR;
- }
-
if (android::modules::sdklevel::IsAtLeastT()) {
+ if (register_com_android_server_BpfNetMaps(env) < 0) {
+ return JNI_ERR;
+ }
+
if (register_com_android_server_connectivity_ClatCoordinator(env) < 0) {
return JNI_ERR;
}
diff --git a/service/src/com/android/server/connectivity/NetworkDiagnostics.java b/service/src/com/android/server/connectivity/NetworkDiagnostics.java
index 4eba74b..15d0925 100644
--- a/service/src/com/android/server/connectivity/NetworkDiagnostics.java
+++ b/service/src/com/android/server/connectivity/NetworkDiagnostics.java
@@ -19,6 +19,9 @@
import static android.system.OsConstants.*;
import static com.android.net.module.util.NetworkStackConstants.ICMP_HEADER_LEN;
+import static com.android.net.module.util.NetworkStackConstants.IPV4_HEADER_MIN_LEN;
+import static com.android.net.module.util.NetworkStackConstants.IPV6_HEADER_LEN;
+import static com.android.net.module.util.NetworkStackConstants.IPV6_MIN_MTU;
import android.annotation.NonNull;
import android.annotation.Nullable;
@@ -35,6 +38,7 @@
import android.system.Os;
import android.system.StructTimeval;
import android.text.TextUtils;
+import android.util.Log;
import android.util.Pair;
import com.android.internal.util.IndentingPrintWriter;
@@ -174,7 +178,7 @@
}
}
- private final Map<InetAddress, Measurement> mIcmpChecks = new HashMap<>();
+ private final Map<Pair<InetAddress, Integer>, Measurement> mIcmpChecks = new HashMap<>();
private final Map<Pair<InetAddress, InetAddress>, Measurement> mExplicitSourceIcmpChecks =
new HashMap<>();
private final Map<InetAddress, Measurement> mDnsUdpChecks = new HashMap<>();
@@ -207,17 +211,21 @@
mLinkProperties.addDnsServer(TEST_DNS6);
}
+ final int mtu = mLinkProperties.getMtu();
for (RouteInfo route : mLinkProperties.getRoutes()) {
if (route.getType() == RouteInfo.RTN_UNICAST && route.hasGateway()) {
InetAddress gateway = route.getGateway();
- prepareIcmpMeasurement(gateway);
+ // Use mtu in the route if exists. Otherwise, use the one in the link property.
+ final int routeMtu = route.getMtu();
+ prepareIcmpMeasurements(gateway, (routeMtu > 0) ? routeMtu : mtu);
if (route.isIPv6Default()) {
prepareExplicitSourceIcmpMeasurements(gateway);
}
}
}
+
for (InetAddress nameserver : mLinkProperties.getDnsServers()) {
- prepareIcmpMeasurement(nameserver);
+ prepareIcmpMeasurements(nameserver, mtu);
prepareDnsMeasurement(nameserver);
// Unlike the DnsResolver which doesn't do certificate validation in opportunistic mode,
@@ -263,11 +271,50 @@
localAddr.getHostAddress(), inetSockAddr.getPort());
}
- private void prepareIcmpMeasurement(InetAddress target) {
- if (!mIcmpChecks.containsKey(target)) {
- Measurement measurement = new Measurement();
- measurement.thread = new Thread(new IcmpCheck(target, 0, measurement));
- mIcmpChecks.put(target, measurement);
+ private static int getHeaderLen(@NonNull InetAddress target) {
+ // Convert IPv4 mapped v6 address to v4 if any.
+ try {
+ final InetAddress addr = InetAddress.getByAddress(target.getAddress());
+ // An ICMPv6 header is technically 4 bytes, but the implementation in IcmpCheck#run()
+ // will always fill in another 4 bytes padding in the v6 diagnostic packets, so the size
+ // before icmp data is always 8 bytes in the implementation of ICMP diagnostics for both
+ // v4 and v6 packets. Thus, it's fine to use the v4 header size in the length
+ // calculation.
+ if (addr instanceof Inet6Address) {
+ return IPV6_HEADER_LEN + ICMP_HEADER_LEN;
+ }
+ } catch (UnknownHostException e) {
+ Log.e(TAG, "Create InetAddress fail(" + target + "): " + e);
+ }
+
+ return IPV4_HEADER_MIN_LEN + ICMP_HEADER_LEN;
+ }
+
+ private void prepareIcmpMeasurements(@NonNull InetAddress target, int targetNetworkMtu) {
+ // Test with different size payload ICMP.
+ // 1. Test with 0 payload.
+ addPayloadIcmpMeasurement(target, 0);
+ final int header = getHeaderLen(target);
+ // 2. Test with full size MTU.
+ addPayloadIcmpMeasurement(target, targetNetworkMtu - header);
+ // 3. If v6, make another measurement with the full v6 min MTU, unless that's what
+ // was done above.
+ if ((target instanceof Inet6Address) && (targetNetworkMtu != IPV6_MIN_MTU)) {
+ addPayloadIcmpMeasurement(target, IPV6_MIN_MTU - header);
+ }
+ }
+
+ private void addPayloadIcmpMeasurement(@NonNull InetAddress target, int payloadLen) {
+ // This can happen if the there is no mtu filled(which is 0) in the link property.
+ // The value becomes negative after minus header length.
+ if (payloadLen < 0) return;
+
+ final Pair<InetAddress, Integer> lenTarget =
+ new Pair<>(target, Integer.valueOf(payloadLen));
+ if (!mIcmpChecks.containsKey(lenTarget)) {
+ final Measurement measurement = new Measurement();
+ measurement.thread = new Thread(new IcmpCheck(target, payloadLen, measurement));
+ mIcmpChecks.put(lenTarget, measurement);
}
}
@@ -336,8 +383,8 @@
ArrayList<Measurement> measurements = new ArrayList(totalMeasurementCount());
// Sort measurements IPv4 first.
- for (Map.Entry<InetAddress, Measurement> entry : mIcmpChecks.entrySet()) {
- if (entry.getKey() instanceof Inet4Address) {
+ for (Map.Entry<Pair<InetAddress, Integer>, Measurement> entry : mIcmpChecks.entrySet()) {
+ if (entry.getKey().first instanceof Inet4Address) {
measurements.add(entry.getValue());
}
}
@@ -359,8 +406,8 @@
}
// IPv6 measurements second.
- for (Map.Entry<InetAddress, Measurement> entry : mIcmpChecks.entrySet()) {
- if (entry.getKey() instanceof Inet6Address) {
+ for (Map.Entry<Pair<InetAddress, Integer>, Measurement> entry : mIcmpChecks.entrySet()) {
+ if (entry.getKey().first instanceof Inet6Address) {
measurements.add(entry.getValue());
}
}
diff --git a/tests/cts/net/src/android/net/cts/NetworkScoreTest.kt b/tests/cts/net/src/android/net/cts/NetworkScoreTest.kt
index fcfecad..2704dd3 100644
--- a/tests/cts/net/src/android/net/cts/NetworkScoreTest.kt
+++ b/tests/cts/net/src/android/net/cts/NetworkScoreTest.kt
@@ -30,6 +30,7 @@
import android.os.Build
import android.os.Handler
import android.os.HandlerThread
+import android.util.Log
import androidx.test.InstrumentationRegistry
import com.android.compatibility.common.util.SystemUtil.runWithShellPermissionIdentity
import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo
@@ -41,6 +42,7 @@
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
+import java.util.Collections
// This test doesn't really have a constraint on how fast the methods should return. If it's
// going to fail, it will simply wait forever, so setting a high timeout lowers the flake ratio
@@ -64,10 +66,11 @@
@IgnoreUpTo(Build.VERSION_CODES.R)
@RunWith(DevSdkIgnoreRunner::class)
class NetworkScoreTest {
+ private val TAG = javaClass.simpleName
private val mCm = testContext.getSystemService(ConnectivityManager::class.java)
- private val mHandlerThread = HandlerThread("${javaClass.simpleName} handler thread")
+ private val mHandlerThread = HandlerThread("$TAG handler thread")
private val mHandler by lazy { Handler(mHandlerThread.looper) }
- private val agentsToCleanUp = mutableListOf<NetworkAgent>()
+ private val agentsToCleanUp = Collections.synchronizedList(mutableListOf<NetworkAgent>())
private val callbacksToCleanUp = mutableListOf<TestableNetworkCallback>()
@Before
@@ -83,15 +86,18 @@
.addTransportType(NetworkCapabilities.TRANSPORT_TEST).build(), cb, mHandler
)
}
+ Log.i(TAG, "Teardown on thread ${System.identityHashCode(Thread.currentThread())} " +
+ "cleaning up ${agentsToCleanUp.size} agents")
agentsToCleanUp.forEach {
+ Log.i(TAG, "Unregister agent for net ${it.network}")
it.unregister()
agentCleanUpCb.eventuallyExpect<CallbackEntry.Lost> { cb -> cb.network == it.network }
}
mCm.unregisterNetworkCallback(agentCleanUpCb)
+ callbacksToCleanUp.forEach { mCm.unregisterNetworkCallback(it) }
mHandlerThread.quitSafely()
mHandlerThread.join()
- callbacksToCleanUp.forEach { mCm.unregisterNetworkCallback(it) }
}
// Returns a networkCallback that sends onAvailable on the best network with TRANSPORT_TEST.
@@ -145,6 +151,8 @@
val agent = object : NetworkAgent(context, looper, "NetworkScore test agent", nc,
LinkProperties(), score, config, NetworkProvider(context, looper,
"NetworkScore test provider")) {}.also {
+ Log.i(TAG, "Add on thread ${System.identityHashCode(Thread.currentThread())} " +
+ "agent to clean up $it")
agentsToCleanUp.add(it)
}
runWithShellPermissionIdentity({ agent.register() }, MANAGE_TEST_NETWORKS)