From 27d9121ef3660dab0ab8bcaaba191bf9119ade38 Mon Sep 17 00:00:00 2001 From: Mark Chien Date: Tue, 26 Mar 2024 06:44:35 +0000 Subject: [PATCH] Remove mWifiTetherRequested Removing "mWifiTetherRequested" because it might cause Wi-Fi tethering to not have an upstream if P2P is turned on. The issue could be recovered when the upstream's capabilities or link properties change, tethering calls "chooseUpstreamType" to select an upstream. If none of the above situations happen, Wi-Fi tethering won't have an upstream. 1. When Wifi P2P is enabled and tethering enters TetherModeAliveState. 2. Turning on wifi tethering, TetherModeAliveState would first receive EVENT_IFACE_SERVING_STATE_INACTIVE when wifi tethering's IpServer is created and enters IpServer's InitialState. Then it will call updateUpstreamWanted and change mUpstreamWanted from false to true before wifi tethering's IpServer actually enters TetheredState. 3. When TetherModeAliveState receives EVENT_IFACE_SERVING_STATE_ACTIVE, which is triggered by wifi tethering's IpServer entering TetheredState, it won't call chooseUpstreamType() because mUpstreamWanted is already true. case EVENT_IFACE_SERVING_STATE_ACTIVE: { IpServer who = (IpServer) message.obj; if (VDBG) Log.d(TAG, "Tether Mode requested by " + who); handleInterfaceServingStateActive(message.arg1, who); who.sendMessage(IpServer.CMD_TETHER_CONNECTION_CHANGED, mCurrentUpstreamIfaceSet); // If there has been a change and an upstream is now // desired, kick off the selection process. final boolean previousUpstreamWanted = updateUpstreamWanted(); if (!previousUpstreamWanted && mUpstreamWanted) { chooseUpstreamType(true); } mWifiTetherRequested is only used to check upstreamWanted() and only be used in TetherModeAliveState so that it is safe to remove mWifiTetherRequested and always use mForwardedDownstreams to check whether upstream is needed for TetherModeAliveState because all the Tethered IpServer is added to mForwardedDownstreams when it enter Tethered State and be removed from mForwardedDownstreams when it exit Tethered State. This change also move the upstreamWanted() function inside TetherMainSM to prevent it from being used incorrectly outside of TetherMainSM. Bug: 329552689 Test: atest TetheringTests Change-Id: I6842170f455b280dd79aae039a1117d5e6d677f3 --- .../networkstack/tethering/Tethering.java | 29 +++++++------------ .../networkstack/tethering/TetheringTest.java | 5 ++-- 2 files changed, 12 insertions(+), 22 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index 873961a7ef..d2fe0ed7ea 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -241,9 +241,6 @@ public class Tethering { private final TetherMainSM mTetherMainSM; private final OffloadController mOffloadController; private final UpstreamNetworkMonitor mUpstreamNetworkMonitor; - // TODO: Figure out how to merge this and other downstream-tracking objects - // into a single coherent structure. - private final HashSet mForwardedDownstreams; private final VersionedBroadcastListener mCarrierConfigChange; private final TetheringDependencies mDeps; private final EntitlementManager mEntitlementMgr; @@ -271,8 +268,6 @@ public class Tethering { private boolean mRndisEnabled; // track the RNDIS function enabled state private boolean mNcmEnabled; // track the NCM function enabled state - // True iff. WiFi tethering should be started when soft AP is ready. - private boolean mWifiTetherRequested; private Network mTetherUpstream; private TetherStatesParcel mTetherStatesParcel; private boolean mDataSaverEnabled = false; @@ -329,7 +324,6 @@ public class Tethering { (what, obj) -> { mTetherMainSM.sendMessage(TetherMainSM.EVENT_UPSTREAM_CALLBACK, what, 0, obj); }); - mForwardedDownstreams = new HashSet<>(); IntentFilter filter = new IntentFilter(); filter.addAction(ACTION_CARRIER_CONFIG_CHANGED); @@ -763,7 +757,6 @@ public class Tethering { } if ((enable && mgr.startTetheredHotspot(null /* use existing softap config */)) || (!enable && mgr.stopSoftAp())) { - mWifiTetherRequested = enable; return TETHER_ERROR_NO_ERROR; } } finally { @@ -1470,10 +1463,6 @@ public class Tethering { } private void disableWifiIpServing(String ifname, int apState) { - // Regardless of whether we requested this transition, the AP has gone - // down. Don't try to tether again unless we're requested to do so. - mWifiTetherRequested = false; - mLog.log("Canceling WiFi tethering request - interface=" + ifname + " state=" + apState); disableWifiIpServingCommon(TETHERING_WIFI, ifname); @@ -1505,8 +1494,7 @@ public class Tethering { private void enableWifiIpServing(String ifname, int wifiIpMode) { mLog.log("request WiFi tethering - interface=" + ifname + " state=" + wifiIpMode); - // Map wifiIpMode values to IpServer.Callback serving states, inferring - // from mWifiTetherRequested as a final "best guess". + // Map wifiIpMode values to IpServer.Callback serving states. final int ipServingMode; switch (wifiIpMode) { case IFACE_IP_MODE_TETHERED: @@ -1653,11 +1641,6 @@ public class Tethering { mLog.log(state.getName() + " got " + sMagicDecoderRing.get(what, Integer.toString(what))); } - private boolean upstreamWanted() { - if (!mForwardedDownstreams.isEmpty()) return true; - return mWifiTetherRequested; - } - // Needed because the canonical source of upstream truth is just the // upstream interface set, |mCurrentUpstreamIfaceSet|. private boolean pertainsToCurrentUpstream(UpstreamNetworkState ns) { @@ -1715,12 +1698,16 @@ public class Tethering { private final ArrayList mNotifyList; private final IPv6TetheringCoordinator mIPv6TetheringCoordinator; private final OffloadWrapper mOffload; + // TODO: Figure out how to merge this and other downstream-tracking objects + // into a single coherent structure. + private final HashSet mForwardedDownstreams; private static final int UPSTREAM_SETTLE_TIME_MS = 10000; TetherMainSM(String name, Looper looper, TetheringDependencies deps) { super(name, looper); + mForwardedDownstreams = new HashSet<>(); mInitialState = new InitialState(); mTetherModeAliveState = new TetherModeAliveState(); mSetIpForwardingEnabledErrorState = new SetIpForwardingEnabledErrorState(); @@ -2056,6 +2043,10 @@ public class Tethering { } } + private boolean upstreamWanted() { + return !mForwardedDownstreams.isEmpty(); + } + class TetherModeAliveState extends State { boolean mUpstreamWanted = false; boolean mTryCell = true; @@ -2651,7 +2642,7 @@ public class Tethering { } pw.println(" - lastError = " + tetherState.lastError); } - pw.println("Upstream wanted: " + upstreamWanted()); + pw.println("Upstream wanted: " + mTetherMainSM.upstreamWanted()); pw.println("Current upstream interface(s): " + mCurrentUpstreamIfaceSet); pw.decreaseIndent(); diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java index f01e1bbd4d..9f430afb07 100644 --- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java +++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java @@ -3653,10 +3653,9 @@ public class TetheringTest { verify(mWifiManager).updateInterfaceIpState(TEST_WLAN_IFNAME, IFACE_IP_MODE_TETHERED); verifyNoMoreInteractions(mWifiManager); - verifyTetheringBroadcast(TEST_WLAN_IFNAME, EXTRA_ACTIVE_TETHER); - // FIXME: wifi tethering doesn't have upstream when P2P is enabled. - verify(mUpstreamNetworkMonitor, never()).setTryCell(true); + + verify(mUpstreamNetworkMonitor).setTryCell(true); } // TODO: Test that a request for hotspot mode doesn't interfere with an