diff --git a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java index 1ac99e4649..1b9f720210 100644 --- a/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java +++ b/service-t/src/com/android/server/ethernet/EthernetNetworkFactory.java @@ -130,23 +130,26 @@ public class EthernetNetworkFactory { * Returns an array of available interface names. The array is sorted: unrestricted interfaces * goes first, then sorted by name. */ - @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) - protected String[] getAvailableInterfaces(boolean includeRestricted) { + public String[] getInterfacesSorted(boolean includeRestricted) { return mTrackingInterfaces.values() .stream() .filter(iface -> !iface.isRestricted() || includeRestricted) .sorted((iface1, iface2) -> { - int r = Boolean.compare(iface1.isRestricted(), iface2.isRestricted()); - return r == 0 ? iface1.name.compareTo(iface2.name) : r; + final int r = Boolean.compare(iface1.isRestricted(), iface2.isRestricted()); + final String ifname1 = iface1.getPort().getInterfaceName(); + final String ifname2 = iface2.getPort().getInterfaceName(); + return r == 0 ? ifname1.compareTo(ifname2) : r; }) - .map(iface -> iface.name) + .map(iface -> iface.getPort().getInterfaceName()) .toArray(String[]::new); } @VisibleForTesting(visibility = VisibleForTesting.Visibility.PACKAGE) - protected void addInterface(@NonNull final String ifaceName, @NonNull final String hwAddress, - @NonNull final IpConfiguration ipConfig, - @NonNull final NetworkCapabilities capabilities) { + protected void addInterface(EthernetPort port, IpConfiguration ipConfig, + NetworkCapabilities capabilities) { + final String ifaceName = port.getInterfaceName(); + final String hwAddress = port.getMacAddress().toString(); + if (mTrackingInterfaces.containsKey(ifaceName)) { Log.e(TAG, "Interface with name " + ifaceName + " already exists."); return; @@ -161,7 +164,7 @@ public class EthernetNetworkFactory { } final NetworkInterfaceState iface = new NetworkInterfaceState( - ifaceName, hwAddress, mHandler, mContext, ipConfig, nc, mProvider, mDeps); + port, mHandler, mContext, ipConfig, nc, mProvider, mDeps); mTrackingInterfaces.put(ifaceName, iface); } @@ -244,14 +247,12 @@ public class EthernetNetworkFactory { } NetworkInterfaceState iface = mTrackingInterfaces.get(ifaceName); - return iface.mHwAddress; + return iface.getPort().getMacAddress().toString(); } @VisibleForTesting static class NetworkInterfaceState { - final String name; - - private final String mHwAddress; + private final EthernetPort mPort; private final Handler mHandler; private final Context mContext; private final NetworkProvider mNetworkProvider; @@ -363,7 +364,7 @@ public class EthernetNetworkFactory { return; } if (DBG) { - Log.d(TAG, String.format("%s: onNetworkNeeded for request: %s", name, request)); + Log.d(TAG, String.format("%s: onNetworkNeeded: %s", mPort, request)); } // When the network offer is first registered, onNetworkNeeded is called with all // existing requests. @@ -380,8 +381,7 @@ public class EthernetNetworkFactory { return; } if (DBG) { - Log.d(TAG, - String.format("%s: onNetworkUnneeded for request: %s", name, request)); + Log.d(TAG, String.format("%s: onNetworkUnneeded: %s", mPort, request)); } if (!mRequestIds.remove(request.requestId)) { // This can only happen if onNetworkNeeded was not called for a request or if @@ -395,10 +395,10 @@ public class EthernetNetworkFactory { } } - NetworkInterfaceState(String ifaceName, String hwAddress, Handler handler, Context context, + NetworkInterfaceState(EthernetPort port, Handler handler, Context context, @NonNull IpConfiguration ipConfig, @NonNull NetworkCapabilities capabilities, NetworkProvider networkProvider, Dependencies deps) { - name = ifaceName; + mPort = port; mIpConfig = Objects.requireNonNull(ipConfig); mCapabilities = Objects.requireNonNull(capabilities); mLegacyType = getLegacyType(mCapabilities); @@ -406,7 +406,11 @@ public class EthernetNetworkFactory { mContext = context; mNetworkProvider = networkProvider; mDeps = deps; - mHwAddress = hwAddress; + } + + /** Returns the EthernetPort object */ + public EthernetPort getPort() { + return mPort; } /** @@ -446,7 +450,7 @@ public class EthernetNetworkFactory { void updateInterface(@Nullable final IpConfiguration ipConfig, @Nullable final NetworkCapabilities capabilities) { if (DBG) { - Log.d(TAG, "updateInterface, iface: " + name + Log.d(TAG, "updateInterface, port: " + mPort + ", ipConfig: " + ipConfig + ", old ipConfig: " + mIpConfig + ", capabilities: " + capabilities + ", old capabilities: " + mCapabilities ); @@ -474,11 +478,11 @@ public class EthernetNetworkFactory { return; } if (DBG) { - Log.d(TAG, String.format("Starting Ethernet IpClient(%s)", name)); + Log.d(TAG, String.format("Starting Ethernet IpClient(%s)", mPort)); } mIpClientCallback = new EthernetIpClientCallback(); - mDeps.makeIpClient(mContext, name, mIpClientCallback); + mDeps.makeIpClient(mContext, mPort.getInterfaceName(), mIpClientCallback); mIpClientCallback.awaitIpClientStart(); if (mIpConfig.getProxySettings() == ProxySettings.STATIC @@ -508,7 +512,7 @@ public class EthernetNetworkFactory { final NetworkAgentConfig config = new NetworkAgentConfig.Builder() .setLegacyType(mLegacyType) .setLegacyTypeName(NETWORK_TYPE) - .setLegacyExtraInfo(mHwAddress) + .setLegacyExtraInfo(mPort.getMacAddress().toString()) .build(); mNetworkAgent = mDeps.makeEthernetNetworkAgent(mContext, mHandler.getLooper(), mCapabilities, mLinkProperties, config, mNetworkProvider, @@ -534,8 +538,9 @@ public class EthernetNetworkFactory { // There is no point in continuing if the interface is gone as stop() will be triggered // by removeInterface() when processed on the handler thread and start() won't // work for a non-existent interface. - if (null == mDeps.getNetworkInterfaceByName(name)) { - if (DBG) Log.d(TAG, name + " is no longer available."); + // TODO: consider removing this functionality entirely and maybeRestart() regardless. + if (null == mDeps.getNetworkInterfaceByName(mPort.getInterfaceName())) { + if (DBG) Log.d(TAG, mPort + " is no longer available."); // Send a callback in case a provisioning request was in progress. return; } @@ -644,7 +649,7 @@ public class EthernetNetworkFactory { // possible that link disappeared in the meantime. In that // case, stop() has already been called and IpClient should not // get restarted to prevent a provisioning failure loop. - Log.i(TAG, String.format("maybeRestart() called on stopped interface %s", name)); + Log.i(TAG, String.format("maybeRestart() called on stopped interface %s", mPort)); return; } if (DBG) Log.d(TAG, "restart IpClient"); @@ -655,9 +660,8 @@ public class EthernetNetworkFactory { @Override public String toString() { return getClass().getSimpleName() + "{ " - + "iface: " + name + ", " + + "port: " + mPort + ", " + "up: " + mLinkUp + ", " - + "hwAddress: " + mHwAddress + ", " + "networkCapabilities: " + mCapabilities + ", " + "networkAgent: " + mNetworkAgent + ", " + "ipClient: " + mIpClient + "," diff --git a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java index 21b9b1d850..3126e48afe 100644 --- a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java +++ b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java @@ -94,7 +94,7 @@ public class EthernetServiceImpl extends IEthernetManager.Stub { @Override public String[] getAvailableInterfaces() throws RemoteException { PermissionUtils.enforceAccessNetworkStatePermission(mContext, TAG); - return mTracker.getClientModeInterfaces(hasUseRestrictedNetworksPermission()); + return mTracker.getClientModeInterfacesSorted(hasUseRestrictedNetworksPermission()); } /** diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java index 672b146f4e..11912eaa0b 100644 --- a/service-t/src/com/android/server/ethernet/EthernetTracker.java +++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java @@ -459,7 +459,7 @@ public class EthernetTracker { private List getAllInterfaces() { final ArrayList interfaces = new ArrayList<>( - List.of(mFactory.getAvailableInterfaces(/* includeRestricted */ true))); + List.of(mFactory.getInterfacesSorted(/* includeRestricted */ true))); if (mTetheringInterfaceMode == INTERFACE_MODE_SERVER && mTetheringInterface != null) { interfaces.add(mTetheringInterface.getInterfaceName()); @@ -467,8 +467,8 @@ public class EthernetTracker { return interfaces; } - String[] getClientModeInterfaces(boolean includeRestricted) { - return mFactory.getAvailableInterfaces(includeRestricted); + String[] getClientModeInterfacesSorted(boolean includeRestricted) { + return mFactory.getInterfacesSorted(includeRestricted); } List getEthernetInterfaceList() { @@ -511,7 +511,7 @@ public class EthernetTracker { // Remote process has already died return; } - for (String iface : getClientModeInterfaces(canUseRestrictedNetworks)) { + for (String iface : getClientModeInterfacesSorted(canUseRestrictedNetworks)) { unicastInterfaceStateChange(listener, iface); } if (mTetheringInterface != null && mTetheringInterfaceMode == INTERFACE_MODE_SERVER) { @@ -703,7 +703,7 @@ public class EthernetTracker { IpConfiguration ipConfiguration = getOrCreateIpConfiguration(iface); Log.d(TAG, "Tracking interface in client mode: " + iface); - mFactory.addInterface(iface, hwAddress, ipConfiguration, nc); + mFactory.addInterface(port, ipConfiguration, nc); // Note: if the interface already has link (e.g., if we crashed and got // restarted while it was running), we need to fake a link up notification so we diff --git a/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java b/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java index 70d4ad8ce4..843e194a9d 100644 --- a/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java +++ b/tests/unit/java/com/android/server/ethernet/EthernetNetworkFactoryTest.java @@ -40,6 +40,7 @@ import android.net.EthernetNetworkSpecifier; import android.net.IpConfiguration; import android.net.LinkAddress; import android.net.LinkProperties; +import android.net.MacAddress; import android.net.Network; import android.net.NetworkAgentConfig; import android.net.NetworkCapabilities; @@ -75,10 +76,11 @@ import java.util.Objects; @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S_V2) public class EthernetNetworkFactoryTest { private static final int TIMEOUT_MS = 2_000; - private static final String TEST_IFACE = "test123"; + private static final String HW_ADDR = "01:02:03:04:05:06"; + private static final EthernetPort TEST_IFACE = + new EthernetPort("test123", MacAddress.fromString(HW_ADDR), 42); 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 TestLooper mLooper; private Handler mHandler; private EthernetNetworkFactory mNetFactory = null; @@ -222,17 +224,17 @@ public class EthernetNetworkFactoryTest { // creates an interface with provisioning in progress (since updating the interface link state // automatically starts the provisioning process) - private void createInterfaceUndergoingProvisioning(String iface) { + private void createInterfaceUndergoingProvisioning(EthernetPort port) { // Default to the ethernet transport type. - createInterfaceUndergoingProvisioning(iface, NetworkCapabilities.TRANSPORT_ETHERNET); + createInterfaceUndergoingProvisioning(port, NetworkCapabilities.TRANSPORT_ETHERNET); } private void createInterfaceUndergoingProvisioning( - @NonNull final String iface, final int transportType) { + EthernetPort port, final int transportType) { final IpConfiguration ipConfig = createDefaultIpConfig(); - mNetFactory.addInterface(iface, HW_ADDR, ipConfig, + mNetFactory.addInterface(port, ipConfig, createInterfaceCapsBuilder(transportType).build()); - assertTrue(mNetFactory.updateInterfaceLinkState(iface, true)); + assertTrue(mNetFactory.updateInterfaceLinkState(port.getInterfaceName(), true)); ArgumentCaptor captor = ArgumentCaptor.forClass( NetworkOfferCallback.class); @@ -248,9 +250,9 @@ public class EthernetNetworkFactoryTest { } // creates a provisioned interface - private void createAndVerifyProvisionedInterface(String iface) throws Exception { + private void createAndVerifyProvisionedInterface(EthernetPort port) throws Exception { // Default to the ethernet transport type. - createAndVerifyProvisionedInterface(iface, NetworkCapabilities.TRANSPORT_ETHERNET, + createAndVerifyProvisionedInterface(port, NetworkCapabilities.TRANSPORT_ETHERNET, ConnectivityManager.TYPE_ETHERNET); } @@ -258,13 +260,13 @@ public class EthernetNetworkFactoryTest { final int expectedLegacyType) throws Exception { createAndVerifyProvisionedInterface(TEST_IFACE, transportType, expectedLegacyType); - mNetFactory.removeInterface(TEST_IFACE); + mNetFactory.removeInterface(TEST_IFACE.getInterfaceName()); } private void createAndVerifyProvisionedInterface( - @NonNull final String iface, final int transportType, final int expectedLegacyType) + EthernetPort port, final int transportType, final int expectedLegacyType) throws Exception { - createInterfaceUndergoingProvisioning(iface, transportType); + createInterfaceUndergoingProvisioning(port, transportType); triggerOnProvisioningSuccess(); // provisioning succeeded, verify that the network agent is created, registered, marked // as connected and legacy type are correctly set. @@ -272,33 +274,21 @@ public class EthernetNetworkFactoryTest { NetworkCapabilities.class); verify(mDeps).makeEthernetNetworkAgent(any(), any(), ncCaptor.capture(), any(), argThat(x -> x.getLegacyType() == expectedLegacyType), any(), any()); - assertEquals( - new EthernetNetworkSpecifier(iface), ncCaptor.getValue().getNetworkSpecifier()); + assertEquals(new EthernetNetworkSpecifier(port.getInterfaceName()), + ncCaptor.getValue().getNetworkSpecifier()); verifyNetworkAgentRegistersAndConnects(); clearInvocations(mDeps); clearInvocations(mNetworkAgent); } - // creates an unprovisioned interface - private void createUnprovisionedInterface(String iface) throws Exception { - // 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. - mNetFactory.addInterface(iface, HW_ADDR, createDefaultIpConfig(), - createInterfaceCapsBuilder(NetworkCapabilities.TRANSPORT_ETHERNET).build()); - assertTrue(mNetFactory.updateInterfaceLinkState(iface, true)); - - clearInvocations(mIpClient); - clearInvocations(mNetworkAgent); - } - @Test public void testUpdateInterfaceLinkStateForActiveProvisioningInterface() throws Exception { initEthernetNetworkFactory(); createInterfaceUndergoingProvisioning(TEST_IFACE); // verify that the IpClient gets shut down when interface state changes to down. - final boolean ret = mNetFactory.updateInterfaceLinkState(TEST_IFACE, false /* up */); + final boolean ret = + mNetFactory.updateInterfaceLinkState(TEST_IFACE.getInterfaceName(), false /* up */); assertTrue(ret); verify(mIpClient).shutdown(); @@ -309,7 +299,8 @@ public class EthernetNetworkFactoryTest { initEthernetNetworkFactory(); // if interface was never added, link state cannot be updated. - final boolean ret = mNetFactory.updateInterfaceLinkState(TEST_IFACE, true /* up */); + final boolean ret = + mNetFactory.updateInterfaceLinkState(TEST_IFACE.getInterfaceName(), true /* up */); assertFalse(ret); verifyNoStopOrStart(); @@ -318,7 +309,8 @@ public class EthernetNetworkFactoryTest { @Test public void testProvisioningLoss() throws Exception { initEthernetNetworkFactory(); - when(mDeps.getNetworkInterfaceByName(TEST_IFACE)).thenReturn(mInterfaceParams); + when(mDeps.getNetworkInterfaceByName(TEST_IFACE.getInterfaceName())) + .thenReturn(mInterfaceParams); createAndVerifyProvisionedInterface(TEST_IFACE); triggerOnProvisioningFailure(); @@ -331,7 +323,7 @@ public class EthernetNetworkFactoryTest { public void testProvisioningLossForDisappearedInterface() throws Exception { initEthernetNetworkFactory(); // mocked method returns null by default, but just to be explicit in the test: - when(mDeps.getNetworkInterfaceByName(eq(TEST_IFACE))).thenReturn(null); + when(mDeps.getNetworkInterfaceByName(eq(TEST_IFACE.getInterfaceName()))).thenReturn(null); createAndVerifyProvisionedInterface(TEST_IFACE); triggerOnProvisioningFailure(); @@ -362,7 +354,8 @@ public class EthernetNetworkFactoryTest { public void testNetworkUnwantedWithStaleNetworkAgent() throws Exception { initEthernetNetworkFactory(); // ensures provisioning is restarted after provisioning loss - when(mDeps.getNetworkInterfaceByName(TEST_IFACE)).thenReturn(mInterfaceParams); + when(mDeps.getNetworkInterfaceByName(TEST_IFACE.getInterfaceName())) + .thenReturn(mInterfaceParams); createAndVerifyProvisionedInterface(TEST_IFACE); EthernetNetworkAgent.Callbacks oldCbs = mNetworkAgent.getCallbacks(); @@ -417,7 +410,7 @@ public class EthernetNetworkFactoryTest { private IpClientCallbacks getStaleIpClientCallbacks() throws Exception { createAndVerifyProvisionedInterface(TEST_IFACE); final IpClientCallbacks staleIpClientCallbacks = mIpClientCallbacks; - mNetFactory.removeInterface(TEST_IFACE); + mNetFactory.removeInterface(TEST_IFACE.getInterfaceName()); verifyStop(); assertNotSame(mIpClientCallbacks, staleIpClientCallbacks); return staleIpClientCallbacks; @@ -438,7 +431,8 @@ public class EthernetNetworkFactoryTest { @Test public void testIgnoreOnIpLayerStoppedCallbackForStaleCallback() throws Exception { initEthernetNetworkFactory(); - when(mDeps.getNetworkInterfaceByName(TEST_IFACE)).thenReturn(mInterfaceParams); + when(mDeps.getNetworkInterfaceByName(TEST_IFACE.getInterfaceName())) + .thenReturn(mInterfaceParams); final IpClientCallbacks staleIpClientCallbacks = getStaleIpClientCallbacks(); staleIpClientCallbacks.onProvisioningFailure(new LinkProperties()); @@ -500,7 +494,7 @@ public class EthernetNetworkFactoryTest { final NetworkCapabilities capabilities = createDefaultFilterCaps(); final IpConfiguration ipConfiguration = createStaticIpConfig(); - mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities); + mNetFactory.updateInterface(TEST_IFACE.getInterfaceName(), ipConfiguration, capabilities); triggerOnProvisioningSuccess(); verify(mDeps).makeEthernetNetworkAgent(any(), any(), @@ -515,7 +509,7 @@ public class EthernetNetworkFactoryTest { final NetworkCapabilities capabilities = createDefaultFilterCaps(); final IpConfiguration ipConfiguration = createStaticIpConfig(); - mNetFactory.updateInterface(TEST_IFACE, ipConfiguration, capabilities); + mNetFactory.updateInterface(TEST_IFACE.getInterfaceName(), ipConfiguration, capabilities); verifyNoStopOrStart(); } @@ -526,7 +520,8 @@ public class EthernetNetworkFactoryTest { createAndVerifyProvisionedInterface(TEST_IFACE); final IpConfiguration initialIpConfig = createStaticIpConfig(); - mNetFactory.updateInterface(TEST_IFACE, initialIpConfig, null /*capabilities*/); + mNetFactory.updateInterface(TEST_IFACE.getInterfaceName(), initialIpConfig, + null /*capabilities*/); triggerOnProvisioningSuccess(); verifyRestart(initialIpConfig); @@ -538,7 +533,8 @@ public class EthernetNetworkFactoryTest { // verify that sending a null ipConfig does not update the current ipConfig. - mNetFactory.updateInterface(TEST_IFACE, null /*ipConfig*/, null /*capabilities*/); + mNetFactory.updateInterface(TEST_IFACE.getInterfaceName(), null /*ipConfig*/, + null /*capabilities*/); triggerOnProvisioningSuccess(); verifyRestart(initialIpConfig); } @@ -547,7 +543,7 @@ public class EthernetNetworkFactoryTest { public void testOnNetworkNeededOnStaleNetworkOffer() throws Exception { initEthernetNetworkFactory(); createAndVerifyProvisionedInterface(TEST_IFACE); - mNetFactory.updateInterfaceLinkState(TEST_IFACE, false); + mNetFactory.updateInterfaceLinkState(TEST_IFACE.getInterfaceName(), false); verify(mNetworkProvider).unregisterNetworkOffer(mNetworkOfferCallback); // It is possible that even after a network offer is unregistered, CS still sends it // onNetworkNeeded() callbacks. @@ -560,7 +556,7 @@ public class EthernetNetworkFactoryTest { initEthernetNetworkFactory(); createAndVerifyProvisionedInterface(TEST_IFACE); - final String result = mNetFactory.getHwAddress(TEST_IFACE); + final String result = mNetFactory.getHwAddress(TEST_IFACE.getInterfaceName()); assertEquals(HW_ADDR, result); } @@ -568,7 +564,7 @@ public class EthernetNetworkFactoryTest { public void testGetMacAddressForNonExistingInterface() { initEthernetNetworkFactory(); - final String result = mNetFactory.getHwAddress(TEST_IFACE); + final String result = mNetFactory.getHwAddress(TEST_IFACE.getInterfaceName()); // No interface exists due to not calling createAndVerifyProvisionedInterface(...). assertNull(result); } diff --git a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java index def064a03f..5013d78c31 100644 --- a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java +++ b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java @@ -79,7 +79,7 @@ public class EthernetTrackerTest { initMockResources(); doReturn(false).when(mFactory).updateInterfaceLinkState(anyString(), anyBoolean()); doReturn(new String[0]).when(mNetd).interfaceGetList(); - doReturn(new String[0]).when(mFactory).getAvailableInterfaces(anyBoolean()); + doReturn(new String[0]).when(mFactory).getInterfacesSorted(anyBoolean()); mHandlerThread = new HandlerThread(THREAD_NAME); mHandlerThread.start(); tracker = new EthernetTracker(mContext, mHandlerThread.getThreadHandler(), mFactory, mNetd,