Fix enable/disableInterface APIs to properly affect administrative state
Previously, enable/disableInterface APIs just called
updateInterfaceState to pretend the link went up / down. This behavior
was fundamentally broken, as any RTM_NEWLINK message would "reset" the
interface to its actual link state.
This change now properly toggles IFF_UP on the interface when
enable/disableInterface() is called using netd APIs. A notable
side-effect of these APIs is that they remove (and later readd) the IPv4
address on the interface. This causes a provisioning failure when trying
to enable an already-enabled interface. However, in its current state,
this should only result in an IpClient restart.
In the longer term, these netd API calls should be replaced with using
netlink directly (the necessary permissions are already in place).
Test: atest EthernetManagerTest
Bug: 316268558
Bug: 225315248
Change-Id: I4a0c7f6eedb8c8768f7736b5033584d520d7e11a
diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java
index 8fb632e..bfcc171 100644
--- a/service-t/src/com/android/server/ethernet/EthernetTracker.java
+++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java
@@ -384,7 +384,7 @@
/** Configure the administrative state of ethernet interface by toggling IFF_UP. */
public void setInterfaceEnabled(String iface, boolean enabled, EthernetCallback cb) {
- mHandler.post(() -> updateInterfaceState(iface, enabled, cb));
+ mHandler.post(() -> setInterfaceAdministrativeState(iface, enabled, cb));
}
IpConfiguration getIpConfiguration(String iface) {
@@ -642,25 +642,40 @@
}
}
- private void updateInterfaceState(String iface, boolean up) {
- updateInterfaceState(iface, up, new EthernetCallback(null /* cb */));
- }
-
- // TODO(b/225315248): enable/disableInterface() should not affect link state.
- private void updateInterfaceState(String iface, boolean up, EthernetCallback cb) {
- final int mode = getInterfaceMode(iface);
- if (mode == INTERFACE_MODE_SERVER || !mFactory.hasInterface(iface)) {
- // The interface is in server mode or is not tracked.
- cb.onError("Failed to set link state " + (up ? "up" : "down") + " for " + iface);
+ private void setInterfaceAdministrativeState(String iface, boolean up, EthernetCallback cb) {
+ if (getInterfaceState(iface) == EthernetManager.STATE_ABSENT) {
+ cb.onError("Failed to enable/disable absent interface: " + iface);
+ return;
+ }
+ if (getInterfaceRole(iface) == EthernetManager.ROLE_SERVER) {
+ // TODO: support setEthernetState for server mode interfaces.
+ cb.onError("Failed to enable/disable interface in server mode: " + iface);
return;
}
+ if (up) {
+ // WARNING! setInterfaceUp() clears the IPv4 address and readds it. Calling
+ // enableInterface() on an active interface can lead to a provisioning failure which
+ // will cause IpClient to be restarted.
+ // TODO: use netlink directly rather than calling into netd.
+ NetdUtils.setInterfaceUp(mNetd, iface);
+ } else {
+ NetdUtils.setInterfaceDown(mNetd, iface);
+ }
+ cb.onResult(iface);
+ }
+
+ private void updateInterfaceState(String iface, boolean up) {
+ final int mode = getInterfaceMode(iface);
+ if (mode == INTERFACE_MODE_SERVER) {
+ // TODO: support tracking link state for interfaces in server mode.
+ return;
+ }
+
+ // If updateInterfaceLinkState returns false, the interface is already in the correct state.
if (mFactory.updateInterfaceLinkState(iface, up)) {
broadcastInterfaceStateChange(iface);
}
- // If updateInterfaceLinkState returns false, the interface is already in the correct state.
- // Always return success.
- cb.onResult(iface);
}
private void maybeUpdateServerModeInterfaceState(String iface, boolean available) {