From aa06b2bf8e910089b2544ccbb96da4d05dd88228 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Mon, 17 Jun 2024 14:18:36 -0700 Subject: [PATCH 01/15] Move SyncStateMachine to staticlibs Test: mm Change-Id: I51685a454a3d0fab578fef1eeb42d600adef06e6 --- Tethering/src/android/net/ip/IpServer.java | 2 +- .../networkstack/tethering/util/StateMachineShim.java | 3 ++- .../networkstack/tethering/util/StateMachineShimTest.kt | 3 ++- staticlibs/Android.bp | 6 ++++-- .../com/android/net/module}/util/SyncStateMachine.java | 2 +- .../com/android/net/module}/util/SyncStateMachineTest.kt | 4 ++-- 6 files changed, 12 insertions(+), 8 deletions(-) rename {Tethering/src/com/android/networkstack/tethering => staticlibs/device/com/android/net/module}/util/SyncStateMachine.java (99%) rename {Tethering/tests/unit/src/com/android/networkstack/tethering => staticlibs/tests/unit/src/com/android/net/module}/util/SyncStateMachineTest.kt (98%) diff --git a/Tethering/src/android/net/ip/IpServer.java b/Tethering/src/android/net/ip/IpServer.java index 9e0c970f54..01f53936b5 100644 --- a/Tethering/src/android/net/ip/IpServer.java +++ b/Tethering/src/android/net/ip/IpServer.java @@ -75,6 +75,7 @@ import com.android.net.module.util.InterfaceParams; import com.android.net.module.util.NetdUtils; import com.android.net.module.util.SdkUtil.LateSdk; import com.android.net.module.util.SharedLog; +import com.android.net.module.util.SyncStateMachine.StateInfo; import com.android.net.module.util.ip.InterfaceController; import com.android.net.module.util.ip.IpNeighborMonitor; import com.android.net.module.util.ip.IpNeighborMonitor.NeighborEvent; @@ -87,7 +88,6 @@ import com.android.networkstack.tethering.metrics.TetheringMetrics; import com.android.networkstack.tethering.util.InterfaceSet; import com.android.networkstack.tethering.util.PrefixUtils; import com.android.networkstack.tethering.util.StateMachineShim; -import com.android.networkstack.tethering.util.SyncStateMachine.StateInfo; import java.net.Inet4Address; import java.net.Inet6Address; diff --git a/Tethering/src/com/android/networkstack/tethering/util/StateMachineShim.java b/Tethering/src/com/android/networkstack/tethering/util/StateMachineShim.java index 078a35f0e2..c236188ce2 100644 --- a/Tethering/src/com/android/networkstack/tethering/util/StateMachineShim.java +++ b/Tethering/src/com/android/networkstack/tethering/util/StateMachineShim.java @@ -22,7 +22,8 @@ import android.os.Looper; import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.State; import com.android.internal.util.StateMachine; -import com.android.networkstack.tethering.util.SyncStateMachine.StateInfo; +import com.android.net.module.util.SyncStateMachine; +import com.android.net.module.util.SyncStateMachine.StateInfo; import java.util.List; diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/util/StateMachineShimTest.kt b/Tethering/tests/unit/src/com/android/networkstack/tethering/util/StateMachineShimTest.kt index f8e98e34b8..24173858d1 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/util/StateMachineShimTest.kt +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/util/StateMachineShimTest.kt @@ -19,9 +19,10 @@ import android.os.Looper import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.SmallTest import com.android.internal.util.State +import com.android.net.module.util.SyncStateMachine +import com.android.net.module.util.SyncStateMachine.StateInfo import com.android.networkstack.tethering.util.StateMachineShim.AsyncStateMachine import com.android.networkstack.tethering.util.StateMachineShim.Dependencies -import com.android.networkstack.tethering.util.SyncStateMachine.StateInfo import kotlin.test.assertFailsWith import org.junit.Test import org.junit.runner.RunWith diff --git a/staticlibs/Android.bp b/staticlibs/Android.bp index e2834b09e4..71f388d026 100644 --- a/staticlibs/Android.bp +++ b/staticlibs/Android.bp @@ -39,12 +39,13 @@ java_library { "device/com/android/net/module/util/DeviceConfigUtils.java", "device/com/android/net/module/util/DomainUtils.java", "device/com/android/net/module/util/FdEventsReader.java", + "device/com/android/net/module/util/FeatureVersions.java", + "device/com/android/net/module/util/HandlerUtils.java", "device/com/android/net/module/util/NetworkMonitorUtils.java", "device/com/android/net/module/util/PacketReader.java", "device/com/android/net/module/util/SharedLog.java", "device/com/android/net/module/util/SocketUtils.java", - "device/com/android/net/module/util/FeatureVersions.java", - "device/com/android/net/module/util/HandlerUtils.java", + "device/com/android/net/module/util/SyncStateMachine.java", // This library is used by system modules, for which the system health impact of Kotlin // has not yet been evaluated. Annotations may need jarjar'ing. // "src_devicecommon/**/*.kt", @@ -68,6 +69,7 @@ java_library { "//packages/modules/CaptivePortalLogin", ], static_libs: [ + "modules-utils-statemachine", "net-utils-framework-common", ], libs: [ diff --git a/Tethering/src/com/android/networkstack/tethering/util/SyncStateMachine.java b/staticlibs/device/com/android/net/module/util/SyncStateMachine.java similarity index 99% rename from Tethering/src/com/android/networkstack/tethering/util/SyncStateMachine.java rename to staticlibs/device/com/android/net/module/util/SyncStateMachine.java index a17eb269ab..cc113a4f6c 100644 --- a/Tethering/src/com/android/networkstack/tethering/util/SyncStateMachine.java +++ b/staticlibs/device/com/android/net/module/util/SyncStateMachine.java @@ -14,7 +14,7 @@ * limitations under the License. */ -package com.android.networkstack.tethering.util; +package com.android.net.module.util; import android.annotation.NonNull; import android.annotation.Nullable; diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/util/SyncStateMachineTest.kt b/staticlibs/tests/unit/src/com/android/net/module/util/SyncStateMachineTest.kt similarity index 98% rename from Tethering/tests/unit/src/com/android/networkstack/tethering/util/SyncStateMachineTest.kt rename to staticlibs/tests/unit/src/com/android/net/module/util/SyncStateMachineTest.kt index 3a57fdd01a..5fe962be1d 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/util/SyncStateMachineTest.kt +++ b/staticlibs/tests/unit/src/com/android/net/module/util/SyncStateMachineTest.kt @@ -13,13 +13,13 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.android.networkstack.tethering.util +package com.android.net.module.util import android.os.Message import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.filters.SmallTest import com.android.internal.util.State -import com.android.networkstack.tethering.util.SyncStateMachine.StateInfo +import com.android.net.module.util.SyncStateMachine.StateInfo import java.util.ArrayDeque import java.util.ArrayList import kotlin.test.assertFailsWith From 6e7c49de1e503867064e5620cb804ccb77ca3c00 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Mon, 17 Jun 2024 14:49:21 -0700 Subject: [PATCH 02/15] Fix typo in SyncStateMachineTest Test: TH Change-Id: Ibeea98446135e16b8ce9f63e0651075b3c109669 --- .../src/com/android/net/module/util/SyncStateMachineTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/staticlibs/tests/unit/src/com/android/net/module/util/SyncStateMachineTest.kt b/staticlibs/tests/unit/src/com/android/net/module/util/SyncStateMachineTest.kt index 5fe962be1d..d5340542b1 100644 --- a/staticlibs/tests/unit/src/com/android/net/module/util/SyncStateMachineTest.kt +++ b/staticlibs/tests/unit/src/com/android/net/module/util/SyncStateMachineTest.kt @@ -45,7 +45,7 @@ private const val ARG_2 = 200 @RunWith(AndroidJUnit4::class) @SmallTest -class SynStateMachineTest { +class SyncStateMachineTest { private val mState1 = spy(object : TestState(MSG_1) {}) private val mState2 = spy(object : TestState(MSG_2) {}) private val mState3 = spy(object : TestState(MSG_3) {}) From 4e6679153a2829b038b32a58ebb239320c640c47 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Fri, 31 May 2024 14:17:23 -0700 Subject: [PATCH 03/15] Add boilerplate to represent interface states in state machine Adds empty boilerplate states to represent ethernet interface states (currently tracked in NetworkInterfaceState) in a StateMachine. Test: TH Change-Id: I1041dc577a9c41b44272dfd534abe705220ff3e0 --- .../EthernetInterfaceStateMachine.java | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java diff --git a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java new file mode 100644 index 0000000000..cf7d9bc6b6 --- /dev/null +++ b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java @@ -0,0 +1,87 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.ethernet; + +import android.os.Handler; + +import com.android.internal.util.State; +import com.android.net.module.util.SyncStateMachine; +import com.android.net.module.util.SyncStateMachine.StateInfo; + +import java.util.ArrayList; +import java.util.List; + +class EthernetInterfaceStateMachine extends SyncStateMachine { + /** Interface is in tethering mode. */ + private class TetheringState extends State { + + } + + /** Link is down */ + private class DisconnectedState extends State { + + } + + /** Parent states of all states that do not cause a NetworkOffer to be extended. */ + private class NetworkOfferExtendedState extends State { + + } + + /** Link is up, network offer is extended */ + private class StoppedState extends State { + } + + /** Network is needed, start provisioning */ + private class ProvisioningState extends State { + + } + + /** Network is needed */ + private class RunningState extends State { + + } + + private final TetheringState mTetheringState = new TetheringState(); + private final DisconnectedState mDisconnectedState = new DisconnectedState(); + private final NetworkOfferExtendedState mOfferExtendedState = new NetworkOfferExtendedState(); + private final StoppedState mStoppedState = new StoppedState(); + private final ProvisioningState mProvisioningState = new ProvisioningState(); + private final RunningState mRunningState = new RunningState(); + + public EthernetInterfaceStateMachine(String iface, Handler handler) { + super(EthernetInterfaceStateMachine.class.getSimpleName() + "." + iface, + handler.getLooper().getThread()); + + // Tethering mode is special as the interface is configured by Tethering, rather than the + // ethernet module. + final List states = new ArrayList<>(); + states.add(new StateInfo(mTetheringState, null)); + + // CHECKSTYLE:OFF IndentationCheck + // Initial state + states.add(new StateInfo(mDisconnectedState, null)); + states.add(new StateInfo(mOfferExtendedState, null)); + states.add(new StateInfo(mStoppedState, mOfferExtendedState)); + states.add(new StateInfo(mProvisioningState, mOfferExtendedState)); + states.add(new StateInfo(mRunningState, mOfferExtendedState)); + // CHECKSTYLE:ON IndentationCheck + + // TODO: set initial state to TetheringState if a tethering interface has been requested and + // this is the first interface to be added. + start(mDisconnectedState); + } +} From 3900b0606cfb707a6f34d773439605ed9beb19c2 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Fri, 31 May 2024 14:52:08 -0700 Subject: [PATCH 04/15] Implement NetworkOffer in EthernetInterfaceStateMachine This approximately does the equivalent of NetworkInterfaceState#registerNetworkOffer, NetworkInterfaceState#unregisterNetworkOfferAndStop, and NetworkInterfaceState#EthernetNetworkOfferCallback. Test: TH Change-Id: I9963e4bd08ab00910fafb258e7213ab01894926a --- .../EthernetInterfaceStateMachine.java | 69 ++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java index cf7d9bc6b6..6ebf84a5b7 100644 --- a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java +++ b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java @@ -16,7 +16,16 @@ package com.android.server.ethernet; +import android.annotation.NonNull; +import android.annotation.Nullable; +import android.net.NetworkCapabilities; +import android.net.NetworkProvider; +import android.net.NetworkProvider.NetworkOfferCallback; +import android.net.NetworkRequest; +import android.net.NetworkScore; import android.os.Handler; +import android.util.ArraySet; +import android.util.Log; import com.android.internal.util.State; import com.android.net.module.util.SyncStateMachine; @@ -24,8 +33,44 @@ import com.android.net.module.util.SyncStateMachine.StateInfo; import java.util.ArrayList; import java.util.List; +import java.util.Set; class EthernetInterfaceStateMachine extends SyncStateMachine { + private static final String TAG = EthernetInterfaceStateMachine.class.getSimpleName(); + + private class EthernetNetworkOfferCallback implements NetworkOfferCallback { + private final Set mRequestIds = new ArraySet<>(); + + @Override + public void onNetworkNeeded(@NonNull NetworkRequest request) { + if (this != mNetworkOfferCallback) { + return; + } + + mRequestIds.add(request.requestId); + // TODO: send ON_NETWORK_NEEDED message if requestIds.size() == 1 + } + + @Override + public void onNetworkUnneeded(@NonNull NetworkRequest request) { + if (this != mNetworkOfferCallback) { + return; + } + + if (!mRequestIds.remove(request.requestId)) { + // This can only happen if onNetworkNeeded was not called for a request or if + // the requestId changed. Both should *never* happen. + Log.wtf(TAG, "onNetworkUnneeded called for unknown request"); + } + // TODO: send ON_NETWORK_UNNEEDED message if requestIds.isEmpty() + } + } + + private @Nullable EthernetNetworkOfferCallback mNetworkOfferCallback; + private final Handler mHandler; + private final NetworkCapabilities mCapabilities; + private final NetworkProvider mNetworkProvider; + /** Interface is in tethering mode. */ private class TetheringState extends State { @@ -38,7 +83,25 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { /** Parent states of all states that do not cause a NetworkOffer to be extended. */ private class NetworkOfferExtendedState extends State { + @Override + public void enter() { + if (mNetworkOfferCallback != null) { + // This should never happen. If it happens anyway, log and move on. + Log.wtf(TAG, "Previous NetworkOffer was never retracted"); + } + mNetworkOfferCallback = new EthernetNetworkOfferCallback(); + final NetworkScore defaultScore = new NetworkScore.Builder().build(); + mNetworkProvider.registerNetworkOffer(defaultScore, + new NetworkCapabilities(mCapabilities), cmd -> mHandler.post(cmd), + mNetworkOfferCallback); + } + + @Override + public void exit() { + mNetworkProvider.unregisterNetworkOffer(mNetworkOfferCallback); + mNetworkOfferCallback = null; + } } /** Link is up, network offer is extended */ @@ -62,10 +125,14 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { private final ProvisioningState mProvisioningState = new ProvisioningState(); private final RunningState mRunningState = new RunningState(); - public EthernetInterfaceStateMachine(String iface, Handler handler) { + public EthernetInterfaceStateMachine(String iface, Handler handler, NetworkCapabilities capabilities, NetworkProvider provider) { super(EthernetInterfaceStateMachine.class.getSimpleName() + "." + iface, handler.getLooper().getThread()); + mHandler = handler; + mCapabilities = capabilities; + mNetworkProvider = provider; + // Tethering mode is special as the interface is configured by Tethering, rather than the // ethernet module. final List states = new ArrayList<>(); From 38c6f7d5cee22225e953873e6f6a745f4a925d76 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Mon, 17 Jun 2024 16:00:02 -0700 Subject: [PATCH 05/15] Add convenience method for processMessage with no arguments Similar to StateMachine, this CL adds a convenience method for processMessage that sets arg1 and arg2 to 0, and obj to null. Test: SyncStateMachineTest Change-Id: I7f5b9c0403c1abf02d03fd26bb2a16c53712c3bb --- .../android/net/module/util/SyncStateMachine.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/staticlibs/device/com/android/net/module/util/SyncStateMachine.java b/staticlibs/device/com/android/net/module/util/SyncStateMachine.java index cc113a4f6c..da184d3e03 100644 --- a/staticlibs/device/com/android/net/module/util/SyncStateMachine.java +++ b/staticlibs/device/com/android/net/module/util/SyncStateMachine.java @@ -165,6 +165,11 @@ public class SyncStateMachine { * The message is processed sequentially, so calling this method recursively is not permitted. * In other words, using this method inside State#enter, State#exit, or State#processMessage * is incorrect and will result in an IllegalStateException. + * + * @param what is assigned to Message.what + * @param arg1 is assigned to Message.arg1 + * @param arg2 is assigned to Message.arg2 + * @param obj is assigned to Message.obj */ public final void processMessage(int what, int arg1, int arg2, @Nullable Object obj) { ensureCorrectThread(); @@ -189,6 +194,15 @@ public class SyncStateMachine { mCurrentlyProcessing = Integer.MIN_VALUE; } + /** + * Synchronously process a message and perform state transition. + * + * @param what is assigned to Message.what. + */ + public final void processMessage(int what) { + processMessage(what, 0, 0, null); + } + private void maybeProcessSelfMessageQueue() { while (!mSelfMsgQueue.isEmpty()) { currentStateProcessMessageThenPerformTransitions(mSelfMsgQueue.poll()); From 9cc02c2e0103b2023fe7b34f9410461db0f6df64 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Tue, 18 Jun 2024 07:10:10 -0700 Subject: [PATCH 06/15] Clean up and document state lifecycle This renames existing states and makes RunningState a child of StartedState. StartedState's purpose is managing the IpClient lifecycle, so when the state machine transitions to RunningState, there is guaranteed to be an IpClient instance. RunningState will then startProvisioning and register a NetworkAgent on provisioning success. Test: builds Change-Id: I7fa41a9be06feaa9cbe78e344242bcedbe91e7ed --- .../EthernetInterfaceStateMachine.java | 47 +++++++++++++++---- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java index 6ebf84a5b7..bc3fc88d77 100644 --- a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java +++ b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java @@ -77,7 +77,7 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { } /** Link is down */ - private class DisconnectedState extends State { + private class LinkDownState extends State { } @@ -108,21 +108,21 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { private class StoppedState extends State { } - /** Network is needed, start provisioning */ - private class ProvisioningState extends State { + /** Network is needed, starts IpClient and manages its lifecycle */ + private class StartedState extends State { } - /** Network is needed */ + /** IpClient is running, starts provisioning and registers NetworkAgent */ private class RunningState extends State { } private final TetheringState mTetheringState = new TetheringState(); - private final DisconnectedState mDisconnectedState = new DisconnectedState(); + private final LinkDownState mLinkDownState = new LinkDownState(); private final NetworkOfferExtendedState mOfferExtendedState = new NetworkOfferExtendedState(); private final StoppedState mStoppedState = new StoppedState(); - private final ProvisioningState mProvisioningState = new ProvisioningState(); + private final StartedState mStartedState = new StartedState(); private final RunningState mRunningState = new RunningState(); public EthernetInterfaceStateMachine(String iface, Handler handler, NetworkCapabilities capabilities, NetworkProvider provider) { @@ -133,6 +133,33 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { mCapabilities = capabilities; mNetworkProvider = provider; + // Interface lifecycle: + // [ LinkDownState ] + // | + // v + // *link comes up* + // | + // v + // [ StoppedState ] + // | + // v + // *network is needed* + // | + // v + // [ StartedState ] + // | + // v + // *IpClient is created* + // | + // v + // [ RunningState ] + // | + // v + // *interface is requested for tethering* + // | + // v + // [TetheringState] + // // Tethering mode is special as the interface is configured by Tethering, rather than the // ethernet module. final List states = new ArrayList<>(); @@ -140,15 +167,15 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { // CHECKSTYLE:OFF IndentationCheck // Initial state - states.add(new StateInfo(mDisconnectedState, null)); + states.add(new StateInfo(mLinkDownState, null)); states.add(new StateInfo(mOfferExtendedState, null)); states.add(new StateInfo(mStoppedState, mOfferExtendedState)); - states.add(new StateInfo(mProvisioningState, mOfferExtendedState)); - states.add(new StateInfo(mRunningState, mOfferExtendedState)); + states.add(new StateInfo(mStartedState, mOfferExtendedState)); + states.add(new StateInfo(mRunningState, mStartedState)); // CHECKSTYLE:ON IndentationCheck // TODO: set initial state to TetheringState if a tethering interface has been requested and // this is the first interface to be added. - start(mDisconnectedState); + start(mLinkDownState); } } From a403056f70ffad5f0e2a653f52b6ecbeada027dd Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Tue, 18 Jun 2024 08:44:11 -0700 Subject: [PATCH 07/15] Introduce CMD_ON_NETWORK_NEEDED and CMD_ON_NETWORK_UNNEEDED messages While the parent state (NetworkOfferExtendedState) registers (and unregisters) the offer, its child states handle the resulting needed / unneeded messages. Since SyncStateMachine logs WTF if a message is processed that is not handled by the current state (or any of its parents), this provides some protection against receiving unexpectected needed / unneeded messages. Test: mm Change-Id: I5d322c9d3b9cea911e1435ef118a3fb03f7d537a --- .../EthernetInterfaceStateMachine.java | 41 +++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java index bc3fc88d77..3039da163e 100644 --- a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java +++ b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java @@ -24,6 +24,7 @@ import android.net.NetworkProvider.NetworkOfferCallback; import android.net.NetworkRequest; import android.net.NetworkScore; import android.os.Handler; +import android.os.Message; import android.util.ArraySet; import android.util.Log; @@ -38,6 +39,9 @@ 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 class EthernetNetworkOfferCallback implements NetworkOfferCallback { private final Set mRequestIds = new ArraySet<>(); @@ -48,7 +52,9 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { } mRequestIds.add(request.requestId); - // TODO: send ON_NETWORK_NEEDED message if requestIds.size() == 1 + if (mRequestIds.size() == 1) { + processMessage(CMD_ON_NETWORK_NEEDED); + } } @Override @@ -62,7 +68,9 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { // the requestId changed. Both should *never* happen. Log.wtf(TAG, "onNetworkUnneeded called for unknown request"); } - // TODO: send ON_NETWORK_UNNEEDED message if requestIds.isEmpty() + if (mRequestIds.isEmpty()) { + processMessage(CMD_ON_NETWORK_UNNEEDED); + } } } @@ -104,13 +112,38 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { } } - /** Link is up, network offer is extended */ + /** + * Offer is extended but has not been requested. + * + * StoppedState's sole purpose is to react to a CMD_ON_NETWORK_NEEDED and transition to + * StartedState when that happens. Note that StoppedState could be rolled into + * NetworkOfferExtendedState. However, keeping the states separate provides some additional + * protection by logging a Log.wtf if a CMD_ON_NETWORK_NEEDED is received in an unexpected state + * (i.e. StartedState or RunningState). StoppedState is a child of NetworkOfferExtendedState. + */ private class StoppedState extends State { + @Override + public boolean processMessage(Message msg) { + switch (msg.what) { + case CMD_ON_NETWORK_NEEDED: + transitionTo(mStartedState); + return HANDLED; + } + return NOT_HANDLED; + } } /** Network is needed, starts IpClient and manages its lifecycle */ private class StartedState extends State { - + @Override + public boolean processMessage(Message msg) { + switch (msg.what) { + case CMD_ON_NETWORK_UNNEEDED: + transitionTo(mStoppedState); + return HANDLED; + } + return NOT_HANDLED; + } } /** IpClient is running, starts provisioning and registers NetworkAgent */ From 2bdae824c5fa94b53b4ec4589c61f2717566c114 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Tue, 18 Jun 2024 08:48:20 -0700 Subject: [PATCH 08/15] Use TAG instead Test: mm Change-Id: Ic0fa5d6c99e804b32f4b47fc2142b6f51c460953 --- .../EthernetInterfaceStateMachine.java | 56 ++++++++++++++++++- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java index 3039da163e..8f42edca92 100644 --- a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java +++ b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java @@ -18,11 +18,16 @@ package com.android.server.ethernet; import android.annotation.NonNull; import android.annotation.Nullable; +import android.content.Context; import android.net.NetworkCapabilities; import android.net.NetworkProvider; import android.net.NetworkProvider.NetworkOfferCallback; import android.net.NetworkRequest; import android.net.NetworkScore; +import android.net.ip.IIpClient; +import android.net.ip.IpClientCallbacks; +import android.net.ip.IpClientManager; +import android.net.ip.IpClientUtil; import android.os.Handler; import android.os.Message; import android.util.ArraySet; @@ -41,6 +46,7 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { 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 class EthernetNetworkOfferCallback implements NetworkOfferCallback { private final Set mRequestIds = new ArraySet<>(); @@ -74,8 +80,31 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { } } + private class EthernetIpClientCallback extends IpClientCallbacks { + private void safelyPostOnHandler(Runnable r) { + mHandler.post(() -> { + if (this != mIpClientCallback) { + return; + } + r.run(); + }); + } + + @Override + public void onIpClientCreated(IIpClient ipClient) { + safelyPostOnHandler(() -> { + // TODO: add a SyncStateMachine#processMessage(cmd, obj) overload. + processMessage(CMD_ON_IPCLIENT_CREATED, 0, 0, new IpClientManager(ipClient, TAG)); + }); + } + } + private @Nullable EthernetNetworkOfferCallback mNetworkOfferCallback; + private @Nullable EthernetIpClientCallback mIpClientCallback; + private @Nullable IpClientManager mIpClient; + private final String mIface; private final Handler mHandler; + private final Context mContext; private final NetworkCapabilities mCapabilities; private final NetworkProvider mNetworkProvider; @@ -135,15 +164,34 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { /** Network is needed, starts IpClient and manages its lifecycle */ private class StartedState extends State { + @Override + public void enter() { + mIpClientCallback = new EthernetIpClientCallback(); + IpClientUtil.makeIpClient(mContext, mIface, mIpClientCallback); + } + @Override public boolean processMessage(Message msg) { switch (msg.what) { case CMD_ON_NETWORK_UNNEEDED: transitionTo(mStoppedState); return HANDLED; + case CMD_ON_IPCLIENT_CREATED: + mIpClient = (IpClientManager) msg.obj; + transitionTo(mRunningState); + return HANDLED; } return NOT_HANDLED; } + + @Override + public void exit() { + mIpClientCallback = null; + if (mIpClient != null) { + mIpClient.shutdown(); + // TODO: either wait for shutdown or add an additional StoppingState. + } + } } /** IpClient is running, starts provisioning and registers NetworkAgent */ @@ -158,11 +206,13 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { private final StartedState mStartedState = new StartedState(); private final RunningState mRunningState = new RunningState(); - public EthernetInterfaceStateMachine(String iface, Handler handler, NetworkCapabilities capabilities, NetworkProvider provider) { - super(EthernetInterfaceStateMachine.class.getSimpleName() + "." + iface, - handler.getLooper().getThread()); + public EthernetInterfaceStateMachine(String iface, Handler handler, Context context, + NetworkCapabilities capabilities, NetworkProvider provider) { + super(TAG + "." + iface, handler.getLooper().getThread()); + mIface = iface; mHandler = handler; + mContext = context; mCapabilities = capabilities; mNetworkProvider = provider; From 49cea0f80ddae9932c6e9c1bcdd8328e94fe2953 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Tue, 18 Jun 2024 10:49:38 -0700 Subject: [PATCH 09/15] Wait on IpClient#onQuit in StartedState#exit This does make the entire operation synchronous, but also temporarily blocks the handler thread (it is however what EthernetNetworkFactory currently does). If that becomes an issue, we can consider adding a StoppingState. Test: mm Change-Id: I799fb027d6eda4106633a047bf13674190ad0e68 --- .../EthernetInterfaceStateMachine.java | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java index 8f42edca92..1c1937dd94 100644 --- a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java +++ b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java @@ -28,6 +28,7 @@ import android.net.ip.IIpClient; import android.net.ip.IpClientCallbacks; import android.net.ip.IpClientManager; import android.net.ip.IpClientUtil; +import android.os.ConditionVariable; import android.os.Handler; import android.os.Message; import android.util.ArraySet; @@ -81,6 +82,8 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { } private class EthernetIpClientCallback extends IpClientCallbacks { + private final ConditionVariable mOnQuitCv = new ConditionVariable(false); + private void safelyPostOnHandler(Runnable r) { mHandler.post(() -> { if (this != mIpClientCallback) { @@ -97,6 +100,15 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { processMessage(CMD_ON_IPCLIENT_CREATED, 0, 0, new IpClientManager(ipClient, TAG)); }); } + + public void waitOnQuit() { + mOnQuitCv.block(5_000 /* timeoutMs */); + } + + @Override + public void onQuit() { + mOnQuitCv.open(); + } } private @Nullable EthernetNetworkOfferCallback mNetworkOfferCallback; @@ -186,11 +198,13 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { @Override public void exit() { - mIpClientCallback = null; if (mIpClient != null) { mIpClient.shutdown(); - // TODO: either wait for shutdown or add an additional StoppingState. + // TODO: consider adding a StoppingState and making the shutdown operation + // asynchronous. + mIpClientCallback.waitOnQuit(); } + mIpClientCallback = null; } } From 903f2308251aeb286a96124df5a3e275dee3a64b Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Tue, 18 Jun 2024 14:42:08 -0700 Subject: [PATCH 10/15] Log.wtf if block times out Test: mm Change-Id: I28e5bd8d208fa77ad04c67e3f66ea36c20ff8917 --- .../server/ethernet/EthernetInterfaceStateMachine.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java index 1c1937dd94..335cc3ed4f 100644 --- a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java +++ b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java @@ -102,7 +102,9 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { } public void waitOnQuit() { - mOnQuitCv.block(5_000 /* timeoutMs */); + if (!mOnQuitCv.block(5_000 /* timeoutMs */)) { + Log.wtf(TAG, "Timed out waiting on IpClient to shutdown."); + } } @Override From e3e9d4bbdcc7e70253ba07707d0bc765e1a6667e Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Tue, 18 Jun 2024 15:45:17 -0700 Subject: [PATCH 11/15] Make class testable Reuses EthernetNetworkFactory.Dependencies. Test: mm Change-Id: I975afb45a84b12e8660ac6b94d97410abb3c7aaa --- .../ethernet/EthernetInterfaceStateMachine.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java index 335cc3ed4f..843d3279ac 100644 --- a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java +++ b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java @@ -27,7 +27,6 @@ import android.net.NetworkScore; import android.net.ip.IIpClient; import android.net.ip.IpClientCallbacks; import android.net.ip.IpClientManager; -import android.net.ip.IpClientUtil; import android.os.ConditionVariable; import android.os.Handler; import android.os.Message; @@ -97,7 +96,8 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { public void onIpClientCreated(IIpClient ipClient) { safelyPostOnHandler(() -> { // TODO: add a SyncStateMachine#processMessage(cmd, obj) overload. - processMessage(CMD_ON_IPCLIENT_CREATED, 0, 0, new IpClientManager(ipClient, TAG)); + processMessage(CMD_ON_IPCLIENT_CREATED, 0, 0, + mDependencies.makeIpClientManager(ipClient)); }); } @@ -121,6 +121,7 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { private final Context mContext; private final NetworkCapabilities mCapabilities; private final NetworkProvider mNetworkProvider; + private final EthernetNetworkFactory.Dependencies mDependencies; /** Interface is in tethering mode. */ private class TetheringState extends State { @@ -181,7 +182,7 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { @Override public void enter() { mIpClientCallback = new EthernetIpClientCallback(); - IpClientUtil.makeIpClient(mContext, mIface, mIpClientCallback); + mDependencies.makeIpClient(mContext, mIface, mIpClientCallback); } @Override @@ -223,7 +224,8 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { private final RunningState mRunningState = new RunningState(); public EthernetInterfaceStateMachine(String iface, Handler handler, Context context, - NetworkCapabilities capabilities, NetworkProvider provider) { + NetworkCapabilities capabilities, NetworkProvider provider, + EthernetNetworkFactory.Dependencies deps) { super(TAG + "." + iface, handler.getLooper().getThread()); mIface = iface; @@ -231,6 +233,7 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { mContext = context; mCapabilities = capabilities; mNetworkProvider = provider; + mDependencies = deps; // Interface lifecycle: // [ LinkDownState ] From 990ea86b462ecaf439a003375661017cb73aa051 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Tue, 18 Jun 2024 15:45:57 -0700 Subject: [PATCH 12/15] Add missing call to add states to state machine Test: EthernetInterfaceStateMachineTest Change-Id: Ia8f42ed03a8d20bd02be2c587976db48e6b8cccd --- .../android/server/ethernet/EthernetInterfaceStateMachine.java | 1 + 1 file changed, 1 insertion(+) diff --git a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java index 843d3279ac..562f03b7f4 100644 --- a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java +++ b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java @@ -275,6 +275,7 @@ class EthernetInterfaceStateMachine extends SyncStateMachine { states.add(new StateInfo(mStartedState, mOfferExtendedState)); states.add(new StateInfo(mRunningState, mStartedState)); // CHECKSTYLE:ON IndentationCheck + addAllStates(states); // TODO: set initial state to TetheringState if a tethering interface has been requested and // this is the first interface to be added. From 7bd5e6dca6d3d721315f9eb4c0350c34f5a9e163 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Tue, 18 Jun 2024 15:48:43 -0700 Subject: [PATCH 13/15] Add unit test class for EthernetInterfaceStateMachine This currently just invokes the constructor and does nothing else. EthernetInterfaceStateMachine does not currently provide a way to transition out of LinkDownState. Test: run test Change-Id: Ica8faced0abe33bb70ec5e3f950bc0af75f78f16 --- .../EthernetInterfaceStateMachineTest.kt | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 tests/unit/java/com/android/server/ethernet/EthernetInterfaceStateMachineTest.kt diff --git a/tests/unit/java/com/android/server/ethernet/EthernetInterfaceStateMachineTest.kt b/tests/unit/java/com/android/server/ethernet/EthernetInterfaceStateMachineTest.kt new file mode 100644 index 0000000000..a4657e2369 --- /dev/null +++ b/tests/unit/java/com/android/server/ethernet/EthernetInterfaceStateMachineTest.kt @@ -0,0 +1,62 @@ +/* + * Copyright (C) 2024 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.ethernet + +import android.content.Context +import android.net.NetworkCapabilities +import android.net.NetworkProvider +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.Mock +import org.mockito.MockitoAnnotations + +private const val IFACE = "eth0" +private val CAPS = NetworkCapabilities.Builder().build() + +@SmallTest +@RunWith(DevSdkIgnoreRunner::class) +@DevSdkIgnoreRule.IgnoreUpTo(Build.VERSION_CODES.S_V2) +class EthernetInterfaceStateMachineTest { + private val looper = TestLooper() + 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 + fun setUp() { + 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) + } +} From 0129a7a11ed6800ede8b8a5b2edfae49c11308bc Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Thu, 20 Jun 2024 12:54:13 -0700 Subject: [PATCH 14/15] 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)) } } From d4c6516b2c9ee94a7cea871436d911e175928193 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Thu, 20 Jun 2024 13:03:58 -0700 Subject: [PATCH 15/15] Add some documentation to EthernetInterfaceStateMachine Test: documentation only Change-Id: I76b0a7ba8633fa9328e1d42de7f7208c84871ca8 --- .../server/ethernet/EthernetInterfaceStateMachine.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java index 7622ac7eb7..b8f68594ee 100644 --- a/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java +++ b/service-t/src/com/android/server/ethernet/EthernetInterfaceStateMachine.java @@ -41,6 +41,14 @@ import java.util.ArrayList; import java.util.List; import java.util.Set; +/** + * EthernetInterfaceStateMachine manages the lifecycle of an ethernet-like network interface which + * includes managing a NetworkOffer, IpClient, and NetworkAgent as well as making the interface + * available as a tethering downstream. + * + * All methods exposed by this class *must* be called on the Handler thread provided in the + * constructor. + */ class EthernetInterfaceStateMachine extends SyncStateMachine { private static final String TAG = EthernetInterfaceStateMachine.class.getSimpleName();