diff options
6 files changed, 163 insertions, 41 deletions
diff --git a/java/com/android/server/ethernet/EthernetNetworkFactory.java b/java/com/android/server/ethernet/EthernetNetworkFactory.java index c610f00..0832b50 100644 --- a/java/com/android/server/ethernet/EthernetNetworkFactory.java +++ b/java/com/android/server/ethernet/EthernetNetworkFactory.java @@ -252,8 +252,13 @@ public class EthernetNetworkFactory extends NetworkFactory { } /** Returns true if state has been modified */ - boolean updateInterfaceLinkState(String ifaceName, boolean up) { + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) + protected boolean updateInterfaceLinkState(@NonNull final String ifaceName, final boolean up, + @Nullable final IInternalNetworkManagementListener listener) { if (!mTrackingInterfaces.containsKey(ifaceName)) { + maybeSendNetworkManagementCallback(listener, null, + new InternalNetworkManagementException( + ifaceName + " can't be updated as it is not available.")); return false; } @@ -262,7 +267,7 @@ public class EthernetNetworkFactory extends NetworkFactory { } NetworkInterfaceState iface = mTrackingInterfaces.get(ifaceName); - return iface.updateLinkState(up); + return iface.updateLinkState(up, listener); } boolean hasInterface(String ifaceName) { @@ -615,16 +620,27 @@ public class EthernetNetworkFactory extends NetworkFactory { } /** Returns true if state has been modified */ - boolean updateLinkState(boolean up) { - if (mLinkUp == up) return false; + boolean updateLinkState(final boolean up, + @Nullable final IInternalNetworkManagementListener listener) { + if (mLinkUp == up) { + EthernetNetworkFactory.maybeSendNetworkManagementCallback(listener, null, + new InternalNetworkManagementException( + "No changes with requested link state " + up + " for " + name)); + return false; + } mLinkUp = up; if (!up) { // was up, goes down + // Save an instance of the current network to use with the callback before stop(). + final Network network = mNetworkAgent != null ? mNetworkAgent.getNetwork() : null; + // Send an abort on a provisioning request callback if necessary before stopping. maybeSendNetworkManagementCallbackForAbort(); stop(); + // If only setting the interface down, send a callback to signal completion. + EthernetNetworkFactory.maybeSendNetworkManagementCallback(listener, network, null); } else { // was down, goes up stop(); - start(); + start(listener); } return true; diff --git a/java/com/android/server/ethernet/EthernetServiceImpl.java b/java/com/android/server/ethernet/EthernetServiceImpl.java index b284477..6f6a04a 100644 --- a/java/com/android/server/ethernet/EthernetServiceImpl.java +++ b/java/com/android/server/ethernet/EthernetServiceImpl.java @@ -243,6 +243,7 @@ public class EthernetServiceImpl extends IEthernetManager.Stub { @Nullable final IInternalNetworkManagementListener listener) { Log.i(TAG, "connectNetwork called with: iface=" + iface + ", listener=" + listener); validateNetworkManagementState(iface, "connectNetwork()"); + mTracker.connectNetwork(iface, listener); } @Override @@ -250,5 +251,6 @@ public class EthernetServiceImpl extends IEthernetManager.Stub { @Nullable final IInternalNetworkManagementListener listener) { Log.i(TAG, "disconnectNetwork called with: iface=" + iface + ", listener=" + listener); validateNetworkManagementState(iface, "disconnectNetwork()"); + mTracker.disconnectNetwork(iface, listener); } } diff --git a/java/com/android/server/ethernet/EthernetTracker.java b/java/com/android/server/ethernet/EthernetTracker.java index 53af955..9784512 100644 --- a/java/com/android/server/ethernet/EthernetTracker.java +++ b/java/com/android/server/ethernet/EthernetTracker.java @@ -190,6 +190,18 @@ public class EthernetTracker { mFactory.updateInterface(iface, ipConfig, capabilities, listener)); } + @VisibleForTesting(visibility = PACKAGE) + protected void connectNetwork(@NonNull final String iface, + @Nullable final IInternalNetworkManagementListener listener) { + mHandler.post(() -> updateInterfaceState(iface, true, listener)); + } + + @VisibleForTesting(visibility = PACKAGE) + protected void disconnectNetwork(@NonNull final String iface, + @Nullable final IInternalNetworkManagementListener listener) { + mHandler.post(() -> updateInterfaceState(iface, false, listener)); + } + IpConfiguration getIpConfiguration(String iface) { return mIpConfigurations.get(iface); } @@ -354,9 +366,14 @@ public class EthernetTracker { } private void updateInterfaceState(String iface, boolean up) { + updateInterfaceState(iface, up, null /* listener */); + } + + private void updateInterfaceState(@NonNull final String iface, final boolean up, + @Nullable final IInternalNetworkManagementListener listener) { final int mode = getInterfaceMode(iface); final boolean factoryLinkStateUpdated = (mode == INTERFACE_MODE_CLIENT) - && mFactory.updateInterfaceLinkState(iface, up); + && mFactory.updateInterfaceLinkState(iface, up, listener); if (factoryLinkStateUpdated) { boolean restricted = isRestrictedInterface(iface); diff --git a/tests/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java b/tests/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java index d569990..b8f0ada 100644 --- a/tests/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java +++ b/tests/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java @@ -23,7 +23,6 @@ import static org.junit.Assert.assertFalse; 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.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; @@ -84,6 +83,7 @@ import java.util.concurrent.TimeUnit; public class EthernetNetworkFactoryTest { private static final int TIMEOUT_MS = 2_000; private static final String TEST_IFACE = "test123"; + private static final IInternalNetworkManagementListener NULL_LISTENER = null; private static final String IP_ADDR = "192.0.2.2/25"; private static final LinkAddress LINK_ADDR = new LinkAddress(IP_ADDR); private static final String HW_ADDR = "01:02:03:04:05:06"; @@ -238,7 +238,7 @@ public class EthernetNetworkFactoryTest { final IpConfiguration ipConfig = createDefaultIpConfig(); mNetFactory.addInterface(iface, HW_ADDR, ipConfig, createInterfaceCapsBuilder(transportType).build()); - assertTrue(mNetFactory.updateInterfaceLinkState(iface, true)); + assertTrue(mNetFactory.updateInterfaceLinkState(iface, true, NULL_LISTENER)); verifyStart(ipConfig); clearInvocations(mDeps); clearInvocations(mIpClient); @@ -309,36 +309,74 @@ public class EthernetNetworkFactoryTest { public void testUpdateInterfaceLinkStateForActiveProvisioningInterface() throws Exception { initEthernetNetworkFactory(); createInterfaceUndergoingProvisioning(TEST_IFACE); + final TestNetworkManagementListener listener = new TestNetworkManagementListener(); + // verify that the IpClient gets shut down when interface state changes to down. - assertTrue(mNetFactory.updateInterfaceLinkState(TEST_IFACE, false)); + final boolean ret = + mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */, listener); + + assertTrue(ret); verify(mIpClient).shutdown(); + assertSuccessfulListener(listener, null); } @Test public void testUpdateInterfaceLinkStateForProvisionedInterface() throws Exception { initEthernetNetworkFactory(); createAndVerifyProvisionedInterface(TEST_IFACE); - assertTrue(mNetFactory.updateInterfaceLinkState(TEST_IFACE, false)); + final TestNetworkManagementListener listener = new TestNetworkManagementListener(); + + final boolean ret = + mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */, listener); + + assertTrue(ret); verifyStop(); + assertSuccessfulListener(listener, mMockNetwork); } @Test public void testUpdateInterfaceLinkStateForUnprovisionedInterface() throws Exception { initEthernetNetworkFactory(); createUnprovisionedInterface(TEST_IFACE); - assertTrue(mNetFactory.updateInterfaceLinkState(TEST_IFACE, false)); + final TestNetworkManagementListener listener = new TestNetworkManagementListener(); + + final boolean ret = + mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */, listener); + + assertTrue(ret); // There should not be an active IPClient or NetworkAgent. verify(mDeps, never()).makeIpClient(any(), any(), any()); verify(mDeps, never()) .makeEthernetNetworkAgent(any(), any(), any(), any(), any(), any(), any()); + assertSuccessfulListener(listener, null); } @Test public void testUpdateInterfaceLinkStateForNonExistingInterface() throws Exception { initEthernetNetworkFactory(); + final TestNetworkManagementListener listener = new TestNetworkManagementListener(); + // if interface was never added, link state cannot be updated. - assertFalse(mNetFactory.updateInterfaceLinkState("eth1", true)); - verify(mDeps, never()).makeIpClient(any(), any(), any()); + final boolean ret = + mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */, listener); + + assertFalse(ret); + verifyNoStopOrStart(); + assertFailedListener(listener, "can't be updated as it is not configured"); + } + + @Test + public void testUpdateInterfaceLinkStateWithNoChanges() throws Exception { + initEthernetNetworkFactory(); + createAndVerifyProvisionedInterface(TEST_IFACE); + final TestNetworkManagementListener listener = new TestNetworkManagementListener(); + + final boolean ret = + mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */, listener); + + assertFalse(ret); + verifyNoStopOrStart(); + assertFailedListener(listener, "No changes"); } @Test @@ -394,6 +432,10 @@ public class EthernetNetworkFactoryTest { // the interface disappeared and getNetworkInterfaceByName returns null, we should not retry verify(mIpClient, never()).startProvisioning(any()); + verifyNoStopOrStart(); + } + + private void verifyNoStopOrStart() { verify(mNetworkAgent, never()).register(); verify(mIpClient, never()).shutdown(); verify(mNetworkAgent, never()).unregister(); @@ -404,7 +446,7 @@ public class EthernetNetworkFactoryTest { public void testIpClientIsNotStartedWhenLinkIsDown() throws Exception { initEthernetNetworkFactory(); createUnprovisionedInterface(TEST_IFACE); - mNetFactory.updateInterfaceLinkState(TEST_IFACE, false); + mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, NULL_LISTENER); mNetFactory.needNetworkFor(createDefaultRequest()); @@ -534,7 +576,7 @@ public class EthernetNetworkFactoryTest { // verifies onLinkPropertiesChange doesn't complete execution for a downed interface. // Posting is necessary as updateInterfaceLinkState with false will set mIpClientCallbacks // to null which will throw an NPE in the test if executed synchronously. - mHandler.post(() -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false)); + mHandler.post(() -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, NULL_LISTENER)); mIpClientCallbacks.onLinkPropertiesChange(lp); mLooper.dispatchAll(); @@ -552,7 +594,7 @@ public class EthernetNetworkFactoryTest { // verifies onReachabilityLost doesn't complete execution for a downed interface. // Posting is necessary as updateInterfaceLinkState with false will set mIpClientCallbacks // to null which will throw an NPE in the test if executed synchronously. - mHandler.post(() -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false)); + mHandler.post(() -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, NULL_LISTENER)); mIpClientCallbacks.onReachabilityLost("Neighbor Lost"); mLooper.dispatchAll(); @@ -616,9 +658,7 @@ public class EthernetNetworkFactoryTest { mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities, listener); triggerOnProvisioningSuccess(); - final Pair<Network, InternalNetworkManagementException> ret = listener.expectOnComplete(); - assertEquals(mMockNetwork, ret.first); - assertNull(ret.second); + assertSuccessfulListener(listener, mMockNetwork); } @DevSdkIgnoreRule.IgnoreUpTo(SC_V2) // TODO: Use to Build.VERSION_CODES.SC_V2 when available @@ -636,7 +676,7 @@ public class EthernetNetworkFactoryTest { initEthernetNetworkFactory(); verifyNetworkManagementCallIsAbortedWhenInterrupted( TEST_IFACE, - () -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false)); + () -> mNetFactory.updateInterfaceLinkState(TEST_IFACE, false, NULL_LISTENER)); } @DevSdkIgnoreRule.IgnoreUpTo(SC_V2) // TODO: Use to Build.VERSION_CODES.SC_V2 when available @@ -658,12 +698,28 @@ public class EthernetNetworkFactoryTest { triggerOnProvisioningSuccess(); }); + assertSuccessfulListener(successfulListener, mMockNetwork); + } + + private void assertSuccessfulListener( + @NonNull final TestNetworkManagementListener successfulListener, + @NonNull final Network expectedNetwork) throws Exception { final Pair<Network, InternalNetworkManagementException> successfulResult = successfulListener.expectOnComplete(); - assertEquals(mMockNetwork, successfulResult.first); + assertEquals(expectedNetwork, successfulResult.first); assertNull(successfulResult.second); } + private void assertFailedListener(@NonNull final TestNetworkManagementListener failedListener, + @NonNull final String errMsg) + throws Exception { + final Pair<Network, InternalNetworkManagementException> failedResult = + failedListener.expectOnComplete(); + assertNull(failedResult.first); + assertNotNull(failedResult.second); + assertTrue(failedResult.second.getMessage().contains(errMsg)); + } + private void verifyNetworkManagementCallIsAbortedWhenInterrupted( @NonNull final String iface, @NonNull final Runnable interruptingRunnable) throws Exception { @@ -676,11 +732,7 @@ public class EthernetNetworkFactoryTest { mNetFactory.updateInterface(iface, ipConfiguration, capabilities, failedListener); interruptingRunnable.run(); - final Pair<Network, InternalNetworkManagementException> failedResult = - failedListener.expectOnComplete(); - assertNull(failedResult.first); - assertNotNull(failedResult.second); - assertTrue(failedResult.second.getMessage().contains("aborted")); + assertFailedListener(failedListener, "aborted"); } @Test diff --git a/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java b/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java index 1c08883..6593aa5 100644 --- a/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java +++ b/tests/java/com/android/server/ethernet/EthernetServiceImplTest.java @@ -65,6 +65,15 @@ public class EthernetServiceImplTest { shouldTrackIface(TEST_IFACE, true); } + private void toggleAutomotiveFeature(final boolean isEnabled) { + doReturn(isEnabled) + .when(mPackageManager).hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE); + } + + private void shouldTrackIface(@NonNull final String iface, final boolean shouldTrack) { + doReturn(shouldTrack).when(mEthernetTracker).isTrackingInterface(iface); + } + @Test public void testSetConfigurationRejectsWhenEthNotStarted() { mEthernetServiceImpl.mStarted.set(false); @@ -143,11 +152,6 @@ public class EthernetServiceImplTest { }); } - private void toggleAutomotiveFeature(final boolean isEnabled) { - doReturn(isEnabled) - .when(mPackageManager).hasSystemFeature(PackageManager.FEATURE_AUTOMOTIVE); - } - @Test public void testUpdateConfigurationRejectsWithUntrackedIface() { shouldTrackIface(TEST_IFACE, false); @@ -172,10 +176,6 @@ public class EthernetServiceImplTest { }); } - private void shouldTrackIface(@NonNull final String iface, final boolean shouldTrack) { - doReturn(shouldTrack).when(mEthernetTracker).isTrackingInterface(iface); - } - @Test public void testUpdateConfiguration() { mEthernetServiceImpl.updateConfiguration(TEST_IFACE, UPDATE_REQUEST, NULL_LISTENER); @@ -184,4 +184,16 @@ public class EthernetServiceImplTest { eq(UPDATE_REQUEST.getIpConfig()), eq(UPDATE_REQUEST.getNetworkCapabilities()), eq(NULL_LISTENER)); } + + @Test + public void testConnectNetwork() { + mEthernetServiceImpl.connectNetwork(TEST_IFACE, NULL_LISTENER); + verify(mEthernetTracker).connectNetwork(eq(TEST_IFACE), eq(NULL_LISTENER)); + } + + @Test + public void testDisconnectNetwork() { + mEthernetServiceImpl.disconnectNetwork(TEST_IFACE, NULL_LISTENER); + verify(mEthernetTracker).disconnectNetwork(eq(TEST_IFACE), eq(NULL_LISTENER)); + } } diff --git a/tests/java/com/android/server/ethernet/EthernetTrackerTest.java b/tests/java/com/android/server/ethernet/EthernetTrackerTest.java index 5aca2e4..5087a9b 100644 --- a/tests/java/com/android/server/ethernet/EthernetTrackerTest.java +++ b/tests/java/com/android/server/ethernet/EthernetTrackerTest.java @@ -19,9 +19,13 @@ package com.android.server.ethernet; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import android.content.Context; import android.content.res.Resources; @@ -55,8 +59,10 @@ import java.util.ArrayList; @SmallTest @RunWith(AndroidJUnit4.class) public class EthernetTrackerTest { + private static final String TEST_IFACE = "test123"; private static final int TIMEOUT_MS = 1_000; private static final String THREAD_NAME = "EthernetServiceThread"; + private static final IInternalNetworkManagementListener NULL_LISTENER = null; private EthernetTracker tracker; private HandlerThread mHandlerThread; @Mock private Context mContext; @@ -68,6 +74,7 @@ public class EthernetTrackerTest { public void setUp() { MockitoAnnotations.initMocks(this); initMockResources(); + when(mFactory.updateInterfaceLinkState(anyString(), anyBoolean(), any())).thenReturn(false); mHandlerThread = new HandlerThread(THREAD_NAME); mHandlerThread.start(); tracker = new EthernetTracker(mContext, mHandlerThread.getThreadHandler(), mFactory, mNetd); @@ -294,16 +301,15 @@ public class EthernetTrackerTest { @Test public void testCreateEthernetTrackerConfigReturnsCorrectValue() { - final String iface = "1"; final String capabilities = "2"; final String ipConfig = "3"; final String transport = "4"; - final String configString = String.join(";", iface, capabilities, ipConfig, transport); + final String configString = String.join(";", TEST_IFACE, capabilities, ipConfig, transport); final EthernetTracker.EthernetTrackerConfig config = EthernetTracker.createEthernetTrackerConfig(configString); - assertEquals(iface, config.mIface); + assertEquals(TEST_IFACE, config.mIface); assertEquals(capabilities, config.mCapabilities); assertEquals(ipConfig, config.mIpConfig); assertEquals(transport, config.mTransport); @@ -317,16 +323,33 @@ public class EthernetTrackerTest { @Test public void testUpdateConfiguration() { - final String iface = "testiface"; final NetworkCapabilities capabilities = new NetworkCapabilities.Builder().build(); final StaticIpConfiguration staticIpConfig = new StaticIpConfiguration(); final IInternalNetworkManagementListener listener = null; - tracker.updateConfiguration(iface, staticIpConfig, capabilities, listener); + tracker.updateConfiguration(TEST_IFACE, staticIpConfig, capabilities, listener); waitForIdle(); verify(mFactory).updateInterface( - eq(iface), eq(EthernetTracker.createIpConfiguration(staticIpConfig)), + eq(TEST_IFACE), eq(EthernetTracker.createIpConfiguration(staticIpConfig)), eq(capabilities), eq(listener)); } + + @Test + public void testConnectNetworkCorrectlyCallsFactory() { + tracker.connectNetwork(TEST_IFACE, NULL_LISTENER); + waitForIdle(); + + verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(true /* up */), + eq(NULL_LISTENER)); + } + + @Test + public void testDisconnectNetworkCorrectlyCallsFactory() { + tracker.disconnectNetwork(TEST_IFACE, NULL_LISTENER); + waitForIdle(); + + verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(false /* up */), + eq(NULL_LISTENER)); + } } |