diff --git a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java index d7800c027a..b556f6c949 100644 --- a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java +++ b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java @@ -21,6 +21,7 @@ import android.annotation.Nullable; import android.content.Context; import android.net.ConnectivityManager; import android.net.EthernetNetworkSpecifier; +import android.net.IInternalNetworkManagementListener; import android.net.IpConfiguration; import android.net.IpConfiguration.IpAssignment; import android.net.IpConfiguration.ProxySettings; @@ -98,14 +99,13 @@ public class EthernetNetworkFactory extends NetworkFactory { } } - public EthernetNetworkFactory(Handler handler, Context context, NetworkCapabilities filter) { - this(handler, context, filter, new Dependencies()); + public EthernetNetworkFactory(Handler handler, Context context) { + this(handler, context, new Dependencies()); } @VisibleForTesting - EthernetNetworkFactory(Handler handler, Context context, NetworkCapabilities filter, - Dependencies deps) { - super(handler.getLooper(), context, NETWORK_TYPE, filter); + EthernetNetworkFactory(Handler handler, Context context, Dependencies deps) { + super(handler.getLooper(), context, NETWORK_TYPE, createDefaultNetworkCapabilities()); mHandler = handler; mContext = context; @@ -166,8 +166,9 @@ public class EthernetNetworkFactory extends NetworkFactory { .toArray(String[]::new); } - void addInterface(String ifaceName, String hwAddress, NetworkCapabilities capabilities, - IpConfiguration ipConfiguration) { + void addInterface(@NonNull final String ifaceName, @NonNull final String hwAddress, + @NonNull final IpConfiguration ipConfig, + @NonNull final NetworkCapabilities capabilities) { if (mTrackingInterfaces.containsKey(ifaceName)) { Log.e(TAG, "Interface with name " + ifaceName + " already exists."); return; @@ -177,14 +178,48 @@ public class EthernetNetworkFactory extends NetworkFactory { Log.d(TAG, "addInterface, iface: " + ifaceName + ", capabilities: " + capabilities); } - NetworkInterfaceState iface = new NetworkInterfaceState( - ifaceName, hwAddress, mHandler, mContext, capabilities, this, mDeps); - iface.setIpConfig(ipConfiguration); + final NetworkInterfaceState iface = new NetworkInterfaceState( + ifaceName, hwAddress, mHandler, mContext, ipConfig, capabilities, this, mDeps); mTrackingInterfaces.put(ifaceName, iface); - updateCapabilityFilter(); } + /** + * Update a network's configuration and restart it if necessary. + * + * @param ifaceName the interface name of the network to be updated. + * @param ipConfig the desired {@link IpConfiguration} for the given network. + * @param capabilities the desired {@link NetworkCapabilities} for the given network. If + * {@code null} is passed, then the network's current + * {@link NetworkCapabilities} will be used in support of existing APIs as + * the public API does not allow this. + * @param listener an optional {@link IInternalNetworkManagementListener} to notify callers of + * completion. + */ + @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) + protected void updateInterface(@NonNull final String ifaceName, + @NonNull final IpConfiguration ipConfig, + @Nullable final NetworkCapabilities capabilities, + @Nullable final IInternalNetworkManagementListener listener) { + enforceInterfaceIsTracked(ifaceName); + final NetworkInterfaceState iface = mTrackingInterfaces.get(ifaceName); + // TODO: The listener will have issues if called in quick succession for the same interface + // before the IP layer restarts. Update the listener logic to address multiple successive + // calls for a particular interface. + iface.mNetworkManagementListener = listener; + if (iface.updateInterface(ipConfig, capabilities)) { + mTrackingInterfaces.put(ifaceName, iface); + updateCapabilityFilter(); + } + } + + private void enforceInterfaceIsTracked(@NonNull final String ifaceName) { + if (!hasInterface(ifaceName)) { + throw new UnsupportedOperationException( + "Interface with name " + ifaceName + " is not being tracked."); + } + } + private static NetworkCapabilities mixInCapabilities(NetworkCapabilities nc, NetworkCapabilities addedNc) { final NetworkCapabilities.Builder builder = new NetworkCapabilities.Builder(nc); @@ -194,11 +229,7 @@ public class EthernetNetworkFactory extends NetworkFactory { } private void updateCapabilityFilter() { - NetworkCapabilities capabilitiesFilter = - NetworkCapabilities.Builder.withoutDefaultCapabilities() - .addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET) - .build(); - + NetworkCapabilities capabilitiesFilter = createDefaultNetworkCapabilities(); for (NetworkInterfaceState iface: mTrackingInterfaces.values()) { capabilitiesFilter = mixInCapabilities(capabilitiesFilter, iface.mCapabilities); } @@ -207,6 +238,12 @@ public class EthernetNetworkFactory extends NetworkFactory { setCapabilityFilter(capabilitiesFilter); } + private static NetworkCapabilities createDefaultNetworkCapabilities() { + return NetworkCapabilities.Builder + .withoutDefaultCapabilities() + .addTransportType(NetworkCapabilities.TRANSPORT_ETHERNET).build(); + } + void removeInterface(String interfaceName) { NetworkInterfaceState iface = mTrackingInterfaces.remove(interfaceName); if (iface != null) { @@ -230,15 +267,8 @@ public class EthernetNetworkFactory extends NetworkFactory { return iface.updateLinkState(up); } - boolean hasInterface(String interfacName) { - return mTrackingInterfaces.containsKey(interfacName); - } - - void updateIpConfiguration(String iface, IpConfiguration ipConfiguration) { - NetworkInterfaceState network = mTrackingInterfaces.get(iface); - if (network != null) { - network.setIpConfig(ipConfiguration); - } + boolean hasInterface(String ifaceName) { + return mTrackingInterfaces.containsKey(ifaceName); } private NetworkInterfaceState networkForRequest(NetworkRequest request) { @@ -272,24 +302,26 @@ public class EthernetNetworkFactory extends NetworkFactory { return network; } - private static class NetworkInterfaceState { + @VisibleForTesting + static class NetworkInterfaceState { final String name; private final String mHwAddress; - private final NetworkCapabilities mCapabilities; private final Handler mHandler; private final Context mContext; private final NetworkFactory mNetworkFactory; private final Dependencies mDeps; - private final int mLegacyType; private static String sTcpBufferSizes = null; // Lazy initialized. private boolean mLinkUp; + private int mLegacyType; private LinkProperties mLinkProperties = new LinkProperties(); private volatile @Nullable IpClientManager mIpClient; + private @NonNull NetworkCapabilities mCapabilities; private @Nullable IpClientCallbacksImpl mIpClientCallback; + private @Nullable IInternalNetworkManagementListener mNetworkManagementListener; private @Nullable EthernetNetworkAgent mNetworkAgent; private @Nullable IpConfiguration mIpConfig; @@ -362,42 +394,17 @@ public class EthernetNetworkFactory extends NetworkFactory { } NetworkInterfaceState(String ifaceName, String hwAddress, Handler handler, Context context, - @NonNull NetworkCapabilities capabilities, NetworkFactory networkFactory, - Dependencies deps) { + @NonNull IpConfiguration ipConfig, @NonNull NetworkCapabilities capabilities, + NetworkFactory networkFactory, Dependencies deps) { name = ifaceName; + mIpConfig = Objects.requireNonNull(ipConfig); mCapabilities = Objects.requireNonNull(capabilities); + mLegacyType = getLegacyType(mCapabilities); mHandler = handler; mContext = context; mNetworkFactory = networkFactory; mDeps = deps; - final int legacyType; - int[] transportTypes = mCapabilities.getTransportTypes(); - - if (transportTypes.length > 0) { - legacyType = getLegacyType(transportTypes[0]); - } else { - // Should never happen as transport is always one of ETHERNET or a valid override - throw new ConfigurationException("Network Capabilities do not have an associated " - + "transport type."); - } - mHwAddress = hwAddress; - mLegacyType = legacyType; - } - - void setIpConfig(IpConfiguration ipConfig) { - if (Objects.equals(this.mIpConfig, ipConfig)) { - if (DBG) Log.d(TAG, "ipConfig have not changed,so ignore setIpConfig"); - return; - } - this.mIpConfig = ipConfig; - if (mNetworkAgent != null) { - restart(); - } - } - - boolean isRestricted() { - return !mCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED); } /** @@ -408,6 +415,52 @@ public class EthernetNetworkFactory extends NetworkFactory { return sTransports.get(transport, ConnectivityManager.TYPE_NONE); } + private static int getLegacyType(@NonNull final NetworkCapabilities capabilities) { + final int[] transportTypes = capabilities.getTransportTypes(); + if (transportTypes.length > 0) { + return getLegacyType(transportTypes[0]); + } + + // Should never happen as transport is always one of ETHERNET or a valid override + throw new ConfigurationException("Network Capabilities do not have an associated " + + "transport type."); + } + + private void setCapabilities(@NonNull final NetworkCapabilities capabilities) { + mCapabilities = new NetworkCapabilities(capabilities); + mLegacyType = getLegacyType(mCapabilities); + } + + boolean updateInterface(@NonNull final IpConfiguration ipConfig, + @Nullable final NetworkCapabilities capabilities) { + final boolean shouldUpdateIpConfig = !Objects.equals(mIpConfig, ipConfig); + final boolean shouldUpdateCapabilities = null != capabilities + && !Objects.equals(mCapabilities, capabilities); + if (DBG) { + Log.d(TAG, "updateInterface, iface: " + name + + ", shouldUpdateIpConfig: " + shouldUpdateIpConfig + + ", shouldUpdateCapabilities: " + shouldUpdateCapabilities + + ", ipConfig: " + ipConfig + ", old ipConfig: " + mIpConfig + + ", capabilities: " + capabilities + ", old capabilities: " + mCapabilities + ); + } + + if (shouldUpdateIpConfig) { mIpConfig = ipConfig; }; + if (shouldUpdateCapabilities) { setCapabilities(capabilities); }; + if (shouldUpdateIpConfig || shouldUpdateCapabilities) { + // TODO: Update this logic to only do a restart if required. Although a restart may + // be required due to the capabilities or ipConfiguration values, not all + // capabilities changes require a restart. + restart(); + return true; + } + return false; + } + + boolean isRestricted() { + return !mCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_RESTRICTED); + } + private void start() { if (mIpClient != null) { if (DBG) Log.d(TAG, "IpClient already started"); @@ -459,6 +512,19 @@ public class EthernetNetworkFactory extends NetworkFactory { }); mNetworkAgent.register(); mNetworkAgent.markConnected(); + sendNetworkManagementCallback(); + } + + private void sendNetworkManagementCallback() { + if (null != mNetworkManagementListener) { + try { + mNetworkManagementListener.onComplete(mNetworkAgent.getNetwork(), null); + } catch (RemoteException e) { + Log.e(TAG, "Can't send onComplete for network management callback", e); + } finally { + mNetworkManagementListener = null; + } + } } void onIpLayerStopped(LinkProperties linkProperties) { @@ -546,7 +612,7 @@ public class EthernetNetworkFactory extends NetworkFactory { } void restart(){ - if (DBG) Log.d(TAG, "reconnecting Etherent"); + if (DBG) Log.d(TAG, "reconnecting Ethernet"); stop(); start(); } diff --git a/service-t/src/com/android/server/ethernet/EthernetService.java b/service-t/src/com/android/server/ethernet/EthernetService.java index 467ab677d8..492a55aff1 100644 --- a/service-t/src/com/android/server/ethernet/EthernetService.java +++ b/service-t/src/com/android/server/ethernet/EthernetService.java @@ -17,11 +17,16 @@ package com.android.server.ethernet; import android.content.Context; +import android.net.INetd; +import android.net.util.NetdService; import android.os.Handler; import android.os.HandlerThread; +import android.os.IBinder; import android.util.Log; import com.android.server.SystemService; +import java.util.Objects; + public final class EthernetService extends SystemService { private static final String TAG = "EthernetService"; @@ -32,9 +37,17 @@ public final class EthernetService extends SystemService { super(context); final HandlerThread handlerThread = new HandlerThread(THREAD_NAME); handlerThread.start(); + final Handler handler = handlerThread.getThreadHandler(); + final EthernetNetworkFactory factory = new EthernetNetworkFactory(handler, context); mImpl = new EthernetServiceImpl( - context, handlerThread.getThreadHandler(), - new EthernetTracker(context, handlerThread.getThreadHandler())); + context, handler, + new EthernetTracker(context, handler, factory, getNetd())); + } + + private INetd getNetd() { + final INetd netd = NetdService.getInstance(); + Objects.requireNonNull(netd, "could not get netd instance"); + return netd; } @Override diff --git a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java index f107865c30..b2844779c8 100644 --- a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java +++ b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java @@ -233,6 +233,9 @@ public class EthernetServiceImpl extends IEthernetManager.Stub { + ", request=" + request + ", listener=" + listener); validateNetworkManagementState(iface, "updateConfiguration()"); // TODO: validate that iface is listed in overlay config_ethernet_interfaces + + mTracker.updateConfiguration( + iface, request.getIpConfig(), request.getNetworkCapabilities(), listener); } @Override diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java index 9660194508..0b6547d145 100644 --- a/service-t/src/com/android/server/ethernet/EthernetTracker.java +++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java @@ -24,6 +24,7 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.content.Context; import android.net.IEthernetServiceListener; +import android.net.IInternalNetworkManagementListener; import android.net.INetd; import android.net.ITetheredInterfaceCallback; import android.net.InterfaceConfigurationParcel; @@ -34,7 +35,6 @@ import android.net.LinkAddress; import android.net.NetworkCapabilities; import android.net.StaticIpConfiguration; import android.os.Handler; -import android.os.IBinder; import android.os.RemoteCallbackList; import android.os.RemoteException; import android.os.ServiceSpecificException; @@ -118,14 +118,12 @@ public class EthernetTracker { } } - EthernetTracker(Context context, Handler handler) { + EthernetTracker(@NonNull final Context context, @NonNull final Handler handler, + @NonNull final EthernetNetworkFactory factory, @NonNull final INetd netd) { mContext = context; mHandler = handler; - - // The services we use. - mNetd = INetd.Stub.asInterface( - (IBinder) context.getSystemService(Context.NETD_SERVICE)); - Objects.requireNonNull(mNetd, "could not get netd instance"); + mFactory = factory; + mNetd = netd; // Interface match regex. updateIfaceMatchRegexp(); @@ -138,9 +136,6 @@ public class EthernetTracker { } mConfigStore = new EthernetConfigStore(); - - NetworkCapabilities nc = createNetworkCapabilities(true /* clear default capabilities */); - mFactory = new EthernetNetworkFactory(handler, context, nc); } void start() { @@ -168,11 +163,30 @@ public class EthernetTracker { if (DBG) { Log.i(TAG, "updateIpConfiguration, iface: " + iface + ", cfg: " + ipConfiguration); } + writeIpConfiguration(iface, ipConfiguration); + mHandler.post(() -> mFactory.updateInterface(iface, ipConfiguration, null, null)); + } - mConfigStore.write(iface, ipConfiguration); - mIpConfigurations.put(iface, ipConfiguration); + private void writeIpConfiguration(@NonNull final String iface, + @NonNull final IpConfiguration ipConfig) { + mConfigStore.write(iface, ipConfig); + mIpConfigurations.put(iface, ipConfig); + } - mHandler.post(() -> mFactory.updateIpConfiguration(iface, ipConfiguration)); + @VisibleForTesting(visibility = PACKAGE) + protected void updateConfiguration(@NonNull final String iface, + @NonNull final StaticIpConfiguration staticIpConfig, + @NonNull final NetworkCapabilities capabilities, + @Nullable final IInternalNetworkManagementListener listener) { + if (DBG) { + Log.i(TAG, "updateConfiguration, iface: " + iface + ", capabilities: " + capabilities + + ", staticIpConfig: " + staticIpConfig); + } + final IpConfiguration ipConfig = createIpConfiguration(staticIpConfig); + writeIpConfiguration(iface, ipConfig); + mNetworkCapabilities.put(iface, capabilities); + mHandler.post(() -> + mFactory.updateInterface(iface, ipConfig, capabilities, listener)); } IpConfiguration getIpConfiguration(String iface) { @@ -325,7 +339,7 @@ public class EthernetTracker { } Log.d(TAG, "Tracking interface in client mode: " + iface); - mFactory.addInterface(iface, hwAddress, nc, ipConfiguration); + mFactory.addInterface(iface, hwAddress, ipConfiguration, nc); } else { maybeUpdateServerModeInterfaceState(iface, true); } @@ -460,11 +474,11 @@ public class EthernetTracker { NetworkCapabilities nc = createNetworkCapabilities( !TextUtils.isEmpty(config.mCapabilities) /* clear default capabilities */, config.mCapabilities, config.mTransport).build(); - mNetworkCapabilities.put(config.mName, nc); + mNetworkCapabilities.put(config.mIface, nc); if (null != config.mIpConfig) { IpConfiguration ipConfig = parseStaticIpConfiguration(config.mIpConfig); - mIpConfigurations.put(config.mName, ipConfig); + mIpConfigurations.put(config.mIface, ipConfig); } } @@ -493,10 +507,6 @@ public class EthernetTracker { return builder.build(); } - private static NetworkCapabilities createNetworkCapabilities(boolean clearDefaultCapabilities) { - return createNetworkCapabilities(clearDefaultCapabilities, null, null).build(); - } - /** * Parses a static list of network capabilities * @@ -623,10 +633,15 @@ public class EthernetTracker { } } } + return createIpConfiguration(staticIpConfigBuilder.build()); + } + + static IpConfiguration createIpConfiguration( + @NonNull final StaticIpConfiguration staticIpConfig) { final IpConfiguration ret = new IpConfiguration(); ret.setIpAssignment(IpAssignment.STATIC); ret.setProxySettings(ProxySettings.NONE); - ret.setStaticIpConfiguration(staticIpConfigBuilder.build()); + ret.setStaticIpConfiguration(staticIpConfig); return ret; } @@ -681,14 +696,14 @@ public class EthernetTracker { @VisibleForTesting static class EthernetTrackerConfig { - final String mName; + final String mIface; final String mCapabilities; final String mIpConfig; final String mTransport; EthernetTrackerConfig(@NonNull final String[] tokens) { Objects.requireNonNull(tokens, "EthernetTrackerConfig requires non-null tokens"); - mName = tokens[0]; + mIface = tokens[0]; mCapabilities = tokens.length > 1 ? tokens[1] : null; mIpConfig = tokens.length > 2 && !TextUtils.isEmpty(tokens[2]) ? tokens[2] : null; mTransport = tokens.length > 3 ? tokens[3] : null; diff --git a/tests/ethernet/Android.bp b/tests/ethernet/Android.bp index 1bc9352f9b..93a8f6ca45 100644 --- a/tests/ethernet/Android.bp +++ b/tests/ethernet/Android.bp @@ -36,6 +36,8 @@ android_test { "ethernet-service", "frameworks-base-testutils", "mockito-target-minus-junit4", + "net-tests-utils", + "services.core", "services.net", ], } diff --git a/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java b/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java index 990ee5fee7..44ed26c9b4 100644 --- a/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java +++ b/tests/ethernet/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java @@ -16,9 +16,11 @@ package com.android.server.ethernet; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; 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; @@ -36,17 +38,24 @@ import android.content.Context; import android.content.res.Resources; import android.net.ConnectivityManager; import android.net.EthernetNetworkSpecifier; +import android.net.IInternalNetworkManagementListener; +import android.net.InternalNetworkManagementException; import android.net.IpConfiguration; +import android.net.LinkAddress; import android.net.LinkProperties; +import android.net.Network; import android.net.NetworkAgentConfig; import android.net.NetworkCapabilities; import android.net.NetworkProvider; import android.net.NetworkRequest; +import android.net.StaticIpConfiguration; import android.net.ip.IpClientCallbacks; import android.net.ip.IpClientManager; import android.os.Handler; +import android.os.IBinder; import android.os.Looper; import android.os.test.TestLooper; +import android.util.Pair; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; @@ -61,10 +70,20 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.Objects; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + @RunWith(AndroidJUnit4.class) @SmallTest public class EthernetNetworkFactoryTest { + private static final int TIMEOUT_MS = 2_000; private static final String TEST_IFACE = "test123"; + 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"; private final TestLooper mLooper = new TestLooper(); private Handler mHandler; private EthernetNetworkFactory mNetFactory = null; @@ -75,13 +94,13 @@ public class EthernetNetworkFactoryTest { @Mock private IpClientManager mIpClient; @Mock private EthernetNetworkAgent mNetworkAgent; @Mock private InterfaceParams mInterfaceParams; + @Mock private Network mMockNetwork; @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); mHandler = new Handler(mLooper.getLooper()); - mNetFactory = new EthernetNetworkFactory(mHandler, mContext, createDefaultFilterCaps(), - mDeps); + mNetFactory = new EthernetNetworkFactory(mHandler, mContext, mDeps); setupNetworkAgentMock(); setupIpClientMock(); @@ -100,6 +119,8 @@ public class EthernetNetworkFactoryTest { NetworkProvider provider, EthernetNetworkAgent.Callbacks cb) { when(mNetworkAgent.getCallbacks()).thenReturn(cb); + when(mNetworkAgent.getNetwork()) + .thenReturn(mMockNetwork); return mNetworkAgent; } } @@ -185,6 +206,19 @@ public class EthernetNetworkFactoryTest { return ipConfig; } + /** + * Create an {@link IpConfiguration} with an associated {@link StaticIpConfiguration}. + * + * @return {@link IpConfiguration} with its {@link StaticIpConfiguration} set. + */ + private IpConfiguration createStaticIpConfig() { + final IpConfiguration ipConfig = new IpConfiguration(); + ipConfig.setIpAssignment(IpConfiguration.IpAssignment.STATIC); + ipConfig.setStaticIpConfiguration( + new StaticIpConfiguration.Builder().setIpAddress(LINK_ADDR).build()); + return ipConfig; + } + // creates an interface with provisioning in progress (since updating the interface link state // automatically starts the provisioning process) private void createInterfaceUndergoingProvisioning(String iface) { @@ -194,10 +228,11 @@ public class EthernetNetworkFactoryTest { private void createInterfaceUndergoingProvisioning( @NonNull final String iface, final int transportType) { - mNetFactory.addInterface(iface, iface, createInterfaceCapsBuilder(transportType).build(), - createDefaultIpConfig()); + final IpConfiguration ipConfig = createDefaultIpConfig(); + mNetFactory.addInterface(iface, HW_ADDR, ipConfig, + createInterfaceCapsBuilder(transportType).build()); assertTrue(mNetFactory.updateInterfaceLinkState(iface, true)); - verifyStart(); + verifyStart(ipConfig); clearInvocations(mDeps); clearInvocations(mIpClient); } @@ -431,13 +466,19 @@ public class EthernetNetworkFactoryTest { triggerOnReachabilityLost(); // Reachability loss should trigger a stop and start, since the interface is still there - verifyStop(); - verifyStart(); + verifyRestart(createDefaultIpConfig()); } - private void verifyStart() { + private void verifyRestart(@NonNull final IpConfiguration ipConfig) { + verifyStop(); + verifyStart(ipConfig); + } + + private void verifyStart(@NonNull final IpConfiguration ipConfig) { verify(mDeps).makeIpClient(any(Context.class), anyString(), any()); - verify(mIpClient).startProvisioning(any()); + verify(mIpClient).startProvisioning( + argThat(x -> Objects.equals(x.mStaticIpConfig, ipConfig.getStaticIpConfiguration())) + ); } private void verifyStop() { @@ -449,4 +490,56 @@ public class EthernetNetworkFactoryTest { verify(mNetworkAgent).register(); verify(mNetworkAgent).markConnected(); } + + private static final class TestNetworkManagementListener + implements IInternalNetworkManagementListener { + private final CompletableFuture> mDone + = new CompletableFuture<>(); + + @Override + public void onComplete(final Network network, + final InternalNetworkManagementException exception) { + mDone.complete(new Pair<>(network, exception)); + } + + Pair expectOnComplete() throws Exception { + return mDone.get(TIMEOUT_MS, TimeUnit.MILLISECONDS); + } + + @Override + public IBinder asBinder() { + return null; + } + } + + @Test + public void testUpdateInterfaceCallsListenerCorrectlyOnSuccess() throws Exception { + createAndVerifyProvisionedInterface(TEST_IFACE); + final NetworkCapabilities capabilities = createDefaultFilterCaps(); + final IpConfiguration ipConfiguration = createStaticIpConfig(); + final TestNetworkManagementListener listener = new TestNetworkManagementListener(); + + mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities, listener); + triggerOnProvisioningSuccess(); + + final Pair ret = listener.expectOnComplete(); + assertEquals(mMockNetwork, ret.first); + assertNull(ret.second); + } + + @Test + public void testUpdateInterfaceRestartsAgentCorrectly() throws Exception { + createAndVerifyProvisionedInterface(TEST_IFACE); + final NetworkCapabilities capabilities = createDefaultFilterCaps(); + final IpConfiguration ipConfiguration = createStaticIpConfig(); + final TestNetworkManagementListener listener = new TestNetworkManagementListener(); + + mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities, listener); + triggerOnProvisioningSuccess(); + + listener.expectOnComplete(); + verify(mDeps).makeEthernetNetworkAgent(any(), any(), + eq(capabilities), any(), any(), any(), any()); + verifyRestart(ipConfiguration); + } } diff --git a/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java b/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java index 516e574522..1c0888398a 100644 --- a/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java +++ b/tests/ethernet/java/com/android/server/ethernet/EthernetServiceImplTest.java @@ -18,13 +18,17 @@ package com.android.server.ethernet; import static org.junit.Assert.assertThrows; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.verify; import android.annotation.NonNull; import android.content.Context; import android.content.pm.PackageManager; +import android.net.IInternalNetworkManagementListener; import android.net.InternalNetworkUpdateRequest; import android.net.IpConfiguration; +import android.net.NetworkCapabilities; import android.net.StaticIpConfiguration; import android.os.Handler; @@ -41,6 +45,10 @@ import org.mockito.MockitoAnnotations; @SmallTest public class EthernetServiceImplTest { private static final String TEST_IFACE = "test123"; + private static final InternalNetworkUpdateRequest UPDATE_REQUEST = + new InternalNetworkUpdateRequest( + new StaticIpConfiguration(), new NetworkCapabilities.Builder().build()); + private static final IInternalNetworkManagementListener NULL_LISTENER = null; private EthernetServiceImpl mEthernetServiceImpl; @Mock private Context mContext; @Mock private Handler mHandler; @@ -69,10 +77,8 @@ public class EthernetServiceImplTest { public void testUpdateConfigurationRejectsWhenEthNotStarted() { mEthernetServiceImpl.mStarted.set(false); assertThrows(IllegalStateException.class, () -> { - final InternalNetworkUpdateRequest r = - new InternalNetworkUpdateRequest(new StaticIpConfiguration(), null); - - mEthernetServiceImpl.updateConfiguration("" /* iface */, r, null /* listener */); + mEthernetServiceImpl.updateConfiguration( + "" /* iface */, UPDATE_REQUEST, null /* listener */); }); } @@ -95,24 +101,21 @@ public class EthernetServiceImplTest { @Test public void testUpdateConfigurationRejectsNullIface() { assertThrows(NullPointerException.class, () -> { - final InternalNetworkUpdateRequest r = - new InternalNetworkUpdateRequest(new StaticIpConfiguration(), null); - - mEthernetServiceImpl.updateConfiguration(null /* iface */, r, null /* listener */); + mEthernetServiceImpl.updateConfiguration(null, UPDATE_REQUEST, NULL_LISTENER); }); } @Test public void testConnectNetworkRejectsNullIface() { assertThrows(NullPointerException.class, () -> { - mEthernetServiceImpl.connectNetwork(null /* iface */, null /* listener */); + mEthernetServiceImpl.connectNetwork(null /* iface */, NULL_LISTENER); }); } @Test public void testDisconnectNetworkRejectsNullIface() { assertThrows(NullPointerException.class, () -> { - mEthernetServiceImpl.disconnectNetwork(null /* iface */, null /* listener */); + mEthernetServiceImpl.disconnectNetwork(null /* iface */, NULL_LISTENER); }); } @@ -120,10 +123,7 @@ public class EthernetServiceImplTest { public void testUpdateConfigurationRejectsWithoutAutomotiveFeature() { toggleAutomotiveFeature(false); assertThrows(UnsupportedOperationException.class, () -> { - final InternalNetworkUpdateRequest r = - new InternalNetworkUpdateRequest(new StaticIpConfiguration(), null); - - mEthernetServiceImpl.updateConfiguration("" /* iface */, r, null /* listener */); + mEthernetServiceImpl.updateConfiguration(TEST_IFACE, UPDATE_REQUEST, NULL_LISTENER); }); } @@ -131,7 +131,7 @@ public class EthernetServiceImplTest { public void testConnectNetworkRejectsWithoutAutomotiveFeature() { toggleAutomotiveFeature(false); assertThrows(UnsupportedOperationException.class, () -> { - mEthernetServiceImpl.connectNetwork("" /* iface */, null /* listener */); + mEthernetServiceImpl.connectNetwork("" /* iface */, NULL_LISTENER); }); } @@ -139,7 +139,7 @@ public class EthernetServiceImplTest { public void testDisconnectNetworkRejectsWithoutAutomotiveFeature() { toggleAutomotiveFeature(false); assertThrows(UnsupportedOperationException.class, () -> { - mEthernetServiceImpl.disconnectNetwork("" /* iface */, null /* listener */); + mEthernetServiceImpl.disconnectNetwork("" /* iface */, NULL_LISTENER); }); } @@ -152,10 +152,7 @@ public class EthernetServiceImplTest { public void testUpdateConfigurationRejectsWithUntrackedIface() { shouldTrackIface(TEST_IFACE, false); assertThrows(UnsupportedOperationException.class, () -> { - final InternalNetworkUpdateRequest r = - new InternalNetworkUpdateRequest(new StaticIpConfiguration(), null); - - mEthernetServiceImpl.updateConfiguration(TEST_IFACE, r, null /* listener */); + mEthernetServiceImpl.updateConfiguration(TEST_IFACE, UPDATE_REQUEST, NULL_LISTENER); }); } @@ -163,7 +160,7 @@ public class EthernetServiceImplTest { public void testConnectNetworkRejectsWithUntrackedIface() { shouldTrackIface(TEST_IFACE, false); assertThrows(UnsupportedOperationException.class, () -> { - mEthernetServiceImpl.connectNetwork(TEST_IFACE, null /* listener */); + mEthernetServiceImpl.connectNetwork(TEST_IFACE, NULL_LISTENER); }); } @@ -171,11 +168,20 @@ public class EthernetServiceImplTest { public void testDisconnectNetworkRejectsWithUntrackedIface() { shouldTrackIface(TEST_IFACE, false); assertThrows(UnsupportedOperationException.class, () -> { - mEthernetServiceImpl.disconnectNetwork(TEST_IFACE, null /* listener */); + mEthernetServiceImpl.disconnectNetwork(TEST_IFACE, NULL_LISTENER); }); } 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); + verify(mEthernetTracker).updateConfiguration( + eq(TEST_IFACE), + eq(UPDATE_REQUEST.getIpConfig()), + eq(UPDATE_REQUEST.getNetworkCapabilities()), eq(NULL_LISTENER)); + } } diff --git a/tests/ethernet/java/com/android/server/ethernet/EthernetTrackerTest.java b/tests/ethernet/java/com/android/server/ethernet/EthernetTrackerTest.java index 6ea2154337..5aca2e40a0 100644 --- a/tests/ethernet/java/com/android/server/ethernet/EthernetTrackerTest.java +++ b/tests/ethernet/java/com/android/server/ethernet/EthernetTrackerTest.java @@ -19,20 +19,35 @@ 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.eq; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.verify; +import android.content.Context; +import android.content.res.Resources; import android.net.InetAddresses; +import android.net.IInternalNetworkManagementListener; +import android.net.INetd; import android.net.IpConfiguration; import android.net.IpConfiguration.IpAssignment; import android.net.IpConfiguration.ProxySettings; import android.net.LinkAddress; import android.net.NetworkCapabilities; import android.net.StaticIpConfiguration; +import android.os.HandlerThread; import androidx.test.filters.SmallTest; import androidx.test.runner.AndroidJUnit4; +import com.android.internal.R; +import com.android.testutils.HandlerUtils; + +import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; import java.net.InetAddress; import java.util.ArrayList; @@ -40,6 +55,39 @@ import java.util.ArrayList; @SmallTest @RunWith(AndroidJUnit4.class) public class EthernetTrackerTest { + private static final int TIMEOUT_MS = 1_000; + private static final String THREAD_NAME = "EthernetServiceThread"; + private EthernetTracker tracker; + private HandlerThread mHandlerThread; + @Mock private Context mContext; + @Mock private EthernetNetworkFactory mFactory; + @Mock private INetd mNetd; + @Mock Resources mResources; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + initMockResources(); + mHandlerThread = new HandlerThread(THREAD_NAME); + mHandlerThread.start(); + tracker = new EthernetTracker(mContext, mHandlerThread.getThreadHandler(), mFactory, mNetd); + } + + @After + public void cleanUp() { + mHandlerThread.quitSafely(); + } + + private void initMockResources() { + doReturn("").when(mResources).getString(R.string.config_ethernet_iface_regex); + doReturn(new String[0]).when(mResources).getStringArray(R.array.config_ethernet_interfaces); + doReturn(mResources).when(mContext).getResources(); + } + + private void waitForIdle() { + HandlerUtils.waitForIdle(mHandlerThread, TIMEOUT_MS); + } + /** * Test: Creation of various valid static IP configurations */ @@ -246,16 +294,16 @@ public class EthernetTrackerTest { @Test public void testCreateEthernetTrackerConfigReturnsCorrectValue() { - final String name = "1"; + final String iface = "1"; final String capabilities = "2"; final String ipConfig = "3"; final String transport = "4"; - final String configString = String.join(";", name, capabilities, ipConfig, transport); + final String configString = String.join(";", iface, capabilities, ipConfig, transport); final EthernetTracker.EthernetTrackerConfig config = EthernetTracker.createEthernetTrackerConfig(configString); - assertEquals(name, config.mName); + assertEquals(iface, config.mIface); assertEquals(capabilities, config.mCapabilities); assertEquals(ipConfig, config.mIpConfig); assertEquals(transport, config.mTransport); @@ -266,4 +314,19 @@ public class EthernetTrackerTest { assertThrows(NullPointerException.class, () -> EthernetTracker.createEthernetTrackerConfig(null)); } + + @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); + waitForIdle(); + + verify(mFactory).updateInterface( + eq(iface), eq(EthernetTracker.createIpConfiguration(staticIpConfig)), + eq(capabilities), eq(listener)); + } }