Merge changes Idd57eb85,I4d457152android-s-v2-preview-2android-s-v2-preview-1android-s-v2-beta-2android-s-v2-preview-1

* changes:
  Add "throw" and "unreachable" routes to NetdBinderTest
  Use route priority only for route cache invalidation

8 files changed, 146 insertions, 80 deletions

diff --git a/server/NetworkController.cpp b/server/NetworkController.cpp
index 1b99556b..cc10a787 100644
--- a/server/NetworkController.cpp
+++ b/server/NetworkController.cpp
@@ -897,11 +897,13 @@ int NetworkController::modifyRoute(unsigned netId, const char* interface, const
 
     switch (op) {
         case ROUTE_ADD:
-            return RouteController::addRoute(interface, destination, nexthop, tableType, mtu);
+            return RouteController::addRoute(interface, destination, nexthop, tableType, mtu,
+                                             0 /* priority */);
         case ROUTE_UPDATE:
             return RouteController::updateRoute(interface, destination, nexthop, tableType, mtu);
         case ROUTE_REMOVE:
-            return RouteController::removeRoute(interface, destination, nexthop, tableType);
+            return RouteController::removeRoute(interface, destination, nexthop, tableType,
+                                                0 /* priority */);
     }
     return -EINVAL;
 }
diff --git a/server/PhysicalNetwork.cpp b/server/PhysicalNetwork.cpp
index 7b9a19a1..f2703175 100644
--- a/server/PhysicalNetwork.cpp
+++ b/server/PhysicalNetwork.cpp
@@ -84,14 +84,20 @@ int PhysicalNetwork::destroySocketsLackingPermission(Permission permission) {
 }
 
 void PhysicalNetwork::invalidateRouteCache(const std::string& interface) {
+    // This method invalidates all socket destination cache entries in the kernel by creating and
+    // removing a low-priority route.
+    // This number is an arbitrary number that need to be higher than any other route created either
+    // by netd or by an IPv6 RouterAdvertisement.
+    int priority = 100000;
+
     for (const auto& dst : { "0.0.0.0/0", "::/0" }) {
         // If any of these operations fail, there's no point in logging because RouteController will
         // have already logged a message. There's also no point returning an error since there's
         // nothing we can do.
         (void)RouteController::addRoute(interface.c_str(), dst, "throw", RouteController::INTERFACE,
-                                        0 /* mtu */);
-        (void) RouteController::removeRoute(interface.c_str(), dst, "throw",
-                                         RouteController::INTERFACE);
+                                        0 /* mtu */, priority);
+        (void)RouteController::removeRoute(interface.c_str(), dst, "throw",
+                                           RouteController::INTERFACE, priority);
     }
 }
 
diff --git a/server/RouteController.cpp b/server/RouteController.cpp
index 14bc926e..bea7c136 100644
--- a/server/RouteController.cpp
+++ b/server/RouteController.cpp
@@ -62,10 +62,6 @@ const char* const ROUTE_TABLE_NAME_LEGACY_SYSTEM  = "legacy_system";
 const char* const ROUTE_TABLE_NAME_LOCAL = "local";
 const char* const ROUTE_TABLE_NAME_MAIN  = "main";
 
-// None of our regular routes specify priority, which causes them to have the default priority.
-// For default throw routes, we use a fixed priority of 100000.
-uint32_t PRIO_THROW = 100000;
-
 const char* const RouteController::LOCAL_MANGLE_INPUT = "routectrl_mangle_INPUT";
 
 const uint8_t AF_FAMILIES[] = {AF_INET, AF_INET6};
