From 333d5e52f5ab262405c96e7aab05e2552fbc52b0 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 14 Feb 2022 09:05:27 +0900 Subject: [PATCH] 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