Enable and disable usb IpServer according to ACTION_USB_STATE
Before this change, usb IpServer would be created if corresponding
interface is available even there is no enable tethering attempt. And
usb IpServer only be disabled when interface is removed. Usb tethering
could not be stopped if interface is always avaialbe.
After this change, usb IpServer would be enabled and disabled according
to ACTION_USB_STATE.
Bug: 185649441
Test: atest TetheringCoverageTests
atest MtsTetheringTestLatestSdk
atest CtsTetheringTest
Change-Id: I8375e3f998d677dc658370b2553facaec0d5ff7c
diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java
index 079bf9c..08170f9 100644
--- a/Tethering/src/com/android/networkstack/tethering/Tethering.java
+++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java
@@ -181,12 +181,16 @@
public final IpServer ipServer;
public int lastState;
public int lastError;
+ // This field only valid for TETHERING_USB and TETHERING_NCM.
+ // TODO: Change this from boolean to int for extension.
+ public final boolean isNcm;
- TetherState(IpServer ipServer) {
+ TetherState(IpServer ipServer, boolean isNcm) {
this.ipServer = ipServer;
// Assume all state machines start out available and with no errors.
lastState = IpServer.STATE_AVAILABLE;
lastError = TETHER_ERROR_NO_ERROR;
+ this.isNcm = isNcm;
}
public boolean isCurrentlyServing() {
@@ -522,9 +526,11 @@
// This method needs to exist because TETHERING_BLUETOOTH and TETHERING_WIGIG can't use
// enableIpServing.
- private void startOrStopIpServer(final String iface, boolean enabled) {
- // TODO: do not listen to USB interface state changes. USB tethering is driven only by
- // USB_ACTION broadcasts.
+ private void processInterfaceStateChange(final String iface, boolean enabled) {
+ // Do not listen to USB interface state changes or USB interface add/removes. USB tethering
+ // is driven only by USB_ACTION broadcasts.
+ final int type = ifaceNameToType(iface);
+ if (type == TETHERING_USB || type == TETHERING_NCM) return;
if (enabled) {
ensureIpServerStarted(iface);
@@ -548,7 +554,7 @@
return;
}
- startOrStopIpServer(iface, up);
+ processInterfaceStateChange(iface, up);
}
void interfaceLinkStateChanged(String iface, boolean up) {
@@ -576,12 +582,12 @@
void interfaceAdded(String iface) {
if (VDBG) Log.d(TAG, "interfaceAdded " + iface);
- startOrStopIpServer(iface, true /* enabled */);
+ processInterfaceStateChange(iface, true /* enabled */);
}
void interfaceRemoved(String iface) {
if (VDBG) Log.d(TAG, "interfaceRemoved " + iface);
- startOrStopIpServer(iface, false /* enabled */);
+ processInterfaceStateChange(iface, false /* enabled */);
}
void startTethering(final TetheringRequestParcel request, final IIntResultListener listener) {
@@ -894,7 +900,7 @@
: IpServer.STATE_TETHERED;
}
- private int getRequestedUsbType(boolean forNcmFunction) {
+ private int getServedUsbType(boolean forNcmFunction) {
// TETHERING_NCM is only used if the device does not use NCM for regular USB tethering.
if (forNcmFunction && !mConfig.isUsingNcm()) return TETHERING_NCM;
@@ -1036,11 +1042,11 @@
private void handleUsbAction(Intent intent) {
final boolean usbConnected = intent.getBooleanExtra(USB_CONNECTED, false);
final boolean usbConfigured = intent.getBooleanExtra(USB_CONFIGURED, false);
- final boolean rndisEnabled = intent.getBooleanExtra(USB_FUNCTION_RNDIS, false);
- final boolean ncmEnabled = intent.getBooleanExtra(USB_FUNCTION_NCM, false);
+ final boolean usbRndis = intent.getBooleanExtra(USB_FUNCTION_RNDIS, false);
+ final boolean usbNcm = intent.getBooleanExtra(USB_FUNCTION_NCM, false);
mLog.i(String.format("USB bcast connected:%s configured:%s rndis:%s ncm:%s",
- usbConnected, usbConfigured, rndisEnabled, ncmEnabled));
+ usbConnected, usbConfigured, usbRndis, usbNcm));
// There are three types of ACTION_USB_STATE:
//
@@ -1057,18 +1063,45 @@
// functions are ready to use.
//
// For more explanation, see b/62552150 .
- if (!usbConnected && (mRndisEnabled || mNcmEnabled)) {
- // Turn off tethering if it was enabled and there is a disconnect.
- disableUsbIpServing(TETHERING_USB);
- mEntitlementMgr.stopProvisioningIfNeeded(TETHERING_USB);
- } else if (usbConfigured && rndisEnabled) {
- // Tether if rndis is enabled and usb is configured.
- enableUsbIpServing(false /* isNcm */);
- } else if (usbConfigured && ncmEnabled) {
- enableUsbIpServing(true /* isNcm */);
+ boolean rndisEnabled = usbConfigured && usbRndis;
+ boolean ncmEnabled = usbConfigured && usbNcm;
+ if (!usbConnected) {
+ // Don't stop provisioning if function is disabled but usb is still connected. The
+ // function may be disable/enable to handle ip conflict condition (disabling the
+ // function is necessary to ensure the connected device sees a disconnect).
+ // Normally the provisioning should be stopped by stopTethering(int)
+ maybeStopUsbProvisioning();
+ rndisEnabled = false;
+ ncmEnabled = false;
}
- mRndisEnabled = usbConfigured && rndisEnabled;
- mNcmEnabled = usbConfigured && ncmEnabled;
+
+ if (mRndisEnabled != rndisEnabled) {
+ changeUsbIpServing(rndisEnabled, false /* forNcmFunction */);
+ mRndisEnabled = rndisEnabled;
+ }
+
+ if (mNcmEnabled != ncmEnabled) {
+ changeUsbIpServing(ncmEnabled, true /* forNcmFunction */);
+ mNcmEnabled = ncmEnabled;
+ }
+ }
+
+ private void changeUsbIpServing(boolean enable, boolean forNcmFunction) {
+ if (enable) {
+ // enable ip serving if function is enabled and usb is configured.
+ enableUsbIpServing(forNcmFunction);
+ } else {
+ disableUsbIpServing(forNcmFunction);
+ }
+ }
+
+ private void maybeStopUsbProvisioning() {
+ for (int i = 0; i < mTetherStates.size(); i++) {
+ final int type = mTetherStates.valueAt(i).ipServer.interfaceType();
+ if (type == TETHERING_USB || type == TETHERING_NCM) {
+ mEntitlementMgr.stopProvisioningIfNeeded(type);
+ }
+ }
}
private void handleWifiApAction(Intent intent) {
@@ -1216,7 +1249,12 @@
}
private void enableIpServing(int tetheringType, String ifname, int ipServingMode) {
- ensureIpServerStarted(ifname, tetheringType);
+ enableIpServing(tetheringType, ifname, ipServingMode, false /* isNcm */);
+ }
+
+ private void enableIpServing(int tetheringType, String ifname, int ipServingMode,
+ boolean isNcm) {
+ ensureIpServerStarted(ifname, tetheringType, isNcm);
changeInterfaceState(ifname, ipServingMode);
}
@@ -1289,15 +1327,22 @@
}
}
- // TODO: Consider renaming to something more accurate in its description.
+ // TODO: Pass TetheringRequest into this method. The code can look at the existing requests
+ // to see which one matches the function that was enabled. That will tell the code what
+ // tethering type was requested, without having to guess it from the configuration.
// This method:
// - allows requesting either tethering or local hotspot serving states
- // - handles both enabling and disabling serving states
// - only tethers the first matching interface in listInterfaces()
// order of a given type
- private void enableUsbIpServing(boolean isNcm) {
- final int interfaceType = getRequestedUsbType(isNcm);
- final int requestedState = getRequestedState(interfaceType);
+ private void enableUsbIpServing(boolean forNcmFunction) {
+ // Note: TetheringConfiguration#isUsingNcm can change between the call to
+ // startTethering(TETHERING_USB) and the ACTION_USB_STATE broadcast. If the USB tethering
+ // function changes from NCM to RNDIS, this can lead to Tethering starting NCM tethering
+ // as local-only. But if this happens, the SettingsObserver will call stopTetheringInternal
+ // for both TETHERING_USB and TETHERING_NCM, so the local-only NCM interface will be
+ // stopped immediately.
+ final int tetheringType = getServedUsbType(forNcmFunction);
+ final int requestedState = getRequestedState(tetheringType);
String[] ifaces = null;
try {
ifaces = mNetd.interfaceGetList();
@@ -1306,49 +1351,28 @@
return;
}
- String chosenIface = null;
if (ifaces != null) {
for (String iface : ifaces) {
- if (ifaceNameToType(iface) == interfaceType) {
- chosenIface = iface;
- break;
+ if (ifaceNameToType(iface) == tetheringType) {
+ enableIpServing(tetheringType, iface, requestedState, forNcmFunction);
+ return;
}
}
}
- if (chosenIface == null) {
- Log.e(TAG, "could not find iface of type " + interfaceType);
- return;
- }
-
- changeInterfaceState(chosenIface, requestedState);
+ mLog.e("could not enable IpServer for function " + (forNcmFunction ? "NCM" : "RNDIS"));
}
- private void disableUsbIpServing(int interfaceType) {
- String[] ifaces = null;
- try {
- ifaces = mNetd.interfaceGetList();
- } catch (RemoteException | ServiceSpecificException e) {
- mLog.e("Cannot disableUsbIpServing due to error listing Interfaces" + e);
- return;
- }
+ private void disableUsbIpServing(boolean forNcmFunction) {
+ for (int i = 0; i < mTetherStates.size(); i++) {
+ final TetherState state = mTetherStates.valueAt(i);
+ final int type = state.ipServer.interfaceType();
+ if (type != TETHERING_USB && type != TETHERING_NCM) continue;
- String chosenIface = null;
- if (ifaces != null) {
- for (String iface : ifaces) {
- if (ifaceNameToType(iface) == interfaceType) {
- chosenIface = iface;
- break;
- }
+ if (state.isNcm == forNcmFunction) {
+ ensureIpServerStopped(state.ipServer.interfaceName());
}
}
-
- if (chosenIface == null) {
- Log.e(TAG, "could not find iface of type " + interfaceType);
- return;
- }
-
- changeInterfaceState(chosenIface, IpServer.STATE_AVAILABLE);
}
private void changeInterfaceState(String ifname, int requestedState) {
@@ -2545,10 +2569,10 @@
return;
}
- ensureIpServerStarted(iface, interfaceType);
+ ensureIpServerStarted(iface, interfaceType, false /* isNcm */);
}
- private void ensureIpServerStarted(final String iface, int interfaceType) {
+ private void ensureIpServerStarted(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");
@@ -2560,7 +2584,7 @@
new IpServer(iface, mLooper, interfaceType, mLog, mNetd, mBpfCoordinator,
makeControlCallback(), mConfig.enableLegacyDhcpServer,
mConfig.isBpfOffloadEnabled(), mPrivateAddressCoordinator,
- mDeps.getIpServerDependencies()));
+ mDeps.getIpServerDependencies()), isNcm);
mTetherStates.put(iface, tetherState);
tetherState.ipServer.start();
}
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 9e0c880..5aebca0 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
@@ -234,6 +234,8 @@
private static final int WIFI_NETID = 101;
private static final int DUN_NETID = 102;
+ private static final int TETHER_USB_RNDIS_NCM_FUNCTIONS = 2;
+
private static final int DHCPSERVER_START_TIMEOUT_MS = 1000;
@Mock private ApplicationInfo mApplicationInfo;
@@ -648,17 +650,17 @@
TetheringConfiguration.TETHER_USB_RNDIS_FUNCTION);
// Setup tetherable configuration.
when(mResources.getStringArray(R.array.config_tether_usb_regexs))
- .thenReturn(new String[] { TEST_RNDIS_REGEX});
+ .thenReturn(new String[] {TEST_RNDIS_REGEX});
when(mResources.getStringArray(R.array.config_tether_wifi_regexs))
- .thenReturn(new String[] { TEST_WIFI_REGEX });
+ .thenReturn(new String[] {TEST_WIFI_REGEX});
when(mResources.getStringArray(R.array.config_tether_wifi_p2p_regexs))
- .thenReturn(new String[] { TEST_P2P_REGEX });
+ .thenReturn(new String[] {TEST_P2P_REGEX});
when(mResources.getStringArray(R.array.config_tether_bluetooth_regexs))
- .thenReturn(new String[] { TEST_BT_REGEX });
+ .thenReturn(new String[] {TEST_BT_REGEX});
when(mResources.getStringArray(R.array.config_tether_ncm_regexs))
- .thenReturn(new String[] { TEST_NCM_REGEX });
+ .thenReturn(new String[] {TEST_NCM_REGEX});
when(mResources.getIntArray(R.array.config_tether_upstream_types)).thenReturn(
- new int[] { TYPE_WIFI, TYPE_MOBILE_DUN });
+ new int[] {TYPE_WIFI, TYPE_MOBILE_DUN});
when(mResources.getBoolean(R.bool.config_tether_upstream_automatic)).thenReturn(true);
}
@@ -738,7 +740,16 @@
mLooper.dispatchAll();
}
+ // enableType:
+ // No function enabled = -1
+ // TETHER_USB_RNDIS_FUNCTION = 0
+ // TETHER_USB_NCM_FUNCTIONS = 1
+ // TETHER_USB_RNDIS_NCM_FUNCTIONS = 2
private boolean tetherUsbFunctionMatches(int function, int enabledType) {
+ if (enabledType < 0) return false;
+
+ if (enabledType == TETHER_USB_RNDIS_NCM_FUNCTIONS) return function < enabledType;
+
return function == enabledType;
}
@@ -822,8 +833,6 @@
mTethering.startTethering(createTetheringRequestParcel(TETHERING_NCM), null);
mLooper.dispatchAll();
verify(mUsbManager, times(1)).setCurrentFunctions(UsbManager.FUNCTION_NCM);
-
- mTethering.interfaceStatusChanged(TEST_NCM_IFNAME, true);
}
private void prepareUsbTethering() {
@@ -1903,7 +1912,6 @@
private void runStopUSBTethering() {
mTethering.stopTethering(TETHERING_USB);
mLooper.dispatchAll();
- mTethering.interfaceRemoved(TEST_RNDIS_IFNAME);
sendUsbBroadcast(true, true, -1 /* function */);
mLooper.dispatchAll();
verify(mUsbManager).setCurrentFunctions(UsbManager.FUNCTION_NONE);
@@ -2087,7 +2095,7 @@
setDataSaverEnabled(true);
// Verify that tethering should be disabled.
verify(mUsbManager, times(1)).setCurrentFunctions(UsbManager.FUNCTION_NONE);
- mTethering.interfaceRemoved(TEST_RNDIS_IFNAME);
+ sendUsbBroadcast(true, true, -1 /* function */);
mLooper.dispatchAll();
assertEquals(mTethering.getTetheredIfaces(), new String[0]);
reset(mUsbManager, mIPv6TetheringCoordinator);
@@ -2350,14 +2358,14 @@
final String ipv4Address = ifaceConfigCaptor.getValue().ipv4Addr;
verify(mDhcpServer, timeout(DHCPSERVER_START_TIMEOUT_MS).times(1)).startWithCallbacks(
any(), any());
- reset(mNetd, mUsbManager);
+ reset(mUsbManager);
// Cause a prefix conflict by assigning a /30 out of the downstream's /24 to the upstream.
updateV4Upstream(new LinkAddress(InetAddresses.parseNumericAddress(ipv4Address), 30),
wifiNetwork, TEST_WIFI_IFNAME, TRANSPORT_WIFI);
// verify turn off usb tethering
verify(mUsbManager).setCurrentFunctions(UsbManager.FUNCTION_NONE);
- mTethering.interfaceRemoved(TEST_RNDIS_IFNAME);
+ sendUsbBroadcast(true, true, -1 /* function */);
mLooper.dispatchAll();
// verify restart usb tethering
verify(mUsbManager).setCurrentFunctions(UsbManager.FUNCTION_RNDIS);
@@ -2398,7 +2406,7 @@
verify(mUsbManager).setCurrentFunctions(UsbManager.FUNCTION_NONE);
// verify turn off ethernet tethering
verify(mockRequest).release();
- mTethering.interfaceRemoved(TEST_RNDIS_IFNAME);
+ sendUsbBroadcast(true, true, -1 /* function */);
ethCallback.onUnavailable();
mLooper.dispatchAll();
// verify restart usb tethering