@@ -346,7 +342,7 @@ int padInterfaceName(const char* input, char* name, size_t* length, uint16_t* pa
 // Adds or deletes an IPv4 or IPv6 route.
 // Returns 0 on success or negative errno on failure.
 int modifyIpRoute(uint16_t action, uint16_t flags, uint32_t table, const char* interface,
-                  const char* destination, const char* nexthop, uint32_t mtu) {
+                  const char* destination, const char* nexthop, uint32_t mtu, uint32_t priority) {
     // At least the destination must be non-null.
     if (!destination) {
         ALOGE("null destination");
@@ -401,8 +397,6 @@ int modifyIpRoute(uint16_t action, uint16_t flags, uint32_t table, const char* i
         }
     }
 
-    bool isDefaultThrowRoute = (type == RTN_THROW && prefixLength == 0);
-
     // Assemble a rtmsg and put it in an array of iovec structures.
     rtmsg route = {
             .rtm_family = family,
@@ -416,21 +410,21 @@ int modifyIpRoute(uint16_t action, uint16_t flags, uint32_t table, const char* i
     rtattr rtaGateway = { U16_RTA_LENGTH(rawLength), RTA_GATEWAY };
 
     iovec iov[] = {
-        { nullptr,         0 },
-        { &route,          sizeof(route) },
-        { &RTATTR_TABLE,   sizeof(RTATTR_TABLE) },
-        { &table,          sizeof(table) },
-        { &rtaDst,         sizeof(rtaDst) },
-        { rawAddress,      static_cast<size_t>(rawLength) },
-        { &RTATTR_OIF,     interface != OIF_NONE ? sizeof(RTATTR_OIF) : 0 },
-        { &ifindex,        interface != OIF_NONE ? sizeof(ifindex) : 0 },
-        { &rtaGateway,     nexthop ? sizeof(rtaGateway) : 0 },
-        { rawNexthop,      nexthop ? static_cast<size_t>(rawLength) : 0 },
-        { &RTATTR_METRICS, mtu != 0 ? sizeof(RTATTR_METRICS) : 0 },
-        { &RTATTRX_MTU,    mtu != 0 ? sizeof(RTATTRX_MTU) : 0 },
-        { &mtu,            mtu != 0 ? sizeof(mtu) : 0 },
-        { &RTATTR_PRIO,    isDefaultThrowRoute ? sizeof(RTATTR_PRIO) : 0 },
-        { &PRIO_THROW,     isDefaultThrowRoute ? sizeof(PRIO_THROW) : 0 },
+            {nullptr, 0},
+            {&route, sizeof(route)},
+            {&RTATTR_TABLE, sizeof(RTATTR_TABLE)},
+            {&table, sizeof(table)},
+            {&rtaDst, sizeof(rtaDst)},
+            {rawAddress, static_cast<size_t>(rawLength)},
+            {&RTATTR_OIF, interface != OIF_NONE ? sizeof(RTATTR_OIF) : 0},
+            {&ifindex, interface != OIF_NONE ? sizeof(ifindex) : 0},
+            {&rtaGateway, nexthop ? sizeof(rtaGateway) : 0},
+            {rawNexthop, nexthop ? static_cast<size_t>(rawLength) : 0},
+            {&RTATTR_METRICS, mtu != 0 ? sizeof(RTATTR_METRICS) : 0},
+            {&RTATTRX_MTU, mtu != 0 ? sizeof(RTATTRX_MTU) : 0},
+            {&mtu, mtu != 0 ? sizeof(mtu) : 0},
+            {&RTATTR_PRIO, priority != 0 ? sizeof(RTATTR_PRIO) : 0},
+            {&priority, priority != 0 ? sizeof(priority) : 0},
     };
 
     // Allow creating multiple link-local routes in the same table, so we can make IPv6
@@ -709,12 +703,12 @@ int RouteController::configureDummyNetwork() {
     }
 
     if ((ret = modifyIpRoute(RTM_NEWROUTE, NETLINK_ROUTE_CREATE_FLAGS, table, interface,
-                             "0.0.0.0/0", nullptr, 0 /* mtu */))) {
+                             "0.0.0.0/0", nullptr, 0 /* mtu */, 0 /* priority */))) {
         return ret;
     }
 
     if ((ret = modifyIpRoute(RTM_NEWROUTE, NETLINK_ROUTE_CREATE_FLAGS, table, interface, "::/0",
-                             nullptr, 0 /* mtu */))) {
+                             nullptr, 0 /* mtu */, 0 /* priority */))) {
         return ret;
     }
 
