shill: Assign "Default" status to the best connection
Set the highest-ranked connection to be the "Default".
As a result, the highest priority default route will
move with the highest-ranked connection in the service
list.
Bonus changes:
* Service now formally holds a reference to the Connection
object, so call a "SetConnection" method instead of a
Create/Destroy of the HTTPProxy.
* Actually start the routing table service, and do a couple
minor fixes due to how the kernel actually accepts metric
changes.
BUG=chromium-os:7607,chromium-os:23993
TEST=New Unit Test + Manual (watch routes while inserting
USB-Ethernet on a machine connected to WiFi)
Change-Id: Iddf1ed766238d9e8adc97bb54fc12b527f86239f
Reviewed-on: https://gerrit.chromium.org/gerrit/12685
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Reviewed-by: Gaurav Shah <gauravsh@chromium.org>
Commit-Ready: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/Makefile b/Makefile
index 82331f9..3b45ee5 100644
--- a/Makefile
+++ b/Makefile
@@ -180,6 +180,7 @@
mock_adaptors.o \
mock_ares.o \
mock_async_connection.o \
+ mock_connection.o \
mock_control.o \
mock_dbus_properties_proxy.o \
mock_device.o \
diff --git a/connection.cc b/connection.cc
index 25291b5..460654d 100644
--- a/connection.cc
+++ b/connection.cc
@@ -75,7 +75,7 @@
}
}
-void Connection::SetDefault(bool is_default) {
+void Connection::SetIsDefault(bool is_default) {
VLOG(2) << __func__;
if (is_default == is_default_) {
return;
diff --git a/connection.h b/connection.h
index 00e61f3..f99447d 100644
--- a/connection.h
+++ b/connection.h
@@ -31,12 +31,12 @@
// Add the contents of an IPConfig reference to the list of managed state.
// This will replace all previous state for this address family.
- void UpdateFromIPConfig(const IPConfigRefPtr &config);
+ virtual void UpdateFromIPConfig(const IPConfigRefPtr &config);
// Sets the current connection as "default", i.e., routes and DNS entries
// should be used by all system components that don't select explicitly.
bool is_default() const { return is_default_; }
- void SetDefault(bool is_default);
+ virtual void SetIsDefault(bool is_default);
const std::string &interface_name() const { return interface_name_; }
const std::vector<std::string> &dns_servers() const { return dns_servers_; }
diff --git a/connection_unittest.cc b/connection_unittest.cc
index 130302c..e5b0cba 100644
--- a/connection_unittest.cc
+++ b/connection_unittest.cc
@@ -109,14 +109,14 @@
ipconfig_->properties().dns_servers,
ipconfig_->properties().domain_search));
- connection_->SetDefault(true);
+ connection_->SetIsDefault(true);
EXPECT_TRUE(connection_->is_default());
EXPECT_CALL(routing_table_,
SetDefaultMetric(kTestDeviceInterfaceIndex0,
Connection::kNonDefaultMetricBase +
kTestDeviceInterfaceIndex0));
- connection_->SetDefault(false);
+ connection_->SetIsDefault(false);
EXPECT_FALSE(connection_->is_default());
}
@@ -125,7 +125,7 @@
Connection::kDefaultMetric));
vector<std::string> empty_list;
EXPECT_CALL(resolver_, SetDNSFromLists(empty_list, empty_list));
- connection_->SetDefault(true);
+ connection_->SetIsDefault(true);
EXPECT_CALL(rtnl_handler_,
AddInterfaceAddress(kTestDeviceInterfaceIndex0, _, _));
diff --git a/device.cc b/device.cc
index 52d270e..87a9a95 100644
--- a/device.cc
+++ b/device.cc
@@ -297,10 +297,15 @@
if (success) {
CreateConnection();
connection_->UpdateFromIPConfig(ipconfig);
- SetServiceState(Service::kStateConnected);
+ // SetConnection must occur after the UpdateFromIPConfig so the
+ // service can use the values derived from the connection.
if (selected_service_.get()) {
- selected_service_->CreateHTTPProxy(connection_);
+ selected_service_->SetConnection(connection_);
}
+ // The service state change needs to happen last, so that at the
+ // time we report the state change to the manager, the service
+ // has its connection.
+ SetServiceState(Service::kStateConnected);
} else {
// TODO(pstew): This logic gets more complex when multiple IPConfig types
// are run in parallel (e.g. DHCP and DHCP6)
@@ -322,7 +327,7 @@
VLOG(2) << __func__;
connection_ = NULL;
if (selected_service_.get()) {
- selected_service_->DestroyHTTPProxy();
+ selected_service_->SetConnection(NULL);
}
}
@@ -339,9 +344,9 @@
}
// TODO(pstew): We need to revisit the model here: should the Device
// subclass be responsible for calling DestroyIPConfig() (which would
- // trigger DestroyConnection() and Service::DestroyHTTPProxy())?
+ // trigger DestroyConnection() and Service::SetConnection(NULL))?
// Ethernet does, but WiFi currently does not. crosbug.com/23929
- selected_service_->DestroyHTTPProxy();
+ selected_service_->SetConnection(NULL);
}
selected_service_ = service;
}
diff --git a/device_unittest.cc b/device_unittest.cc
index 5b9cc3c..0a10427 100644
--- a/device_unittest.cc
+++ b/device_unittest.cc
@@ -180,6 +180,14 @@
EXPECT_EQ(string::npos, to_process.find('/'));
}
+MATCHER(IsNullRefPtr, "") {
+ return !arg;
+}
+
+MATCHER(NotNullRefPtr, "") {
+ return arg;
+}
+
TEST_F(DeviceTest, SelectedService) {
EXPECT_FALSE(device_->selected_service_.get());
device_->SetServiceState(Service::kStateAssociating);
@@ -199,14 +207,14 @@
EXPECT_CALL(*service.get(), state())
.WillOnce(Return(Service::kStateUnknown));
EXPECT_CALL(*service.get(), SetState(Service::kStateIdle));
- EXPECT_CALL(*service.get(), DestroyHTTPProxy());
+ EXPECT_CALL(*service.get(), SetConnection(IsNullRefPtr()));
device_->SelectService(NULL);
// A service in the "Failure" state should not be reset to "Idle"
device_->SelectService(service);
EXPECT_CALL(*service.get(), state())
.WillOnce(Return(Service::kStateFailure));
- EXPECT_CALL(*service.get(), DestroyHTTPProxy());
+ EXPECT_CALL(*service.get(), SetConnection(IsNullRefPtr()));
device_->SelectService(NULL);
}
@@ -217,7 +225,7 @@
manager()));
device_->SelectService(service);
EXPECT_CALL(*service.get(), SetState(Service::kStateDisconnected));
- EXPECT_CALL(*service.get(), DestroyHTTPProxy());
+ EXPECT_CALL(*service.get(), SetConnection(IsNullRefPtr()));
device_->IPConfigUpdatedCallback(NULL, false);
}
@@ -231,7 +239,7 @@
scoped_refptr<MockIPConfig> ipconfig = new MockIPConfig(control_interface(),
kDeviceName);
EXPECT_CALL(*service.get(), SetState(Service::kStateConnected));
- EXPECT_CALL(*service.get(), CreateHTTPProxy(_));
+ EXPECT_CALL(*service.get(), SetConnection(NotNullRefPtr()));
device_->IPConfigUpdatedCallback(ipconfig.get(), true);
}
diff --git a/manager.cc b/manager.cc
index 5d07df0..fe5c36b 100644
--- a/manager.cc
+++ b/manager.cc
@@ -21,6 +21,7 @@
#include <chromeos/dbus/service_constants.h>
#include "shill/adaptor_interfaces.h"
+#include "shill/connection.h"
#include "shill/control_interface.h"
#include "shill/dbus_adaptor.h"
#include "shill/default_profile.h"
@@ -392,6 +393,7 @@
vector<ServiceRefPtr>::iterator it;
for (it = services_.begin(); it != services_.end(); ++it) {
if (to_forget->UniqueName() == (*it)->UniqueName()) {
+ DCHECK(!(*it)->connection());
services_.erase(it);
SortServices();
return;
@@ -449,6 +451,12 @@
}
void Manager::SortServices() {
+ VLOG(4) << "In " << __func__;
+ ConnectionRefPtr default_connection;
+ if (!services_.empty()) {
+ // Keep track of the connection that was last considered default.
+ default_connection = services_[0]->connection();
+ }
sort(services_.begin(), services_.end(), ServiceSorter(technology_order_));
vector<string> service_paths;
@@ -466,6 +474,16 @@
ConnectedTechnologies(&error));
adaptor_->EmitStringChanged(flimflam::kDefaultTechnologyProperty,
DefaultTechnology(&error));
+
+ if (!services_.empty()) {
+ if (default_connection.get() &&
+ (services_[0]->connection().get() != default_connection.get())) {
+ default_connection->SetIsDefault(false);
+ }
+ if (services_[0]->connection().get()) {
+ services_[0]->connection()->SetIsDefault(true);
+ }
+ }
}
string Manager::CalculateState(Error */*error*/) {
diff --git a/manager.h b/manager.h
index b31c6bc..9a56c64 100644
--- a/manager.h
+++ b/manager.h
@@ -106,6 +106,7 @@
FRIEND_TEST(ManagerTest, DeviceRegistrationAndStart);
FRIEND_TEST(ManagerTest, PushPopProfile);
FRIEND_TEST(ManagerTest, SortServices);
+ FRIEND_TEST(ManagerTest, SortServicesWithConnection);
FRIEND_TEST(ManagerTest, AvailableTechnologies);
FRIEND_TEST(ManagerTest, ConnectedTechnologies);
FRIEND_TEST(ManagerTest, DefaultTechnology);
diff --git a/manager_unittest.cc b/manager_unittest.cc
index 6aebdb5..ab9372b 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -23,8 +23,10 @@
#include "shill/key_file_store.h"
#include "shill/key_value_store.h"
#include "shill/mock_adaptors.h"
+#include "shill/mock_connection.h"
#include "shill/mock_control.h"
#include "shill/mock_device.h"
+#include "shill/mock_device_info.h"
#include "shill/mock_glib.h"
#include "shill/mock_profile.h"
#include "shill/mock_service.h"
@@ -57,7 +59,12 @@
manager(),
"wifi0",
"addr4",
- 4)) {
+ 4)),
+ device_info_(new NiceMock<MockDeviceInfo>(
+ control_interface(),
+ reinterpret_cast<EventDispatcher*>(NULL),
+ reinterpret_cast<Manager*>(NULL))),
+ manager_adaptor_(new NiceMock<ManagerMockAdaptor>()) {
mock_devices_.push_back(new NiceMock<MockDevice>(control_interface(),
dispatcher(),
manager(),
@@ -83,6 +90,10 @@
"addr3",
3));
manager()->connect_profiles_to_rpc_ = false;
+
+ // Replace the manager's adaptor with a quieter one, and one
+ // we can do EXPECT*() against. Passes ownership.
+ manager()->adaptor_.reset(manager_adaptor_);
}
virtual ~ManagerTest() {}
@@ -152,6 +163,10 @@
protected:
scoped_refptr<MockWiFi> mock_wifi_;
vector<scoped_refptr<MockDevice> > mock_devices_;
+ scoped_ptr<MockDeviceInfo> device_info_;
+
+ // This pointer is owned by the manager, and only tracked here for EXPECT*()
+ ManagerMockAdaptor *manager_adaptor_;
};
bool ManagerTest::ServiceOrderIs(ServiceRefPtr svc0, ServiceRefPtr svc1) {
@@ -740,6 +755,43 @@
manager()->DeregisterService(mock_service1);
}
+TEST_F(ManagerTest, SortServicesWithConnection) {
+ scoped_refptr<MockService> mock_service0(
+ new NiceMock<MockService>(control_interface(),
+ dispatcher(),
+ manager()));
+ scoped_refptr<MockService> mock_service1(
+ new NiceMock<MockService>(control_interface(),
+ dispatcher(),
+ manager()));
+
+ scoped_refptr<MockConnection> mock_connection0(
+ new NiceMock<MockConnection>(device_info_.get()));
+ scoped_refptr<MockConnection> mock_connection1(
+ new NiceMock<MockConnection>(device_info_.get()));
+
+ manager()->RegisterService(mock_service0);
+ manager()->RegisterService(mock_service1);
+
+ mock_service0->connection_ = mock_connection0;
+ mock_service1->connection_ = mock_connection1;
+
+ EXPECT_CALL(*mock_connection0.get(), SetIsDefault(true));
+ manager()->SortServices();
+
+ mock_service1->set_priority(1);
+ EXPECT_CALL(*mock_connection0.get(), SetIsDefault(false));
+ EXPECT_CALL(*mock_connection1.get(), SetIsDefault(true));
+ manager()->SortServices();
+
+ EXPECT_CALL(*mock_connection0.get(), SetIsDefault(true));
+ mock_service1->connection_ = NULL;
+ manager()->DeregisterService(mock_service1);
+
+ mock_service0->connection_ = NULL;
+ manager()->DeregisterService(mock_service0);
+}
+
TEST_F(ManagerTest, AvailableTechnologies) {
mock_devices_.push_back(new NiceMock<MockDevice>(control_interface(),
dispatcher(),
diff --git a/mock_connection.cc b/mock_connection.cc
new file mode 100644
index 0000000..6715994
--- /dev/null
+++ b/mock_connection.cc
@@ -0,0 +1,16 @@
+// Copyright (c) 2011 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.
+
+#include "shill/mock_connection.h"
+
+#include "shill/ipconfig.h"
+
+namespace shill {
+
+MockConnection::MockConnection(const DeviceInfo *device_info)
+ : Connection(0, std::string(), device_info) {}
+
+MockConnection::~MockConnection() {}
+
+} // namespace shill
diff --git a/mock_connection.h b/mock_connection.h
new file mode 100644
index 0000000..06f067f
--- /dev/null
+++ b/mock_connection.h
@@ -0,0 +1,29 @@
+// Copyright (c) 2011 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.
+
+#ifndef SHILL_MOCK_CONNECTION_H_
+#define SHILL_MOCK_CONNECTION_H_
+
+#include <base/basictypes.h>
+#include <gmock/gmock.h>
+
+#include "shill/connection.h"
+
+namespace shill {
+
+class MockConnection : public Connection {
+ public:
+ MockConnection(const DeviceInfo *device_info);
+ virtual ~MockConnection();
+
+ MOCK_METHOD1(UpdateFromIPConfig, void(const IPConfigRefPtr &config));
+ MOCK_METHOD1(SetIsDefault, void(bool is_default));
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MockConnection);
+};
+
+} // namespace shill
+
+#endif // SHILL_MOCK_CONNECTION_H_
diff --git a/mock_service.h b/mock_service.h
index 3d7ce28..d03e6c2 100644
--- a/mock_service.h
+++ b/mock_service.h
@@ -41,8 +41,7 @@
MOCK_CONST_METHOD0(GetStorageIdentifier, std::string());
MOCK_METHOD1(Load, bool(StoreInterface *store_interface));
MOCK_METHOD1(Save, bool(StoreInterface *store_interface));
- MOCK_METHOD1(CreateHTTPProxy, void(ConnectionRefPtr connection));
- MOCK_METHOD0(DestroyHTTPProxy, void());
+ MOCK_METHOD1(SetConnection, void(ConnectionRefPtr connection));
MOCK_CONST_METHOD0(technology, Technology::Identifier());
// Set a string for this Service via |store|. Can be wired to Save() for
// test purposes.
diff --git a/routing_table.cc b/routing_table.cc
index 447cba0..a32549a 100644
--- a/routing_table.cc
+++ b/routing_table.cc
@@ -130,9 +130,7 @@
&old_entry)) {
if (old_entry.gateway.Equals(gateway_address)) {
if (old_entry.metric != metric) {
- old_entry.metric = metric;
- ApplyRoute(interface_index, old_entry, RTNLMessage::kModeAdd,
- NLM_F_CREATE | NLM_F_REPLACE);
+ ReplaceMetric(interface_index, old_entry, metric);
}
return true;
} else {
@@ -183,22 +181,16 @@
if (GetDefaultRoute(interface_index, IPAddress::kFamilyIPv4, &entry) &&
entry.metric != metric) {
- entry.metric = metric;
- ApplyRoute(interface_index, entry, RTNLMessage::kModeAdd,
- NLM_F_CREATE | NLM_F_REPLACE);
+ ReplaceMetric(interface_index, entry, metric);
}
if (GetDefaultRoute(interface_index, IPAddress::kFamilyIPv6, &entry) &&
entry.metric != metric) {
- entry.metric = metric;
- ApplyRoute(interface_index, entry, RTNLMessage::kModeAdd,
- NLM_F_CREATE | NLM_F_REPLACE);
+ ReplaceMetric(interface_index, entry, metric);
}
}
void RoutingTable::RouteMsgHandler(const RTNLMessage &msg) {
- VLOG(2) << __func__;
-
if (msg.type() != RTNLMessage::kTypeRoute ||
msg.family() == IPAddress::kFamilyUnknown ||
!msg.HasAttribute(RTA_OIF)) {
@@ -254,9 +246,10 @@
nent->src.Equals(entry.src) &&
nent->gateway.Equals(entry.gateway) &&
nent->scope == entry.scope) {
- if (msg.mode() == RTNLMessage::kModeDelete) {
+ if (msg.mode() == RTNLMessage::kModeDelete &&
+ nent->metric == entry.metric) {
table.erase(nent);
- } else {
+ } else if (msg.mode() == RTNLMessage::kModeAdd) {
nent->from_rtnl = true;
nent->metric = entry.metric;
}
@@ -307,6 +300,24 @@
return RTNLHandler::GetInstance()->SendMessage(&msg);
}
+// Somewhat surprisingly, the kernel allows you to create multiple routes
+// to the same destination through the same interface with different metrics.
+// Therefore, to change the metric on a route, we can't just use the
+// NLM_F_REPLACE flag by itself. We have to explicitly remove the old route.
+// We do so after creating the route at a new metric so there is no traffic
+// disruption to existing network streams.
+void RoutingTable::ReplaceMetric(uint32 interface_index,
+ const RoutingTableEntry &entry,
+ uint32 metric) {
+ RoutingTableEntry new_entry = entry;
+ new_entry.metric = metric;
+ // First create the route at the new metric.
+ ApplyRoute(interface_index, new_entry, RTNLMessage::kModeAdd,
+ NLM_F_CREATE | NLM_F_REPLACE);
+ // Then delete the route at the old metric.
+ ApplyRoute(interface_index, entry, RTNLMessage::kModeDelete, 0);
+}
+
bool RoutingTable::FlushCache() {
static const char *kPaths[2] = { kRouteFlushPath4, kRouteFlushPath6 };
bool ret = true;
diff --git a/routing_table.h b/routing_table.h
index 84f87a3..08a0fa9 100644
--- a/routing_table.h
+++ b/routing_table.h
@@ -70,6 +70,9 @@
const RoutingTableEntry &entry,
RTNLMessage::Mode mode,
unsigned int flags);
+ void ReplaceMetric(uint32 interface_index,
+ const RoutingTableEntry &entry,
+ uint32 metric);
bool FlushCache();
static const char kRouteFlushPath4[];
diff --git a/routing_table_unittest.cc b/routing_table_unittest.cc
index 0ba1f5b..1ca7a09 100644
--- a/routing_table_unittest.cc
+++ b/routing_table_unittest.cc
@@ -169,7 +169,7 @@
StartRTNLHandler();
routing_table_->Start();
- // Expect the tables to be empty by default
+ // Expect the tables to be empty by default.
EXPECT_EQ(0, GetRoutingTables()->size());
IPAddress default_address(IPAddress::kFamilyIPv4);
@@ -186,7 +186,7 @@
metric,
RT_SCOPE_UNIVERSE,
true);
- // Add a single entry
+ // Add a single entry.
SendRouteMsg(RTNLMessage::kModeAdd,
kTestDeviceIndex0,
entry0);
@@ -194,7 +194,7 @@
base::hash_map<int, std::vector<RoutingTableEntry> > *tables =
GetRoutingTables();
- // Should have a single table, which should in turn have a single entry
+ // We should have a single table, which should in turn have a single entry.
EXPECT_EQ(1, tables->size());
EXPECT_TRUE(ContainsKey(*tables, kTestDeviceIndex0));
EXPECT_EQ(1, (*tables)[kTestDeviceIndex0].size());
@@ -202,12 +202,12 @@
RoutingTableEntry test_entry = (*tables)[kTestDeviceIndex0][0];
EXPECT_TRUE(entry0.Equals(test_entry));
- // Add a second entry for a different interface
+ // Add a second entry for a different interface.
SendRouteMsg(RTNLMessage::kModeAdd,
kTestDeviceIndex1,
entry0);
- // Should have two tables, which should have a single entry each
+ // We should have two tables, which should have a single entry each.
EXPECT_EQ(2, tables->size());
EXPECT_TRUE(ContainsKey(*tables, kTestDeviceIndex1));
EXPECT_EQ(1, (*tables)[kTestDeviceIndex0].size());
@@ -226,12 +226,13 @@
RT_SCOPE_UNIVERSE,
true);
- // Add a second gateway route to the second interface
+ // Add a second gateway route to the second interface.
SendRouteMsg(RTNLMessage::kModeAdd,
kTestDeviceIndex1,
entry1);
- // Should have two tables, one of which has a single entry, the other has two
+ // We should have two tables, one of which has a single entry, the other has
+ // two.
EXPECT_EQ(2, tables->size());
EXPECT_EQ(1, (*tables)[kTestDeviceIndex0].size());
EXPECT_EQ(2, (*tables)[kTestDeviceIndex1].size());
@@ -239,12 +240,12 @@
test_entry = (*tables)[kTestDeviceIndex1][1];
EXPECT_TRUE(entry1.Equals(test_entry));
- // Remove the first gateway route from the second interface
+ // Remove the first gateway route from the second interface.
SendRouteMsg(RTNLMessage::kModeDelete,
kTestDeviceIndex1,
entry0);
- // We should be back to having one route per table
+ // We should be back to having one route per table.
EXPECT_EQ(2, tables->size());
EXPECT_EQ(1, (*tables)[kTestDeviceIndex0].size());
EXPECT_EQ(1, (*tables)[kTestDeviceIndex1].size());
@@ -252,30 +253,30 @@
test_entry = (*tables)[kTestDeviceIndex1][0];
EXPECT_TRUE(entry1.Equals(test_entry));
- // Send a duplicate of the second gatway route message, changing the metric
+ // Send a duplicate of the second gatway route message, changing the metric.
RoutingTableEntry entry2(entry1);
entry2.metric++;
SendRouteMsg(RTNLMessage::kModeAdd,
kTestDeviceIndex1,
entry2);
- // Routing table size shouldn't change, but the new metric should match
+ // Routing table size shouldn't change, but the new metric should match.
EXPECT_EQ(1, (*tables)[kTestDeviceIndex1].size());
test_entry = (*tables)[kTestDeviceIndex1][0];
EXPECT_TRUE(entry2.Equals(test_entry));
- // Find a matching entry
+ // Find a matching entry.
EXPECT_TRUE(routing_table_->GetDefaultRoute(kTestDeviceIndex1,
IPAddress::kFamilyIPv4,
&test_entry));
EXPECT_TRUE(entry2.Equals(test_entry));
- // Test that a search for a non-matching family fails
+ // Test that a search for a non-matching family fails.
EXPECT_FALSE(routing_table_->GetDefaultRoute(kTestDeviceIndex1,
IPAddress::kFamilyIPv6,
&test_entry));
- // Remove last entry from an existing interface and test that we now fail
+ // Remove last entry from an existing interface and test that we now fail.
SendRouteMsg(RTNLMessage::kModeDelete,
kTestDeviceIndex1,
entry2);
@@ -284,7 +285,7 @@
IPAddress::kFamilyIPv4,
&test_entry));
- // Add a route from an IPConfig entry
+ // Add a route from an IPConfig entry.
MockControl control;
IPConfigRefPtr ipconfig(new IPConfig(&control, kTestDeviceName0));
IPConfig::Properties properties;
@@ -305,7 +306,8 @@
ipconfig,
metric));
- // The table entry should look much like entry0, except with from_rtnl = false
+ // The table entry should look much like entry0, except with
+ // from_rtnl = false.
RoutingTableEntry entry3(entry0);
entry3.from_rtnl = false;
EXPECT_TRUE(routing_table_->GetDefaultRoute(kTestDeviceIndex1,
@@ -314,7 +316,8 @@
EXPECT_TRUE(entry3.Equals(test_entry));
// Setting the same route on the interface with a different metric should
- // push the route with different flags to indicate we are replacing it.
+ // push the route with different flags to indicate we are replacing it,
+ // then it should delete the old entry.
RoutingTableEntry entry4(entry3);
entry4.metric += 10;
EXPECT_CALL(sockets_,
@@ -325,11 +328,19 @@
NLM_F_CREATE | NLM_F_REPLACE),
_,
0));
+ EXPECT_CALL(sockets_,
+ Send(kTestSocket,
+ IsRoutingPacket(RTNLMessage::kModeDelete,
+ kTestDeviceIndex1,
+ entry3,
+ 0),
+ _,
+ 0));
EXPECT_TRUE(routing_table_->SetDefaultRoute(kTestDeviceIndex1,
ipconfig,
entry4.metric));
- // Test that removing the table causes the route to disappear
+ // Test that removing the table causes the route to disappear.
routing_table_->ResetTable(kTestDeviceIndex1);
EXPECT_FALSE(ContainsKey(*tables, kTestDeviceIndex1));
EXPECT_FALSE(routing_table_->GetDefaultRoute(kTestDeviceIndex1,
@@ -337,7 +348,8 @@
&test_entry));
EXPECT_EQ(1, GetRoutingTables()->size());
- // When we set the metric on an existing route, a new add message appears
+ // When we set the metric on an existing route, a new add and delete
+ // operation should occur.
RoutingTableEntry entry5(entry4);
entry5.metric += 10;
EXPECT_CALL(sockets_,
@@ -348,9 +360,17 @@
NLM_F_CREATE | NLM_F_REPLACE),
_,
0));
+ EXPECT_CALL(sockets_,
+ Send(kTestSocket,
+ IsRoutingPacket(RTNLMessage::kModeDelete,
+ kTestDeviceIndex0,
+ entry0,
+ 0),
+ _,
+ 0));
routing_table_->SetDefaultMetric(kTestDeviceIndex0, entry5.metric);
- // Ask to flush table0. We should see a delete message sent
+ // Ask to flush table0. We should see a delete message sent.
EXPECT_CALL(sockets_,
Send(kTestSocket,
IsRoutingPacket(RTNLMessage::kModeDelete,
@@ -361,7 +381,7 @@
0));
routing_table_->FlushRoutes(kTestDeviceIndex0);
- // Test that the routing table size returns to zero
+ // Test that the routing table size returns to zero.
EXPECT_EQ(1, GetRoutingTables()->size());
routing_table_->ResetTable(kTestDeviceIndex0);
EXPECT_EQ(0, GetRoutingTables()->size());
diff --git a/service.cc b/service.cc
index d868b59..a0f3347 100644
--- a/service.cc
+++ b/service.cc
@@ -318,14 +318,15 @@
favorite_ = true;
}
-void Service::CreateHTTPProxy(ConnectionRefPtr connection) {
- http_proxy_.reset(new HTTPProxy(connection->interface_name(),
- connection->dns_servers()));
- http_proxy_->Start(dispatcher_, &sockets_);
-}
-
-void Service::DestroyHTTPProxy() {
- http_proxy_.reset();
+void Service::SetConnection(ConnectionRefPtr connection) {
+ if (connection.get()) {
+ http_proxy_.reset(new HTTPProxy(connection->interface_name(),
+ connection->dns_servers()));
+ http_proxy_->Start(dispatcher_, &sockets_);
+ } else {
+ http_proxy_.reset();
+ }
+ connection_ = connection;
}
// static
diff --git a/service.h b/service.h
index 687f5dc..7f96ab2 100644
--- a/service.h
+++ b/service.h
@@ -148,10 +148,10 @@
virtual void MakeFavorite();
- // Create an HTTP Proxy that will utilize this service's connection
- // (interface, DNS servers, default route) to serve requests.
- virtual void CreateHTTPProxy(ConnectionRefPtr connection);
- virtual void DestroyHTTPProxy();
+ // Set the connection for this service. If the connection is
+ // non-NULL, create an HTTP Proxy that will utilize this service's
+ // connection to serve requests.
+ virtual void SetConnection(ConnectionRefPtr connection);
bool auto_connect() const { return auto_connect_; }
void set_auto_connect(bool connect) { auto_connect_ = connect; }
@@ -197,6 +197,7 @@
PropertyStore *mutable_store() { return &store_; }
const PropertyStore &store() const { return store_; }
+ const ConnectionRefPtr &connection() const { return connection_; }
protected:
// Returns true if a character is allowed to be in a service storage id.
@@ -242,6 +243,7 @@
private:
friend class ServiceAdaptorInterface;
FRIEND_TEST(DeviceTest, SelectedService);
+ FRIEND_TEST(ManagerTest, SortServicesWithConnection);
FRIEND_TEST(ServiceTest, Constructor);
FRIEND_TEST(ServiceTest, Save);
FRIEND_TEST(ServiceTest, SaveString);
@@ -314,6 +316,7 @@
Configuration *configuration_;
scoped_ptr<ServiceAdaptorInterface> adaptor_;
scoped_ptr<HTTPProxy> http_proxy_;
+ ConnectionRefPtr connection_;
Manager *manager_;
Sockets sockets_;
diff --git a/shill_daemon.cc b/shill_daemon.cc
index 8f62248..233b24d 100644
--- a/shill_daemon.cc
+++ b/shill_daemon.cc
@@ -15,6 +15,7 @@
#include "shill/dhcp_provider.h"
#include "shill/error.h"
#include "shill/proxy_factory.h"
+#include "shill/routing_table.h"
#include "shill/rtnl_handler.h"
#include "shill/shill_config.h"
@@ -60,6 +61,7 @@
glib_.TypeInit();
ProxyFactory::GetInstance()->Init();
RTNLHandler::GetInstance()->Start(&dispatcher_, &sockets_);
+ RoutingTable::GetInstance()->Start();
DHCPProvider::GetInstance()->Init(control_, &dispatcher_, &glib_);
manager_.Start();
}