Merge changes Id33fb720,Ic1a6e60d,If1b88754,I290ee07d into main

* changes:
  Rename getClientModeInterfaces to getClientModeInterfacesSorted
  Rename getAvailableInterfaces to getInterfacesSorted
  Pass EthernetPort into NetworkInterfaceState
  Pass EthernetPort into EthernetNetworkFactory#addInterface
This commit is contained in:
Patrick Rohr 2025-06-06 12:17:33 -07:00 • committed by Android (Google) Code Review
commit eec49e4bb6
5 changed files with 76 additions and 76 deletions

View file

@ -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 + ","

View file

@ -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());
}
/**

View file

@ -459,7 +459,7 @@ public class EthernetTracker {
private List<String> getAllInterfaces() {
final ArrayList<String> 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<String> 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

View file

@ -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<NetworkOfferCallback> 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);
}

View file

@ -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,