ethernet: read running flag before bringing up interface
Previously, the "running" flag was checked right after calling
setInterfaceUp(), which caused an updateInterfaceState(UP) event to be
generated at the end of addInterface(). This "artificial" link up
notification is only needed for interfaces that already had link before
being added (as in this case, no RTM_NEWLINK callbacks are received, and
link state needs to "manually" be set to UP to start configuring the
interface). This could happen when system server crashes and gets
restarted, or when ethernet is temporarily disabled via the
EthernetManager API.
Reading the running flag before bringing up the interface ensures the
updateInterfaceState(UP) event is not generated for interfaces that were
just brought up.
This fix gets rid of the up/up/down/up callback when a new interface is
added.
Test: atest EthernetManagerTest
Change-Id: I122dc886a94222c575f44182b943e64b435c03e8
diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java
index 77addcf..00dff5b 100644
--- a/service-t/src/com/android/server/ethernet/EthernetTracker.java
+++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java
@@ -578,8 +578,14 @@
// Bring up the interface so we get link status indications.
try {
PermissionUtils.enforceNetworkStackPermission(mContext);
- NetdUtils.setInterfaceUp(mNetd, iface);
+ // 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)) {
+ // 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);
+ }
} catch (IllegalStateException e) {
// Either the system is crashing or the interface has disappeared. Just ignore the
// error; we haven't modified any state because we only do that if our calls succeed.
@@ -615,7 +621,7 @@
// 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
// start configuring it.
- if (NetdUtils.hasFlag(config, "running")) {
+ if (NetdUtils.hasFlag(config, INetd.IF_FLAG_RUNNING)) {
updateInterfaceState(iface, true);
}
}
diff --git a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt
index 57df3eb..1c92e4e 100644
--- a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt
+++ b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt
@@ -497,9 +497,7 @@
// If an interface appears, existing callbacks see it.
val iface2 = createInterface()
- // TODO: fix the up/up/down/up callbacks and only send down/up. Change to expectCallback
- // once that is fixed.
- listener1.eventuallyExpect(iface2, STATE_LINK_DOWN, ROLE_CLIENT)
+ listener1.expectCallback(iface2, STATE_LINK_DOWN, ROLE_CLIENT)
listener1.expectCallback(iface2, STATE_LINK_UP, ROLE_CLIENT)
// Register a new listener, it should see state of all existing interfaces immediately.