@@ -1026,7 +1020,7 @@ int RouteController::modifyTetheredNetwork(uint16_t action, const char* inputInt
 // Returns 0 on success or negative errno on failure.
 int RouteController::modifyRoute(uint16_t action, uint16_t flags, const char* interface,
                                  const char* destination, const char* nexthop, TableType tableType,
-                                 int mtu) {
+                                 int mtu, int priority) {
     uint32_t table;
     switch (tableType) {
         case RouteController::INTERFACE: {
@@ -1050,7 +1044,7 @@ int RouteController::modifyRoute(uint16_t action, uint16_t flags, const char* in
         }
     }
 
-    int ret = modifyIpRoute(action, flags, table, interface, destination, nexthop, mtu);
+    int ret = modifyIpRoute(action, flags, table, interface, destination, nexthop, mtu, priority);
     // Trying to add a route that already exists shouldn't cause an error.
     if (ret && !(action == RTM_NEWROUTE && ret == -EEXIST)) {
         return ret;
@@ -1284,21 +1278,21 @@ int RouteController::removeInterfaceFromDefaultNetwork(const char* interface,
 }
 
 int RouteController::addRoute(const char* interface, const char* destination, const char* nexthop,
-                              TableType tableType, int mtu) {
+                              TableType tableType, int mtu, int priority) {
     return modifyRoute(RTM_NEWROUTE, NETLINK_ROUTE_CREATE_FLAGS, interface, destination, nexthop,
-                       tableType, mtu);
+                       tableType, mtu, priority);
 }
 
 int RouteController::removeRoute(const char* interface, const char* destination,
-                                 const char* nexthop, TableType tableType) {
+                                 const char* nexthop, TableType tableType, int priority) {
     return modifyRoute(RTM_DELROUTE, NETLINK_REQUEST_FLAGS, interface, destination, nexthop,
-                       tableType, 0);
+                       tableType, 0 /* mtu */, priority);
 }
 
 int RouteController::updateRoute(const char* interface, const char* destination,
                                  const char* nexthop, TableType tableType, int mtu) {
     return modifyRoute(RTM_NEWROUTE, NETLINK_ROUTE_REPLACE_FLAGS, interface, destination, nexthop,
-                       tableType, mtu);
+                       tableType, mtu, 0 /* priority */);
 }
 
 int RouteController::enableTethering(const char* inputInterface, const char* outputInterface) {
diff --git a/server/RouteController.h b/server/RouteController.h
index 38d2d621..039ef3c1 100644
--- a/server/RouteController.h
+++ b/server/RouteController.h
@@ -142,9 +142,10 @@ public:
     // |nexthop| can be NULL (to indicate a directly-connected route), "unreachable" (to indicate a
     // route that's blocked), "throw" (to indicate the lack of a match), or a regular IP address.
     [[nodiscard]] static int addRoute(const char* interface, const char* destination,
-                                      const char* nexthop, TableType tableType, int mtu);
+                                      const char* nexthop, TableType tableType, int mtu,
+                                      int priority);
     [[nodiscard]] static int removeRoute(const char* interface, const char* destination,
-                                         const char* nexthop, TableType tableType);
+                                         const char* nexthop, TableType tableType, int priority);
     [[nodiscard]] static int updateRoute(const char* interface, const char* destination,
                                          const char* nexthop, TableType tableType, int mtu);
 
@@ -195,7 +196,7 @@ private:
     static int modifyUnreachableNetwork(unsigned netId, const UidRangeMap& uidRangeMap, bool add);
     static int modifyRoute(uint16_t action, uint16_t flags, const char* interface,
                            const char* destination, const char* nexthop, TableType tableType,
-                           int mtu);
+                           int mtu, int priority);
     static int modifyTetheredNetwork(uint16_t action, const char* inputInterface,
                                      const char* outputInterface);
     static int modifyVpnFallthroughRule(uint16_t action, unsigned vpnNetId,
@@ -211,7 +212,7 @@ private:
 // functions public.
 [[nodiscard]] int modifyIpRoute(uint16_t action, uint16_t flags, uint32_t table,
                                 const char* interface, const char* destination, const char* nexthop,
-                                uint32_t mtu);
+                                uint32_t mtu, uint32_t priority);
 uint32_t getRulePriority(const nlmsghdr *nlh);
 [[nodiscard]] int modifyIncomingPacketMark(unsigned netId, const char* interface,
                                            Permission permission, bool add);
