diff options
2 files changed, 33 insertions, 32 deletions
diff --git a/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java b/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java index 37221a971ad1..5e5157913f20 100644 --- a/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java +++ b/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java @@ -250,31 +250,33 @@ public class TetherInterfaceStateMachine extends StateMachine { } private void cleanupUpstream() { - if (mMyUpstreamIfaceName != null) { - // note that we don't care about errors here. - // sometimes interfaces are gone before we get - // to remove their rules, which generates errors. - // just do the best we can. - try { - // about to tear down NAT; gather remaining statistics - mStatsService.forceUpdate(); - } catch (Exception e) { - if (VDBG) Log.e(TAG, "Exception in forceUpdate: " + e.toString()); - } - try { - mNMService.stopInterfaceForwarding(mIfaceName, mMyUpstreamIfaceName); - } catch (Exception e) { - if (VDBG) Log.e( - TAG, "Exception in removeInterfaceForward: " + e.toString()); - } - try { - mNMService.disableNat(mIfaceName, mMyUpstreamIfaceName); - } catch (Exception e) { - if (VDBG) Log.e(TAG, "Exception in disableNat: " + e.toString()); - } - mMyUpstreamIfaceName = null; + if (mMyUpstreamIfaceName == null) return; + + cleanupUpstreamInterface(mMyUpstreamIfaceName); + mMyUpstreamIfaceName = null; + } + + private void cleanupUpstreamInterface(String upstreamIface) { + // Note that we don't care about errors here. + // Sometimes interfaces are gone before we get + // to remove their rules, which generates errors. + // Just do the best we can. + try { + // About to tear down NAT; gather remaining statistics. + mStatsService.forceUpdate(); + } catch (Exception e) { + if (VDBG) Log.e(TAG, "Exception in forceUpdate: " + e.toString()); + } + try { + mNMService.stopInterfaceForwarding(mIfaceName, upstreamIface); + } catch (Exception e) { + if (VDBG) Log.e(TAG, "Exception in removeInterfaceForward: " + e.toString()); + } + try { + mNMService.disableNat(mIfaceName, upstreamIface); + } catch (Exception e) { + if (VDBG) Log.e(TAG, "Exception in disableNat: " + e.toString()); } - return; } @Override @@ -306,6 +308,7 @@ public class TetherInterfaceStateMachine extends StateMachine { newUpstreamIfaceName); } catch (Exception e) { Log.e(TAG, "Exception enabling Nat: " + e.toString()); + cleanupUpstreamInterface(newUpstreamIfaceName); mLastError = ConnectivityManager.TETHER_ERROR_ENABLE_NAT_ERROR; transitionTo(mInitialState); return true; diff --git a/tests/net/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachineTest.java b/tests/net/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachineTest.java index 0fa45454fa70..2b31eb6bf825 100644 --- a/tests/net/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachineTest.java +++ b/tests/net/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachineTest.java @@ -210,10 +210,9 @@ public class TetherInterfaceStateMachineTest { inOrder.verify(mNMService).stopInterfaceForwarding(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNMService).disableNat(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNMService).enableNat(IFACE_NAME, UPSTREAM_IFACE2); - // TODO: Verify proper cleanup is performed: - // inOrder.verify(mStatsService).forceUpdate(); - // inOrder.verify(mNMService).stopInterfaceForwarding(IFACE_NAME, UPSTREAM_IFACE2); - // inOrder.verify(mNMService).disableNat(IFACE_NAME, UPSTREAM_IFACE2); + inOrder.verify(mStatsService).forceUpdate(); + inOrder.verify(mNMService).stopInterfaceForwarding(IFACE_NAME, UPSTREAM_IFACE2); + inOrder.verify(mNMService).disableNat(IFACE_NAME, UPSTREAM_IFACE2); } @Test @@ -230,10 +229,9 @@ public class TetherInterfaceStateMachineTest { inOrder.verify(mNMService).disableNat(IFACE_NAME, UPSTREAM_IFACE); inOrder.verify(mNMService).enableNat(IFACE_NAME, UPSTREAM_IFACE2); inOrder.verify(mNMService).startInterfaceForwarding(IFACE_NAME, UPSTREAM_IFACE2); - // TODO: Verify proper cleanup is performed: - // inOrder.verify(mStatsService).forceUpdate(); - // inOrder.verify(mNMService).stopInterfaceForwarding(IFACE_NAME, UPSTREAM_IFACE2); - // inOrder.verify(mNMService).disableNat(IFACE_NAME, UPSTREAM_IFACE2); + inOrder.verify(mStatsService).forceUpdate(); + inOrder.verify(mNMService).stopInterfaceForwarding(IFACE_NAME, UPSTREAM_IFACE2); + inOrder.verify(mNMService).disableNat(IFACE_NAME, UPSTREAM_IFACE2); } @Test |