From 333d5e52f5ab262405c96e7aab05e2552fbc52b0 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 14 Feb 2022 09:05:27 +0900 Subject: [PATCH 1/2] Add a method to create a TAP interface without bringing it up. In S, the behaviour of createTunInterface and createTapInterface changed so that they bring up the interface before returning it. This makes it difficult to test code that brings interfaces up itself, such as IpClient or EthernetManager, because the tests cannot predict whether that code will see the interface up or not. This leads to flaky tests and can even make it impossible to reliably test some behaviour. Add a method that allows the caller to specify whether to bring up the interface or not. Test: new codepath tested by other CL in topic Test: existing codepaths already well-covered Change-Id: I0f7698f4dad132f201db4203e65a78c6af564ab2 --- .../src/android/net/ITestNetworkManager.aidl | 3 +- .../src/android/net/TestNetworkManager.java | 27 ++++++++++++++-- .../android/server/TestNetworkService.java | 32 ++++--------------- 3 files changed, 33 insertions(+), 29 deletions(-) diff --git a/framework/src/android/net/ITestNetworkManager.aidl b/framework/src/android/net/ITestNetworkManager.aidl index 2a863adde5..847f14e367 100644 --- a/framework/src/android/net/ITestNetworkManager.aidl +++ b/framework/src/android/net/ITestNetworkManager.aidl @@ -29,8 +29,7 @@ import android.os.ParcelFileDescriptor; */ interface ITestNetworkManager { - TestNetworkInterface createTunInterface(in LinkAddress[] linkAddrs); - TestNetworkInterface createTapInterface(); + TestNetworkInterface createInterface(boolean isTun, boolean bringUp, in LinkAddress[] addrs); void setupTestNetwork(in String iface, in LinkProperties lp, in boolean isMetered, in int[] administratorUids, in IBinder binder); diff --git a/framework/src/android/net/TestNetworkManager.java b/framework/src/android/net/TestNetworkManager.java index 9ddd2f5767..280e497996 100644 --- a/framework/src/android/net/TestNetworkManager.java +++ b/framework/src/android/net/TestNetworkManager.java @@ -49,6 +49,11 @@ public class TestNetworkManager { @NonNull private final ITestNetworkManager mService; + private static final boolean TAP = false; + private static final boolean TUN = true; + private static final boolean BRING_UP = true; + private static final LinkAddress[] NO_ADDRS = new LinkAddress[0]; + /** @hide */ public TestNetworkManager(@NonNull ITestNetworkManager service) { mService = Objects.requireNonNull(service, "missing ITestNetworkManager"); @@ -155,7 +160,7 @@ public class TestNetworkManager { public TestNetworkInterface createTunInterface(@NonNull Collection linkAddrs) { try { final LinkAddress[] arr = new LinkAddress[linkAddrs.size()]; - return mService.createTunInterface(linkAddrs.toArray(arr)); + return mService.createInterface(TUN, BRING_UP, linkAddrs.toArray(arr)); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } @@ -173,10 +178,28 @@ public class TestNetworkManager { @NonNull public TestNetworkInterface createTapInterface() { try { - return mService.createTapInterface(); + return mService.createInterface(TAP, BRING_UP, NO_ADDRS); } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } } + /** + * Create a tap interface for testing purposes + * + * @param bringUp whether to bring up the interface before returning it. + * + * @return A ParcelFileDescriptor of the underlying TAP interface. Close this to tear down the + * TAP interface. + * @hide + */ + @RequiresPermission(Manifest.permission.MANAGE_TEST_NETWORKS) + @NonNull + public TestNetworkInterface createTapInterface(boolean bringUp) { + try { + return mService.createInterface(TAP, bringUp, NO_ADDRS); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); + } + } } diff --git a/service/src/com/android/server/TestNetworkService.java b/service/src/com/android/server/TestNetworkService.java index fffd2be35e..a0bfb4a7a6 100644 --- a/service/src/com/android/server/TestNetworkService.java +++ b/service/src/com/android/server/TestNetworkService.java @@ -99,12 +99,14 @@ class TestNetworkService extends ITestNetworkManager.Stub { } /** - * Create a TUN or TAP interface with the given interface name and link addresses + * Create a TUN or TAP interface with the specified parameters. * *

This method will return the FileDescriptor to the interface. Close it to tear down the * interface. */ - private TestNetworkInterface createInterface(boolean isTun, LinkAddress[] linkAddrs) { + @Override + public TestNetworkInterface createInterface(boolean isTun, boolean bringUp, + LinkAddress[] linkAddrs) { enforceTestNetworkPermissions(mContext); Objects.requireNonNull(linkAddrs, "missing linkAddrs"); @@ -122,7 +124,9 @@ class TestNetworkService extends ITestNetworkManager.Stub { addr.getPrefixLength()); } - NetdUtils.setInterfaceUp(mNetd, iface); + if (bringUp) { + NetdUtils.setInterfaceUp(mNetd, iface); + } return new TestNetworkInterface(tunIntf, iface); } catch (RemoteException e) { @@ -132,28 +136,6 @@ class TestNetworkService extends ITestNetworkManager.Stub { } } - /** - * Create a TUN interface with the given interface name and link addresses - * - *

This method will return the FileDescriptor to the TUN interface. Close it to tear down the - * TUN interface. - */ - @Override - public TestNetworkInterface createTunInterface(@NonNull LinkAddress[] linkAddrs) { - return createInterface(true, linkAddrs); - } - - /** - * Create a TAP interface with the given interface name - * - *

This method will return the FileDescriptor to the TAP interface. Close it to tear down the - * TAP interface. - */ - @Override - public TestNetworkInterface createTapInterface() { - return createInterface(false, new LinkAddress[0]); - } - // Tracker for TestNetworkAgents @GuardedBy("mTestNetworkTracker") @NonNull From 9e560d24c2652b4b5e2d39e57d5c8bb506945923 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Wed, 9 Feb 2022 21:52:00 +0900 Subject: [PATCH 2/2] Add a CTS test for EthernetManager.InterfaceStateListener. Test: CtsNetTestCases:android.net.cts.EthernetManagerTest Change-Id: Ic956b5e52575a467b405fecf142c7f25925358d4 --- .../android/net/cts/EthernetManagerTest.kt | 184 ++++++++++++++++++ 1 file changed, 184 insertions(+) create mode 100644 tests/cts/net/src/android/net/cts/EthernetManagerTest.kt diff --git a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt new file mode 100644 index 0000000000..0a32f09bfc --- /dev/null +++ b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt @@ -0,0 +1,184 @@ +/* + * Copyright (C) 2022 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 android.net.cts + +import android.Manifest.permission.MANAGE_TEST_NETWORKS +import android.Manifest.permission.NETWORK_SETTINGS +import android.net.IpConfiguration +import android.net.TestNetworkInterface +import android.net.TestNetworkManager +import android.platform.test.annotations.AppModeFull +import androidx.test.platform.app.InstrumentationRegistry +import androidx.test.runner.AndroidJUnit4 +import com.android.net.module.util.ArrayTrackRecord +import com.android.net.module.util.TrackRecord +import com.android.testutils.DevSdkIgnoreRule +import com.android.testutils.SC_V2 +import com.android.testutils.runAsShell +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import kotlin.test.assertNull +import kotlin.test.fail +import android.net.cts.EthernetManagerTest.EthernetStateListener.CallbackEntry.InterfaceStateChanged +import android.os.Handler +import android.os.HandlerExecutor +import android.os.Looper +import com.android.networkstack.apishim.common.EthernetManagerShim.InterfaceStateListener +import com.android.networkstack.apishim.common.EthernetManagerShim.STATE_ABSENT +import com.android.networkstack.apishim.common.EthernetManagerShim.STATE_LINK_DOWN +import com.android.networkstack.apishim.common.EthernetManagerShim.STATE_LINK_UP +import com.android.networkstack.apishim.common.EthernetManagerShim.ROLE_CLIENT +import com.android.networkstack.apishim.common.EthernetManagerShim.ROLE_NONE +import com.android.networkstack.apishim.EthernetManagerShimImpl +import java.util.concurrent.Executor +import kotlin.test.assertEquals + +private const val TIMEOUT_MS = 1000L +private const val NO_CALLBACK_TIMEOUT_MS = 200L +private val DEFAULT_IP_CONFIGURATION = IpConfiguration(IpConfiguration.IpAssignment.DHCP, + IpConfiguration.ProxySettings.NONE, null, null) + +@AppModeFull(reason = "Instant apps can't access EthernetManager") +@RunWith(AndroidJUnit4::class) +class EthernetManagerTest { + // EthernetManager is not updatable before T, so tests do not need to be backwards compatible + @get:Rule + val ignoreRule = DevSdkIgnoreRule(ignoreClassUpTo = SC_V2) + + private val context by lazy { InstrumentationRegistry.getInstrumentation().context } + private val em by lazy { EthernetManagerShimImpl.newInstance(context) } + + private val createdIfaces = ArrayList() + private val addedListeners = ArrayList() + + private open class EthernetStateListener private constructor( + private val history: ArrayTrackRecord + ) : InterfaceStateListener, + TrackRecord by history { + constructor() : this(ArrayTrackRecord()) + + val events = history.newReadHead() + + sealed class CallbackEntry { + data class InterfaceStateChanged( + val iface: String, + val state: Int, + val role: Int, + val configuration: IpConfiguration? + ) : CallbackEntry() + } + + override fun onInterfaceStateChanged( + iface: String, + state: Int, + role: Int, + cfg: IpConfiguration? + ) { + add(InterfaceStateChanged(iface, state, role, cfg)) + } + + fun expectCallback(expected: T): T { + val event = pollForNextCallback() + assertEquals(expected, event) + return event as T + } + + fun expectCallback(iface: TestNetworkInterface, state: Int, role: Int) { + expectCallback(InterfaceStateChanged(iface.interfaceName, state, role, + if (state != STATE_ABSENT) DEFAULT_IP_CONFIGURATION else null)) + } + + fun pollForNextCallback(): CallbackEntry { + return events.poll(TIMEOUT_MS) ?: fail("Did not receive callback after ${TIMEOUT_MS}ms") + } + + fun assertNoCallback() { + val cb = events.poll(NO_CALLBACK_TIMEOUT_MS) + assertNull(cb, "Expected no callback but got $cb") + } + } + + @Test + public fun testCallbacks() { + val executor = HandlerExecutor(Handler(Looper.getMainLooper())) + + // If an interface exists when the callback is registered, it is reported on registration. + val iface = runAsShell(MANAGE_TEST_NETWORKS) { + createInterface() + } + val listener = EthernetStateListener() + addInterfaceStateListener(executor, listener) + listener.expectCallback(iface, STATE_LINK_UP, ROLE_CLIENT) + + // If an interface appears, existing callbacks see it. + // TODO: fix the up/up/down/up callbacks and only send down/up. + val iface2 = runAsShell(MANAGE_TEST_NETWORKS) { + createInterface() + } + listener.expectCallback(iface2, STATE_LINK_UP, ROLE_CLIENT) + listener.expectCallback(iface2, STATE_LINK_UP, ROLE_CLIENT) + listener.expectCallback(iface2, STATE_LINK_DOWN, ROLE_CLIENT) + listener.expectCallback(iface2, STATE_LINK_UP, ROLE_CLIENT) + + // Removing interfaces first sends link down, then STATE_ABSENT/ROLE_NONE. + removeInterface(iface) + listener.expectCallback(iface, STATE_LINK_DOWN, ROLE_CLIENT) + listener.expectCallback(iface, STATE_ABSENT, ROLE_NONE) + + removeInterface(iface2) + listener.expectCallback(iface2, STATE_LINK_DOWN, ROLE_CLIENT) + listener.expectCallback(iface2, STATE_ABSENT, ROLE_NONE) + listener.assertNoCallback() + } + + @Before + fun setUp() { + runAsShell(MANAGE_TEST_NETWORKS, NETWORK_SETTINGS) { + em.setIncludeTestInterfaces(true) + } + } + + @After + fun tearDown() { + runAsShell(MANAGE_TEST_NETWORKS, NETWORK_SETTINGS) { + em.setIncludeTestInterfaces(false) + for (iface in createdIfaces) { + if (iface.fileDescriptor.fileDescriptor.valid()) iface.fileDescriptor.close() + } + for (listener in addedListeners) { + em.removeInterfaceStateListener(listener) + } + } + } + + private fun addInterfaceStateListener(executor: Executor, listener: InterfaceStateListener) { + em.addInterfaceStateListener(executor, listener) + addedListeners.add(listener) + } + + private fun createInterface(): TestNetworkInterface { + val tnm = context.getSystemService(TestNetworkManager::class.java) + return tnm.createTapInterface(false /* bringUp */).also { createdIfaces.add(it) } + } + + private fun removeInterface(iface: TestNetworkInterface) { + iface.fileDescriptor.close() + createdIfaces.remove(iface) + } +} \ No newline at end of file