diff --git a/server/RouteControllerTest.cpp b/server/RouteControllerTest.cpp
index e85a83c7..4d1da06f 100644
--- a/server/RouteControllerTest.cpp
+++ b/server/RouteControllerTest.cpp
@@ -80,23 +80,20 @@ TEST_F(RouteControllerTest, TestRouteFlush) {
                   "Test table2 number too large");
 
     EXPECT_EQ(0, modifyIpRoute(RTM_NEWROUTE, NETLINK_ROUTE_CREATE_FLAGS, table1, "lo",
-              "192.0.2.2/32", nullptr, 0 /* mtu */));
+                               "192.0.2.2/32", nullptr, 0 /* mtu */, 0 /* priority */));
     EXPECT_EQ(0, modifyIpRoute(RTM_NEWROUTE, NETLINK_ROUTE_CREATE_FLAGS, table1, "lo",
-              "192.0.2.3/32", nullptr, 0 /* mtu */));
+                               "192.0.2.3/32", nullptr, 0 /* mtu */, 0 /* priority */));
     EXPECT_EQ(0, modifyIpRoute(RTM_NEWROUTE, NETLINK_ROUTE_CREATE_FLAGS, table2, "lo",
-              "192.0.2.4/32", nullptr, 0 /* mtu */));
+                               "192.0.2.4/32", nullptr, 0 /* mtu */, 0 /* priority */));
 
     EXPECT_EQ(0, flushRoutes(table1));
 
