Fix addRoute replace default route unexpectedly
In aosp/1203789, if two routes are with the same destination, it will be replaced instead of added when calling addRoute. This breaks scenarios which rely on the ability to add multiple default routes, such as multiple IPv6 default routes learned via address autoconfiguration. This change treats the route is an update if the destination and nexthop are the same, but different in other properties. Test: atest OffloadControllerTest#testSetUpstreamLinkPropertiesWorking Test: atest LinkPropertiesUtilsTest#testLinkPropertiesIdenticalEqual Test: atest ConnectivityServiceTest#testStackedLinkProperties Test: atest ConnectivityServiceTest#testRouteAddDeleteUpdate (only directly related tests are listed) Fix: 152170074 Fix: 151911339 Bug: 142892223 Change-Id: I7153ec9866f14a109ba8155c905e5d9e4f85eb64
This commit is contained in:
@@ -690,9 +690,9 @@ public final class LinkProperties implements Parcelable {
|
|||||||
route.getMtu());
|
route.getMtu());
|
||||||
}
|
}
|
||||||
|
|
||||||
private int findRouteIndexByDestination(RouteInfo route) {
|
private int findRouteIndexByRouteKey(RouteInfo route) {
|
||||||
for (int i = 0; i < mRoutes.size(); i++) {
|
for (int i = 0; i < mRoutes.size(); i++) {
|
||||||
if (mRoutes.get(i).isSameDestinationAs(route)) {
|
if (mRoutes.get(i).getRouteKey().equals(route.getRouteKey())) {
|
||||||
return i;
|
return i;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -701,11 +701,11 @@ public final class LinkProperties implements Parcelable {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Adds a {@link RouteInfo} to this {@code LinkProperties}, if a {@link RouteInfo}
|
* Adds a {@link RouteInfo} to this {@code LinkProperties}, if a {@link RouteInfo}
|
||||||
* with the same destination exists with different properties (e.g., different MTU),
|
* with the same {@link RouteInfo.RouteKey} with different properties
|
||||||
* it will be updated. If the {@link RouteInfo} had an interface name set and
|
* (e.g., different MTU), it will be updated. If the {@link RouteInfo} had an
|
||||||
* that differs from the interface set for this {@code LinkProperties} an
|
* interface name set and that differs from the interface set for this
|
||||||
* {@link IllegalArgumentException} will be thrown. The proper
|
* {@code LinkProperties} an {@link IllegalArgumentException} will be thrown.
|
||||||
* course is to add either un-named or properly named {@link RouteInfo}.
|
* The proper course is to add either un-named or properly named {@link RouteInfo}.
|
||||||
*
|
*
|
||||||
* @param route A {@link RouteInfo} to add to this object.
|
* @param route A {@link RouteInfo} to add to this object.
|
||||||
* @return {@code true} was added or updated, false otherwise.
|
* @return {@code true} was added or updated, false otherwise.
|
||||||
@@ -719,7 +719,7 @@ public final class LinkProperties implements Parcelable {
|
|||||||
}
|
}
|
||||||
route = routeWithInterface(route);
|
route = routeWithInterface(route);
|
||||||
|
|
||||||
int i = findRouteIndexByDestination(route);
|
int i = findRouteIndexByRouteKey(route);
|
||||||
if (i == -1) {
|
if (i == -1) {
|
||||||
// Route was not present. Add it.
|
// Route was not present. Add it.
|
||||||
mRoutes.add(route);
|
mRoutes.add(route);
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ import android.net.util.NetUtils;
|
|||||||
import android.os.Build;
|
import android.os.Build;
|
||||||
import android.os.Parcel;
|
import android.os.Parcel;
|
||||||
import android.os.Parcelable;
|
import android.os.Parcelable;
|
||||||
|
import android.util.Pair;
|
||||||
|
|
||||||
import java.lang.annotation.Retention;
|
import java.lang.annotation.Retention;
|
||||||
import java.lang.annotation.RetentionPolicy;
|
import java.lang.annotation.RetentionPolicy;
|
||||||
@@ -527,23 +528,27 @@ public final class RouteInfo implements Parcelable {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Compares this RouteInfo object against the specified object and indicates if the
|
* A helper class that contains the destination and the gateway in a {@code RouteInfo},
|
||||||
* destinations of both routes are equal.
|
* used by {@link ConnectivityService#updateRoutes} or
|
||||||
* @return {@code true} if the route destinations are equal, {@code false} otherwise.
|
* {@link LinkProperties#addRoute} to calculate the list to be updated.
|
||||||
*
|
*
|
||||||
* @hide
|
* @hide
|
||||||
*/
|
*/
|
||||||
public boolean isSameDestinationAs(@Nullable Object obj) {
|
public static class RouteKey extends Pair<IpPrefix, InetAddress> {
|
||||||
if (this == obj) return true;
|
RouteKey(@NonNull IpPrefix destination, @Nullable InetAddress gateway) {
|
||||||
|
super(destination, gateway);
|
||||||
if (!(obj instanceof RouteInfo)) return false;
|
|
||||||
|
|
||||||
RouteInfo target = (RouteInfo) obj;
|
|
||||||
|
|
||||||
if (Objects.equals(mDestination, target.getDestination())) {
|
|
||||||
return true;
|
|
||||||
}
|
}
|
||||||
return false;
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Get {@code RouteKey} of this {@code RouteInfo}.
|
||||||
|
* @return a {@code RouteKey} object.
|
||||||
|
*
|
||||||
|
* @hide
|
||||||
|
*/
|
||||||
|
@NonNull
|
||||||
|
public RouteKey getRouteKey() {
|
||||||
|
return new RouteKey(mDestination, mGateway);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@@ -236,7 +236,6 @@ import java.util.SortedSet;
|
|||||||
import java.util.StringJoiner;
|
import java.util.StringJoiner;
|
||||||
import java.util.TreeSet;
|
import java.util.TreeSet;
|
||||||
import java.util.concurrent.atomic.AtomicInteger;
|
import java.util.concurrent.atomic.AtomicInteger;
|
||||||
import java.util.function.Function;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @hide
|
* @hide
|
||||||
@@ -5982,12 +5981,12 @@ public class ConnectivityService extends IConnectivityManager.Stub
|
|||||||
* @return true if routes changed between oldLp and newLp
|
* @return true if routes changed between oldLp and newLp
|
||||||
*/
|
*/
|
||||||
private boolean updateRoutes(LinkProperties newLp, LinkProperties oldLp, int netId) {
|
private boolean updateRoutes(LinkProperties newLp, LinkProperties oldLp, int netId) {
|
||||||
Function<RouteInfo, IpPrefix> getDestination = (r) -> r.getDestination();
|
|
||||||
// compare the route diff to determine which routes have been updated
|
// compare the route diff to determine which routes have been updated
|
||||||
CompareOrUpdateResult<IpPrefix, RouteInfo> routeDiff = new CompareOrUpdateResult<>(
|
final CompareOrUpdateResult<RouteInfo.RouteKey, RouteInfo> routeDiff =
|
||||||
oldLp != null ? oldLp.getAllRoutes() : null,
|
new CompareOrUpdateResult<>(
|
||||||
newLp != null ? newLp.getAllRoutes() : null,
|
oldLp != null ? oldLp.getAllRoutes() : null,
|
||||||
getDestination);
|
newLp != null ? newLp.getAllRoutes() : null,
|
||||||
|
(r) -> r.getRouteKey());
|
||||||
|
|
||||||
// add routes before removing old in case it helps with continuous connectivity
|
// add routes before removing old in case it helps with continuous connectivity
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user