From ddce7ee20fcad91bd45f77222d762136587bc192 Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Mon, 21 Aug 2017 12:34:50 +0900 Subject: [PATCH 1/2] Don't completely stop offload if setting data limit fails. Currently, if setting a data limit fails, we completely stop offload in order to avoid data overages. However, the next thing we do is try to fetch the stats and crash, because once offload is stopped all our local state is cleared. Fix this by fetching stats before we stop offload. Bug: 29337859 Bug: 32163131 Bug: 64867836 Test: OffloadControllerTest passes Test: no crash when disabling wifi tethering with BT tethering active Merged-In: I7fc47e60b2da5f39c26fb22c1325618f9948dd38 Merged-In: I464dd2a6d1996b1cfb8bbf82b6ee453fd0747569 Change-Id: I260f5450f8b67f055983af68fb23a5f3cfc0bc69 (cherry picked from commit d743601a002ac12c02da58e92ebd0544ab0b77ea) --- .../tethering/OffloadController.java | 24 ++++++++++++++----- .../tethering/OffloadControllerTest.java | 10 ++++++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/tethering/OffloadController.java b/services/core/java/com/android/server/connectivity/tethering/OffloadController.java index bcc74c0d49e40..ef18e4e0ba75b 100644 --- a/services/core/java/com/android/server/connectivity/tethering/OffloadController.java +++ b/services/core/java/com/android/server/connectivity/tethering/OffloadController.java @@ -131,21 +131,25 @@ public class OffloadController { new OffloadHardwareInterface.ControlCallback() { @Override public void onStarted() { + if (!started()) return; mLog.log("onStarted"); } @Override public void onStoppedError() { + if (!started()) return; mLog.log("onStoppedError"); } @Override public void onStoppedUnsupported() { + if (!started()) return; mLog.log("onStoppedUnsupported"); } @Override public void onSupportAvailable() { + if (!started()) return; mLog.log("onSupportAvailable"); // [1] Poll for statistics and notify NetworkStats @@ -153,11 +157,12 @@ public class OffloadController { // [a] push local prefixes // [b] push downstreams // [c] push upstream parameters - pushUpstreamParameters(); + pushUpstreamParameters(null); } @Override public void onStoppedLimitReached() { + if (!started()) return; mLog.log("onStoppedLimitReached"); // We cannot reliably determine on which interface the limit was reached, @@ -185,6 +190,7 @@ public class OffloadController { public void onNatTimeoutUpdate(int proto, String srcAddr, int srcPort, String dstAddr, int dstPort) { + if (!started()) return; mLog.log(String.format("NAT timeout update: %s (%s,%s) -> (%s,%s)", proto, srcAddr, srcPort, dstAddr, dstPort)); } @@ -197,6 +203,9 @@ public class OffloadController { } public void stop() { + // Completely stops tethering offload. After this method is called, it is no longer safe to + // call any HAL method, no callbacks from the hardware will be delivered, and any in-flight + // callbacks must be ignored. Offload may be started again by calling start(). final boolean wasStarted = started(); updateStatsForCurrentUpstream(); mUpstreamLinkProperties = null; @@ -305,10 +314,7 @@ public class OffloadController { // onOffloadEvent() callback to tell us offload is available again and // then reapply all state). computeAndPushLocalPrefixes(); - pushUpstreamParameters(); - - // Update stats after we've told the hardware to change routing so we don't miss packets. - maybeUpdateStats(prevUpstream); + pushUpstreamParameters(prevUpstream); } public void setLocalPrefixes(Set localPrefixes) { @@ -342,8 +348,9 @@ public class OffloadController { return mConfigInitialized && mControlInitialized; } - private boolean pushUpstreamParameters() { + private boolean pushUpstreamParameters(String prevUpstream) { if (mUpstreamLinkProperties == null) { + maybeUpdateStats(prevUpstream); return mHwInterface.setUpstreamParameters(null, null, null, null); } @@ -382,9 +389,14 @@ public class OffloadController { return success; } + // Update stats after we've told the hardware to change routing so we don't miss packets. + maybeUpdateStats(prevUpstream); + // Data limits can only be set once offload is running on the upstream. success = maybeUpdateDataLimit(iface); if (!success) { + // If we failed to set a data limit, don't use this upstream, because we don't want to + // blow through the data limit that we were told to apply. mLog.log("Setting data limit for " + iface + " failed, disabling offload."); stop(); } diff --git a/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java b/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java index 3f17b9eb98178..c769492424a46 100644 --- a/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java +++ b/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java @@ -112,7 +112,9 @@ public class OffloadControllerTest { when(mHardware.initOffloadConfig()).thenReturn(true); when(mHardware.initOffloadControl(mControlCallbackCaptor.capture())) .thenReturn(true); + when(mHardware.setUpstreamParameters(anyString(), any(), any(), any())).thenReturn(true); when(mHardware.getForwardedStats(any())).thenReturn(new ForwardedStats()); + when(mHardware.setDataLimit(anyString(), anyLong())).thenReturn(true); } private void enableOffload() { @@ -260,6 +262,7 @@ public class OffloadControllerTest { inOrder.verify(mHardware, never()).setLocalPrefixes(mStringArrayCaptor.capture()); inOrder.verify(mHardware, times(1)).setUpstreamParameters( eq(testIfName), eq(null), eq(null), eq(null)); + inOrder.verify(mHardware, times(1)).setDataLimit(eq(testIfName), eq(Long.MAX_VALUE)); inOrder.verifyNoMoreInteractions(); final String ipv4Addr = "192.0.2.5"; @@ -277,6 +280,7 @@ public class OffloadControllerTest { inOrder.verify(mHardware, times(1)).setUpstreamParameters( eq(testIfName), eq(ipv4Addr), eq(null), eq(null)); inOrder.verify(mHardware, times(1)).getForwardedStats(eq(testIfName)); + inOrder.verify(mHardware, times(1)).setDataLimit(eq(testIfName), eq(Long.MAX_VALUE)); inOrder.verifyNoMoreInteractions(); final String ipv4Gateway = "192.0.2.1"; @@ -287,6 +291,7 @@ public class OffloadControllerTest { inOrder.verify(mHardware, times(1)).setUpstreamParameters( eq(testIfName), eq(ipv4Addr), eq(ipv4Gateway), eq(null)); inOrder.verify(mHardware, times(1)).getForwardedStats(eq(testIfName)); + inOrder.verify(mHardware, times(1)).setDataLimit(eq(testIfName), eq(Long.MAX_VALUE)); inOrder.verifyNoMoreInteractions(); final String ipv6Gw1 = "fe80::cafe"; @@ -300,6 +305,7 @@ public class OffloadControllerTest { ArrayList v6gws = mStringArrayCaptor.getValue(); assertEquals(1, v6gws.size()); assertTrue(v6gws.contains(ipv6Gw1)); + inOrder.verify(mHardware, times(1)).setDataLimit(eq(testIfName), eq(Long.MAX_VALUE)); inOrder.verifyNoMoreInteractions(); final String ipv6Gw2 = "fe80::d00d"; @@ -314,6 +320,7 @@ public class OffloadControllerTest { assertEquals(2, v6gws.size()); assertTrue(v6gws.contains(ipv6Gw1)); assertTrue(v6gws.contains(ipv6Gw2)); + inOrder.verify(mHardware, times(1)).setDataLimit(eq(testIfName), eq(Long.MAX_VALUE)); inOrder.verifyNoMoreInteractions(); final LinkProperties stacked = new LinkProperties(); @@ -332,6 +339,7 @@ public class OffloadControllerTest { assertEquals(2, v6gws.size()); assertTrue(v6gws.contains(ipv6Gw1)); assertTrue(v6gws.contains(ipv6Gw2)); + inOrder.verify(mHardware, times(1)).setDataLimit(eq(testIfName), eq(Long.MAX_VALUE)); inOrder.verifyNoMoreInteractions(); // Add in some IPv6 upstream info. When there is a tethered downstream @@ -363,6 +371,7 @@ public class OffloadControllerTest { assertTrue(v6gws.contains(ipv6Gw1)); assertTrue(v6gws.contains(ipv6Gw2)); inOrder.verify(mHardware, times(1)).getForwardedStats(eq(testIfName)); + inOrder.verify(mHardware, times(1)).setDataLimit(eq(testIfName), eq(Long.MAX_VALUE)); inOrder.verifyNoMoreInteractions(); // Completely identical LinkProperties updates are de-duped. @@ -524,6 +533,7 @@ public class OffloadControllerTest { offload.setUpstreamLinkProperties(lp); provider.setInterfaceQuota(mobileIface, mobileLimit); waitForIdle(); + inOrder.verify(mHardware).getForwardedStats(ethernetIface); inOrder.verify(mHardware).stopOffloadControl(); } From b3bb26eaa790cc18f6383387975630c1ae35752a Mon Sep 17 00:00:00 2001 From: Erik Kline Date: Thu, 6 Jul 2017 19:49:35 +0900 Subject: [PATCH 2/2] Send add/removeDownstream info to offload HAL Test: as follows - built - flashed - booted - "runtest frameworks-net" passed Bug: 29337859 Bug: 32163131 Merged-In: I0cb81ac054fc2bf6c8b8bfe658e9404a15091d7a Merged-In: I7abcdcc2d7d967179c47081a6db2b417164891f3 Change-Id: I6c59aa7cb80b54f376f294b24c1409710c553d74 (cherry picked from commit ed962a84122ab950a103fbbc0bf911f61b6b9500) --- .../server/connectivity/Tethering.java | 128 +++++++++++++----- .../tethering/OffloadController.java | 56 ++++++-- .../tethering/OffloadHardwareInterface.java | 38 ++++++ .../TetherInterfaceStateMachine.java | 5 + .../android/net/util/NetworkConstants.java | 2 + .../java/android/net/util/PrefixUtils.java | 9 ++ .../tethering/OffloadControllerTest.java | 101 ++++++++++++-- 7 files changed, 283 insertions(+), 56 deletions(-) diff --git a/services/core/java/com/android/server/connectivity/Tethering.java b/services/core/java/com/android/server/connectivity/Tethering.java index 836d61aa58eed..5583e86cde705 100644 --- a/services/core/java/com/android/server/connectivity/Tethering.java +++ b/services/core/java/com/android/server/connectivity/Tethering.java @@ -49,6 +49,7 @@ import android.net.ConnectivityManager; import android.net.INetworkPolicyManager; import android.net.INetworkStatsService; import android.net.IpPrefix; +import android.net.LinkAddress; import android.net.LinkProperties; import android.net.Network; import android.net.NetworkCapabilities; @@ -1252,6 +1253,7 @@ public class Tethering extends BaseNetworkObserver { // to tear itself down. private final ArrayList mNotifyList; private final IPv6TetheringCoordinator mIPv6TetheringCoordinator; + private final OffloadWrapper mOffload; private static final int UPSTREAM_SETTLE_TIME_MS = 10000; @@ -1276,33 +1278,11 @@ public class Tethering extends BaseNetworkObserver { mNotifyList = new ArrayList<>(); mIPv6TetheringCoordinator = new IPv6TetheringCoordinator(mNotifyList, mLog); + mOffload = new OffloadWrapper(); setInitialState(mInitialState); } - private void startOffloadController() { - mOffloadController.start(); - sendOffloadExemptPrefixes(); - } - - private void sendOffloadExemptPrefixes() { - sendOffloadExemptPrefixes(mUpstreamNetworkMonitor.getLocalPrefixes()); - } - - private void sendOffloadExemptPrefixes(Set localPrefixes) { - // Add in well-known minimum set. - PrefixUtils.addNonForwardablePrefixes(localPrefixes); - // Add tragically hardcoded prefixes. - localPrefixes.add(PrefixUtils.DEFAULT_WIFI_P2P_PREFIX); - - // Add prefixes for all downstreams, regardless of IP serving mode. - for (TetherInterfaceStateMachine tism : mNotifyList) { - localPrefixes.addAll(PrefixUtils.localPrefixesFrom(tism.linkProperties())); - } - - mOffloadController.setLocalPrefixes(localPrefixes); - } - class InitialState extends State { @Override public boolean processMessage(Message message) { @@ -1460,7 +1440,7 @@ public class Tethering extends BaseNetworkObserver { protected void handleNewUpstreamNetworkState(NetworkState ns) { mIPv6TetheringCoordinator.updateUpstreamNetworkState(ns); - mOffloadController.setUpstreamLinkProperties((ns != null) ? ns.linkProperties : null); + mOffload.updateUpstreamNetworkState(ns); } private void handleInterfaceServingStateActive(int mode, TetherInterfaceStateMachine who) { @@ -1470,9 +1450,12 @@ public class Tethering extends BaseNetworkObserver { } if (mode == IControlsTethering.STATE_TETHERED) { + // No need to notify OffloadController just yet as there are no + // "offload-able" prefixes to pass along. This will handled + // when the TISM informs Tethering of its LinkProperties. mForwardedDownstreams.add(who); } else { - mOffloadController.removeDownstreamInterface(who.interfaceName()); + mOffload.excludeDownstreamInterface(who.interfaceName()); mForwardedDownstreams.remove(who); } @@ -1497,7 +1480,7 @@ public class Tethering extends BaseNetworkObserver { private void handleInterfaceServingStateInactive(TetherInterfaceStateMachine who) { mNotifyList.remove(who); mIPv6TetheringCoordinator.removeActiveDownstream(who); - mOffloadController.removeDownstreamInterface(who.interfaceName()); + mOffload.excludeDownstreamInterface(who.interfaceName()); mForwardedDownstreams.remove(who); // If this is a Wi-Fi interface, tell WifiManager of any errors. @@ -1511,7 +1494,7 @@ public class Tethering extends BaseNetworkObserver { private void handleUpstreamNetworkMonitorCallback(int arg1, Object o) { if (arg1 == UpstreamNetworkMonitor.NOTIFY_LOCAL_PREFIXES) { - sendOffloadExemptPrefixes((Set) o); + mOffload.sendOffloadExemptPrefixes((Set) o); return; } @@ -1581,7 +1564,7 @@ public class Tethering extends BaseNetworkObserver { // TODO: De-duplicate with updateUpstreamWanted() below. if (upstreamWanted()) { mUpstreamWanted = true; - startOffloadController(); + mOffload.start(); chooseUpstreamType(true); mTryCell = false; } @@ -1589,7 +1572,7 @@ public class Tethering extends BaseNetworkObserver { @Override public void exit() { - mOffloadController.stop(); + mOffload.stop(); mUpstreamNetworkMonitor.stop(); mSimChange.stopListening(); notifyDownstreamsOfNewUpstreamIface(null); @@ -1601,9 +1584,9 @@ public class Tethering extends BaseNetworkObserver { mUpstreamWanted = upstreamWanted(); if (mUpstreamWanted != previousUpstreamWanted) { if (mUpstreamWanted) { - startOffloadController(); + mOffload.start(); } else { - mOffloadController.stop(); + mOffload.stop(); } } return previousUpstreamWanted; @@ -1658,12 +1641,9 @@ public class Tethering extends BaseNetworkObserver { case EVENT_IFACE_UPDATE_LINKPROPERTIES: { final LinkProperties newLp = (LinkProperties) message.obj; if (message.arg1 == IControlsTethering.STATE_TETHERED) { - mOffloadController.notifyDownstreamLinkProperties(newLp); + mOffload.updateDownstreamLinkProperties(newLp); } else { - mOffloadController.removeDownstreamInterface(newLp.getInterfaceName()); - // Another interface might be in local-only hotspot mode; - // resend all local prefixes to the OffloadController. - sendOffloadExemptPrefixes(); + mOffload.excludeDownstreamInterface(newLp.getInterfaceName()); } break; } @@ -1778,6 +1758,82 @@ public class Tethering extends BaseNetworkObserver { } catch (Exception e) {} } } + + // A wrapper class to handle multiple situations where several calls to + // the OffloadController need to happen together. + // + // TODO: This suggests that the interface between OffloadController and + // Tethering is in need of improvement. Refactor these calls into the + // OffloadController implementation. + class OffloadWrapper { + public void start() { + mOffloadController.start(); + sendOffloadExemptPrefixes(); + } + + public void stop() { + mOffloadController.stop(); + } + + public void updateUpstreamNetworkState(NetworkState ns) { + mOffloadController.setUpstreamLinkProperties( + (ns != null) ? ns.linkProperties : null); + } + + public void updateDownstreamLinkProperties(LinkProperties newLp) { + // Update the list of offload-exempt prefixes before adding + // new prefixes on downstream interfaces to the offload HAL. + sendOffloadExemptPrefixes(); + mOffloadController.notifyDownstreamLinkProperties(newLp); + } + + public void excludeDownstreamInterface(String ifname) { + // This and other interfaces may be in local-only hotspot mode; + // resend all local prefixes to the OffloadController. + sendOffloadExemptPrefixes(); + mOffloadController.removeDownstreamInterface(ifname); + } + + public void sendOffloadExemptPrefixes() { + sendOffloadExemptPrefixes(mUpstreamNetworkMonitor.getLocalPrefixes()); + } + + public void sendOffloadExemptPrefixes(final Set localPrefixes) { + // Add in well-known minimum set. + PrefixUtils.addNonForwardablePrefixes(localPrefixes); + // Add tragically hardcoded prefixes. + localPrefixes.add(PrefixUtils.DEFAULT_WIFI_P2P_PREFIX); + + // Maybe add prefixes or addresses for downstreams, depending on + // the IP serving mode of each. + for (TetherInterfaceStateMachine tism : mNotifyList) { + final LinkProperties lp = tism.linkProperties(); + + switch (tism.servingMode()) { + case IControlsTethering.STATE_UNAVAILABLE: + case IControlsTethering.STATE_AVAILABLE: + // No usable LinkProperties in these states. + continue; + case IControlsTethering.STATE_TETHERED: + // Only add IPv4 /32 and IPv6 /128 prefixes. The + // directly-connected prefixes will be sent as + // downstream "offload-able" prefixes. + for (LinkAddress addr : lp.getAllLinkAddresses()) { + final InetAddress ip = addr.getAddress(); + if (ip.isLinkLocalAddress()) continue; + localPrefixes.add(PrefixUtils.ipAddressAsPrefix(ip)); + } + break; + case IControlsTethering.STATE_LOCAL_ONLY: + // Add prefixes covering all local IPs. + localPrefixes.addAll(PrefixUtils.localPrefixesFrom(lp)); + break; + } + } + + mOffloadController.setLocalPrefixes(localPrefixes); + } + } } @Override diff --git a/services/core/java/com/android/server/connectivity/tethering/OffloadController.java b/services/core/java/com/android/server/connectivity/tethering/OffloadController.java index ef18e4e0ba75b..6d5c428e58f8c 100644 --- a/services/core/java/com/android/server/connectivity/tethering/OffloadController.java +++ b/services/core/java/com/android/server/connectivity/tethering/OffloadController.java @@ -48,6 +48,7 @@ import java.net.InetAddress; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -69,6 +70,7 @@ public class OffloadController { private final INetworkManagementService mNms; private final ITetheringStatsProvider mStatsProvider; private final SharedLog mLog; + private final HashMap mDownstreams; private boolean mConfigInitialized; private boolean mControlInitialized; private LinkProperties mUpstreamLinkProperties; @@ -100,6 +102,7 @@ public class OffloadController { mNms = nms; mStatsProvider = new OffloadTetheringStatsProvider(); mLog = log.forSubComponent(TAG); + mDownstreams = new HashMap<>(); mExemptPrefixes = new HashSet<>(); mLastLocalPrefixStrs = new HashSet<>(); @@ -257,6 +260,11 @@ public class OffloadController { } } + private String currentUpstreamInterface() { + return (mUpstreamLinkProperties != null) + ? mUpstreamLinkProperties.getInterfaceName() : null; + } + private void maybeUpdateStats(String iface) { if (TextUtils.isEmpty(iface)) { return; @@ -281,9 +289,7 @@ public class OffloadController { private boolean maybeUpdateDataLimit(String iface) { // setDataLimit may only be called while offload is occuring on this upstream. - if (!started() || - mUpstreamLinkProperties == null || - !TextUtils.equals(iface, mUpstreamLinkProperties.getInterfaceName())) { + if (!started() || !TextUtils.equals(iface, currentUpstreamInterface())) { return true; } @@ -296,9 +302,7 @@ public class OffloadController { } private void updateStatsForCurrentUpstream() { - if (mUpstreamLinkProperties != null) { - maybeUpdateStats(mUpstreamLinkProperties.getInterfaceName()); - } + maybeUpdateStats(currentUpstreamInterface()); } public void setUpstreamLinkProperties(LinkProperties lp) { @@ -325,17 +329,42 @@ public class OffloadController { } public void notifyDownstreamLinkProperties(LinkProperties lp) { + final String ifname = lp.getInterfaceName(); + final LinkProperties oldLp = mDownstreams.put(ifname, new LinkProperties(lp)); + if (Objects.equals(oldLp, lp)) return; + if (!started()) return; - // TODO: Cache LinkProperties on a per-ifname basis and compute the - // deltas, calling addDownstream()/removeDownstream() accordingly. + final List oldRoutes = (oldLp != null) ? oldLp.getRoutes() : new ArrayList<>(); + final List newRoutes = lp.getRoutes(); + + // For each old route, if not in new routes: remove. + for (RouteInfo oldRoute : oldRoutes) { + if (shouldIgnoreDownstreamRoute(oldRoute)) continue; + if (!newRoutes.contains(oldRoute)) { + mHwInterface.removeDownstreamPrefix(ifname, oldRoute.getDestination().toString()); + } + } + + // For each new route, if not in old routes: add. + for (RouteInfo newRoute : newRoutes) { + if (shouldIgnoreDownstreamRoute(newRoute)) continue; + if (!oldRoutes.contains(newRoute)) { + mHwInterface.addDownstreamPrefix(ifname, newRoute.getDestination().toString()); + } + } } public void removeDownstreamInterface(String ifname) { + final LinkProperties lp = mDownstreams.remove(ifname); + if (lp == null) return; + if (!started()) return; - // TODO: Check cache for LinkProperties of ifname and, if present, - // call removeDownstream() accordingly. + for (RouteInfo route : lp.getRoutes()) { + if (shouldIgnoreDownstreamRoute(route)) continue; + mHwInterface.removeDownstreamPrefix(ifname, route.getDestination().toString()); + } } private boolean isOffloadDisabled() { @@ -442,6 +471,13 @@ public class OffloadController { return localPrefixStrs; } + private static boolean shouldIgnoreDownstreamRoute(RouteInfo route) { + // Ignore any link-local routes. + if (!route.getDestinationLinkAddress().isGlobalPreferred()) return true; + + return false; + } + public void dump(IndentingPrintWriter pw) { if (isOffloadDisabled()) { pw.println("Offload disabled"); diff --git a/services/core/java/com/android/server/connectivity/tethering/OffloadHardwareInterface.java b/services/core/java/com/android/server/connectivity/tethering/OffloadHardwareInterface.java index 86ff0a6077004..865a98902d0b1 100644 --- a/services/core/java/com/android/server/connectivity/tethering/OffloadHardwareInterface.java +++ b/services/core/java/com/android/server/connectivity/tethering/OffloadHardwareInterface.java @@ -236,6 +236,44 @@ public class OffloadHardwareInterface { return results.success; } + public boolean addDownstreamPrefix(String ifname, String prefix) { + final String logmsg = String.format("addDownstreamPrefix(%s, %s)", ifname, prefix); + + final CbResults results = new CbResults(); + try { + mOffloadControl.addDownstream(ifname, prefix, + (boolean success, String errMsg) -> { + results.success = success; + results.errMsg = errMsg; + }); + } catch (RemoteException e) { + record(logmsg, e); + return false; + } + + record(logmsg, results); + return results.success; + } + + public boolean removeDownstreamPrefix(String ifname, String prefix) { + final String logmsg = String.format("removeDownstreamPrefix(%s, %s)", ifname, prefix); + + final CbResults results = new CbResults(); + try { + mOffloadControl.removeDownstream(ifname, prefix, + (boolean success, String errMsg) -> { + results.success = success; + results.errMsg = errMsg; + }); + } catch (RemoteException e) { + record(logmsg, e); + return false; + } + + record(logmsg, results); + return results.success; + } + private void record(String msg, Throwable t) { mLog.e(msg + YIELDS + "exception: " + t); } 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 69678df4543d5..57d2502c6dc7f 100644 --- a/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java +++ b/services/core/java/com/android/server/connectivity/tethering/TetherInterfaceStateMachine.java @@ -115,6 +115,7 @@ public class TetherInterfaceStateMachine extends StateMachine { private final LinkProperties mLinkProperties; private int mLastError; + private int mServingMode; private String mMyUpstreamIfaceName; // may change over time private NetworkInterface mNetworkInterface; private byte[] mHwAddr; @@ -142,6 +143,7 @@ public class TetherInterfaceStateMachine extends StateMachine { mLinkProperties = new LinkProperties(); resetLinkProperties(); mLastError = ConnectivityManager.TETHER_ERROR_NO_ERROR; + mServingMode = IControlsTethering.STATE_AVAILABLE; mInitialState = new InitialState(); mLocalHotspotState = new LocalHotspotState(); @@ -161,6 +163,8 @@ public class TetherInterfaceStateMachine extends StateMachine { public int lastError() { return mLastError; } + public int servingMode() { return mServingMode; } + public LinkProperties linkProperties() { return new LinkProperties(mLinkProperties); } public void stop() { sendMessage(CMD_INTERFACE_DOWN); } @@ -448,6 +452,7 @@ public class TetherInterfaceStateMachine extends StateMachine { } private void sendInterfaceState(int newInterfaceState) { + mServingMode = newInterfaceState; mTetherController.updateInterfaceState( TetherInterfaceStateMachine.this, newInterfaceState, mLastError); sendLinkProperties(); diff --git a/services/net/java/android/net/util/NetworkConstants.java b/services/net/java/android/net/util/NetworkConstants.java index 9b3bc3f0301ae..606526816368c 100644 --- a/services/net/java/android/net/util/NetworkConstants.java +++ b/services/net/java/android/net/util/NetworkConstants.java @@ -87,6 +87,7 @@ public final class NetworkConstants { public static final int IPV4_PROTOCOL_OFFSET = 9; public static final int IPV4_SRC_ADDR_OFFSET = 12; public static final int IPV4_DST_ADDR_OFFSET = 16; + public static final int IPV4_ADDR_BITS = 32; public static final int IPV4_ADDR_LEN = 4; /** @@ -99,6 +100,7 @@ public final class NetworkConstants { public static final int IPV6_PROTOCOL_OFFSET = 6; public static final int IPV6_SRC_ADDR_OFFSET = 8; public static final int IPV6_DST_ADDR_OFFSET = 24; + public static final int IPV6_ADDR_BITS = 128; public static final int IPV6_ADDR_LEN = 16; public static final int IPV6_MIN_MTU = 1280; public static final int RFC7421_PREFIX_LENGTH = 64; diff --git a/services/net/java/android/net/util/PrefixUtils.java b/services/net/java/android/net/util/PrefixUtils.java index 962aab459a19a..f60694aaedc9c 100644 --- a/services/net/java/android/net/util/PrefixUtils.java +++ b/services/net/java/android/net/util/PrefixUtils.java @@ -20,6 +20,8 @@ import android.net.IpPrefix; import android.net.LinkAddress; import android.net.LinkProperties; +import java.net.Inet4Address; +import java.net.InetAddress; import java.util.Collections; import java.util.HashSet; import java.util.Set; @@ -68,6 +70,13 @@ public class PrefixUtils { return new IpPrefix(addr.getAddress(), addr.getPrefixLength()); } + public static IpPrefix ipAddressAsPrefix(InetAddress ip) { + final int bitLength = (ip instanceof Inet4Address) + ? NetworkConstants.IPV4_ADDR_BITS + : NetworkConstants.IPV6_ADDR_BITS; + return new IpPrefix(ip, bitLength); + } + private static IpPrefix pfx(String prefixStr) { return new IpPrefix(prefixStr); } diff --git a/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java b/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java index c769492424a46..679c369c1a871 100644 --- a/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java +++ b/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java @@ -79,6 +79,15 @@ import org.mockito.MockitoAnnotations; @RunWith(AndroidJUnit4.class) @SmallTest public class OffloadControllerTest { + private static final String RNDIS0 = "test_rndis0"; + private static final String RMNET0 = "test_rmnet_data0"; + private static final String WLAN0 = "test_wlan0"; + + private static final String IPV6_LINKLOCAL = "fe80::/64"; + private static final String IPV6_DOC_PREFIX = "2001:db8::/64"; + private static final String IPV6_DISCARD_PREFIX = "100::/64"; + private static final String USB_PREFIX = "192.168.42.0/24"; + private static final String WIFI_PREFIX = "192.168.43.0/24"; @Mock private OffloadHardwareInterface mHardware; @Mock private ApplicationInfo mApplicationInfo; @@ -238,10 +247,8 @@ public class OffloadControllerTest { inOrder.verify(mHardware, times(1)).setLocalPrefixes(mStringArrayCaptor.capture()); ArrayList localPrefixes = mStringArrayCaptor.getValue(); assertEquals(4, localPrefixes.size()); - assertTrue(localPrefixes.contains("127.0.0.0/8")); - assertTrue(localPrefixes.contains("192.0.2.0/24")); - assertTrue(localPrefixes.contains("fe80::/64")); - assertTrue(localPrefixes.contains("2001:db8::/64")); + assertArrayListContains(localPrefixes, + "127.0.0.0/8", "192.0.2.0/24", "fe80::/64", "2001:db8::/64"); inOrder.verifyNoMoreInteractions(); offload.setUpstreamLinkProperties(null); @@ -356,12 +363,9 @@ public class OffloadControllerTest { inOrder.verify(mHardware, times(1)).setLocalPrefixes(mStringArrayCaptor.capture()); localPrefixes = mStringArrayCaptor.getValue(); assertEquals(6, localPrefixes.size()); - assertTrue(localPrefixes.contains("127.0.0.0/8")); - assertTrue(localPrefixes.contains("192.0.2.0/24")); - assertTrue(localPrefixes.contains("fe80::/64")); - assertTrue(localPrefixes.contains("2001:db8::/64")); - assertTrue(localPrefixes.contains("2001:db8::6173:7369:676e:6564/128")); - assertTrue(localPrefixes.contains("2001:db8::7261:6e64:6f6d/128")); + assertArrayListContains(localPrefixes, + "127.0.0.0/8", "192.0.2.0/24", "fe80::/64", "2001:db8::/64", + "2001:db8::6173:7369:676e:6564/128", "2001:db8::7261:6e64:6f6d/128"); // The relevant parts of the LinkProperties have not changed, but at the // moment we do not de-dup upstream LinkProperties this carefully. inOrder.verify(mHardware, times(1)).setUpstreamParameters( @@ -445,6 +449,8 @@ public class OffloadControllerTest { waitForIdle(); // There is no current upstream, so no stats are fetched. inOrder.verify(mHardware, never()).getForwardedStats(eq(ethernetIface)); + inOrder.verify(mHardware, times(1)).setUpstreamParameters( + eq(null), eq(null), eq(null), eq(null)); inOrder.verifyNoMoreInteractions(); assertEquals(2, stats.size()); @@ -549,4 +555,79 @@ public class OffloadControllerTest { callback.onStoppedLimitReached(); verify(mNMService, times(1)).tetherLimitReached(mTetherStatsProviderCaptor.getValue()); } + + @Test + public void testAddRemoveDownstreams() throws Exception { + setupFunctioningHardwareInterface(); + enableOffload(); + + final OffloadController offload = makeOffloadController(); + offload.start(); + + final InOrder inOrder = inOrder(mHardware); + inOrder.verify(mHardware, times(1)).initOffloadConfig(); + inOrder.verify(mHardware, times(1)).initOffloadControl( + any(OffloadHardwareInterface.ControlCallback.class)); + inOrder.verifyNoMoreInteractions(); + + // Tethering makes several calls to setLocalPrefixes() before add/remove + // downstream calls are made. This is not tested here; only the behavior + // of notifyDownstreamLinkProperties() and removeDownstreamInterface() + // are tested. + + // [1] USB tethering is started. + final LinkProperties usbLinkProperties = new LinkProperties(); + usbLinkProperties.setInterfaceName(RNDIS0); + usbLinkProperties.addLinkAddress(new LinkAddress("192.168.42.1/24")); + usbLinkProperties.addRoute(new RouteInfo(new IpPrefix(USB_PREFIX))); + offload.notifyDownstreamLinkProperties(usbLinkProperties); + inOrder.verify(mHardware, times(1)).addDownstreamPrefix(RNDIS0, USB_PREFIX); + inOrder.verifyNoMoreInteractions(); + + // [2] Routes for IPv6 link-local prefixes should never be added. + usbLinkProperties.addRoute(new RouteInfo(new IpPrefix(IPV6_LINKLOCAL))); + offload.notifyDownstreamLinkProperties(usbLinkProperties); + inOrder.verify(mHardware, never()).addDownstreamPrefix(eq(RNDIS0), anyString()); + inOrder.verifyNoMoreInteractions(); + + // [3] Add an IPv6 prefix for good measure. Only new offload-able + // prefixes should be passed to the HAL. + usbLinkProperties.addLinkAddress(new LinkAddress("2001:db8::1/64")); + usbLinkProperties.addRoute(new RouteInfo(new IpPrefix(IPV6_DOC_PREFIX))); + offload.notifyDownstreamLinkProperties(usbLinkProperties); + inOrder.verify(mHardware, times(1)).addDownstreamPrefix(RNDIS0, IPV6_DOC_PREFIX); + inOrder.verifyNoMoreInteractions(); + + // [4] Adding addresses doesn't affect notifyDownstreamLinkProperties(). + // The address is passed in by a separate setLocalPrefixes() invocation. + usbLinkProperties.addLinkAddress(new LinkAddress("2001:db8::2/64")); + offload.notifyDownstreamLinkProperties(usbLinkProperties); + inOrder.verify(mHardware, never()).addDownstreamPrefix(eq(RNDIS0), anyString()); + + // [5] Differences in local routes are converted into addDownstream() + // and removeDownstream() invocations accordingly. + usbLinkProperties.removeRoute(new RouteInfo(new IpPrefix(IPV6_DOC_PREFIX), null, RNDIS0)); + usbLinkProperties.addRoute(new RouteInfo(new IpPrefix(IPV6_DISCARD_PREFIX))); + offload.notifyDownstreamLinkProperties(usbLinkProperties); + inOrder.verify(mHardware, times(1)).removeDownstreamPrefix(RNDIS0, IPV6_DOC_PREFIX); + inOrder.verify(mHardware, times(1)).addDownstreamPrefix(RNDIS0, IPV6_DISCARD_PREFIX); + inOrder.verifyNoMoreInteractions(); + + // [6] Removing a downstream interface which was never added causes no + // interactions with the HAL. + offload.removeDownstreamInterface(WLAN0); + inOrder.verifyNoMoreInteractions(); + + // [7] Removing an active downstream removes all remaining prefixes. + offload.removeDownstreamInterface(RNDIS0); + inOrder.verify(mHardware, times(1)).removeDownstreamPrefix(RNDIS0, USB_PREFIX); + inOrder.verify(mHardware, times(1)).removeDownstreamPrefix(RNDIS0, IPV6_DISCARD_PREFIX); + inOrder.verifyNoMoreInteractions(); + } + + private static void assertArrayListContains(ArrayList list, String... elems) { + for (String element : elems) { + assertTrue(list.contains(element)); + } + } }