Merge "have OffloaderController store and re-push more state"

2 files changed, 75 insertions, 20 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 5eafe5f9f64f..057704a56c4c 100644
--- a/services/core/java/com/android/server/connectivity/tethering/OffloadController.java
+++ b/services/core/java/com/android/server/connectivity/tethering/OffloadController.java
@@ -52,6 +52,7 @@ import java.net.Inet4Address;
 import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -73,6 +74,8 @@ public class OffloadController {
     private static final String ANYIP = "0.0.0.0";
     private static final ForwardedStats EMPTY_STATS = new ForwardedStats();
 
+    private static enum UpdateType { IF_NEEDED, FORCE };
+
     private final Handler mHandler;
     private final OffloadHardwareInterface mHwInterface;
     private final ContentResolver mContentResolver;
@@ -185,8 +188,8 @@ public class OffloadController {
                         updateStatsForAllUpstreams();
                         forceTetherStatsPoll();
                         // [2] (Re)Push all state.
-                        // TODO: computeAndPushLocalPrefixes()
-                        // TODO: push all downstream state.
+                        computeAndPushLocalPrefixes(UpdateType.FORCE);
+                        pushAllDownstreamState();
                         pushUpstreamParameters(null);
                     }
 
@@ -319,7 +322,7 @@ public class OffloadController {
     }
 
     private boolean maybeUpdateDataLimit(String iface) {
-        // setDataLimit may only be called while offload is occuring on this upstream.
+        // setDataLimit may only be called while offload is occurring on this upstream.
         if (!started() || !TextUtils.equals(iface, currentUpstreamInterface())) {
             return true;
         }
@@ -368,15 +371,15 @@ public class OffloadController {
         // upstream parameters fails (probably just wait for a subsequent
         // onOffloadEvent() callback to tell us offload is available again and
         // then reapply all state).
-        computeAndPushLocalPrefixes();
+        computeAndPushLocalPrefixes(UpdateType.IF_NEEDED);
         pushUpstreamParameters(prevUpstream);
     }
 
     public void setLocalPrefixes(Set<IpPrefix> localPrefixes) {
-        if (!started()) return;
-
         mExemptPrefixes = localPrefixes;
-        computeAndPushLocalPrefixes();
+
+        if (!started()) return;
+        computeAndPushLocalPrefixes(UpdateType.IF_NEEDED);
     }
 
     public void notifyDownstreamLinkProperties(LinkProperties lp) {
@@ -385,27 +388,38 @@ public class OffloadController {
         if (Objects.equals(oldLp, lp)) return;
 
         if (!started()) return;
+        pushDownstreamState(oldLp, lp);
+    }
 
-        final List<RouteInfo> oldRoutes = (oldLp != null) ? oldLp.getRoutes() : new ArrayList<>();
-        final List<RouteInfo> newRoutes = lp.getRoutes();
+    private void pushDownstreamState(LinkProperties oldLp, LinkProperties newLp) {
+        final String ifname = newLp.getInterfaceName();
+        final List<RouteInfo> oldRoutes =
+                (oldLp != null) ? oldLp.getRoutes() : Collections.EMPTY_LIST;
+        final List<RouteInfo> newRoutes = newLp.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 (RouteInfo ri : oldRoutes) {
+            if (shouldIgnoreDownstreamRoute(ri)) continue;
+            if (!newRoutes.contains(ri)) {
+                mHwInterface.removeDownstreamPrefix(ifname, ri.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());
+        for (RouteInfo ri : newRoutes) {
+            if (shouldIgnoreDownstreamRoute(ri)) continue;
+            if (!oldRoutes.contains(ri)) {
+                mHwInterface.addDownstreamPrefix(ifname, ri.getDestination().toString());
             }
         }
     }
 
+    private void pushAllDownstreamState() {
+        for (LinkProperties lp : mDownstreams.values()) {
+            pushDownstreamState(null, lp);
+        }
+    }
+
     public void removeDownstreamInterface(String ifname) {
         final LinkProperties lp = mDownstreams.remove(ifname);
         if (lp == null) return;
@@ -484,10 +498,11 @@ public class OffloadController {
         return success;
     }
 
-    private boolean computeAndPushLocalPrefixes() {
+    private boolean computeAndPushLocalPrefixes(UpdateType how) {
+        final boolean force = (how == UpdateType.FORCE);
         final Set<String> localPrefixStrs = computeLocalPrefixStrings(
                 mExemptPrefixes, mUpstreamLinkProperties);
-        if (mLastLocalPrefixStrs.equals(localPrefixStrs)) return true;
+        if (!force && mLastLocalPrefixStrs.equals(localPrefixStrs)) return true;
 
         mLastLocalPrefixStrs = localPrefixStrs;
         return mHwInterface.setLocalPrefixes(new ArrayList<>(localPrefixStrs));
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 2199a134d657..2a8efc046ee3 100644
--- a/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java
+++ b/tests/net/java/com/android/server/connectivity/tethering/OffloadControllerTest.java
@@ -676,6 +676,35 @@ public class OffloadControllerTest {
             offload.setUpstreamLinkProperties(upstreamLp);
         }
 
+        // Pretend that some local prefixes and downstreams have been added
+        // (and removed, for good measure).
+        final Set<IpPrefix> minimumLocalPrefixes = new HashSet<>();
+        for (String s : new String[]{
+                "127.0.0.0/8", "192.0.2.0/24", "fe80::/64", "2001:db8::/64"}) {
+            minimumLocalPrefixes.add(new IpPrefix(s));
+        }
+        offload.setLocalPrefixes(minimumLocalPrefixes);
+
+        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);
+
+        final LinkProperties wifiLinkProperties = new LinkProperties();
+        wifiLinkProperties.setInterfaceName(WLAN0);
+        wifiLinkProperties.addLinkAddress(new LinkAddress("192.168.43.1/24"));
+        wifiLinkProperties.addRoute(new RouteInfo(new IpPrefix(WIFI_PREFIX)));
+        wifiLinkProperties.addRoute(new RouteInfo(new IpPrefix(IPV6_LINKLOCAL)));
+        // Use a benchmark prefix (RFC 5180 + erratum), since the documentation
+        // prefix is included in the excluded prefix list.
+        wifiLinkProperties.addLinkAddress(new LinkAddress("2001:2::1/64"));
+        wifiLinkProperties.addLinkAddress(new LinkAddress("2001:2::2/64"));
+        wifiLinkProperties.addRoute(new RouteInfo(new IpPrefix("2001:2::/64")));
+        offload.notifyDownstreamLinkProperties(wifiLinkProperties);
+
+        offload.removeDownstreamInterface(RNDIS0);
+
         // Clear invocation history, especially the getForwardedStats() calls
         // that happen with setUpstreamParameters().
         clearInvocations(mHardware);
@@ -690,6 +719,17 @@ public class OffloadControllerTest {
         verifyNoMoreInteractions(mNMService);
 
         // TODO: verify local prefixes and downstreams are also pushed to the HAL.
+        verify(mHardware, times(1)).setLocalPrefixes(mStringArrayCaptor.capture());
+        ArrayList<String> localPrefixes = mStringArrayCaptor.getValue();
+        assertEquals(4, localPrefixes.size());
+        assertArrayListContains(localPrefixes,
+                // TODO: The logic to find and exclude downstream IP prefixes
+                // is currently in Tethering's OffloadWrapper but must be moved
+                // into OffloadController proper. After this, also check for:
+                //     "192.168.43.1/32", "2001:2::1/128", "2001:2::2/128"
+                "127.0.0.0/8", "192.0.2.0/24", "fe80::/64", "2001:db8::/64");
+        verify(mHardware, times(1)).addDownstreamPrefix(WLAN0, "192.168.43.0/24");
+        verify(mHardware, times(1)).addDownstreamPrefix(WLAN0, "2001:2::/64");
         verify(mHardware, times(1)).setUpstreamParameters(eq(RMNET0), any(), any(), any());
         verify(mHardware, times(1)).setDataLimit(eq(RMNET0), anyLong());
         verifyNoMoreInteractions(mHardware);
@@ -697,7 +737,7 @@ public class OffloadControllerTest {
 
     private static void assertArrayListContains(ArrayList<String> list, String... elems) {
         for (String element : elems) {
-            assertTrue(list.contains(element));
+            assertTrue(element + " not in list", list.contains(element));
         }
     }
 }