-    EXPECT_EQ(-ESRCH,
-              modifyIpRoute(RTM_DELROUTE, NETLINK_ROUTE_CREATE_FLAGS, table1, "lo", "192.0.2.2/32",
-                            nullptr, 0 /* mtu */));
-    EXPECT_EQ(-ESRCH,
-              modifyIpRoute(RTM_DELROUTE, NETLINK_ROUTE_CREATE_FLAGS, table1, "lo", "192.0.2.3/32",
-                            nullptr, 0 /* mtu */));
-    EXPECT_EQ(0,
-              modifyIpRoute(RTM_DELROUTE, NETLINK_ROUTE_CREATE_FLAGS, table2, "lo", "192.0.2.4/32",
-                            nullptr, 0 /* mtu */));
+    EXPECT_EQ(-ESRCH, modifyIpRoute(RTM_DELROUTE, NETLINK_ROUTE_CREATE_FLAGS, table1, "lo",
+                                    "192.0.2.2/32", nullptr, 0 /* mtu */, 0 /* priority */));
+    EXPECT_EQ(-ESRCH, modifyIpRoute(RTM_DELROUTE, NETLINK_ROUTE_CREATE_FLAGS, table1, "lo",
+                                    "192.0.2.3/32", nullptr, 0 /* mtu */, 0 /* priority */));
+    EXPECT_EQ(0, modifyIpRoute(RTM_DELROUTE, NETLINK_ROUTE_CREATE_FLAGS, table2, "lo",
+                               "192.0.2.4/32", nullptr, 0 /* mtu */, 0 /* priority */));
 }
 
 TEST_F(RouteControllerTest, TestModifyIncomingPacketMark) {
diff --git a/tests/binder_test.cpp b/tests/binder_test.cpp
index 9b66a979..d0dc5ccd 100644
--- a/tests/binder_test.cpp
+++ b/tests/binder_test.cpp
@@ -24,6 +24,7 @@
 #include <cstdlib>
 #include <iostream>
 #include <mutex>
+#include <numeric>
 #include <regex>
 #include <set>
 #include <string>
@@ -1764,39 +1765,86 @@ TEST_F(NetdBinderTest, BandwidthSetGlobalAlert) {
 
 namespace {
 
-std::string ipRouteString(const std::string& ifName, const std::string& dst,
-                          const std::string& nextHop, const std::string& mtu) {
-    std::string dstString = (dst == "0.0.0.0/0" || dst == "::/0") ? "default" : dst;
+// Output looks like this:
+//
+// IPv4:
+//
+// throw        dst                         proto static    scope link
+// unreachable  dst                         proto static    scope link
+//              dst via nextHop dev ifName  proto static
+//              dst             dev ifName  proto static    scope link
+//
+// IPv6:
+//
+// throw        dst             dev lo      proto static    metric 1024
+// unreachable  dst             dev lo      proto static    metric 1024
+//              dst via nextHop dev ifName  proto static    metric 1024
+//              dst             dev ifName  proto static    metric 1024
+std::string ipRoutePrefix(const std::string& ifName, const std::string& dst,
+                          const std::string& nextHop) {
+    std::string prefixString;
+
+    bool isThrow = nextHop == "throw";
+    bool isUnreachable = nextHop == "unreachable";
+    bool isDefault = (dst == "0.0.0.0/0" || dst == "::/0");
+    bool isIPv6 = dst.find(':') != std::string::npos;
+    bool isThrowOrUnreachable = isThrow || isUnreachable;
+
+    if (isThrowOrUnreachable) {
+        prefixString += nextHop + " ";
+    }
+
+    prefixString += isDefault ? "default" : dst;
 
-    if (!nextHop.empty()) {
-        dstString += " via " + nextHop;
+    if (!nextHop.empty() && !isThrowOrUnreachable) {
+        prefixString += " via " + nextHop;
     }
 
-    dstString += " dev " + ifName;
+    if (isThrowOrUnreachable) {
+        if (isIPv6) {
+            prefixString += " dev lo";
+        }
+    } else {
+        prefixString += " dev " + ifName;
+    }
 
-    if (!mtu.empty()) {
-        dstString += " proto static";
-        // IPv6 routes report the metric, IPv4 routes report the scope.
-        // TODO: move away from specifying the entire string and use a regexp instead.
-        if (dst.find(':') != std::string::npos) {
-            dstString += " metric 1024";
-        } else {
-            if (nextHop.empty()) {
-                dstString += " scope link";
-            }
+    prefixString += " proto static";
+
+    // IPv6 routes report the metric, IPv4 routes report the scope.
+    if (isIPv6) {
+        prefixString += " metric 1024";
+    } else {
+        if (nextHop.empty() || isThrowOrUnreachable) {
+            prefixString += " scope link";
         }
-        dstString += " mtu " + mtu;
     }
 
-    return dstString;
+    return prefixString;
+}
+
+std::vector<std::string> ipRouteSubstrings(const std::string& ifName, const std::string& dst,
+                                           const std::string& nextHop, const std::string& mtu) {
+    std::vector<std::string> routeSubstrings;
+
+    routeSubstrings.push_back(ipRoutePrefix(ifName, dst, nextHop));
+
+    if (!mtu.empty()) {
+        // Add separate substring to match mtu value.
+        // This is needed because on some devices "error -11"/"error -113" appears between ip prefix
+        // and mtu for throw/unreachable routes.
+        routeSubstrings.push_back("mtu " + mtu);
+    }
+
+    return routeSubstrings;
 }
 
 void expectNetworkRouteExistsWithMtu(const char* ipVersion, const std::string& ifName,
                                      const std::string& dst, const std::string& nextHop,
                                      const std::string& mtu, const char* table) {
-    std::string routeString = ipRouteString(ifName, dst, nextHop, mtu);
-    EXPECT_TRUE(ipRouteExists(ipVersion, table, routeString))
-            << "Couldn't find route to " << dst << ": '" << routeString << "' in table " << table;
+    std::vector<std::string> routeSubstrings = ipRouteSubstrings(ifName, dst, nextHop, mtu);
+    EXPECT_TRUE(ipRouteExists(ipVersion, table, routeSubstrings))
+            << "Couldn't find route to " << dst << ": [" << Join(routeSubstrings, ", ")
+            << "] in table " << table;
 }
 
 void expectNetworkRouteExists(const char* ipVersion, const std::string& ifName,
@@ -1808,9 +1856,9 @@ void expectNetworkRouteExists(const char* ipVersion, const std::string& ifName,
 void expectNetworkRouteDoesNotExist(const char* ipVersion, const std::string& ifName,
                                     const std::string& dst, const std::string& nextHop,
                                     const char* table) {
-    std::string routeString = ipRouteString(ifName, dst, nextHop, "");
-    EXPECT_FALSE(ipRouteExists(ipVersion, table, routeString))
-            << "Found unexpected route " << routeString << " in table " << table;
+    std::vector<std::string> routeSubstrings = ipRouteSubstrings(ifName, dst, nextHop, "");
+    EXPECT_FALSE(ipRouteExists(ipVersion, table, routeSubstrings))
+            << "Found unexpected route [" << Join(routeSubstrings, ", ") << "] in table " << table;
 }
 
 bool ipRuleExists(const char* ipVersion, const std::string& ipRule) {
@@ -1922,6 +1970,14 @@ TEST_F(NetdBinderTest, NetworkAddRemoveRouteUserPermission) {
             {IP_RULE_V6, "::/0", "2001:db8::", true},
             {IP_RULE_V6, "2001:db8:cafe::/64", "2001:db8::", true},
             {IP_RULE_V4, "fe80::/64", "0.0.0.0", false},
+            {IP_RULE_V4, "10.251.10.2/31", "throw", true},
+            {IP_RULE_V4, "10.251.10.2/31", "unreachable", true},
+            {IP_RULE_V4, "0.0.0.0/0", "throw", true},
+            {IP_RULE_V4, "0.0.0.0/0", "unreachable", true},
+            {IP_RULE_V6, "::/0", "throw", true},
+            {IP_RULE_V6, "::/0", "unreachable", true},
+            {IP_RULE_V6, "2001:db8:cafe::/64", "throw", true},
+            {IP_RULE_V6, "2001:db8:cafe::/64", "unreachable", true},
     };
 
     static const struct {
@@ -4713,4 +4769,4 @@ TEST_F(NetdBinderTest, UidRangeSubPriority_ImplicitlySelectNetwork) {
             EXPECT_TRUE(mNetd->networkRemoveUidRangesParcel(uidRangeConfig).isOk());
         }
     }
-}
\ No newline at end of file
+}
diff --git a/tests/test_utils.cpp b/tests/test_utils.cpp
index 27b17c53..93c7dd8c 100644
--- a/tests/test_utils.cpp
+++ b/tests/test_utils.cpp
@@ -88,10 +88,19 @@ std::vector<std::string> listIpRoutes(const char* ipVersion, const char* table)
     return runCommand(command);
 }
 
-bool ipRouteExists(const char* ipVersion, const char* table, const std::string& ipRoute) {
+bool ipRouteExists(const char* ipVersion, const char* table,
+                   const std::vector<std::string>& ipRouteSubstrings) {
     std::vector<std::string> routes = listIpRoutes(ipVersion, table);
     for (const auto& route : routes) {
-        if (route.find(ipRoute) != std::string::npos) {
+        bool matched = true;
+        for (const auto& substring : ipRouteSubstrings) {
+            if (route.find(substring) == std::string::npos) {
+                matched = false;
+                break;
+            }
+        }
+
+        if (matched) {
             return true;
         }
     }
diff --git a/tests/test_utils.h b/tests/test_utils.h
index c8cd6ce6..de6c221f 100644
--- a/tests/test_utils.h
+++ b/tests/test_utils.h
@@ -35,4 +35,5 @@ bool iptablesRuleExists(const char* binary, const char* chainName, const std::st
 
 std::vector<std::string> listIpRoutes(const char* ipVersion, const char* table);
 
-bool ipRouteExists(const char* ipVersion, const char* table, const std::string& ipRoute);
+bool ipRouteExists(const char* ipVersion, const char* table,
+                   const std::vector<std::string>& ipRouteSubstrings);