From 0129a7a11ed6800ede8b8a5b2edfae49c11308bc Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Thu, 20 Jun 2024 12:54:13 -0700 Subject: [PATCH] Add support for link state tracking Currently, TetheringState just ignores these callbacks (though it must return HANDLED, otherwise a wtf is logged). This is in line with what EthernetTracker currently does. Test: EthernetInterfaceStateMachineTest Change-Id: If7308f1aa4b333552a79dfe05864c5cef1621582 --- .../EthernetInterfaceStateMachine.java | 62 +++++++++++++++++-- .../EthernetInterfaceStateMachineTest.kt | 41 +++++++++--- 2 files changed, 90 insertions(+), 13 deletions(-) diff --git a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java index 562f03b7f4..7622ac7eb7 100644 --- a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java +++ b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java @@ -44,9 +44,11 @@ import java.util.Set; class EthernetInterfaceStateMachine extends SyncStateMachine { private static final String TAG = EthernetInterfaceStateMachine.class.getSimpleName(); - private static final int CMD_ON_NETWORK_NEEDED = 1; - private static final int CMD_ON_NETWORK_UNNEEDED = 2; - private static final int CMD_ON_IPCLIENT_CREATED = 3; + private static final int CMD_ON_LINK_UP = 1; + private static final int CMD_ON_LINK_DOWN = 2; + private static final int CMD_ON_NETWORK_NEEDED = 3; + private static final int CMD_ON_NETWORK_UNNEEDED = 4; + private static final int CMD_ON_IPCLIENT_CREATED = 5; private class EthernetNetworkOfferCallback implements NetworkOfferCallback { private final Set mRequestIds = new ArraySet<>(); @@ -122,15 +124,36 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { private final NetworkCapabilities mCapabilities; private final NetworkProvider mNetworkProvider; private final EthernetNetworkFactory.Dependencies mDependencies; + private boolean mLinkUp = false; /** Interface is in tethering mode. */ private class TetheringState extends State { - + @Override + public boolean processMessage(Message msg) { + switch (msg.what) { + case CMD_ON_LINK_UP: + case CMD_ON_LINK_DOWN: + // TODO: think about what to do here. + return HANDLED; + } + return NOT_HANDLED; + } } /** Link is down */ private class LinkDownState extends State { - + @Override + public boolean processMessage(Message msg) { + switch (msg.what) { + case CMD_ON_LINK_UP: + transitionTo(mStoppedState); + return HANDLED; + case CMD_ON_LINK_DOWN: + // do nothing, already in the correct state. + return HANDLED; + } + return NOT_HANDLED; + } } /** Parent states of all states that do not cause a NetworkOffer to be extended. */ @@ -149,6 +172,19 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { mNetworkOfferCallback); } + @Override + public boolean processMessage(Message msg) { + switch (msg.what) { + case CMD_ON_LINK_UP: + // do nothing, already in the correct state. + return HANDLED; + case CMD_ON_LINK_DOWN: + transitionTo(mLinkDownState); + return HANDLED; + } + return NOT_HANDLED; + } + @Override public void exit() { mNetworkProvider.unregisterNetworkOffer(mNetworkOfferCallback); @@ -281,4 +317,20 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { // this is the first interface to be added. start(mLinkDownState); } + + public boolean updateLinkState(boolean up) { + if (mLinkUp == up) { + return false; + } + + // TODO: consider setting mLinkUp as part of processMessage(). + mLinkUp = up; + if (!up) { // was up, goes down + processMessage(CMD_ON_LINK_DOWN); + } else { // was down, comes up + processMessage(CMD_ON_LINK_UP); + } + + return true; + } } diff --git a/tests/unit/java/com/android/server/ethernet/EthernetInterfaceStateMachineTest.kt b/tests/unit/java/com/android/server/ethernet/EthernetInterfaceStateMachineTest.kt index a4657e2369..c8b2f65c06 100644 --- a/tests/unit/java/com/android/server/ethernet/EthernetInterfaceStateMachineTest.kt +++ b/tests/unit/java/com/android/server/ethernet/EthernetInterfaceStateMachineTest.kt @@ -13,23 +13,30 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +// ktlint does not allow annotating function argument literals inline. Disable the specific rule +// since this negatively affects readability. +@file:Suppress("ktlint:standard:comment-wrapping") package com.android.server.ethernet import android.content.Context import android.net.NetworkCapabilities import android.net.NetworkProvider +import android.net.NetworkProvider.NetworkOfferCallback import android.os.Build import android.os.Handler import android.os.test.TestLooper import androidx.test.filters.SmallTest import com.android.testutils.DevSdkIgnoreRule import com.android.testutils.DevSdkIgnoreRunner -import org.junit.Assert.assertTrue -import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentCaptor +import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.eq import org.mockito.Mock +import org.mockito.Mockito.inOrder +import org.mockito.Mockito.verify import org.mockito.MockitoAnnotations private const val IFACE = "eth0" @@ -39,24 +46,42 @@ private val CAPS = NetworkCapabilities.Builder().build() @RunWith(DevSdkIgnoreRunner::class) @DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S_V2) class EthernetInterfaceStateMachineTest { - private val looper = TestLooper() + private lateinit var looper: TestLooper + private lateinit var handler: Handler private lateinit var ifaceState: EthernetInterfaceStateMachine @Mock private lateinit var context: Context @Mock private lateinit var provider: NetworkProvider @Mock private lateinit var deps: EthernetNetworkFactory.Dependencies - @Before + // There seems to be no (obvious) way to force execution of @Before and @Test annotation on the + // same thread. Since SyncStateMachine requires all interactions to be called from the same + // thread that is provided at construction time (in this case, the thread that TestLooper() is + // called on), setUp() must be called directly from the @Test method. + // TODO: find a way to fix this in the test runner. fun setUp() { + looper = TestLooper() + handler = Handler(looper.looper) MockitoAnnotations.initMocks(this) - val handler = Handler(looper.looper) ifaceState = EthernetInterfaceStateMachine(IFACE, handler, context, CAPS, provider, deps) } - // TODO: actually test something. @Test - fun doNothing() { - assertTrue(true) + fun testUpdateLinkState_networkOfferRegisteredAndRetracted() { + setUp() + + ifaceState.updateLinkState(/* up= */ true) + + // link comes up: validate the NetworkOffer is registered and capture callback object. + val inOrder = inOrder(provider) + val networkOfferCb = ArgumentCaptor.forClass(NetworkOfferCallback::class.java).also { + inOrder.verify(provider).registerNetworkOffer(any(), any(), any(), it.capture()) + }.value + + ifaceState.updateLinkState(/* up */ false) + + // link goes down: validate the NetworkOffer is retracted + inOrder.verify(provider).unregisterNetworkOffer(eq(networkOfferCb)) } }