From 48fbdf4a368b763a916edc190f1b1f7765a9ed06 Mon Sep 17 00:00:00 2001 From: Quang Anh Luong Date: Wed, 12 Feb 2025 11:43:42 +0900 Subject: [PATCH 1/2] no-op: Refactor bluetooth tethering listener enableTetheringInternal immediately sends the tether result for all types except for TETHERING_BLUETOOTH, which relies on setBluetoothTethering to sends the result immediately if the Pan service is connected, or defer the result until after the Pan service connects. To keep better track of the active listeners and simplify the logic for further listener behavior changes (i.e. notifying the listener after the link layer event), refactor the bluetooth listener to be called immediately within enableTetheringInternal if the Pan service is connected like the other tethering types. Bug: 216524590 Test: covered by existing tests Change-Id: I91196e430b44ef2391de662ff0ed6e051b453d98 --- .../src/android/net/TetheringManager.java | 5 +++ .../networkstack/tethering/Tethering.java | 37 +++++++++---------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/Tethering/common/TetheringLib/src/android/net/TetheringManager.java b/Tethering/common/TetheringLib/src/android/net/TetheringManager.java index f123dca0dd..0ac97f06e5 100644 --- a/Tethering/common/TetheringLib/src/android/net/TetheringManager.java +++ b/Tethering/common/TetheringLib/src/android/net/TetheringManager.java @@ -333,6 +333,11 @@ public class TetheringManager { public static final int TETHER_ERROR_UNKNOWN_REQUEST = 17; @FlaggedApi(Flags.FLAG_TETHERING_WITH_SOFT_AP_CONFIG) public static final int TETHER_ERROR_DUPLICATE_REQUEST = 18; + /** + * Never used outside Tethering.java. + * @hide + */ + public static final int TETHER_ERROR_BLUETOOTH_SERVICE_PENDING = 19; /** @hide */ @Retention(RetentionPolicy.SOURCE) diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index c0192ddc7e..58220835e0 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -43,6 +43,7 @@ import static android.net.TetheringManager.TETHERING_VIRTUAL; import static android.net.TetheringManager.TETHERING_WIFI; import static android.net.TetheringManager.TETHERING_WIFI_P2P; import static android.net.TetheringManager.TETHERING_WIGIG; +import static android.net.TetheringManager.TETHER_ERROR_BLUETOOTH_SERVICE_PENDING; import static android.net.TetheringManager.TETHER_ERROR_INTERNAL_ERROR; import static android.net.TetheringManager.TETHER_ERROR_NO_ERROR; import static android.net.TetheringManager.TETHER_ERROR_SERVICE_UNAVAIL; @@ -776,7 +777,7 @@ public class Tethering { */ private void enableTetheringInternal(int type, boolean enable, String iface, final IIntResultListener listener) { - int result = TETHER_ERROR_NO_ERROR; + final int result; switch (type) { case TETHERING_WIFI: result = setWifiTethering(enable); @@ -785,7 +786,7 @@ public class Tethering { result = setUsbTethering(enable); break; case TETHERING_BLUETOOTH: - setBluetoothTethering(enable, listener); + result = setBluetoothTethering(enable, listener); break; case TETHERING_NCM: result = setNcmTethering(enable); @@ -801,10 +802,10 @@ public class Tethering { result = TETHER_ERROR_UNKNOWN_TYPE; } - // The result of Bluetooth tethering will be sent by #setBluetoothTethering. - if (type != TETHERING_BLUETOOTH) { - sendTetherResult(listener, result, type); - } + // The result of Bluetooth tethering will be sent after the pan service connects. + if (result == TETHER_ERROR_BLUETOOTH_SERVICE_PENDING) return; + + sendTetherResult(listener, result, type); } private void sendTetherResult(final IIntResultListener listener, final int result, @@ -843,13 +844,12 @@ public class Tethering { return TETHER_ERROR_INTERNAL_ERROR; } - private void setBluetoothTethering(final boolean enable, final IIntResultListener listener) { + private int setBluetoothTethering(final boolean enable, final IIntResultListener listener) { final BluetoothAdapter adapter = mDeps.getBluetoothAdapter(); if (adapter == null || !adapter.isEnabled()) { Log.w(TAG, "Tried to enable bluetooth tethering with null or disabled adapter. null: " + (adapter == null)); - sendTetherResult(listener, TETHER_ERROR_SERVICE_UNAVAIL, TETHERING_BLUETOOTH); - return; + return TETHER_ERROR_SERVICE_UNAVAIL; } if (mBluetoothPanListener != null && mBluetoothPanListener.isConnected()) { @@ -857,8 +857,7 @@ public class Tethering { // When bluetooth tethering is enabled, any time a PAN client pairs with this // host, bluetooth will bring up a bt-pan interface and notify tethering to // enable IP serving. - setBluetoothTetheringSettings(mBluetoothPan, enable, listener); - return; + return setBluetoothTetheringSettings(mBluetoothPan, enable); } if (!enable) { @@ -870,8 +869,7 @@ public class Tethering { TETHERING_BLUETOOTH); } mPendingPanRequestListeners.clear(); - sendTetherResult(listener, TETHER_ERROR_NO_ERROR, TETHERING_BLUETOOTH); - return; + return TETHER_ERROR_NO_ERROR; } mPendingPanRequestListeners.add(listener); @@ -883,6 +881,7 @@ public class Tethering { mBluetoothPanListener = new PanServiceListener(); adapter.getProfileProxy(mContext, mBluetoothPanListener, BluetoothProfile.PAN); } + return TETHER_ERROR_BLUETOOTH_SERVICE_PENDING; } private class PanServiceListener implements ServiceListener { @@ -900,8 +899,9 @@ public class Tethering { mIsConnected = true; for (IIntResultListener pendingListener : mPendingPanRequestListeners) { - setBluetoothTetheringSettings(mBluetoothPan, true /* enable */, - pendingListener); + final int result = setBluetoothTetheringSettings(mBluetoothPan, + true /* enable */); + sendTetherResult(pendingListener, result, TETHERING_BLUETOOTH); } mPendingPanRequestListeners.clear(); }); @@ -930,8 +930,8 @@ public class Tethering { } } - private void setBluetoothTetheringSettings(@NonNull final BluetoothPan bluetoothPan, - final boolean enable, final IIntResultListener listener) { + private int setBluetoothTetheringSettings(@NonNull final BluetoothPan bluetoothPan, + final boolean enable) { if (SdkLevel.isAtLeastT()) { changeBluetoothTetheringSettings(bluetoothPan, enable); } else { @@ -940,9 +940,8 @@ public class Tethering { // Enabling bluetooth tethering settings can silently fail. Send internal error if the // result is not expected. - final int result = bluetoothPan.isTetheringOn() == enable + return bluetoothPan.isTetheringOn() == enable ? TETHER_ERROR_NO_ERROR : TETHER_ERROR_INTERNAL_ERROR; - sendTetherResult(listener, result, TETHERING_BLUETOOTH); } private void changeBluetoothTetheringSettingsPreT(@NonNull final BluetoothPan bluetoothPan, From 394c88969963775922259c50c1115c03af2768f5 Mon Sep 17 00:00:00 2001 From: Quang Anh Luong Date: Wed, 12 Feb 2025 15:23:06 +0900 Subject: [PATCH 2/2] no-op: Pipe TetheringRequest through enableTetheringInternal Add a TetheringRequest argument to enableTetheringInternal and sendTetherResult so we can pass the request to Wifi in a later CL. Also pass through for TETHERING_VIRTUAL to use the request and interface directly. Note that removing getOrCreatePendingTetheringRequest in setVirtualTethering is also a no-op: - If setVirtualTethering is called from startTethering, the request that is now passed in as a parameter is the exact same one that was just inserted into the pending list, and which the old code would have obtained from getOrCreatePendingTetheringRequest. - If setVirtualTethering is called due to an IP conflict, both the old and the new code use a placeholder request with no interface. This codepath was already broken before this CL (and likely has never worked). Bug: 216524590 Test: covered by existing tests Change-Id: I3cb9b5ed44e83bb6bbf4ed77c37cbf35e306f08b --- .../networkstack/tethering/Tethering.java | 48 +++++++++++++------ 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index 58220835e0..b50831d56a 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -708,8 +708,7 @@ public class Tethering { // If tethering is already enabled with a different request, // disable before re-enabling. if (unfinishedRequest != null && !unfinishedRequest.equalsIgnoreUidPackage(request)) { - enableTetheringInternal(type, false /* disabled */, - unfinishedRequest.getInterfaceName(), null); + enableTetheringInternal(false /* disabled */, unfinishedRequest, null); mEntitlementMgr.stopProvisioningIfNeeded(type); } mPendingTetheringRequests.put(type, request); @@ -720,7 +719,7 @@ public class Tethering { mEntitlementMgr.startProvisioningIfNeeded(type, request.getShouldShowEntitlementUi()); } - enableTetheringInternal(type, true /* enabled */, request.getInterfaceName(), listener); + enableTetheringInternal(true /* enabled */, request, listener); mTetheringMetrics.createBuilder(type, callerPkg); }); } @@ -767,7 +766,10 @@ public class Tethering { void stopTetheringInternal(int type) { mPendingTetheringRequests.remove(type); - enableTetheringInternal(type, false /* disabled */, null, null); + // Using a placeholder here is ok since none of the disable APIs use the request for + // anything. We simply need the tethering type to know which link layer to poke for removal. + // TODO: Remove the placeholder here and loop through each pending/serving request. + enableTetheringInternal(false /* disabled */, createPlaceholderRequest(type), null); mEntitlementMgr.stopProvisioningIfNeeded(type); } @@ -775,8 +777,9 @@ public class Tethering { * Enables or disables tethering for the given type. If provisioning is required, it will * schedule provisioning rechecks for the specified interface. */ - private void enableTetheringInternal(int type, boolean enable, - String iface, final IIntResultListener listener) { + private void enableTetheringInternal(boolean enable, @NonNull final TetheringRequest request, + final IIntResultListener listener) { + final int type = request.getTetheringType(); final int result; switch (type) { case TETHERING_WIFI: @@ -795,7 +798,7 @@ public class Tethering { result = setEthernetTethering(enable); break; case TETHERING_VIRTUAL: - result = setVirtualMachineTethering(enable, iface); + result = setVirtualMachineTethering(enable, request); break; default: Log.w(TAG, "Invalid tether type."); @@ -1060,14 +1063,15 @@ public class Tethering { } } - private int setVirtualMachineTethering(final boolean enable, String iface) { + private int setVirtualMachineTethering(final boolean enable, + @NonNull final TetheringRequest request) { + final String iface = request.getInterfaceName(); if (enable) { if (TextUtils.isEmpty(iface)) { mConfiguredVirtualIface = "avf_tap_fixed"; } else { mConfiguredVirtualIface = iface; } - final TetheringRequest request = getOrCreatePendingTetheringRequest(TETHERING_VIRTUAL); enableIpServing(request, mConfiguredVirtualIface); } else if (mConfiguredVirtualIface != null) { ensureIpServerStopped(mConfiguredVirtualIface); @@ -1099,6 +1103,19 @@ public class Tethering { return request; } + /** + * Create a placeholder request. This is used in case we try to find a pending request but there + * is none (e.g. stopTethering removed a pending request), or for cases where we only have the + * tethering type (e.g. stopTethering(int)). + */ + @NonNull + private TetheringRequest createPlaceholderRequest(int type) { + final TetheringRequest request = new TetheringRequest.Builder(type).build(); + request.getParcel().requestType = TetheringRequest.REQUEST_TYPE_LEGACY; + request.getParcel().connectivityScope = CONNECTIVITY_SCOPE_GLOBAL; + return request; + } + /** * Gets the TetheringRequest that #startTethering was called with but is waiting for the link * layer event to indicate the interface is available to tether. @@ -1115,9 +1132,7 @@ public class Tethering { Log.w(TAG, "No pending TetheringRequest for type " + type + " found, creating a placeholder" + " request"); - TetheringRequest placeholder = new TetheringRequest.Builder(type).build(); - placeholder.getParcel().requestType = REQUEST_TYPE_PLACEHOLDER; - return placeholder; + return createPlaceholderRequest(type); } private void handleLegacyTether(String iface, final IIntResultListener listener) { @@ -2413,9 +2428,14 @@ public class Tethering { break; } case EVENT_REQUEST_CHANGE_DOWNSTREAM: { - final int tetheringType = message.arg1; + final int type = message.arg1; final Boolean enabled = (Boolean) message.obj; - enableTetheringInternal(tetheringType, enabled, null, null); + // Using a placeholder here is ok since we just need to the type of + // tethering to poke the link layer. When the link layer comes up, we won't + // have a pending request to use, but this matches the historical behavior. + // TODO: Get the TetheringRequest from IpServer and make sure to put it in + // the pending list too. + enableTetheringInternal(enabled, createPlaceholderRequest(type), null); break; } default: