Merge "mtu: Add MTU parameter to Routes"

core/java/android/net/LinkProperties.java

@@ -686,17 +686,29 @@ public final class LinkProperties implements Parcelable {
             route.getDestination(),
             route.getGateway(),
             mIfaceName,
-            route.getType());
+            route.getType(),
+            route.getMtu());
+    }
+
+    private int findRouteIndexByDestination(RouteInfo route) {
+        for (int i = 0; i < mRoutes.size(); i++) {
+            if (mRoutes.get(i).isSameDestinationAs(route)) {
+                return i;
+            }
+        }
+        return -1;
     }
 
     /**
-     * Adds a {@link RouteInfo} to this {@code LinkProperties}, if not present. If the
+     * Adds a {@link RouteInfo} to this {@code LinkProperties}, if a {@link RouteInfo}
-     * {@link RouteInfo} had an interface name set and that differs from the interface set for this
+     * with the same destination exists with different properties (e.g., different MTU),
-     * {@code LinkProperties} an {@link IllegalArgumentException} will be thrown.  The proper
+     * it will be updated. If the {@link RouteInfo} had an interface name set and
+     * that differs from the interface set for this {@code LinkProperties} an
+     * {@link IllegalArgumentException} will be thrown.  The proper
      * course is to add either un-named or properly named {@link RouteInfo}.
      *
      * @param route A {@link RouteInfo} to add to this object.
-     * @return {@code false} if the route was already present, {@code true} if it was added.
+     * @return {@code true} was added or updated, false otherwise.
      */
     public boolean addRoute(@NonNull RouteInfo route) {
         String routeIface = route.getInterface();
@@ -706,11 +718,20 @@ public final class LinkProperties implements Parcelable {
                             + " vs. " + mIfaceName);
         }
         route = routeWithInterface(route);
-        if (!mRoutes.contains(route)) {
+
+        int i = findRouteIndexByDestination(route);
+        if (i == -1) {
+            // Route was not present. Add it.
             mRoutes.add(route);
             return true;
+        } else if (mRoutes.get(i).equals(route)) {
+            // Route was present and has same properties. Do nothing.
+            return false;
+        } else {
+            // Route was present and has different properties. Update it.
+            mRoutes.set(i, route);
+            return true;
         }
-        return false;
     }
 
     /**
@@ -718,6 +739,7 @@ public final class LinkProperties implements Parcelable {
      * specify an interface and the interface must match the interface of this
      * {@code LinkProperties}, or it will not be removed.
      *
+     * @param route A {@link RouteInfo} specifying the route to remove.
      * @return {@code true} if the route was removed, {@code false} if it was not present.
      *
      * @hide

core/java/android/net/RouteInfo.java

@@ -526,6 +526,26 @@ public final class RouteInfo implements Parcelable {
                 mType == target.getType() && mMtu == target.getMtu();
     }
 
+    /**
+     * Compares this RouteInfo object against the specified object and indicates if the
+     * destinations of both routes are equal.
+     * @return {@code true} if the route destinations are equal, {@code false} otherwise.
+     *
+     * @hide
+     */
+    public boolean isSameDestinationAs(@Nullable Object obj) {
+        if (this == obj) return true;
+
+        if (!(obj instanceof RouteInfo)) return false;
+
+        RouteInfo target = (RouteInfo) obj;
+
+        if (Objects.equals(mDestination, target.getDestination())) {
+            return true;
+        }
+        return false;
+    }
+
     /**
      *  Returns a hashcode for this <code>RouteInfo</code> object.
      */

services/core/java/com/android/server/ConnectivityService.java

@@ -112,6 +112,7 @@ import android.net.NetworkWatchlistManager;
 import android.net.PrivateDnsConfigParcel;
 import android.net.ProxyInfo;
 import android.net.RouteInfo;
+import android.net.RouteInfoParcel;
 import android.net.SocketKeepalive;
 import android.net.TetheringManager;
 import android.net.UidRange;
@@ -122,6 +123,7 @@ import android.net.metrics.IpConnectivityLog;
 import android.net.metrics.NetworkEvent;
 import android.net.netlink.InetDiagMessage;
 import android.net.shared.PrivateDnsConfig;
