Merge changes Ib0c4a52e,Iff7d2981,Ie8af8b71,I7884faf8 into main
* changes:
Rename mActiveTetheringRequests to mPendingTetheringRequests
Give the two variants of ensureIpServerStarted different names.
Move the TetheringRequest further up the stack.
Un-nest legacyTether by moving lambda contents to new method.
diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java
index 11b14ed..3fcf356 100644
--- a/Tethering/src/com/android/networkstack/tethering/Tethering.java
+++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java
@@ -241,7 +241,7 @@
// Currently active tethering requests per tethering type. Only one of each type can be
// requested at a time. After a tethering type is requested, the map keeps tethering parameters
// to be used after the interface comes up asynchronously.
- private final SparseArray<TetheringRequest> mActiveTetheringRequests =
+ private final SparseArray<TetheringRequest> mPendingTetheringRequests =
new SparseArray<>();
private final Context mContext;
@@ -640,7 +640,7 @@
}
if (enabled) {
- ensureIpServerStarted(iface);
+ ensureIpServerStartedForInterface(iface);
} else {
ensureIpServerStopped(iface);
}
@@ -701,7 +701,7 @@
final IIntResultListener listener) {
mHandler.post(() -> {
final int type = request.getTetheringType();
- final TetheringRequest unfinishedRequest = mActiveTetheringRequests.get(type);
+ final TetheringRequest unfinishedRequest = mPendingTetheringRequests.get(type);
// If tethering is already enabled with a different request,
// disable before re-enabling.
if (unfinishedRequest != null && !unfinishedRequest.equalsIgnoreUidPackage(request)) {
@@ -709,7 +709,7 @@
unfinishedRequest.getInterfaceName(), null);
mEntitlementMgr.stopProvisioningIfNeeded(type);
}
- mActiveTetheringRequests.put(type, request);
+ mPendingTetheringRequests.put(type, request);
if (request.isExemptFromEntitlementCheck()) {
mEntitlementMgr.setExemptedDownstreamType(type);
@@ -728,7 +728,7 @@
});
}
void stopTetheringInternal(int type) {
- mActiveTetheringRequests.remove(type);
+ mPendingTetheringRequests.remove(type);
enableTetheringInternal(type, false /* disabled */, null, null);
mEntitlementMgr.stopProvisioningIfNeeded(type);
@@ -782,7 +782,7 @@
// If changing tethering fail, remove corresponding request
// no matter who trigger the start/stop.
if (result != TETHER_ERROR_NO_ERROR) {
- mActiveTetheringRequests.remove(type);
+ mPendingTetheringRequests.remove(type);
mTetheringMetrics.updateErrorCode(type, result);
mTetheringMetrics.sendReport(type);
}
@@ -944,7 +944,9 @@
public void onAvailable(String iface) {
if (this != mBluetoothCallback) return;
- enableIpServing(TETHERING_BLUETOOTH, iface, getRequestedState(TETHERING_BLUETOOTH));
+ final TetheringRequest request = getPendingTetheringRequest(TETHERING_BLUETOOTH);
+ enableIpServing(request, TETHERING_BLUETOOTH, iface,
+ getRequestedState(TETHERING_BLUETOOTH));
mConfiguredBluetoothIface = iface;
}
@@ -999,7 +1001,10 @@
// Ethernet callback arrived after Ethernet tethering stopped. Ignore.
return;
}
- enableIpServing(TETHERING_ETHERNET, iface, getRequestedState(TETHERING_ETHERNET));
+
+ final TetheringRequest request = getPendingTetheringRequest(TETHERING_ETHERNET);
+ enableIpServing(request, TETHERING_ETHERNET, iface,
+ getRequestedState(TETHERING_ETHERNET));
mConfiguredEthernetIface = iface;
}
@@ -1020,7 +1025,9 @@
} else {
mConfiguredVirtualIface = iface;
}
+ final TetheringRequest request = getPendingTetheringRequest(TETHERING_VIRTUAL);
enableIpServing(
+ request,
TETHERING_VIRTUAL,
mConfiguredVirtualIface,
getRequestedState(TETHERING_VIRTUAL));
@@ -1064,7 +1071,7 @@
*/
@NonNull
private TetheringRequest getOrCreatePendingTetheringRequest(int type) {
- TetheringRequest pending = mActiveTetheringRequests.get(type);
+ TetheringRequest pending = mPendingTetheringRequests.get(type);
if (pending != null) return pending;
Log.w(TAG, "No pending TetheringRequest for type " + type + " found, creating a placeholder"
@@ -1074,6 +1081,59 @@
return placeholder;
}
+ private void handleLegacyTether(String iface, int requestedState,
+ final IIntResultListener listener) {
+ if (Build.VERSION.SDK_INT > Build.VERSION_CODES.VANILLA_ICE_CREAM) {
+ // After V, the TetheringManager and ConnectivityManager tether and untether methods
+ // throw UnsupportedOperationException, so this cannot happen in normal use. Ensure
+ // that this code cannot run even if callers use raw binder calls or other
+ // unsupported methods.
+ return;
+ }
+
+ final int type = ifaceNameToType(iface);
+ if (type == TETHERING_INVALID) {
+ Log.e(TAG, "Ignoring call to legacy tether for unknown iface " + iface);
+ try {
+ listener.onResult(TETHER_ERROR_UNKNOWN_IFACE);
+ } catch (RemoteException e) { }
+ }
+
+ int result = tetherInternal(null, iface, requestedState);
+ switch (type) {
+ case TETHERING_WIFI:
+ TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
+ "Legacy tether API called on Wifi iface " + iface,
+ CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
+ CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI);
+ if (result == TETHER_ERROR_NO_ERROR) {
+ TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
+ "Legacy tether API succeeded on Wifi iface " + iface,
+ CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
+ CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_SUCCESS);
+ }
+ break;
+ case TETHERING_WIFI_P2P:
+ TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
+ "Legacy tether API called on Wifi P2P iface " + iface,
+ CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
+ CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_P2P);
+ if (result == TETHER_ERROR_NO_ERROR) {
+ TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
+ "Legacy tether API succeeded on Wifi P2P iface " + iface,
+ CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
+ CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_P2P_SUCCESS);
+ }
+ break;
+ default:
+ // Do nothing
+ break;
+ }
+ try {
+ listener.onResult(result);
+ } catch (RemoteException e) { }
+ }
+
/**
* Legacy tether API that starts tethering with CONNECTIVITY_SCOPE_GLOBAL on the given iface.
*
@@ -1088,51 +1148,14 @@
* those broadcasts are disabled by OEM.
*/
void legacyTether(String iface, int requestedState, final IIntResultListener listener) {
- if (Build.VERSION.SDK_INT > Build.VERSION_CODES.VANILLA_ICE_CREAM) {
- // After V, the TetheringManager and ConnectivityManager tether and untether methods
- // throw UnsupportedOperationException, so this cannot happen in normal use. Ensure
- // that this code cannot run even if callers use raw binder calls or other
- // unsupported methods.
- return;
- }
- mHandler.post(() -> {
- int result = tetherInternal(iface, requestedState);
- switch (ifaceNameToType(iface)) {
- case TETHERING_WIFI:
- TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
- "Legacy tether API called on Wifi iface " + iface,
- CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
- CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI);
- if (result == TETHER_ERROR_NO_ERROR) {
- TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
- "Legacy tether API succeeded on Wifi iface " + iface,
- CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
- CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_SUCCESS);
- }
- break;
- case TETHERING_WIFI_P2P:
- TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
- "Legacy tether API called on Wifi P2P iface " + iface,
- CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
- CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_P2P);
- if (result == TETHER_ERROR_NO_ERROR) {
- TerribleErrorLog.logTerribleError(TetheringStatsLog::write,
- "Legacy tether API succeeded on Wifi P2P iface " + iface,
- CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED,
- CORE_NETWORKING_TERRIBLE_ERROR_OCCURRED__ERROR_TYPE__TYPE_LEGACY_TETHER_WITH_TYPE_WIFI_P2P_SUCCESS);
- }
- break;
- default:
- // Do nothing
- break;
- }
- try {
- listener.onResult(result);
- } catch (RemoteException e) { }
- });
+ mHandler.post(() -> handleLegacyTether(iface, requestedState, listener));
}
- private int tetherInternal(String iface, int requestedState) {
+ // TODO: is it possible to make the request @NonNull here?
+ // Because the IpServer doesn't actually look at (almost anything in) the request, we can just
+ // make callers synthesize a reasonable-looking request and make this @NonNull.
+ private int tetherInternal(@Nullable TetheringRequest request, String iface,
+ int requestedState) {
if (DBG) Log.d(TAG, "Tethering " + iface);
TetherState tetherState = mTetherStates.get(iface);
if (tetherState == null) {
@@ -1145,15 +1168,19 @@
Log.e(TAG, "Tried to Tether an unavailable iface: " + iface + ", ignoring");
return TETHER_ERROR_UNAVAIL_IFACE;
}
- // NOTE: If a CMD_TETHER_REQUESTED message is already in the TISM's queue but not yet
+ // NOTE: If a CMD_TETHER_REQUESTED message is already in the IpServer's queue but not yet
// processed, this will be a no-op and it will not return an error.
//
// This code cannot race with untether() because they both run on the handler thread.
+ //
+ // TODO: once the request is @NonNull, it will be correct to fetch the type from it, because
+ // the type used to fetch (or synthesize) the request is the same as the type passed in at
+ // IpServer creation time.
+ // BUG: the connectivity scope in the request is ignored. This matches current behaviour.
+ // TODO: see if the connectivity scope can be made (or is already) correct, and fetch the
+ // requested state from there.
final int type = tetherState.ipServer.interfaceType();
- final TetheringRequest request = mActiveTetheringRequests.get(type, null);
- if (request != null) {
- mActiveTetheringRequests.delete(type);
- }
+ mPendingTetheringRequests.remove(type);
tetherState.ipServer.enable(requestedState, request);
return TETHER_ERROR_NO_ERROR;
}
@@ -1213,8 +1240,9 @@
return mEntitlementMgr.isTetherProvisioningRequired(cfg);
}
+ // TODO: make this take a TetheringRequest instead.
private int getRequestedState(int type) {
- final TetheringRequest request = mActiveTetheringRequests.get(type);
+ final TetheringRequest request = mPendingTetheringRequests.get(type);
// The request could have been deleted before we had a chance to complete it.
// If so, assume that the scope is the default scope for this tethering type.
@@ -1529,8 +1557,8 @@
}
@VisibleForTesting
- SparseArray<TetheringRequest> getActiveTetheringRequests() {
- return mActiveTetheringRequests;
+ SparseArray<TetheringRequest> getPendingTetheringRequests() {
+ return mPendingTetheringRequests;
}
@VisibleForTesting
@@ -1590,14 +1618,21 @@
}
}
- private void enableIpServing(int tetheringType, String ifname, int ipServingMode) {
- enableIpServing(tetheringType, ifname, ipServingMode, false /* isNcm */);
+ final TetheringRequest getPendingTetheringRequest(int type) {
+ return mPendingTetheringRequests.get(type, null);
}
- private void enableIpServing(int tetheringType, String ifname, int ipServingMode,
- boolean isNcm) {
- ensureIpServerStarted(ifname, tetheringType, isNcm);
- if (tetherInternal(ifname, ipServingMode) != TETHER_ERROR_NO_ERROR) {
+ // TODO: make the request @NonNull and move the tetheringType and ipServingMode into it.
+ private void enableIpServing(@Nullable TetheringRequest request, int tetheringType,
+ String ifname, int ipServingMode) {
+ enableIpServing(request, tetheringType, ifname, ipServingMode, false /* isNcm */);
+ }
+
+ // TODO: make the request @NonNull and move the tetheringType and ipServingMode into it.
+ private void enableIpServing(@Nullable TetheringRequest request, int tetheringType,
+ String ifname, int ipServingMode, boolean isNcm) {
+ ensureIpServerStartedForType(ifname, tetheringType, isNcm);
+ if (tetherInternal(request, ifname, ipServingMode) != TETHER_ERROR_NO_ERROR) {
Log.e(TAG, "unable start tethering on iface " + ifname);
}
}
@@ -1649,7 +1684,9 @@
mLog.e(ifname + " is not a tetherable iface, ignoring");
return;
}
- enableIpServing(type, ifname, IpServer.STATE_LOCAL_ONLY);
+ // No need to look for active requests. There can never be explicit requests for
+ // TETHERING_WIFI_P2P because enableTetheringInternal ignores that type.
+ enableIpServing(null, type, ifname, IpServer.STATE_LOCAL_ONLY);
}
private void disableWifiP2pIpServingIfNeeded(String ifname) {
@@ -1659,17 +1696,35 @@
disableWifiIpServingCommon(TETHERING_WIFI_P2P, ifname);
}
+ // TODO: fold this in to enableWifiIpServing. We cannot do this at the moment because there
+ // are tests that send wifi AP broadcasts with a null interface. But if this can't happen on
+ // real devices, we should fix those tests to always pass in an interface.
+ private int maybeInferWifiTetheringType(String ifname) {
+ return SdkLevel.isAtLeastT() ? TETHERING_WIFI : ifaceNameToType(ifname);
+ }
+
private void enableWifiIpServing(String ifname, int wifiIpMode) {
mLog.log("request WiFi tethering - interface=" + ifname + " state=" + wifiIpMode);
// Map wifiIpMode values to IpServer.Callback serving states.
+ TetheringRequest request;
final int ipServingMode;
+ final int type;
switch (wifiIpMode) {
case IFACE_IP_MODE_TETHERED:
ipServingMode = IpServer.STATE_TETHERED;
+ type = maybeInferWifiTetheringType(ifname);
+ request = getPendingTetheringRequest(type);
break;
case IFACE_IP_MODE_LOCAL_ONLY:
ipServingMode = IpServer.STATE_LOCAL_ONLY;
+ type = maybeInferWifiTetheringType(ifname);
+ // BUG: this request is incorrect - instead of LOHS, it will reflect whatever
+ // request (if any) is being processed for TETHERING_WIFI. However, this is the
+ // historical behaviour. It mostly works because a) most of the time there is no
+ // such request b) tetherinternal doesn't look at the connectivity scope of the
+ // request, it takes the scope from requestedState.
+ request = getPendingTetheringRequest(type);
break;
default:
mLog.e("Cannot enable IP serving in unknown WiFi mode: " + wifiIpMode);
@@ -1678,14 +1733,13 @@
// After T, tethering always trust the iface pass by state change intent. This allow
// tethering to deprecate tetherable wifi regexs after T.
- final int type = SdkLevel.isAtLeastT() ? TETHERING_WIFI : ifaceNameToType(ifname);
if (!checkTetherableType(type)) {
mLog.e(ifname + " is not a tetherable iface, ignoring");
return;
}
if (!TextUtils.isEmpty(ifname)) {
- enableIpServing(type, ifname, ipServingMode);
+ enableIpServing(request, type, ifname, ipServingMode);
} else {
mLog.e("Cannot enable IP serving on missing interface name");
}
@@ -1715,10 +1769,11 @@
return;
}
+ final TetheringRequest request = getPendingTetheringRequest(tetheringType);
if (ifaces != null) {
for (String iface : ifaces) {
if (ifaceNameToType(iface) == tetheringType) {
- enableIpServing(tetheringType, iface, requestedState, forNcmFunction);
+ enableIpServing(request, tetheringType, iface, requestedState, forNcmFunction);
return;
}
}
@@ -2950,7 +3005,7 @@
return type != TETHERING_INVALID;
}
- private void ensureIpServerStarted(final String iface) {
+ private void ensureIpServerStartedForInterface(final String iface) {
// If we don't care about this type of interface, ignore.
final int interfaceType = ifaceNameToType(iface);
if (!checkTetherableType(interfaceType)) {
@@ -2959,10 +3014,11 @@
return;
}
- ensureIpServerStarted(iface, interfaceType, false /* isNcm */);
+ ensureIpServerStartedForType(iface, interfaceType, false /* isNcm */);
}
- private void ensureIpServerStarted(final String iface, int interfaceType, boolean isNcm) {
+ private void ensureIpServerStartedForType(final String iface, int interfaceType,
+ boolean isNcm) {
// If we have already started a TISM for this interface, skip.
if (mTetherStates.containsKey(iface)) {
mLog.log("active iface (" + iface + ") reported as added, ignoring");
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 e50a7fd..866b219 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
@@ -957,8 +957,8 @@
mTethering.startTethering(request, TEST_CALLER_PKG, null);
mLooper.dispatchAll();
- assertEquals(1, mTethering.getActiveTetheringRequests().size());
- assertEquals(request, mTethering.getActiveTetheringRequests().get(TETHERING_USB));
+ assertEquals(1, mTethering.getPendingTetheringRequests().size());
+ assertEquals(request, mTethering.getPendingTetheringRequests().get(TETHERING_USB));
if (mTethering.getTetheringConfiguration().isUsingNcm()) {
verify(mUsbManager).setCurrentFunctions(UsbManager.FUNCTION_NCM);
@@ -2170,7 +2170,7 @@
runUsbTethering(upstreamState);
assertContains(Arrays.asList(mTethering.getTetheredIfaces()), TEST_RNDIS_IFNAME);
assertTrue(mTethering.isTetheringActive());
- assertEquals(0, mTethering.getActiveTetheringRequests().size());
+ assertEquals(0, mTethering.getPendingTetheringRequests().size());
final Tethering.UserRestrictionActionListener ural = makeUserRestrictionActionListener(
mTethering, false /* currentDisallow */, true /* nextDisallow */);