Merge changes I0b88bc8b,I60d884db,I436858e7,Iaaace0e6,Ie7ee84c3, ... am: 0a822f8f41
Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/2193535
Change-Id: Id7100fd83fcb877cda623b80807672d99dbb8d9c
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
diff --git a/service-t/src/com/android/server/ethernet/EthernetTracker.java b/service-t/src/com/android/server/ethernet/EthernetTracker.java
index 00dff5b..6ec478b 100644
--- a/service-t/src/com/android/server/ethernet/EthernetTracker.java
+++ b/service-t/src/com/android/server/ethernet/EthernetTracker.java
@@ -622,31 +622,32 @@
// restarted while it was running), we need to fake a link up notification so we
// start configuring it.
if (NetdUtils.hasFlag(config, INetd.IF_FLAG_RUNNING)) {
- updateInterfaceState(iface, true);
+ // no need to send an interface state change as this is not a true "state change". The
+ // callers (maybeTrackInterface() and setTetheringInterfaceMode()) already broadcast the
+ // state change.
+ mFactory.updateInterfaceLinkState(iface, true);
}
}
private void updateInterfaceState(String iface, boolean up) {
- // TODO: pull EthernetCallbacks out of EthernetNetworkFactory.
updateInterfaceState(iface, up, new EthernetCallback(null /* cb */));
}
- private void updateInterfaceState(@NonNull final String iface, final boolean up,
- @Nullable final EthernetCallback 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);
- final boolean factoryLinkStateUpdated = (mode == INTERFACE_MODE_CLIENT)
- && mFactory.updateInterfaceLinkState(iface, up);
-
- if (factoryLinkStateUpdated) {
- broadcastInterfaceStateChange(iface);
- cb.onResult(iface);
- } else {
- // The interface may already be in the correct state. While usually this should not be
- // an error, since updateInterfaceState is used in setInterfaceEnabled() /
- // setInterfaceDisabled() it has to be reported as such.
- // It is also possible that the interface disappeared or is in server mode.
+ 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);
+ return;
}
+
+ 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) {
diff --git a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt
index b21c5b4..8909e1a 100644
--- a/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt
+++ b/tests/cts/net/src/android/net/cts/EthernetManagerTest.kt
@@ -55,6 +55,7 @@
import android.os.Handler
import android.os.Looper
import android.os.OutcomeReceiver
+import android.os.SystemProperties
import android.platform.test.annotations.AppModeFull
import android.util.ArraySet
import androidx.test.platform.app.InstrumentationRegistry
@@ -75,6 +76,7 @@
import com.android.testutils.runAsShell
import com.android.testutils.waitForIdle
import org.junit.After
+import org.junit.Assume.assumeFalse
import org.junit.Assume.assumeTrue
import org.junit.Before
import org.junit.Ignore
@@ -312,10 +314,12 @@
private val result = CompletableFuture<String>()
override fun onResult(iface: String) {
+ assertFalse(result.isDone())
result.complete(iface)
}
override fun onError(e: EthernetNetworkManagementException) {
+ assertFalse(result.isDone())
result.completeExceptionally(e)
}
@@ -375,6 +379,18 @@
// Setting the carrier up / down relies on TUNSETCARRIER which was added in kernel version 5.0.
private fun assumeChangingCarrierSupported() = assumeTrue(isKernelVersionAtLeast("5.0.0"))
+ private fun isAdbOverEthernet(): Boolean {
+ // If no ethernet interface is available, adb is not connected over ethernet.
+ if (em.getInterfaceList().isEmpty()) return false
+
+ // cuttlefish is special and does not connect adb over ethernet.
+ if (SystemProperties.get("ro.product.board", "") == "cutf") return false
+
+ // Check if adb is connected over the network.
+ return (SystemProperties.getInt("persist.adb.tcp.port", -1) > -1 ||
+ SystemProperties.getInt("service.adb.tcp.port", -1) > -1)
+ }
+
private fun addInterfaceStateListener(listener: EthernetStateListener) {
runAsShell(CONNECTIVITY_USE_RESTRICTED_NETWORKS) {
em.addInterfaceStateListener(handler::post, listener)
@@ -471,6 +487,7 @@
}
}
+ // WARNING: check that isAdbOverEthernet() is false before calling setEthernetEnabled(false).
private fun setEthernetEnabled(enabled: Boolean) {
runAsShell(NETWORK_SETTINGS) { em.setEthernetEnabled(enabled) }
@@ -515,7 +532,7 @@
it.networkSpecifier == EthernetNetworkSpecifier(name)
}
- private fun TestableNetworkCallback.expectCapabilitiesWithCapability(cap: Int) =
+ private fun TestableNetworkCallback.expectCapabilitiesWith(cap: Int) =
expectCapabilitiesThat(anyNetwork(), TIMEOUT_MS) {
it.hasCapability(cap)
}
@@ -560,6 +577,25 @@
}
}
+ @Test
+ fun testCallbacks_withRunningInterface() {
+ // This test disables ethernet, so check that adb is not connected over ethernet.
+ assumeFalse(isAdbOverEthernet())
+ assumeTrue(em.getInterfaceList().isEmpty())
+ 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.
@@ -804,6 +840,26 @@
}
@Test
+ fun testEnableDisableInterface_withoutStateChange() {
+ val iface = createInterface()
+ // Interface is already enabled, so enableInterface() should return success
+ enableInterface(iface).expectResult(iface.name)
+
+ disableInterface(iface).expectResult(iface.name)
+ // Interface is already disabled, so disableInterface() should return success.
+ disableInterface(iface).expectResult(iface.name)
+ }
+
+ @Test
+ fun testEnableDisableInterface_withMissingInterface() {
+ val iface = createInterface()
+ removeInterface(iface)
+ // Interface does not exist, enable/disableInterface() should both return an error.
+ enableInterface(iface).expectError()
+ disableInterface(iface).expectError()
+ }
+
+ @Test
fun testUpdateConfiguration_forBothIpConfigAndCapabilities() {
val iface = createInterface()
val cb = requestNetwork(ETH_REQUEST.createCopyWithEthernetSpecifier(iface.name))
@@ -821,14 +877,14 @@
// will be expected first before available, as part of the restart.
cb.expectLost(network)
cb.expectAvailable()
- cb.expectCapabilitiesWithCapability(testCapability)
+ cb.expectCapabilitiesWith(testCapability)
cb.expectLinkPropertiesWithLinkAddress(
STATIC_IP_CONFIGURATION.staticIpConfiguration.ipAddress!!)
}
@Test
fun testUpdateConfiguration_forOnlyIpConfig() {
- val iface: EthernetTestInterface = createInterface()
+ val iface = createInterface()
val cb = requestNetwork(ETH_REQUEST.createCopyWithEthernetSpecifier(iface.name))
val network = cb.expectAvailable()
cb.assertNeverLost()
@@ -849,7 +905,7 @@
@Ignore
@Test
fun testUpdateConfiguration_forOnlyCapabilities() {
- val iface: EthernetTestInterface = createInterface()
+ val iface = createInterface()
val cb = requestNetwork(ETH_REQUEST.createCopyWithEthernetSpecifier(iface.name))
val network = cb.expectAvailable()
cb.assertNeverLost()
@@ -865,6 +921,6 @@
// will be expected first before available, as part of the restart.
cb.expectLost(network)
cb.expectAvailable()
- cb.expectCapabilitiesWithCapability(testCapability)
+ cb.expectCapabilitiesWith(testCapability)
}
}
diff --git a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java
index ea3d392..dde1d94 100644
--- a/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java
+++ b/tests/unit/java/com/android/server/ethernet/EthernetTrackerTest.java
@@ -353,22 +353,6 @@
}
@Test
- public void testEnableInterfaceCorrectlyCallsFactory() {
- tracker.enableInterface(TEST_IFACE, NULL_CB);
- waitForIdle();
-
- verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(true /* up */));
- }
-
- @Test
- public void testDisableInterfaceCorrectlyCallsFactory() {
- tracker.disableInterface(TEST_IFACE, NULL_CB);
- waitForIdle();
-
- verify(mFactory).updateInterfaceLinkState(eq(TEST_IFACE), eq(false /* up */));
- }
-
- @Test
public void testIsValidTestInterfaceIsFalseWhenTestInterfacesAreNotIncluded() {
final String validIfaceName = TEST_TAP_PREFIX + "123";
tracker.setIncludeTestInterfaces(false);