diff --git a/core/java/android/net/LinkProperties.java b/core/java/android/net/LinkProperties.java
index fe3f91940e..2c356e43d9 100644
--- a/core/java/android/net/LinkProperties.java
+++ b/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
- * {@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
+ * Adds a {@link RouteInfo} to this {@code LinkProperties}, if a {@link RouteInfo}
+ * with the same destination exists with different properties (e.g., different MTU),
+ * 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
diff --git a/core/java/android/net/RouteInfo.java b/core/java/android/net/RouteInfo.java
index 2b9e9fe81b..fec2df412a 100644
--- a/core/java/android/net/RouteInfo.java
+++ b/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 RouteInfo object.
*/
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index d14d98c914..4dbbd0d9f0 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/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.
- CompareResult routeDiff = new CompareResult<>(
+ Function getDestination = (r) -> r.getDestination();
+ // compare the route diff to determine which routes have been updated
+ CompareOrUpdateResult 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) {
diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java
index d8f32e1c45..e04051fcac 100644
--- a/tests/net/java/com/android/server/ConnectivityServiceTest.java
+++ b/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 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 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 captor = ArgumentCaptor.forClass(RouteInfoParcel.class);
+ verify(mMockNetd).networkUpdateRouteParcel(eq(netId), captor.capture());
+ assertRouteInfoParcelMatches(defaultWithMtu, captor.getValue());
+
+
+ mCm.unregisterNetworkCallback(networkCallback);
+ }
}