From 2f5ea0e99e9a436cd43901b1772b77a410a62f8d Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Tue, 29 Jul 2014 15:46:56 +0900 Subject: [PATCH] Stop copying directly-connected routes to the main table. For a long time we have thought that copying directly-connected routes to the main table was necessary to add gatewayed routes to other routing tables. However, this is not necessary when the directly-connected routes are properly created with "scope link" as we do in http://ag/513100 . Delete the copying code, but keep dumping the main table in bugreports, so we can see if third-party code such as RIL daemons is putting anything in it. Change-Id: Iddd531daaf9881ffd82f0a4b4f6cc857ce8788fd --- server/RouteController.cpp | 40 +++++----------------------------------- 1 file changed, 5 insertions(+), 35 deletions(-) diff --git a/server/RouteController.cpp b/server/RouteController.cpp index 943a4c1..6a8ec7e 100644 --- a/server/RouteController.cpp +++ b/server/RouteController.cpp @@ -48,8 +48,7 @@ const uint32_t RULE_PRIORITY_IMPLICIT_NETWORK = 19000; const uint32_t RULE_PRIORITY_BYPASSABLE_VPN = 20000; const uint32_t RULE_PRIORITY_VPN_FALLTHROUGH = 21000; const uint32_t RULE_PRIORITY_DEFAULT_NETWORK = 22000; -const uint32_t RULE_PRIORITY_DIRECTLY_CONNECTED = 23000; -const uint32_t RULE_PRIORITY_UNREACHABLE = 24000; +const uint32_t RULE_PRIORITY_UNREACHABLE = 32000; const uint32_t ROUTE_TABLE_LOCAL_NETWORK = 97; const uint32_t ROUTE_TABLE_LEGACY_NETWORK = 98; @@ -638,24 +637,10 @@ WARN_UNUSED_RESULT int addLocalNetworkRules(unsigned localNetId) { fwmark.intValue, mask.intValue); } -// Add a new rule to look up the 'main' table, with the same selectors as the "default network" -// rule, but with a lower priority. Since the default network rule points to a table with a default -// route, the rule we're adding will never be used for normal routing lookups. However, the kernel -// may fall-through to it to find directly-connected routes when it validates that a nexthop (in a -// route being added) is reachable. -WARN_UNUSED_RESULT int addDirectlyConnectedRule() { - Fwmark fwmark; - Fwmark mask; - - fwmark.netId = NETID_UNSET; - mask.netId = FWMARK_NET_ID_MASK; - - return modifyIpRule(RTM_NEWRULE, RULE_PRIORITY_DIRECTLY_CONNECTED, RT_TABLE_MAIN, - fwmark.intValue, mask.intValue, IIF_NONE, OIF_NONE, UID_ROOT, UID_ROOT); -} - -// Add a rule to preempt the pre-defined "from all lookup main" rule. Packets that reach this rule -// will be null-routed, and won't fall-through to the main table. +// Add an explicit unreachable rule close to the end of the prioriy list to make it clear that +// relying on the kernel-default "from all lookup main" rule at priority 32766 is not intended +// behaviour, and that no routes should be in the main table. We do flush the kernel-default rules +// at startup, but having an explicit unreachable rule will hopefully make things even clearer. WARN_UNUSED_RESULT int addUnreachableRule() { return modifyIpRule(RTM_NEWRULE, RULE_PRIORITY_UNREACHABLE, RT_TABLE_UNSPEC, MARK_UNSET, MARK_UNSET); @@ -813,18 +798,6 @@ WARN_UNUSED_RESULT int modifyRoute(uint16_t action, const char* interface, const return ret; } - // If there's no nexthop, this is a directly connected route. Add it to the main table also, to - // let the kernel find it when validating nexthops when global routes are added. - if (!nexthop) { - ret = modifyIpRoute(action, RT_TABLE_MAIN, interface, destination, NULL); - // A failure with action == ADD && errno == EEXIST means that the route already exists in - // the main table, perhaps because the kernel added it automatically as part of adding the - // IP address to the interface. Ignore this, but complain about everything else. - if (ret && !(action == RTM_NEWROUTE && ret == -EEXIST)) { - return ret; - } - } - return 0; } @@ -869,9 +842,6 @@ int RouteController::Init(unsigned localNetId) { if (int ret = addLocalNetworkRules(localNetId)) { return ret; } - if (int ret = addDirectlyConnectedRule()) { - return ret; - } if (int ret = addUnreachableRule()) { return ret; } -- 2.11.0