Use NetworkInterface API to retrieve interface list in EthernetTracker
Currently EthernetTracker uses netd to interact with interfaces, such as
retrieving all interfaces, updating interface configurations etc.
However, the usage of netd can bring some unexpected side effects, such
as clearing v4 address when toggling on/off interface.
This change moves forward with one less usage of netd to use
NetworkInterface.getNetworkInterfaces() to directly retrieve interface
list in EthernetTracker and added test to test the interface list can be
retrieved successfully when ethernet is disabled.
Bug: 379021505
Test: atest EthernetManager
Change-Id: I32f285a62f8f62180043052dd8790eed419dd226
diff --git a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java
index b8689d6..21b9b1d 100644
--- a/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java
+++ b/service-t/src/com/android/server/ethernet/EthernetServiceImpl.java
@@ -317,6 +317,6 @@
@Override
public List<String> getInterfaceList() {
PermissionUtils.enforceAccessNetworkStatePermission(mContext, TAG);
- return mTracker.getInterfaceList();
+ return mTracker.getEthernetInterfaceList();
}
}
diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java
index 5228aab..adfb694 100644
--- a/service-t/src/com/android/server/ethernet/EthernetTracker.java
+++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java
@@ -40,7 +40,6 @@
import android.os.Handler;
import android.os.RemoteCallbackList;
import android.os.RemoteException;
-import android.os.ServiceSpecificException;
import android.system.OsConstants;
import android.text.TextUtils;
import android.util.ArrayMap;
@@ -62,7 +61,10 @@
import java.io.FileDescriptor;
import java.net.InetAddress;
+import java.net.NetworkInterface;
+import java.net.SocketException;
import java.util.ArrayList;
+import java.util.Enumeration;
import java.util.Iterator;
import java.util.List;
import java.util.Objects;
@@ -105,7 +107,7 @@
/**
* Track test interfaces if true, don't track otherwise.
- * Volatile is needed as getInterfaceList() does not run on the handler thread.
+ * Volatile is needed as getEthernetInterfaceList() does not run on the handler thread.
*/
private volatile boolean mIncludeTestInterfaces = false;
@@ -398,26 +400,27 @@
return mFactory.getAvailableInterfaces(includeRestricted);
}
- List<String> getInterfaceList() {
+ List<String> getEthernetInterfaceList() {
final List<String> interfaceList = new ArrayList<String>();
- final String[] ifaces;
+ final Enumeration<NetworkInterface> ifaces;
try {
- ifaces = mNetd.interfaceGetList();
- } catch (RemoteException e) {
- Log.e(TAG, "Could not get list of interfaces " + e);
+ ifaces = NetworkInterface.getNetworkInterfaces();
+ } catch (SocketException e) {
+ Log.e(TAG, "Failed to get ethernet interfaces: ", e);
return interfaceList;
}
// There is a possible race with setIncludeTestInterfaces() which can affect
// isValidEthernetInterface (it returns true for test interfaces if setIncludeTestInterfaces
// is set to true).
- // setIncludeTestInterfaces() is only used in tests, and since getInterfaceList() does not
- // run on the handler thread, the behavior around setIncludeTestInterfaces() is
+ // setIncludeTestInterfaces() is only used in tests, and since getEthernetInterfaceList()
+ // does not run on the handler thread, the behavior around setIncludeTestInterfaces() is
// indeterminate either way. This can easily be circumvented by waiting on a callback from
// a test interface after calling setIncludeTestInterfaces() before calling this function.
// In production code, this has no effect.
- for (String iface : ifaces) {
- if (isValidEthernetInterface(iface)) interfaceList.add(iface);
+ while (ifaces.hasMoreElements()) {
+ NetworkInterface iface = ifaces.nextElement();
+ if (isValidEthernetInterface(iface.getName())) interfaceList.add(iface.getName());
}
return interfaceList;
}
@@ -707,10 +710,6 @@
}
private void maybeTrackInterface(String iface) {
- if (!isValidEthernetInterface(iface)) {
- return;
- }
-
// If we don't already track this interface, and if this interface matches
// our regex, start tracking it.
if (mFactory.hasInterface(iface) || iface.equals(mTetheringInterface)) {
@@ -730,13 +729,9 @@
}
private void trackAvailableInterfaces() {
- try {
- final String[] ifaces = mNetd.interfaceGetList();
- for (String iface : ifaces) {
- maybeTrackInterface(iface);
- }
- } catch (RemoteException | ServiceSpecificException e) {
- Log.e(TAG, "Could not get list of interfaces " + e);
+ final List<String> ifaces = getEthernetInterfaceList();
+ for (String iface : ifaces) {
+ maybeTrackInterface(iface);
}
}
diff --git a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt
index 9be579b..5b2c9f7 100644
--- a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt
+++ b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt
@@ -77,6 +77,7 @@
import com.android.testutils.assertThrows
import com.android.testutils.runAsShell
import com.android.testutils.waitForIdle
+import com.google.common.truth.Truth.assertThat
import java.io.IOException
import java.net.Inet6Address
import java.net.Socket
@@ -1089,4 +1090,24 @@
setEthernetEnabled(true)
listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_CLIENT)
}
+
+ @Test
+ fun testGetInterfaceList_disableEnableEthernet() {
+ // Test that interface list can be obtained when ethernet is disabled.
+ setEthernetEnabled(false)
+ // Create two test interfaces and check the return list contains the interface names.
+ val iface1 = createInterface()
+ val iface2 = createInterface()
+ var ifaces = em.getInterfaceList()
+ assertThat(ifaces).containsAtLeast(iface1.name, iface2.name)
+
+ // Remove one existing test interface and check the return list doesn't contain the
+ // removed interface name.
+ removeInterface(iface1)
+ ifaces = em.getInterfaceList()
+ assertThat(ifaces).doesNotContain(iface1.name)
+ assertThat(ifaces).contains(iface2.name)
+
+ removeInterface(iface2)
+ }
}