From 3c089e67f15cc6657b80c638b21c1990d1b8db66 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Fri, 20 May 2022 12:28:04 -0700 Subject: [PATCH 1/3] Improve waiting for interface added or removed In a previous CL, code was added to wait for interfaces being properly registered with the EthernetTracker before continuing the test. The same logic applies to removingInterfaces (which prevents a previous tests Lost callbacks to show up in a subsequent test). The createInterface listener was improved to expect the down callback first. EthernetTracker receives down and up callbacks from netd which it posts on its handler. While the initial up-up callbacks (KI) can be flaky, down-up should always be the correct signal that the interface is ready and properly registered with the ethernet code. Test: atest EthernetManagerTest Bug: 227008268 Change-Id: Id2a078da732009411934166df7ae08bedd17a19d --- tests/cts/net/src/android/net/cts/EthernetManagerTest.kt | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt index 792aa8373a..adafd8d4e2 100644 --- a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt +++ b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt @@ -195,7 +195,11 @@ class EthernetManagerTest { context, Handler(Looper.getMainLooper()) ).also { createdIfaces.add(it) } - ifaceListener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_CLIENT) + with(ifaceListener) { + // when an interface comes up, we should always see a down cb before an up cb. + eventuallyExpect(iface, STATE_LINK_DOWN, ROLE_CLIENT) + expectCallback(iface, STATE_LINK_UP, ROLE_CLIENT) + } return iface } @@ -208,6 +212,7 @@ class EthernetManagerTest { private fun removeInterface(iface: EthernetTestInterface) { iface.destroy() createdIfaces.remove(iface) + ifaceListener.eventuallyExpect(iface, STATE_ABSENT, ROLE_NONE) } @Test From daca9ca74f04852ee699940c0fc2eabb55cb102b Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Fri, 8 Apr 2022 12:04:34 +0200 Subject: [PATCH 2/3] Move EthernetNetworkFactory to using the NetworkProvider API This CL makes EthernetNetworkFactory inherit from NetworkProvider rather than NetworkFactory. The name of the class is purposefully unchanged to make the code review easier (it will be changed to EthernetNetworkProvider in a follow up). As part of the conversion, NetworkInterfaceState now registers a NetworkOffer when the link comes up and unregisters it when the link goes down. It updates the existing offer when capabilities change (by calling registerNetworkOffer with an already registered NetworkOfferCallback). This change should fix existing refCount issues. When a NetworkOffer is first registered, it receives callbacks for all existing requests. This is the main problem with the NetworkFactory implementation where only one NetworkOffer is registered when the factory is first created; so when interfaces come up, they do not receive callbacks for existing requests. Test: atest EthernetNetworkTest Bug: 197548738 Change-Id: I5e8e4673d2ed04bc1a0c8d232a8772edfff65b5d --- .../ethernet/EthernetNetworkFactory.java | 155 ++++++------------ .../ethernet/EthernetNetworkFactoryTest.java | 87 ++-------- 2 files changed, 62 insertions(+), 180 deletions(-) diff --git a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java index e2cceda353..79802fbd86 100644 --- a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java +++ b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java @@ -31,10 +31,9 @@ import android.net.IpConfiguration.ProxySettings; import android.net.LinkProperties; import android.net.NetworkAgentConfig; import android.net.NetworkCapabilities; -import android.net.NetworkFactory; import android.net.NetworkProvider; import android.net.NetworkRequest; -import android.net.NetworkSpecifier; +import android.net.NetworkScore; import android.net.ip.IIpClient; import android.net.ip.IpClientCallbacks; import android.net.ip.IpClientManager; @@ -61,22 +60,19 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; /** - * {@link NetworkFactory} that represents Ethernet networks. - * - * This class reports a static network score of 70 when it is tracking an interface and that - * interface's link is up, and a score of 0 otherwise. + * {@link NetworkProvider} that manages NetworkOffers for Ethernet networks. */ -public class EthernetNetworkFactory extends NetworkFactory { +public class EthernetNetworkFactory { private final static String TAG = EthernetNetworkFactory.class.getSimpleName(); final static boolean DBG = true; - private final static int NETWORK_SCORE = 70; private static final String NETWORK_TYPE = "Ethernet"; private final ConcurrentHashMap mTrackingInterfaces = new ConcurrentHashMap<>(); private final Handler mHandler; private final Context mContext; + private final NetworkProvider mProvider; final Dependencies mDeps; public static class Dependencies { @@ -111,54 +107,24 @@ public class EthernetNetworkFactory extends NetworkFactory { } public EthernetNetworkFactory(Handler handler, Context context) { - this(handler, context, new Dependencies()); + this(handler, context, new NetworkProvider(context, handler.getLooper(), TAG), + new Dependencies()); } @VisibleForTesting - EthernetNetworkFactory(Handler handler, Context context, Dependencies deps) { - super(handler.getLooper(), context, NETWORK_TYPE, createDefaultNetworkCapabilities()); - + EthernetNetworkFactory(Handler handler, Context context, NetworkProvider provider, + Dependencies deps) { mHandler = handler; mContext = context; + mProvider = provider; mDeps = deps; - - setScoreFilter(NETWORK_SCORE); } - @Override - public boolean acceptRequest(NetworkRequest request) { - if (DBG) { - Log.d(TAG, "acceptRequest, request: " + request); - } - - return networkForRequest(request) != null; - } - - @Override - protected void needNetworkFor(NetworkRequest networkRequest) { - NetworkInterfaceState network = networkForRequest(networkRequest); - - if (network == null) { - Log.e(TAG, "needNetworkFor, failed to get a network for " + networkRequest); - return; - } - - if (++network.refCount == 1) { - network.start(); - } - } - - @Override - protected void releaseNetworkFor(NetworkRequest networkRequest) { - NetworkInterfaceState network = networkForRequest(networkRequest); - if (network == null) { - Log.e(TAG, "releaseNetworkFor, failed to get a network for " + networkRequest); - return; - } - - if (--network.refCount == 0) { - network.stop(); - } + /** + * Registers the network provider with the system. + */ + public void register() { + mContext.getSystemService(ConnectivityManager.class).registerNetworkProvider(mProvider); } /** @@ -196,9 +162,8 @@ public class EthernetNetworkFactory extends NetworkFactory { } final NetworkInterfaceState iface = new NetworkInterfaceState( - ifaceName, hwAddress, mHandler, mContext, ipConfig, nc, getProvider(), mDeps); + ifaceName, hwAddress, mHandler, mContext, ipConfig, nc, mProvider, mDeps); mTrackingInterfaces.put(ifaceName, iface); - updateCapabilityFilter(); } @VisibleForTesting @@ -239,7 +204,6 @@ public class EthernetNetworkFactory extends NetworkFactory { final NetworkInterfaceState iface = mTrackingInterfaces.get(ifaceName); iface.updateInterface(ipConfig, capabilities, listener); mTrackingInterfaces.put(ifaceName, iface); - updateCapabilityFilter(); } private static NetworkCapabilities mixInCapabilities(NetworkCapabilities nc, @@ -250,16 +214,6 @@ public class EthernetNetworkFactory extends NetworkFactory { return builder.build(); } - private void updateCapabilityFilter() { - NetworkCapabilities capabilitiesFilter = createDefaultNetworkCapabilities(); - for (NetworkInterfaceState iface: mTrackingInterfaces.values()) { - capabilitiesFilter = mixInCapabilities(capabilitiesFilter, iface.mCapabilities); - } - - if (DBG) Log.d(TAG, "updateCapabilityFilter: " + capabilitiesFilter); - setCapabilityFilter(capabilitiesFilter); - } - private static NetworkCapabilities createDefaultNetworkCapabilities() { return NetworkCapabilities.Builder .withoutDefaultCapabilities() @@ -270,11 +224,8 @@ public class EthernetNetworkFactory extends NetworkFactory { protected void removeInterface(String interfaceName) { NetworkInterfaceState iface = mTrackingInterfaces.remove(interfaceName); if (iface != null) { - iface.maybeSendNetworkManagementCallbackForAbort(); - iface.stop(); + iface.destroy(); } - - updateCapabilityFilter(); } /** Returns true if state has been modified */ @@ -306,37 +257,6 @@ public class EthernetNetworkFactory extends NetworkFactory { return mTrackingInterfaces.containsKey(ifaceName); } - private NetworkInterfaceState networkForRequest(NetworkRequest request) { - String requestedIface = null; - - NetworkSpecifier specifier = request.getNetworkSpecifier(); - if (specifier instanceof EthernetNetworkSpecifier) { - requestedIface = ((EthernetNetworkSpecifier) specifier) - .getInterfaceName(); - } - - NetworkInterfaceState network = null; - if (!TextUtils.isEmpty(requestedIface)) { - NetworkInterfaceState n = mTrackingInterfaces.get(requestedIface); - if (n != null && request.canBeSatisfiedBy(n.mCapabilities)) { - network = n; - } - } else { - for (NetworkInterfaceState n : mTrackingInterfaces.values()) { - if (request.canBeSatisfiedBy(n.mCapabilities) && n.mLinkUp) { - network = n; - break; - } - } - } - - if (DBG) { - Log.i(TAG, "networkForRequest, request: " + request + ", network: " + network); - } - - return network; - } - private static void maybeSendNetworkManagementCallback( @Nullable final INetworkInterfaceOutcomeReceiver listener, @Nullable final String iface, @@ -401,8 +321,6 @@ public class EthernetNetworkFactory extends NetworkFactory { ConnectivityManager.TYPE_NONE); } - long refCount = 0; - private class EthernetIpClientCallback extends IpClientCallbacks { private final ConditionVariable mIpClientStartCv = new ConditionVariable(false); private final ConditionVariable mIpClientShutdownCv = new ConditionVariable(false); @@ -476,6 +394,9 @@ public class EthernetNetworkFactory extends NetworkFactory { private class EthernetNetworkOfferCallback implements NetworkProvider.NetworkOfferCallback { @Override public void onNetworkNeeded(@NonNull NetworkRequest request) { + if (DBG) { + Log.d(TAG, String.format("%s: onNetworkNeeded for request: %s", name, request)); + } // When the network offer is first registered, onNetworkNeeded is called with all // existing requests. // ConnectivityService filters requests for us based on the NetworkCapabilities @@ -487,6 +408,10 @@ public class EthernetNetworkFactory extends NetworkFactory { @Override public void onNetworkUnneeded(@NonNull NetworkRequest request) { + if (DBG) { + Log.d(TAG, + String.format("%s: onNetworkUnneeded for request: %s", name, request)); + } mRequests.remove(request); if (mRequests.isEmpty()) { // not currently serving any requests, stop the network. @@ -529,9 +454,21 @@ public class EthernetNetworkFactory extends NetworkFactory { + "transport type."); } + private static NetworkScore getBestNetworkScore() { + return new NetworkScore.Builder().build(); + } + private void setCapabilities(@NonNull final NetworkCapabilities capabilities) { mCapabilities = new NetworkCapabilities(capabilities); mLegacyType = getLegacyType(mCapabilities); + + if (mLinkUp) { + // registering a new network offer will update the existing one, not install a + // new one. + mNetworkProvider.registerNetworkOffer(getBestNetworkScore(), + new NetworkCapabilities(capabilities), cmd -> mHandler.post(cmd), + mNetworkOfferCallback); + } } void updateInterface(@Nullable final IpConfiguration ipConfig, @@ -693,20 +630,21 @@ public class EthernetNetworkFactory extends NetworkFactory { mLinkUp = up; if (!up) { // was up, goes down - // Send an abort on a provisioning request callback if necessary before stopping. - maybeSendNetworkManagementCallbackForAbort(); - stop(); + // retract network offer and stop IpClient. + destroy(); // If only setting the interface down, send a callback to signal completion. EthernetNetworkFactory.maybeSendNetworkManagementCallback(listener, name, null); } else { // was down, goes up - stop(); - start(listener); + // register network offer + mNetworkProvider.registerNetworkOffer(getBestNetworkScore(), + new NetworkCapabilities(mCapabilities), (cmd) -> mHandler.post(cmd), + mNetworkOfferCallback); } return true; } - void stop() { + private void stop() { // Invalidate all previous start requests if (mIpClient != null) { mIpClient.shutdown(); @@ -722,6 +660,13 @@ public class EthernetNetworkFactory extends NetworkFactory { mLinkProperties.clear(); } + public void destroy() { + mNetworkProvider.unregisterNetworkOffer(mNetworkOfferCallback); + maybeSendNetworkManagementCallbackForAbort(); + stop(); + mRequests.clear(); + } + private static void provisionIpClient(@NonNull final IpClientManager ipClient, @NonNull final IpConfiguration config, @NonNull final String tcpBufferSizes) { if (config.getProxySettings() == ProxySettings.STATIC || @@ -761,7 +706,6 @@ public class EthernetNetworkFactory extends NetworkFactory { @Override public String toString() { return getClass().getSimpleName() + "{ " - + "refCount: " + refCount + ", " + "iface: " + name + ", " + "up: " + mLinkUp + ", " + "hwAddress: " + mHwAddress + ", " @@ -774,7 +718,6 @@ public class EthernetNetworkFactory extends NetworkFactory { } void dump(FileDescriptor fd, IndentingPrintWriter pw, String[] args) { - super.dump(fd, pw, args); pw.println(getClass().getSimpleName()); pw.println("Tracking interfaces:"); pw.increaseIndent(); diff --git a/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java b/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java index dfb4fccbd7..568d40fb23 100644 --- a/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java +++ b/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java @@ -32,7 +32,6 @@ import static org.mockito.ArgumentMatchers.same; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -51,6 +50,7 @@ import android.net.Network; import android.net.NetworkAgentConfig; import android.net.NetworkCapabilities; import android.net.NetworkProvider; +import android.net.NetworkProvider.NetworkOfferCallback; import android.net.NetworkRequest; import android.net.StaticIpConfiguration; import android.net.ip.IpClientCallbacks; @@ -99,6 +99,7 @@ public class EthernetNetworkFactoryTest { @Mock private EthernetNetworkAgent mNetworkAgent; @Mock private InterfaceParams mInterfaceParams; @Mock private Network mMockNetwork; + @Mock private NetworkProvider mNetworkProvider; @Before public void setUp() throws Exception { @@ -112,7 +113,7 @@ public class EthernetNetworkFactoryTest { private void initEthernetNetworkFactory() { mLooper = new TestLooper(); mHandler = new Handler(mLooper.getLooper()); - mNetFactory = new EthernetNetworkFactory(mHandler, mContext, mDeps); + mNetFactory = new EthernetNetworkFactory(mHandler, mContext, mNetworkProvider, mDeps); } private void setupNetworkAgentMock() { @@ -239,9 +240,16 @@ public class EthernetNetworkFactoryTest { mNetFactory.addInterface(iface, HW_ADDR, ipConfig, createInterfaceCapsBuilder(transportType).build()); assertTrue(mNetFactory.updateInterfaceLinkState(iface, true, NULL_LISTENER)); + + ArgumentCaptor captor = ArgumentCaptor.forClass( + NetworkOfferCallback.class); + verify(mNetworkProvider).registerNetworkOffer(any(), any(), any(), captor.capture()); + captor.getValue().onNetworkNeeded(createDefaultRequest()); + verifyStart(ipConfig); clearInvocations(mDeps); clearInvocations(mIpClient); + clearInvocations(mNetworkProvider); } // creates a provisioned interface @@ -281,28 +289,14 @@ public class EthernetNetworkFactoryTest { // To create an unprovisioned interface, provision and then "stop" it, i.e. stop its // NetworkAgent and IpClient. One way this can be done is by provisioning an interface and // then calling onNetworkUnwanted. - createAndVerifyProvisionedInterface(iface); - - mNetworkAgent.getCallbacks().onNetworkUnwanted(); - mLooper.dispatchAll(); - verifyStop(); + mNetFactory.addInterface(iface, HW_ADDR, createDefaultIpConfig(), + createInterfaceCapsBuilder(NetworkCapabilities.TRANSPORT_ETHERNET).build()); + assertTrue(mNetFactory.updateInterfaceLinkState(iface, true, NULL_LISTENER)); clearInvocations(mIpClient); clearInvocations(mNetworkAgent); } - @Test - public void testAcceptRequest() throws Exception { - initEthernetNetworkFactory(); - createInterfaceUndergoingProvisioning(TEST_IFACE); - assertTrue(mNetFactory.acceptRequest(createDefaultRequest())); - - NetworkRequest wifiRequest = createDefaultRequestBuilder() - .removeTransportType(NetworkCapabilities.TRANSPORT_ETHERNET) - .addTransportType(NetworkCapabilities.TRANSPORT_WIFI).build(); - assertFalse(mNetFactory.acceptRequest(wifiRequest)); - } - @Test public void testUpdateInterfaceLinkStateForActiveProvisioningInterface() throws Exception { initEthernetNetworkFactory(); @@ -377,36 +371,6 @@ public class EthernetNetworkFactoryTest { listener.expectOnError(); } - @Test - public void testNeedNetworkForOnProvisionedInterface() throws Exception { - initEthernetNetworkFactory(); - createAndVerifyProvisionedInterface(TEST_IFACE); - mNetFactory.needNetworkFor(createDefaultRequest()); - verify(mIpClient, never()).startProvisioning(any()); - } - - @Test - public void testNeedNetworkForOnUnprovisionedInterface() throws Exception { - initEthernetNetworkFactory(); - createUnprovisionedInterface(TEST_IFACE); - mNetFactory.needNetworkFor(createDefaultRequest()); - verify(mIpClient).startProvisioning(any()); - - triggerOnProvisioningSuccess(); - verifyNetworkAgentRegistersAndConnects(); - } - - @Test - public void testNeedNetworkForOnInterfaceUndergoingProvisioning() throws Exception { - initEthernetNetworkFactory(); - createInterfaceUndergoingProvisioning(TEST_IFACE); - mNetFactory.needNetworkFor(createDefaultRequest()); - verify(mIpClient, never()).startProvisioning(any()); - - triggerOnProvisioningSuccess(); - verifyNetworkAgentRegistersAndConnects(); - } - @Test public void testProvisioningLoss() throws Exception { initEthernetNetworkFactory(); @@ -440,31 +404,6 @@ public class EthernetNetworkFactoryTest { verify(mIpClient, never()).startProvisioning(any()); } - @Test - public void testIpClientIsNotStartedWhenLinkIsDown() throws Exception { - initEthernetNetworkFactory(); - createUnprovisionedInterface(TEST_IFACE); - mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, NULL_LISTENER); - - mNetFactory.needNetworkFor(createDefaultRequest()); - - verify(mDeps, never()).makeIpClient(any(), any(), any()); - - // BUG(b/191854824): requesting a network with a specifier (Android Auto use case) should - // not start an IpClient when the link is down, but fixing this may make matters worse by - // tiggering b/197548738. - NetworkRequest specificNetRequest = new NetworkRequest.Builder() - .addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET) - .setNetworkSpecifier(new EthernetNetworkSpecifier(TEST_IFACE)) - .build(); - mNetFactory.needNetworkFor(specificNetRequest); - mNetFactory.releaseNetworkFor(specificNetRequest); - - mNetFactory.updateInterfaceLinkState(TEST_IFACE, true, NULL_LISTENER); - // TODO: change to once when b/191854824 is fixed. - verify(mDeps, times(2)).makeIpClient(any(), eq(TEST_IFACE), any()); - } - @Test public void testLinkPropertiesChanged() throws Exception { initEthernetNetworkFactory(); From 97bdb655d9c86547da911f8e3906f0037922d203 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Fri, 20 May 2022 12:25:04 -0700 Subject: [PATCH 3/3] Add CTS tests for EthernetNetworkProvider changes Adds CTS test to validate behavior of EthernetNetworkProvider API. These tests are currently still pretty slow due to the fact that newly registered tracked interfaces are brought up, back down, and back up before they are available (this is a bug and will hopefully get fixed in the near future). Test: atest EthernetManagerTest Bug: 197548738 Change-Id: I8e806b3b884f2e0b6c1a1d2fffdb9a99c5dd60e8 --- .../android/net/cts/EthernetManagerTest.kt | 149 ++++++++++++++++++ 1 file changed, 149 insertions(+) diff --git a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt index adafd8d4e2..0a02593c87 100644 --- a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt +++ b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt @@ -19,9 +19,16 @@ import android.Manifest.permission.CONNECTIVITY_USE_RESTRICTED_NETWORKS import android.Manifest.permission.MANAGE_TEST_NETWORKS import android.Manifest.permission.NETWORK_SETTINGS import android.content.Context +import android.net.ConnectivityManager +import android.net.EthernetNetworkSpecifier import android.net.InetAddresses import android.net.IpConfiguration import android.net.MacAddress +import android.net.Network +import android.net.NetworkCapabilities.NET_CAPABILITY_TRUSTED +import android.net.NetworkCapabilities.TRANSPORT_ETHERNET +import android.net.NetworkCapabilities.TRANSPORT_TEST +import android.net.NetworkRequest import android.net.TestNetworkInterface import android.net.TestNetworkManager import android.net.cts.EthernetManagerTest.EthernetStateListener.CallbackEntry.InterfaceStateChanged @@ -41,10 +48,14 @@ import com.android.networkstack.apishim.common.EthernetManagerShim.ROLE_NONE 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.testutils.anyNetwork import com.android.testutils.DevSdkIgnoreRule +import com.android.testutils.RecorderCallback.CallbackEntry.Available +import com.android.testutils.RecorderCallback.CallbackEntry.Lost import com.android.testutils.RouterAdvertisementResponder import com.android.testutils.SC_V2 import com.android.testutils.TapPacketReader +import com.android.testutils.TestableNetworkCallback import com.android.testutils.runAsShell import com.android.testutils.waitForIdle import org.junit.After @@ -64,6 +75,11 @@ 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) +private val ETH_REQUEST: NetworkRequest = NetworkRequest.Builder() + .addTransportType(TRANSPORT_TEST) + .addTransportType(TRANSPORT_ETHERNET) + .removeCapability(NET_CAPABILITY_TRUSTED) + .build() @AppModeFull(reason = "Instant apps can't access EthernetManager") @RunWith(AndroidJUnit4::class) @@ -74,10 +90,12 @@ class EthernetManagerTest { private val context by lazy { InstrumentationRegistry.getInstrumentation().context } private val em by lazy { EthernetManagerShimImpl.newInstance(context) } + private val cm by lazy { context.getSystemService(ConnectivityManager::class.java) } private val ifaceListener = EthernetStateListener() private val createdIfaces = ArrayList() private val addedListeners = ArrayList() + private val networkRequests = ArrayList() private class EthernetTestInterface( context: Context, @@ -177,10 +195,12 @@ class EthernetManagerTest { setIncludeTestInterfaces(false) for (iface in createdIfaces) { iface.destroy() + ifaceListener.eventuallyExpect(iface, STATE_ABSENT, ROLE_NONE) } for (listener in addedListeners) { em.removeInterfaceStateListener(listener) } + networkRequests.forEach { cm.unregisterNetworkCallback(it) } } private fun addInterfaceStateListener(listener: EthernetStateListener) { @@ -215,6 +235,34 @@ class EthernetManagerTest { ifaceListener.eventuallyExpect(iface, STATE_ABSENT, ROLE_NONE) } + private fun requestNetwork(request: NetworkRequest): TestableNetworkCallback { + return TestableNetworkCallback().also { + cm.requestNetwork(request, it) + networkRequests.add(it) + } + } + + private fun releaseNetwork(cb: TestableNetworkCallback) { + cm.unregisterNetworkCallback(cb) + networkRequests.remove(cb) + } + + private fun NetworkRequest.createCopyWithEthernetSpecifier(ifaceName: String) = + NetworkRequest.Builder(NetworkRequest(ETH_REQUEST)) + .setNetworkSpecifier(EthernetNetworkSpecifier(ifaceName)).build() + + // It can take multiple seconds for the network to become available. + private fun TestableNetworkCallback.expectAvailable() = + expectCallback(anyNetwork(), 5000/*ms timeout*/).network + + // b/233534110: eventuallyExpect() does not advance ReadHead, use + // eventuallyExpect(Lost::class) instead. + private fun TestableNetworkCallback.eventuallyExpectLost(n: Network? = null) = + eventuallyExpect(Lost::class, TIMEOUT_MS) { n?.equals(it.network) ?: true } + + private fun TestableNetworkCallback.assertNotLost(n: Network? = null) = + assertNoCallbackThat() { it is Lost && (n?.equals(it.network) ?: true) } + @Test fun testCallbacks() { // If an interface exists when the callback is registered, it is reported on registration. @@ -294,4 +342,105 @@ class EthernetManagerTest { removeInterface(iface2) } + + @Test + fun testNetworkRequest_withSingleExistingInterface() { + setIncludeTestInterfaces(true) + createInterface() + + // install a listener which will later be used to verify the Lost callback + val listenerCb = TestableNetworkCallback() + cm.registerNetworkCallback(ETH_REQUEST, listenerCb) + networkRequests.add(listenerCb) + + val cb = requestNetwork(ETH_REQUEST) + val network = cb.expectAvailable() + + cb.assertNotLost() + releaseNetwork(cb) + listenerCb.eventuallyExpectLost(network) + } + + @Test + fun testNetworkRequest_beforeSingleInterfaceIsUp() { + setIncludeTestInterfaces(true) + + val cb = requestNetwork(ETH_REQUEST) + + // bring up interface after network has been requested + val iface = createInterface() + val network = cb.expectAvailable() + + // remove interface before network request has been removed + cb.assertNotLost() + removeInterface(iface) + cb.eventuallyExpectLost() + + releaseNetwork(cb) + } + + @Test + fun testNetworkRequest_withMultipleInterfaces() { + setIncludeTestInterfaces(true) + + val iface1 = createInterface() + val iface2 = createInterface() + + val cb = requestNetwork(ETH_REQUEST.createCopyWithEthernetSpecifier(iface2.interfaceName)) + + val network = cb.expectAvailable() + cb.expectCapabilitiesThat(network) { + it.networkSpecifier == EthernetNetworkSpecifier(iface2.interfaceName) + } + + removeInterface(iface1) + cb.assertNotLost() + removeInterface(iface2) + cb.eventuallyExpectLost() + + releaseNetwork(cb) + } + + @Test + fun testNetworkRequest_withInterfaceBeingReplaced() { + setIncludeTestInterfaces(true) + val iface1 = createInterface() + + val cb = requestNetwork(ETH_REQUEST) + val network = cb.expectAvailable() + + // create another network and verify the request sticks to the current network + val iface2 = createInterface() + cb.assertNotLost() + + // remove iface1 and verify the request brings up iface2 + removeInterface(iface1) + cb.eventuallyExpectLost(network) + val network2 = cb.expectAvailable() + + releaseNetwork(cb) + } + + @Test + fun testNetworkRequest_withMultipleInterfacesAndRequests() { + setIncludeTestInterfaces(true) + val iface1 = createInterface() + val iface2 = createInterface() + + val cb1 = requestNetwork(ETH_REQUEST.createCopyWithEthernetSpecifier(iface1.interfaceName)) + val cb2 = requestNetwork(ETH_REQUEST.createCopyWithEthernetSpecifier(iface2.interfaceName)) + val cb3 = requestNetwork(ETH_REQUEST) + + cb1.expectAvailable() + cb2.expectAvailable() + cb3.expectAvailable() + + cb1.assertNotLost() + cb2.assertNotLost() + cb3.assertNotLost() + + releaseNetwork(cb1) + releaseNetwork(cb2) + releaseNetwork(cb3) + } }