+import android.net.util.LinkPropertiesUtils.CompareOrUpdateResult;
 import android.net.util.LinkPropertiesUtils.CompareResult;
 import android.net.util.MultinetworkPolicyTracker;
 import android.net.util.NetdService;
@@ -234,6 +236,7 @@ import java.util.SortedSet;
 import java.util.StringJoiner;
 import java.util.TreeSet;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Function;
 
 /**
  * @hide
@@ -5944,15 +5947,49 @@ public class ConnectivityService extends IConnectivityManager.Stub
         }
     }
 
+    // TODO: move to frameworks/libs/net.
+    private RouteInfoParcel convertRouteInfo(RouteInfo route) {
+        final String nextHop;
+
+        switch (route.getType()) {
+            case RouteInfo.RTN_UNICAST:
+                if (route.hasGateway()) {
+                    nextHop = route.getGateway().getHostAddress();
+                } else {
+                    nextHop = INetd.NEXTHOP_NONE;
+                }
+                break;
+            case RouteInfo.RTN_UNREACHABLE:
+                nextHop = INetd.NEXTHOP_UNREACHABLE;
+                break;
+            case RouteInfo.RTN_THROW:
+                nextHop = INetd.NEXTHOP_THROW;
+                break;
+            default:
+                nextHop = INetd.NEXTHOP_NONE;
+                break;
+        }
+
+        final RouteInfoParcel rip = new RouteInfoParcel();
+        rip.ifName = route.getInterface();
+        rip.destination = route.getDestination().toString();
+        rip.nextHop = nextHop;
+        rip.mtu = route.getMtu();
+
+        return rip;
+    }
+
     /**
      * Have netd update routes from oldLp to newLp.
      * @return true if routes changed between oldLp and newLp
      */
     private boolean updateRoutes(LinkProperties newLp, LinkProperties oldLp, int netId) {
-        // Compare the route diff to determine which routes should be added and removed.
+        Function<RouteInfo, IpPrefix> getDestination = (r) -> r.getDestination();
-        CompareResult<RouteInfo> routeDiff = new CompareResult<>(
+        // compare the route diff to determine which routes have been updated
+        CompareOrUpdateResult<IpPrefix, RouteInfo> routeDiff = new CompareOrUpdateResult<>(
                 oldLp != null ? oldLp.getAllRoutes() : null,
-                newLp != null ? newLp.getAllRoutes() : null);
+                newLp != null ? newLp.getAllRoutes() : null,
+                getDestination);
 
         // add routes before removing old in case it helps with continuous connectivity
 
@@ -5961,10 +5998,10 @@ public class ConnectivityService extends IConnectivityManager.Stub
             if (route.hasGateway()) continue;
             if (VDBG || DDBG) log("Adding Route [" + route + "] to network " + netId);
             try {
-                mNMS.addRoute(netId, route);
+                mNetd.networkAddRouteParcel(netId, convertRouteInfo(route));
             } catch (Exception e) {
                 if ((route.getDestination().getAddress() instanceof Inet4Address) || VDBG) {
-                    loge("Exception in addRoute for non-gateway: " + e);
+                    loge("Exception in networkAddRouteParcel for non-gateway: " + e);
                 }
             }
         }
@@ -5972,10 +6009,10 @@ public class ConnectivityService extends IConnectivityManager.Stub
             if (!route.hasGateway()) continue;
             if (VDBG || DDBG) log("Adding Route [" + route + "] to network " + netId);
             try {
-                mNMS.addRoute(netId, route);
+                mNetd.networkAddRouteParcel(netId, convertRouteInfo(route));
             } catch (Exception e) {
                 if ((route.getGateway() instanceof Inet4Address) || VDBG) {
-                    loge("Exception in addRoute for gateway: " + e);
+                    loge("Exception in networkAddRouteParcel for gateway: " + e);
                 }
             }
         }
