Fix setEthernetEnabled API to properly affect administrative state
Previously, setEthernetEnabled(false) untracks all interfaces and
ignores future RTM_NEWLINK callbacks, which means that the ethernet
module loses all knowledge of existing interfaces.
This change now properly toggles IFF_UP on all interfaces including
those in server mode when setEthernetEnabled API is called, and returns
on error on callback when trying to enable an interface when ethernet is
disabled.
Test: atest EthernetManagerTest
Bug: 316268558
Bug: 225315248
Change-Id: Ic7785e7e0c87c1437627661401e66201dd60802a
diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java
index 67d0891..07469b1 100644
--- a/service-t/src/com/android/server/ethernet/EthernetTracker.java
+++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java
@@ -215,9 +215,6 @@
// Note: processNetlinkMessage is called on the handler thread.
@Override
protected void processNetlinkMessage(NetlinkMessage nlMsg, long whenMs) {
- // ignore all updates when ethernet is disabled.
- if (mEthernetState == ETHERNET_STATE_DISABLED) return;
-
if (nlMsg instanceof RtNetlinkLinkMessage) {
processRtNetlinkLinkMessage((RtNetlinkLinkMessage) nlMsg);
} else {
@@ -596,10 +593,13 @@
// Read the flags before attempting to bring up the interface. If the interface is
// already running an UP event is created after adding the interface.
config = NetdUtils.getInterfaceConfigParcel(mNetd, iface);
- if (NetdUtils.hasFlag(config, INetd.IF_STATE_DOWN)) {
+ // Only bring the interface up when ethernet is enabled.
+ if (mEthernetState == ETHERNET_STATE_ENABLED) {
// As a side-effect, NetdUtils#setInterfaceUp() also clears the interface's IPv4
// address and readds it which *could* lead to unexpected behavior in the future.
NetdUtils.setInterfaceUp(mNetd, iface);
+ } else if (mEthernetState == ETHERNET_STATE_DISABLED) {
+ NetdUtils.setInterfaceDown(mNetd, iface);
}
} catch (IllegalStateException e) {
// Either the system is crashing or the interface has disappeared. Just ignore the
@@ -646,6 +646,10 @@
}
private void setInterfaceAdministrativeState(String iface, boolean up, EthernetCallback cb) {
+ if (mEthernetState == ETHERNET_STATE_DISABLED) {
+ cb.onError("Cannot enable/disable interface when ethernet is disabled");
+ return;
+ }
if (getInterfaceState(iface) == EthernetManager.STATE_ABSENT) {
cb.onError("Failed to enable/disable absent interface: " + iface);
return;
@@ -965,22 +969,26 @@
mEthernetState = newState;
- if (enabled) {
- trackAvailableInterfaces();
- } else {
- // TODO: maybe also disable server mode interface as well.
- untrackFactoryInterfaces();
+ // Interface in server mode should also be included.
+ ArrayList<String> interfaces =
+ new ArrayList<>(
+ List.of(mFactory.getAvailableInterfaces(/* includeRestricted */ true)));
+
+ if (mTetheringInterfaceMode == INTERFACE_MODE_SERVER) {
+ interfaces.add(mTetheringInterface);
+ }
+
+ for (String iface : interfaces) {
+ if (enabled) {
+ NetdUtils.setInterfaceUp(mNetd, iface);
+ } else {
+ NetdUtils.setInterfaceDown(mNetd, iface);
+ }
}
broadcastEthernetStateChange(mEthernetState);
});
}
- private void untrackFactoryInterfaces() {
- for (String iface : mFactory.getAvailableInterfaces(true /* includeRestricted */)) {
- stopTrackingInterface(iface);
- }
- }
-
private void unicastEthernetStateChange(@NonNull IEthernetServiceListener listener,
int state) {
ensureRunningOnEthernetServiceThread();
diff --git a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt
index 1e2a212..9be579b 100644
--- a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt
+++ b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt
@@ -145,6 +145,8 @@
private var tetheredInterfaceRequest: TetheredInterfaceRequest? = null
+ private var ethernetEnabled = true
+
private class EthernetTestInterface(
context: Context,
private val handler: Handler,
@@ -428,7 +430,7 @@
// when an interface comes up, we should always see a down cb before an up cb.
ifaceListener.eventuallyExpect(iface, STATE_LINK_DOWN, ROLE_CLIENT)
- if (hasCarrier) {
+ if (hasCarrier && ethernetEnabled) {
ifaceListener.expectCallback(iface, STATE_LINK_UP, ROLE_CLIENT)
}
return iface
@@ -514,6 +516,7 @@
private fun setEthernetEnabled(enabled: Boolean) {
runAsShell(NETWORK_SETTINGS) { em.setEthernetEnabled(enabled) }
+ ethernetEnabled = enabled
val listener = EthernetStateListener()
addEthernetStateListener(listener)
listener.eventuallyExpect(if (enabled) ETHERNET_STATE_ENABLED else ETHERNET_STATE_DISABLED)
@@ -600,26 +603,6 @@
}
}
- @Test
- fun testCallbacks_withRunningInterface() {
- assumeFalse(isAdbOverEthernet())
- // Only run this test when no non-restricted / physical interfaces are present.
- assumeNoInterfaceForTetheringAvailable()
-
- val iface = createInterface()
- val listener = EthernetStateListener()
- addInterfaceStateListener(listener)
- listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_CLIENT)
-
- // Remove running interface. The interface stays running but is no longer tracked.
- setEthernetEnabled(false)
- listener.expectCallback(iface, STATE_ABSENT, ROLE_NONE)
-
- setEthernetEnabled(true)
- listener.expectCallback(iface, STATE_LINK_UP, ROLE_CLIENT)
- listener.assertNoCallback()
- }
-
private fun assumeNoInterfaceForTetheringAvailable() {
// Interfaces that have configured NetworkCapabilities will never be used for tethering,
// see aosp/2123900.
@@ -911,6 +894,30 @@
}
@Test
+ fun testEnableDisableInterface_disableEnableEthernet() {
+ val iface = createInterface()
+ val listener = EthernetStateListener()
+ addInterfaceStateListener(listener)
+ listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_CLIENT)
+
+ // When ethernet is disabled, interface should be down and enable/disableInterface()
+ // should not bring the interfaces up.
+ setEthernetEnabled(false)
+ listener.eventuallyExpect(iface, STATE_LINK_DOWN, ROLE_CLIENT)
+ enableInterface(iface).expectError()
+ disableInterface(iface).expectError()
+ listener.assertNoCallback()
+
+ // When ethernet is enabled, enable/disableInterface() should succeed.
+ setEthernetEnabled(true)
+ listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_CLIENT)
+ disableInterface(iface).expectResult(iface.name)
+ listener.eventuallyExpect(iface, STATE_LINK_DOWN, ROLE_CLIENT)
+ enableInterface(iface).expectResult(iface.name)
+ listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_CLIENT)
+ }
+
+ @Test
fun testUpdateConfiguration_forBothIpConfigAndCapabilities() {
val iface = createInterface()
val cb = requestNetwork(ETH_REQUEST.copyWithEthernetSpecifier(iface.name))
@@ -1018,4 +1025,68 @@
cb.eventuallyExpectCapabilities(TEST_CAPS)
cb.eventuallyExpectLpForStaticConfig(STATIC_IP_CONFIGURATION.staticIpConfiguration)
}
+
+ @Test
+ fun testAddInterface_disableEnableEthernet() {
+ val listener = EthernetStateListener()
+ addInterfaceStateListener(listener)
+
+ // When ethernet is disabled, newly added interfaces should not be brought up.
+ setEthernetEnabled(false)
+ val iface = createInterface(/* hasCarrier */ true)
+ listener.eventuallyExpect(iface, STATE_LINK_DOWN, ROLE_CLIENT)
+
+ // When ethernet is re-enabled after interface is added, it will be brought up.
+ setEthernetEnabled(true)
+ listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_CLIENT)
+ }
+
+
+ @Test
+ fun testRemoveInterface_disableEnableEthernet() {
+ // Set up 2 interfaces for testing
+ val iface1 = createInterface()
+ val listener = EthernetStateListener()
+ addInterfaceStateListener(listener)
+ listener.eventuallyExpect(iface1, STATE_LINK_UP, ROLE_CLIENT)
+ val iface2 = createInterface()
+ listener.eventuallyExpect(iface2, STATE_LINK_UP, ROLE_CLIENT)
+
+ // Removing interfaces when ethernet is enabled will first send link down, then
+ // STATE_ABSENT/ROLE_NONE.
+ removeInterface(iface1)
+ listener.expectCallback(iface1, STATE_LINK_DOWN, ROLE_CLIENT)
+ listener.expectCallback(iface1, STATE_ABSENT, ROLE_NONE)
+
+ // Removing interfaces after ethernet is disabled will first send link down when ethernet is
+ // disabled, then STATE_ABSENT/ROLE_NONE when interface is removed.
+ setEthernetEnabled(false)
+ listener.expectCallback(iface2, STATE_LINK_DOWN, ROLE_CLIENT)
+ removeInterface(iface2)
+ listener.expectCallback(iface2, STATE_ABSENT, ROLE_NONE)
+ }
+
+ @Test
+ fun testSetTetheringInterfaceMode_disableEnableEthernet() {
+ val listener = EthernetStateListener()
+ addInterfaceStateListener(listener)
+
+ val iface = createInterface()
+ requestTetheredInterface().expectOnAvailable()
+ listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_SERVER)
+
+ // (b/234743836): Currently the state of server mode interfaces always returns true due to
+ // that interface state for server mode interfaces is not tracked properly.
+ // So we do not get any state change when disabling ethernet.
+ setEthernetEnabled(false)
+ listener.assertNoCallback()
+
+ // When ethernet is disabled, change interface mode will not bring the interface up.
+ releaseTetheredInterface()
+ listener.eventuallyExpect(iface, STATE_LINK_DOWN, ROLE_CLIENT)
+
+ // When ethernet is re-enabled, interface will be brought up.
+ setEthernetEnabled(true)
+ listener.eventuallyExpect(iface, STATE_LINK_UP, ROLE_CLIENT)
+ }
}