diff --git a/Tethering/src/com/android/networkstack/tethering/Tethering.java b/Tethering/src/com/android/networkstack/tethering/Tethering.java index 079bf9c475..08170f9a69 100644 --- a/Tethering/src/com/android/networkstack/tethering/Tethering.java +++ b/Tethering/src/com/android/networkstack/tethering/Tethering.java @@ -181,12 +181,16 @@ public class Tethering { 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 @@ public class Tethering { // 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 @@ public class Tethering { return; } - startOrStopIpServer(iface, up); + processInterfaceStateChange(iface, up); } void interfaceLinkStateChanged(String iface, boolean up) { @@ -576,12 +582,12 @@ public class Tethering { 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 @@ public class Tethering { : 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 @@ public class Tethering { 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 @@ public class Tethering { // 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; + } + + 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); + } } - mRndisEnabled = usbConfigured && rndisEnabled; - mNcmEnabled = usbConfigured && ncmEnabled; } private void handleWifiApAction(Intent intent) { @@ -1216,7 +1249,12 @@ public class Tethering { } 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 @@ public class Tethering { } } - // 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 @@ public class Tethering { 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 @@ public class Tethering { 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 @@ public class Tethering { 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 9e0c880f18..5aebca06d4 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 @@ public class TetheringTest { 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 @@ public class TetheringTest { 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 @@ public class TetheringTest { 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 @@ public class TetheringTest { 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 @@ public class TetheringTest { 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 @@ public class TetheringTest { 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 @@ public class TetheringTest { 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 @@ public class TetheringTest { 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