@@ -5983,12 +6020,22 @@ public class ConnectivityService extends IConnectivityManager.Stub
         for (RouteInfo route : routeDiff.removed) {
             if (VDBG || DDBG) log("Removing Route [" + route + "] from network " + netId);
             try {
-                mNMS.removeRoute(netId, route);
+                mNetd.networkRemoveRouteParcel(netId, convertRouteInfo(route));
             } catch (Exception e) {
-                loge("Exception in removeRoute: " + e);
+                loge("Exception in networkRemoveRouteParcel: " + e);
             }
         }
-        return !routeDiff.added.isEmpty() || !routeDiff.removed.isEmpty();
+
+        for (RouteInfo route : routeDiff.updated) {
+            if (VDBG || DDBG) log("Updating Route [" + route + "] from network " + netId);
+            try {
+                mNetd.networkUpdateRouteParcel(netId, convertRouteInfo(route));
+            } catch (Exception e) {
+                loge("Exception in networkUpdateRouteParcel: " + e);
+            }
+        }
+        return !routeDiff.added.isEmpty() || !routeDiff.removed.isEmpty()
+                || !routeDiff.updated.isEmpty();
     }
 
     private void updateDnses(LinkProperties newLp, LinkProperties oldLp, int netId) {

tests/net/java/com/android/server/ConnectivityServiceTest.java

@@ -175,6 +175,7 @@ import android.net.NetworkUtils;
 import android.net.ProxyInfo;
 import android.net.ResolverParamsParcel;
 import android.net.RouteInfo;
+import android.net.RouteInfoParcel;
 import android.net.SocketKeepalive;
 import android.net.UidRange;
 import android.net.Uri;
@@ -6064,6 +6065,7 @@ public class ConnectivityServiceTest {
             verify(mBatteryStatsService).noteNetworkInterfaceType(stackedLp.getInterfaceName(),
                     TYPE_MOBILE);
         }
+        reset(mMockNetd);
 
         // Add ipv4 address, expect that clatd and prefix discovery are stopped and stacked
         // linkproperties are cleaned up.
@@ -6115,7 +6117,6 @@ public class ConnectivityServiceTest {
         networkCallback.expectCallback(CallbackEntry.LINK_PROPERTIES_CHANGED, mCellNetworkAgent);
         verify(mMockNetd, times(1)).clatdStart(MOBILE_IFNAME, kNat64Prefix.toString());
 
-
         // Clat iface comes up. Expect stacked link to be added.
         clat.interfaceLinkStateChanged(CLAT_PREFIX + MOBILE_IFNAME, true);
         networkCallback.expectLinkPropertiesThat(mCellNetworkAgent,
@@ -6701,17 +6702,45 @@ public class ConnectivityServiceTest {
         }
     }
 
+    private void assertRouteInfoParcelMatches(RouteInfo route, RouteInfoParcel parcel) {
+        assertEquals(route.getDestination().toString(), parcel.destination);
+        assertEquals(route.getInterface(), parcel.ifName);
+        assertEquals(route.getMtu(), parcel.mtu);
+
+        switch (route.getType()) {
+            case RouteInfo.RTN_UNICAST:
+                if (route.hasGateway()) {
+                    assertEquals(route.getGateway().getHostAddress(), parcel.nextHop);
+                } else {
+                    assertEquals(INetd.NEXTHOP_NONE, parcel.nextHop);
+                }
+                break;
+            case RouteInfo.RTN_UNREACHABLE:
+                assertEquals(INetd.NEXTHOP_UNREACHABLE, parcel.nextHop);
+                break;
+            case RouteInfo.RTN_THROW:
+                assertEquals(INetd.NEXTHOP_THROW, parcel.nextHop);
+                break;
+            default:
+                assertEquals(INetd.NEXTHOP_NONE, parcel.nextHop);
+                break;
+        }
+    }
+
     private void assertRoutesAdded(int netId, RouteInfo... routes) throws Exception {
-        InOrder inOrder = inOrder(mNetworkManagementService);
+        ArgumentCaptor<RouteInfoParcel> captor = ArgumentCaptor.forClass(RouteInfoParcel.class);
+        verify(mMockNetd, times(routes.length)).networkAddRouteParcel(eq(netId), captor.capture());
         for (int i = 0; i < routes.length; i++) {
-            inOrder.verify(mNetworkManagementService).addRoute(eq(netId), eq(routes[i]));
+            assertRouteInfoParcelMatches(routes[i], captor.getAllValues().get(i));
         }
     }
 
     private void assertRoutesRemoved(int netId, RouteInfo... routes) throws Exception {
-        InOrder inOrder = inOrder(mNetworkManagementService);
+        ArgumentCaptor<RouteInfoParcel> captor = ArgumentCaptor.forClass(RouteInfoParcel.class);
+        verify(mMockNetd, times(routes.length)).networkRemoveRouteParcel(eq(netId),
+                captor.capture());
         for (int i = 0; i < routes.length; i++) {
-            inOrder.verify(mNetworkManagementService).removeRoute(eq(netId), eq(routes[i]));
+            assertRouteInfoParcelMatches(routes[i], captor.getAllValues().get(i));
         }
     }
 
@@ -6973,4 +7002,60 @@ public class ConnectivityServiceTest {
         verify(mConnectivityDiagnosticsCallback)
                 .onNetworkConnectivityReported(eq(n), eq(noConnectivity));
     }
+
+    @Test
+    public void testRouteAddDeleteUpdate() throws Exception {
+        final NetworkRequest request = new NetworkRequest.Builder().build();
+        final TestNetworkCallback networkCallback = new TestNetworkCallback();
+        mCm.registerNetworkCallback(request, networkCallback);
+        mCellNetworkAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR);
+        reset(mMockNetd);
+        mCellNetworkAgent.connect(false);
+        networkCallback.expectAvailableCallbacksUnvalidated(mCellNetworkAgent);
+        final int netId = mCellNetworkAgent.getNetwork().netId;
+
+        final String iface = "rmnet_data0";
+        final InetAddress gateway = InetAddress.getByName("fe80::5678");
+        RouteInfo direct = RouteInfo.makeHostRoute(gateway, iface);
+        RouteInfo rio1 = new RouteInfo(new IpPrefix("2001:db8:1::/48"), gateway, iface);
+        RouteInfo rio2 = new RouteInfo(new IpPrefix("2001:db8:2::/48"), gateway, iface);
+        RouteInfo defaultRoute = new RouteInfo((IpPrefix) null, gateway, iface);
+        RouteInfo defaultWithMtu = new RouteInfo(null, gateway, iface, RouteInfo.RTN_UNICAST,
+                                                 1280 /* mtu */);
+
+        // Send LinkProperties and check that we ask netd to add routes.
+        LinkProperties lp = new LinkProperties();
+        lp.setInterfaceName(iface);
+        lp.addRoute(direct);
+        lp.addRoute(rio1);
+        lp.addRoute(defaultRoute);
+        mCellNetworkAgent.sendLinkProperties(lp);
+        networkCallback.expectLinkPropertiesThat(mCellNetworkAgent, x -> x.getRoutes().size() == 3);
+
+        assertRoutesAdded(netId, direct, rio1, defaultRoute);
+        reset(mMockNetd);
+
+        // Send updated LinkProperties and check that we ask netd to add, remove, update routes.
+        assertTrue(lp.getRoutes().contains(defaultRoute));
+        lp.removeRoute(rio1);
+        lp.addRoute(rio2);
+        lp.addRoute(defaultWithMtu);
+        // Ensure adding the same route with a different MTU replaces the previous route.
+        assertFalse(lp.getRoutes().contains(defaultRoute));
+        assertTrue(lp.getRoutes().contains(defaultWithMtu));
+
+        mCellNetworkAgent.sendLinkProperties(lp);
+        networkCallback.expectLinkPropertiesThat(mCellNetworkAgent,
+                x -> x.getRoutes().contains(rio2));
+
+        assertRoutesRemoved(netId, rio1);
+        assertRoutesAdded(netId, rio2);
+
+        ArgumentCaptor<RouteInfoParcel> captor = ArgumentCaptor.forClass(RouteInfoParcel.class);
+        verify(mMockNetd).networkUpdateRouteParcel(eq(netId), captor.capture());
+        assertRouteInfoParcelMatches(defaultWithMtu, captor.getValue());
+
+
+        mCm.unregisterNetworkCallback(networkCallback);
+    }
 }