shill: Delete foreign route entries when adding new default route.
BUG=chromium-os:26587
TEST=Unit test, network_WiFiManager, manually add foreign route and
verify that shill deletes it
Change-Id: I9fa11c227dbdf21baab3a2c7af95b2b4f6f6511e
Reviewed-on: https://gerrit.chromium.org/gerrit/17024
Commit-Ready: Thieu Le <thieule@chromium.org>
Tested-by: Thieu Le <thieule@chromium.org>
Reviewed-by: Thieu Le <thieule@chromium.org>
diff --git a/connection.cc b/connection.cc
index 5d549a4..bfd1962 100644
--- a/connection.cc
+++ b/connection.cc
@@ -39,7 +39,7 @@
VLOG(2) << __func__ << " " << interface_name_;
DCHECK(!routing_request_count_);
- routing_table_->FlushRoutes(interface_index_);
+ routing_table_->FlushRoutes(interface_index_, true);
device_info_->FlushAddresses(interface_index_);
}
diff --git a/connection_unittest.cc b/connection_unittest.cc
index 491cb1f..ec9807a 100644
--- a/connection_unittest.cc
+++ b/connection_unittest.cc
@@ -174,14 +174,14 @@
connection->ReleaseRouting();
// The destructor will remove the routes and addresses.
- EXPECT_CALL(routing_table_, FlushRoutes(kTestDeviceInterfaceIndex0));
+ EXPECT_CALL(routing_table_, FlushRoutes(kTestDeviceInterfaceIndex0, true));
EXPECT_CALL(*device_info_.get(),
FlushAddresses(kTestDeviceInterfaceIndex0));
}
}
TEST_F(ConnectionTest, Destructor) {
- EXPECT_CALL(routing_table_, FlushRoutes(kTestDeviceInterfaceIndex1));
+ EXPECT_CALL(routing_table_, FlushRoutes(kTestDeviceInterfaceIndex1, true));
EXPECT_CALL(*device_info_, FlushAddresses(kTestDeviceInterfaceIndex1));
{
ConnectionRefPtr connection(new Connection(kTestDeviceInterfaceIndex1,
diff --git a/ipconfig.h b/ipconfig.h
index 6082d6c..ed2fc35 100644
--- a/ipconfig.h
+++ b/ipconfig.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -104,6 +104,7 @@
FRIEND_TEST(ResolverTest, Empty);
FRIEND_TEST(ResolverTest, NonEmpty);
FRIEND_TEST(RoutingTableTest, RouteAddDelete);
+ FRIEND_TEST(RoutingTableTest, RouteDeleteForeign);
static const char kStorageType[];
static const char kType[];
diff --git a/mock_routing_table.h b/mock_routing_table.h
index 9eaa870..55cf583 100644
--- a/mock_routing_table.h
+++ b/mock_routing_table.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -27,7 +27,7 @@
MOCK_METHOD3(SetDefaultRoute, bool(int interface_index,
const IPConfigRefPtr &ipconfig,
uint32 metric));
- MOCK_METHOD1(FlushRoutes, void(int interface_index));
+ MOCK_METHOD2(FlushRoutes, void(int interface_index, bool all_routes));
MOCK_METHOD0(FlushCache, bool());
MOCK_METHOD1(ResetTable, void(int interface_index));
MOCK_METHOD2(SetDefaultMetric, void(int interface_index, uint32 metric));
diff --git a/routing_table.cc b/routing_table.cc
index 9a68052..b0f606d 100644
--- a/routing_table.cc
+++ b/routing_table.cc
@@ -136,6 +136,9 @@
uint32 metric) {
VLOG(2) << __func__ << " index " << interface_index << " metric " << metric;
+ // Delete routes that we did not explicitly create.
+ FlushRoutes(interface_index, false);
+
const IPConfig::Properties &ipconfig_props = ipconfig->properties();
RoutingTableEntry *old_entry;
IPAddress gateway_address(ipconfig_props.address_family);
@@ -172,7 +175,7 @@
false));
}
-void RoutingTable::FlushRoutes(int interface_index) {
+void RoutingTable::FlushRoutes(int interface_index, bool all_routes) {
VLOG(2) << __func__;
base::hash_map<int, vector<RoutingTableEntry> >::iterator table =
@@ -185,7 +188,9 @@
vector<RoutingTableEntry>::iterator nent;
for (nent = table->second.begin(); nent != table->second.end(); ++nent) {
- ApplyRoute(interface_index, *nent, RTNLMessage::kModeDelete, 0);
+ if (all_routes ||
+ (nent->from_rtnl && nent->dst.family() == IPAddress::kFamilyIPv4))
+ ApplyRoute(interface_index, *nent, RTNLMessage::kModeDelete, 0);
}
}
diff --git a/routing_table.h b/routing_table.h
index 90eccbe..d8c80fd 100644
--- a/routing_table.h
+++ b/routing_table.h
@@ -50,8 +50,10 @@
const IPConfigRefPtr &ipconfig,
uint32 metric);
- // Remove all routes associated with interface.
- virtual void FlushRoutes(int interface_index);
+ // Remove routes associated with interface.
+ // If |all_routes| is true, delete all routes.
+ // If |all_routes| is false, delete routes that we did not create.
+ virtual void FlushRoutes(int interface_index, bool all_routes);
// Flush the routing cache for all interfaces.
virtual bool FlushCache();
diff --git a/routing_table_unittest.cc b/routing_table_unittest.cc
index e5b2960..0722529 100644
--- a/routing_table_unittest.cc
+++ b/routing_table_unittest.cc
@@ -54,6 +54,11 @@
static const uint32 kTestDeviceIndex0;
static const uint32 kTestDeviceIndex1;
static const char kTestDeviceName0[];
+ static const char kTestDeviceNetAddress4[];
+ static const char kTestForeignNetAddress4[];
+ static const char kTestForeignNetGateway4[];
+ static const char kTestForeignNetAddress6[];
+ static const char kTestForeignNetGateway6[];
static const char kTestNetAddress0[];
static const char kTestNetAddress1[];
@@ -69,6 +74,11 @@
const uint32 RoutingTableTest::kTestDeviceIndex0 = 12345;
const uint32 RoutingTableTest::kTestDeviceIndex1 = 67890;
const char RoutingTableTest::kTestDeviceName0[] = "test-device0";
+const char RoutingTableTest::kTestDeviceNetAddress4[] = "192.168.2.0/24";
+const char RoutingTableTest::kTestForeignNetAddress4[] = "192.168.2.2";
+const char RoutingTableTest::kTestForeignNetGateway4[] = "192.168.2.1";
+const char RoutingTableTest::kTestForeignNetAddress6[] = "2000::/3";
+const char RoutingTableTest::kTestForeignNetGateway6[] = "fe80:::::1";
const char RoutingTableTest::kTestNetAddress0[] = "192.168.1.1";
const char RoutingTableTest::kTestNetAddress1[] = "192.168.1.2";
@@ -386,7 +396,7 @@
0),
_,
0));
- routing_table_->FlushRoutes(kTestDeviceIndex0);
+ routing_table_->FlushRoutes(kTestDeviceIndex0, true);
// Test that the routing table size returns to zero.
EXPECT_EQ(1, GetRoutingTables()->size());
@@ -397,4 +407,151 @@
StopRTNLHandler();
}
+TEST_F(RoutingTableTest, RouteDeleteForeign) {
+ EXPECT_CALL(sockets_, Send(kTestSocket, _, _, 0));
+ StartRTNLHandler();
+ routing_table_->Start();
+
+ // Expect the tables to be empty by default.
+ EXPECT_EQ(0, GetRoutingTables()->size());
+
+ // Add a foreign IPv4 entry.
+ IPAddress default_address4(IPAddress::kFamilyIPv4);
+ default_address4.SetAddressToDefault();
+
+ IPAddress foreign_address4(IPAddress::kFamilyIPv4);
+ foreign_address4.SetAddressFromString(kTestForeignNetAddress4);
+
+ IPAddress foreign_gateway4(IPAddress::kFamilyIPv4);
+ foreign_gateway4.SetAddressFromString(kTestForeignNetGateway4);
+
+ const int metric = 10;
+
+ RoutingTableEntry foreign_entry4(foreign_address4,
+ default_address4,
+ foreign_gateway4,
+ metric,
+ RT_SCOPE_UNIVERSE,
+ true);
+ SendRouteMsg(RTNLMessage::kModeAdd,
+ kTestDeviceIndex0,
+ foreign_entry4);
+
+ base::hash_map<int, std::vector<RoutingTableEntry> > *tables =
+ GetRoutingTables();
+ EXPECT_EQ(1, tables->size());
+ EXPECT_EQ(1, (*tables)[kTestDeviceIndex0].size());
+ RoutingTableEntry test_entry = (*tables)[kTestDeviceIndex0][0];
+ EXPECT_TRUE(test_entry.from_rtnl);
+
+ // Add a foreign IPv6 entry.
+ // This entry should not be deleted.
+ IPAddress default_address6(IPAddress::kFamilyIPv6);
+ default_address6.SetAddressToDefault();
+
+ IPAddress foreign_address6(IPAddress::kFamilyIPv6);
+ foreign_address6.SetAddressFromString(kTestForeignNetAddress6);
+
+ IPAddress foreign_gateway6(IPAddress::kFamilyIPv6);
+ foreign_gateway6.SetAddressFromString(kTestForeignNetGateway6);
+
+ RoutingTableEntry foreign_entry6(foreign_address6,
+ default_address6,
+ foreign_gateway6,
+ metric,
+ RT_SCOPE_UNIVERSE,
+ true);
+ SendRouteMsg(RTNLMessage::kModeAdd,
+ kTestDeviceIndex0,
+ foreign_entry6);
+
+ EXPECT_EQ(2, (*tables)[kTestDeviceIndex0].size());
+ test_entry = (*tables)[kTestDeviceIndex0][1];
+ EXPECT_TRUE(test_entry.from_rtnl);
+
+ // Add device route.
+ IPAddress device_net_address4(IPAddress::kFamilyIPv4);
+ device_net_address4.SetAddressFromString(kTestDeviceNetAddress4);
+ RTNLMessage msg(
+ RTNLMessage::kTypeRoute,
+ RTNLMessage::kModeAdd,
+ 0,
+ 0,
+ 0,
+ 0,
+ device_net_address4.family());
+ msg.set_route_status(RTNLMessage::RouteStatus(
+ device_net_address4.prefix(),
+ default_address4.prefix(),
+ RT_TABLE_MAIN,
+ RTPROT_KERNEL,
+ RT_SCOPE_UNIVERSE,
+ RTN_UNICAST,
+ 0));
+
+ msg.SetAttribute(RTA_DST, device_net_address4.address());
+ msg.SetAttribute(RTA_SRC, default_address4.address());
+ msg.SetAttribute(RTA_GATEWAY, foreign_gateway4.address());
+ msg.SetAttribute(RTA_PRIORITY, ByteString::CreateFromCPUUInt32(metric));
+ msg.SetAttribute(RTA_OIF, ByteString::CreateFromCPUUInt32(kTestDeviceIndex0));
+
+ ByteString msgdata = msg.Encode();
+ EXPECT_NE(0, msgdata.GetLength());
+
+ InputData data(msgdata.GetData(), msgdata.GetLength());
+ RTNLHandler::GetInstance()->ParseRTNL(&data);
+
+ // The device route should not make it into our routing table.
+ EXPECT_EQ(2, (*tables)[kTestDeviceIndex0].size());
+
+ // Add an entry from IPConfig.
+ MockControl control;
+ IPConfigRefPtr ipconfig(new IPConfig(&control, kTestDeviceName0));
+ IPConfig::Properties properties;
+ properties.address_family = IPAddress::kFamilyIPv4;
+ properties.gateway = kTestNetAddress0;
+ properties.address = kTestNetAddress1;
+ ipconfig->UpdateProperties(properties, true);
+
+ IPAddress gateway_address4(IPAddress::kFamilyIPv4);
+ gateway_address4.SetAddressFromString(kTestNetAddress0);
+ RoutingTableEntry entry(default_address4,
+ default_address4,
+ gateway_address4,
+ metric,
+ RT_SCOPE_UNIVERSE,
+ false);
+
+ EXPECT_CALL(sockets_,
+ Send(kTestSocket,
+ IsRoutingPacket(RTNLMessage::kModeDelete,
+ kTestDeviceIndex0,
+ foreign_entry4,
+ 0),
+ _,
+ 0));
+ EXPECT_CALL(sockets_,
+ Send(kTestSocket,
+ IsRoutingPacket(RTNLMessage::kModeDelete,
+ kTestDeviceIndex0,
+ foreign_entry6,
+ 0),
+ _,
+ 0)).Times(0);
+ EXPECT_CALL(sockets_,
+ Send(kTestSocket,
+ IsRoutingPacket(RTNLMessage::kModeAdd,
+ kTestDeviceIndex0,
+ entry,
+ NLM_F_CREATE | NLM_F_EXCL),
+ _,
+ 0));
+ EXPECT_TRUE(routing_table_->SetDefaultRoute(kTestDeviceIndex0,
+ ipconfig,
+ metric));
+
+ routing_table_->Stop();
+ StopRTNLHandler();
+}
+
} // namespace shill
diff --git a/rtnl_handler.h b/rtnl_handler.h
index c119472..b945840 100644
--- a/rtnl_handler.h
+++ b/rtnl_handler.h
@@ -105,6 +105,7 @@
FRIEND_TEST(RTNLListenerTest, NoRun);
FRIEND_TEST(RTNLListenerTest, Run);
+ FRIEND_TEST(RoutingTableTest, RouteDeleteForeign);
// This stops the event-monitoring function of the RTNL handler -- it is
// private since it will never happen in normal running, but is useful for