Merge changes from topic "offload_cherrypick"

* changes:
  Send add/removeDownstream info to offload HAL
  Don't completely stop offload if setting data limit fails.

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<TetherInterfaceStateMachine> 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<IpPrefix> 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<IpPrefix>) o);
+                mOffload.sendOffloadExemptPrefixes((Set<IpPrefix>) 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<IpPrefix> 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

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<String, LinkProperties> 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<>();
 
@@ -131,21 +134,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 +160,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 +193,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 +206,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;
@@ -248,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;
@@ -272,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;
         }
 
@@ -287,9 +302,7 @@ public class OffloadController {
     }
 
     private void updateStatsForCurrentUpstream() {
-        if (mUpstreamLinkProperties != null) {
-            maybeUpdateStats(mUpstreamLinkProperties.getInterfaceName());
-        }
+        maybeUpdateStats(currentUpstreamInterface());
     }
 
     public void setUpstreamLinkProperties(LinkProperties lp) {
@@ -305,10 +318,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<IpPrefix> localPrefixes) {
@@ -319,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<RouteInfo> oldRoutes = (oldLp != null) ? oldLp.getRoutes() : new ArrayList<>();
+        final List<RouteInfo> 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() {
@@ -342,8 +377,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 +418,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();
         }
@@ -430,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");

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);
     }

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();

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;

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);
     }

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;
@@ -112,7 +121,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() {
@@ -236,10 +247,8 @@ public class OffloadControllerTest {
         inOrder.verify(mHardware, times(1)).setLocalPrefixes(mStringArrayCaptor.capture());
         ArrayList<String> 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);
@@ -260,6 +269,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 +287,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 +298,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 +312,7 @@ public class OffloadControllerTest {
         ArrayList<String> 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 +327,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 +346,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
@@ -348,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(
@@ -363,6 +375,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.
@@ -436,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());
@@ -524,6 +539,7 @@ public class OffloadControllerTest {
         offload.setUpstreamLinkProperties(lp);
         provider.setInterfaceQuota(mobileIface, mobileLimit);
         waitForIdle();
+        inOrder.verify(mHardware).getForwardedStats(ethernetIface);
         inOrder.verify(mHardware).stopOffloadControl();
     }
 
@@ -539,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<String> list, String... elems) {
+        for (String element : elems) {
+            assertTrue(list.contains(element));
+        }
+    }
 }