shill: device_info: Revamp address/route flushing
The route flushing for devices ended up floating around in the device
code with a number of changes and lodged itself in a place where it
might not even be called in some situations. The address flush code
was only being called on connection close, and only applied to
addresses that were active before the system started up.
Implemented the following fixes:
- Move FlushRoutes out of Device to the initialization code in
DeviceInfo.
- Add Address flushing code in the same place.
- Capture the list of IP addresses per-interface earlier at startup
so they can be flushed at device discovery. This involved re-ordering
the RTNL "Dump" path to dump addresses before links, and adding code
in DeviceInfo to capture these addresses in its Info structures before
the actual link was discovered. This way, when the link is discovered,
it's possible to flush the old addresses.
- Use the NLM_ECHO flag when creating and deleting IP addresses in RTNL,
so RTNL events are generated even for operations that originate from
shill, and the tables are better kept up to date, so when Connections
flush IP addresses on disconnect, this properly includes those that
were created locally.
- Break apart DeviceInfo::AddLinkMsgHandler so it can be tested.
BUG=chromium-os:30358
TEST=New unit tests + plug/unplug between different networks / kill/restart
shill.
Change-Id: Ie1a38e0e86ba3a52aaaaf9c3f0e0dfcb3c8fc276
Reviewed-on: https://gerrit.chromium.org/gerrit/21719
Commit-Ready: Paul Stewart <pstew@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/Makefile b/Makefile
index a16698e..47e5501 100644
--- a/Makefile
+++ b/Makefile
@@ -275,6 +275,7 @@
mock_modem_cdma_proxy.o \
mock_modem_gsm_card_proxy.o \
mock_modem_gsm_network_proxy.o \
+ mock_modem_info.o \
mock_modem_manager_proxy.o \
mock_modem_proxy.o \
mock_modem_simple_proxy.o \
@@ -299,6 +300,7 @@
mock_time.o \
mock_vpn.o \
mock_vpn_driver.o \
+ mock_vpn_provider.o \
mock_vpn_service.o \
mock_wifi.o \
mock_wifi_service.o \
diff --git a/device.cc b/device.cc
index e89c118..e76ed36 100644
--- a/device.cc
+++ b/device.cc
@@ -31,7 +31,6 @@
#include "shill/metrics.h"
#include "shill/property_accessor.h"
#include "shill/refptr_types.h"
-#include "shill/routing_table.h"
#include "shill/rtnl_handler.h"
#include "shill/scope_logger.h"
#include "shill/service.h"
@@ -98,7 +97,6 @@
technology_(technology),
portal_attempts_to_online_(0),
dhcp_provider_(DHCPProvider::GetInstance()),
- routing_table_(RoutingTable::GetInstance()),
rtnl_handler_(RTNLHandler::GetInstance()) {
store_.RegisterConstString(flimflam::kAddressProperty, &hardware_address_);
@@ -729,7 +727,6 @@
weak_ptr_factory_.GetWeakPtr(), callback);
if (enable) {
running_ = true;
- routing_table_->FlushRoutes(interface_index());
Start(error, enabled_callback);
} else {
running_ = false;
diff --git a/device.h b/device.h
index 23c92f1..75ac743 100644
--- a/device.h
+++ b/device.h
@@ -35,7 +35,6 @@
class EventDispatcher;
class Manager;
class Metrics;
-class RoutingTable;
class RTNLHandler;
// Device superclass. Individual network interfaces types will inherit from
@@ -169,6 +168,7 @@
FRIEND_TEST(DeviceTest, Save);
FRIEND_TEST(DeviceTest, SelectedService);
FRIEND_TEST(DeviceTest, SetServiceConnectedState);
+ FRIEND_TEST(DeviceTest, Start);
FRIEND_TEST(DeviceTest, Stop);
FRIEND_TEST(DeviceTest, OnEnabledStateChanged);
FRIEND_TEST(ManagerTest, DeviceRegistrationAndStart);
@@ -386,7 +386,6 @@
// Cache singleton pointers for performance and test purposes.
DHCPProvider *dhcp_provider_;
- RoutingTable *routing_table_;
RTNLHandler *rtnl_handler_;
DISALLOW_COPY_AND_ASSIGN(Device);
diff --git a/device_info.cc b/device_info.cc
index a294f79..c724f52 100644
--- a/device_info.cc
+++ b/device_info.cc
@@ -34,6 +34,7 @@
#include "shill/device_stub.h"
#include "shill/ethernet.h"
#include "shill/manager.h"
+#include "shill/routing_table.h"
#include "shill/rtnl_handler.h"
#include "shill/rtnl_listener.h"
#include "shill/rtnl_message.h"
@@ -82,6 +83,7 @@
link_listener_(NULL),
address_listener_(NULL),
device_info_root_(kDeviceInfoRoot),
+ routing_table_(RoutingTable::GetInstance()),
rtnl_handler_(RTNLHandler::GetInstance()) {
}
@@ -107,6 +109,7 @@
void DeviceInfo::Stop() {
link_listener_.reset();
address_listener_.reset();
+ infos_.clear();
}
void DeviceInfo::RegisterDevice(const DeviceRefPtr &device) {
@@ -323,6 +326,78 @@
return false;
}
+DeviceRefPtr DeviceInfo::CreateDevice(const string &link_name,
+ const string &address,
+ int interface_index,
+ Technology::Identifier technology) {
+ DeviceRefPtr device;
+
+ switch (technology) {
+ case Technology::kCellular:
+ // Cellular devices are managed by ModemInfo.
+ SLOG(Device, 2) << "Cellular link " << link_name
+ << " at index " << interface_index
+ << " -- notifying ModemInfo.";
+ manager_->modem_info()->OnDeviceInfoAvailable(link_name);
+ break;
+ case Technology::kEthernet:
+ device = new Ethernet(control_interface_, dispatcher_, metrics_,
+ manager_, link_name, address, interface_index);
+ device->EnableIPv6Privacy();
+ break;
+ case Technology::kVirtioEthernet:
+ device = new VirtioEthernet(control_interface_, dispatcher_, metrics_,
+ manager_, link_name, address,
+ interface_index);
+ device->EnableIPv6Privacy();
+ break;
+ case Technology::kWifi:
+ device = new WiFi(control_interface_, dispatcher_, metrics_, manager_,
+ link_name, address, interface_index);
+ device->EnableIPv6Privacy();
+ break;
+ case Technology::kPPP:
+ case Technology::kTunnel:
+ // Tunnel and PPP devices are managed by the VPN code (PPP for
+ // l2tpipsec). Notify the VPN Provider of the interface's presence.
+ // Since CreateDevice is only called once in the lifetime of an
+ // interface index, this notification will only occur the first
+ // time the device is seen.
+ SLOG(Device, 2) << "Tunnel / PPP link " << link_name
+ << " at index " << interface_index
+ << " -- notifying VPNProvider.";
+ if (!manager_->vpn_provider()->OnDeviceInfoAvailable(link_name,
+ interface_index) &&
+ technology == Technology::kTunnel) {
+ // If VPN does not know anything about this tunnel, it is probably
+ // left over from a previous instance and should not exist.
+ SLOG(Device, 2) << "Tunnel link is unused. Deleting.";
+ DeleteInterface(interface_index);
+ }
+ break;
+ case Technology::kLoopback:
+ // Loopback devices are largely ignored, but we should make sure the
+ // link is enabled.
+ SLOG(Device, 2) << "Bringing up loopback device " << link_name
+ << " at index " << interface_index;
+ rtnl_handler_->SetInterfaceFlags(interface_index, IFF_UP, IFF_UP);
+ return NULL;
+ default:
+ // We will not manage this device in shill. Do not create a device
+ // object or do anything to change its state. We create a stub object
+ // which is useful for testing.
+ return new DeviceStub(control_interface_, dispatcher_, metrics_,
+ manager_, link_name, address, interface_index,
+ technology);;
+ }
+
+ // Reset the routing table and addresses.
+ routing_table_->FlushRoutes(interface_index);
+ FlushAddresses(interface_index);
+
+ return device;
+}
+
void DeviceInfo::AddLinkMsgHandler(const RTNLMessage &msg) {
DCHECK(msg.type() == RTNLMessage::kTypeLink &&
msg.mode() == RTNLMessage::kModeAdd);
@@ -331,16 +406,19 @@
unsigned int flags = msg.link_status().flags;
unsigned int change = msg.link_status().change;
- bool new_device = !ContainsKey(infos_, dev_index);
+ bool new_device =
+ !ContainsKey(infos_, dev_index) || infos_[dev_index].has_addresses_only;
SLOG(Device, 2) << __func__ << "(index=" << dev_index
<< std::showbase << std::hex
<< ", flags=" << flags << ", change=" << change << ")"
<< std::dec << std::noshowbase
<< ", new_device=" << new_device;
+ infos_[dev_index].has_addresses_only = false;
infos_[dev_index].flags = flags;
DeviceRefPtr device = GetDevice(dev_index);
- if (!device.get()) {
+ if (new_device) {
+ CHECK(!device);
if (!msg.HasAttribute(IFLA_IFNAME)) {
LOG(ERROR) << "Add Link message does not have IFLA_IFNAME!";
return;
@@ -369,64 +447,14 @@
LOG(ERROR) << "Add Link message does not have IFLA_ADDRESS!";
return;
}
- switch (technology) {
- case Technology::kCellular:
- // Cellular devices are managed by ModemInfo.
- SLOG(Device, 2) << "Cellular link " << link_name
- << " at index " << dev_index
- << " -- notifying ModemInfo.";
- manager_->modem_info()->OnDeviceInfoAvailable(link_name);
- return;
- case Technology::kEthernet:
- device = new Ethernet(control_interface_, dispatcher_, metrics_,
- manager_, link_name, address, dev_index);
- device->EnableIPv6Privacy();
- break;
- case Technology::kVirtioEthernet:
- device = new VirtioEthernet(control_interface_, dispatcher_, metrics_,
- manager_, link_name, address, dev_index);
- device->EnableIPv6Privacy();
- break;
- case Technology::kWifi:
- device = new WiFi(control_interface_, dispatcher_, metrics_, manager_,
- link_name, address, dev_index);
- device->EnableIPv6Privacy();
- break;
- case Technology::kPPP:
- case Technology::kTunnel:
- // Tunnel and PPP devices are managed by the VPN code (PPP for
- // l2tpipsec). Notify the VPN Provider only if this is the first
- // time we have seen this device index.
- if (new_device) {
- SLOG(Device, 2) << "Tunnel / PPP link " << link_name
- << " at index " << dev_index
- << " -- notifying VPNProvider.";
- if (!manager_->vpn_provider()->OnDeviceInfoAvailable(link_name,
- dev_index) &&
- technology == Technology::kTunnel) {
- // If VPN does not know anything about this tunnel, it is probably
- // left over from a previous instance and should not exist.
- SLOG(Device, 2) << "Tunnel link is unused. Deleting.";
- DeleteInterface(dev_index);
- }
- }
- return;
- case Technology::kLoopback:
- // Loopback devices are largely ignored, but we should make sure the
- // link is enabled.
- SLOG(Device, 2) << "Bringing up loopback device " << link_name
- << " at index " << dev_index;
- rtnl_handler_->SetInterfaceFlags(dev_index, IFF_UP, IFF_UP);
- return;
- default:
- device = new DeviceStub(control_interface_, dispatcher_, metrics_,
- manager_, link_name, address, dev_index,
- technology);
- break;
+ device = CreateDevice(link_name, address, dev_index, technology);
+ if (device) {
+ RegisterDevice(device);
}
- RegisterDevice(device);
}
- device->LinkEvent(flags, change);
+ if (device) {
+ device->LinkEvent(flags, change);
+ }
}
void DeviceInfo::DelLinkMsgHandler(const RTNLMessage &msg) {
@@ -482,8 +510,9 @@
if (iter->address.family() == IPAddress::kFamilyIPv4 ||
(iter->scope == RT_SCOPE_UNIVERSE &&
(iter->flags & ~IFA_F_TEMPORARY) == 0)) {
- SLOG(Device, 2) << __func__ << ": removing ip address from interface "
- << interface_index;
+ SLOG(Device, 2) << __func__ << ": removing ip address "
+ << iter->address.ToString()
+ << " from interface " << interface_index;
rtnl_handler_->RemoveInterfaceAddress(interface_index, iter->address);
}
}
@@ -567,9 +596,9 @@
DCHECK(msg.type() == RTNLMessage::kTypeAddress);
int interface_index = msg.interface_index();
if (!ContainsKey(infos_, interface_index)) {
- LOG(ERROR) << "Got address type message for unknown index "
- << interface_index;
- return;
+ SLOG(Device, 2) << "Got advance address information for unknown index "
+ << interface_index;
+ infos_[interface_index].has_addresses_only = true;
}
const RTNLMessage::AddressStatus &status = msg.address_status();
IPAddress address(msg.family(),
@@ -593,7 +622,8 @@
}
} else if (msg.mode() == RTNLMessage::kModeAdd) {
address_list.push_back(AddressData(address, status.flags, status.scope));
- SLOG(Device, 2) << "Add address for interface " << interface_index;
+ SLOG(Device, 2) << "Add address " << address.ToString()
+ << " for interface " << interface_index;
}
}
diff --git a/device_info.h b/device_info.h
index 5579dd2..261a010 100644
--- a/device_info.h
+++ b/device_info.h
@@ -26,6 +26,7 @@
class Manager;
class Metrics;
+class RoutingTable;
class RTNLHandler;
class RTNLMessage;
@@ -84,15 +85,19 @@
friend class DeviceInfoTest;
FRIEND_TEST(CellularTest, StartLinked);
FRIEND_TEST(DeviceInfoTest, HasSubdir); // For HasSubdir.
+ FRIEND_TEST(DeviceInfoTest, StartStop);
struct Info {
- Info() : flags(0) {}
+ Info() : flags(0), has_addresses_only(false) {}
DeviceRefPtr device;
std::string name;
ByteString mac_address;
std::vector<AddressData> ip_addresses;
unsigned int flags;
+ // This flag indicates that link information has not been retrieved yet;
+ // only the ip_addresses field is valid.
+ bool has_addresses_only;
};
// Root of the kernel sysfs directory holding network device info.
@@ -120,6 +125,14 @@
// Path to the tun device.
static const char kTunDeviceName[];
+ // Create a Device object for the interface named |linkname|, with a
+ // string-form MAC address |address|, whose kernel interface index
+ // is |interface_index| and detected technology is |technology|.
+ DeviceRefPtr CreateDevice(const std::string &link_name,
+ const std::string &address,
+ int interface_index,
+ Technology::Identifier technology);
+
// Return the FilePath for a given |path_name| in the device sysinfo for
// a specific interface |iface_name|.
FilePath GetDeviceInfoPath(const std::string &iface_name,
@@ -173,7 +186,8 @@
std::set<std::string> black_list_;
FilePath device_info_root_;
- // Cache copy of singleton
+ // Cache copy of singleton pointers.
+ RoutingTable *routing_table_;
RTNLHandler *rtnl_handler_;
DISALLOW_COPY_AND_ASSIGN(DeviceInfo);
diff --git a/device_info_unittest.cc b/device_info_unittest.cc
index 1145e62..90cd1f2 100644
--- a/device_info_unittest.cc
+++ b/device_info_unittest.cc
@@ -28,8 +28,11 @@
#include "shill/mock_glib.h"
#include "shill/mock_manager.h"
#include "shill/mock_metrics.h"
+#include "shill/mock_modem_info.h"
+#include "shill/mock_routing_table.h"
#include "shill/mock_rtnl_handler.h"
#include "shill/mock_sockets.h"
+#include "shill/mock_vpn_provider.h"
#include "shill/rtnl_message.h"
using base::Callback;
@@ -37,6 +40,7 @@
using std::string;
using std::vector;
using testing::_;
+using testing::Mock;
using testing::Return;
using testing::StrictMock;
using testing::Test;
@@ -62,10 +66,31 @@
virtual void SetUp() {
device_info_.rtnl_handler_ = &rtnl_handler_;
- EXPECT_CALL(rtnl_handler_, RequestDump(RTNLHandler::kRequestLink |
- RTNLHandler::kRequestAddr));
+ device_info_.routing_table_ = &routing_table_;
}
+ IPAddress CreateInterfaceAddress() {
+ // Create an IP address entry (as if left-over from a previous connection
+ // manager).
+ IPAddress address(IPAddress::kFamilyIPv4);
+ EXPECT_TRUE(address.SetAddressFromString(kTestIPAddress0));
+ address.set_prefix(kTestIPAddressPrefix0);
+ vector<DeviceInfo::AddressData> &addresses =
+ device_info_.infos_[kTestDeviceIndex].ip_addresses;
+ addresses.push_back(DeviceInfo::AddressData(address, 0, RT_SCOPE_UNIVERSE));
+ EXPECT_EQ(1, addresses.size());
+ return address;
+ }
+
+ DeviceRefPtr CreateDevice(const std::string &link_name,
+ const std::string &address,
+ int interface_index,
+ Technology::Identifier technology) {
+ return device_info_.CreateDevice(link_name, address, interface_index,
+ technology);
+ }
+
+
protected:
static const int kTestDeviceIndex;
static const char kTestDeviceName[];
@@ -93,6 +118,7 @@
StrictMock<MockManager> manager_;
DeviceInfo device_info_;
TestEventDispatcher dispatcher_;
+ MockRoutingTable routing_table_;
StrictMock<MockRTNLHandler> rtnl_handler_;
};
@@ -163,9 +189,29 @@
return address.Equals(arg);
}
-TEST_F(DeviceInfoTest, DeviceEnumeration) {
- // Start our own private device_info
+TEST_F(DeviceInfoTest, StartStop) {
+ EXPECT_FALSE(device_info_.link_listener_.get());
+ EXPECT_FALSE(device_info_.address_listener_.get());
+ EXPECT_TRUE(device_info_.infos_.empty());
+
+ EXPECT_CALL(rtnl_handler_, RequestDump(RTNLHandler::kRequestLink |
+ RTNLHandler::kRequestAddr));
device_info_.Start();
+ EXPECT_TRUE(device_info_.link_listener_.get());
+ EXPECT_TRUE(device_info_.address_listener_.get());
+ EXPECT_TRUE(device_info_.infos_.empty());
+ Mock::VerifyAndClearExpectations(&rtnl_handler_);
+
+ CreateInterfaceAddress();
+ EXPECT_FALSE(device_info_.infos_.empty());
+
+ device_info_.Stop();
+ EXPECT_FALSE(device_info_.link_listener_.get());
+ EXPECT_FALSE(device_info_.address_listener_.get());
+ EXPECT_TRUE(device_info_.infos_.empty());
+}
+
+TEST_F(DeviceInfoTest, DeviceEnumeration) {
scoped_ptr<RTNLMessage> message(BuildLinkMessage(RTNLMessage::kModeAdd));
message->set_link_status(RTNLMessage::LinkStatus(0, IFF_LOWER_UP, 0));
EXPECT_FALSE(device_info_.GetDevice(kTestDeviceIndex).get());
@@ -189,29 +235,161 @@
EXPECT_EQ(IFF_UP | IFF_RUNNING, flags);
message.reset(BuildLinkMessage(RTNLMessage::kModeDelete));
+ EXPECT_CALL(manager_, DeregisterDevice(_)).Times(1);
SendMessageToDeviceInfo(*message);
EXPECT_FALSE(device_info_.GetDevice(kTestDeviceIndex).get());
EXPECT_FALSE(device_info_.GetFlags(kTestDeviceIndex, NULL));
EXPECT_EQ(-1, device_info_.GetIndex(kTestDeviceName));
+}
- device_info_.Stop();
+TEST_F(DeviceInfoTest, CreateDeviceCellular) {
+ IPAddress address = CreateInterfaceAddress();
+
+ // A cellular device should be offered to ModemInfo.
+ StrictMock<MockModemInfo> modem_info;
+ EXPECT_CALL(manager_, modem_info()).WillOnce(Return(&modem_info));
+ EXPECT_CALL(modem_info, OnDeviceInfoAvailable(kTestDeviceName)).Times(1);
+ EXPECT_CALL(routing_table_, FlushRoutes(kTestDeviceIndex)).Times(1);
+ EXPECT_CALL(rtnl_handler_, RemoveInterfaceAddress(kTestDeviceIndex,
+ IsIPAddress(address)));
+ EXPECT_FALSE(CreateDevice(
+ kTestDeviceName, "address", kTestDeviceIndex, Technology::kCellular));
+}
+
+TEST_F(DeviceInfoTest, CreateDeviceEthernet) {
+ IPAddress address = CreateInterfaceAddress();
+
+ // An Ethernet device should cause routes and addresses to be flushed.
+ EXPECT_CALL(routing_table_, FlushRoutes(kTestDeviceIndex)).Times(1);
+ EXPECT_CALL(rtnl_handler_, RemoveInterfaceAddress(kTestDeviceIndex,
+ IsIPAddress(address)));
+ DeviceRefPtr device = CreateDevice(
+ kTestDeviceName, "address", kTestDeviceIndex, Technology::kEthernet);
+ EXPECT_TRUE(device);
+ Mock::VerifyAndClearExpectations(&routing_table_);
+ Mock::VerifyAndClearExpectations(&rtnl_handler_);
+
+ // The Ethernet device destructor notifies the manager.
+ EXPECT_CALL(manager_, UpdateEnabledTechnologies()).Times(1);
+ device = NULL;
+}
+
+TEST_F(DeviceInfoTest, CreateDeviceVirtioEthernet) {
+ IPAddress address = CreateInterfaceAddress();
+
+ // VirtioEthernet is identical to Ethernet from the perspective of this test.
+ EXPECT_CALL(routing_table_, FlushRoutes(kTestDeviceIndex)).Times(1);
+ EXPECT_CALL(rtnl_handler_, RemoveInterfaceAddress(kTestDeviceIndex,
+ IsIPAddress(address)));
+ DeviceRefPtr device = CreateDevice(
+ kTestDeviceName, "address", kTestDeviceIndex,
+ Technology::kVirtioEthernet);
+ EXPECT_TRUE(device);
+ Mock::VerifyAndClearExpectations(&routing_table_);
+ Mock::VerifyAndClearExpectations(&rtnl_handler_);
+
+ // The Ethernet device destructor notifies the manager.
+ EXPECT_CALL(manager_, UpdateEnabledTechnologies()).Times(1);
+ device = NULL;
+}
+
+TEST_F(DeviceInfoTest, CreateDeviceWiFi) {
+ IPAddress address = CreateInterfaceAddress();
+
+ // WiFi looks a lot like Ethernet too.
+ EXPECT_CALL(routing_table_, FlushRoutes(kTestDeviceIndex)).Times(1);
+ EXPECT_CALL(rtnl_handler_, RemoveInterfaceAddress(kTestDeviceIndex,
+ IsIPAddress(address)));
+ EXPECT_TRUE(CreateDevice(
+ kTestDeviceName, "address", kTestDeviceIndex, Technology::kWifi));
+}
+
+TEST_F(DeviceInfoTest, CreateDeviceTunnelAccepted) {
+ IPAddress address = CreateInterfaceAddress();
+
+ // A VPN device should be offered to VPNProvider.
+ StrictMock<MockVPNProvider> vpn_provider;
+ EXPECT_CALL(manager_, vpn_provider()).WillOnce(Return(&vpn_provider));
+ EXPECT_CALL(vpn_provider,
+ OnDeviceInfoAvailable(kTestDeviceName, kTestDeviceIndex))
+ .WillOnce(Return(true));
+ EXPECT_CALL(routing_table_, FlushRoutes(kTestDeviceIndex)).Times(1);
+ EXPECT_CALL(rtnl_handler_, RemoveInterfaceAddress(kTestDeviceIndex,
+ IsIPAddress(address)));
+ EXPECT_CALL(rtnl_handler_, RemoveInterface(_)).Times(0);
+ EXPECT_FALSE(CreateDevice(
+ kTestDeviceName, "address", kTestDeviceIndex, Technology::kTunnel));
+}
+
+TEST_F(DeviceInfoTest, CreateDeviceTunnelRejected) {
+ IPAddress address = CreateInterfaceAddress();
+
+ // A VPN device should be offered to VPNProvider.
+ StrictMock<MockVPNProvider> vpn_provider;
+ EXPECT_CALL(manager_, vpn_provider()).WillOnce(Return(&vpn_provider));
+ EXPECT_CALL(vpn_provider,
+ OnDeviceInfoAvailable(kTestDeviceName, kTestDeviceIndex))
+ .WillOnce(Return(false));
+ EXPECT_CALL(routing_table_, FlushRoutes(kTestDeviceIndex)).Times(1);
+ EXPECT_CALL(rtnl_handler_, RemoveInterfaceAddress(kTestDeviceIndex,
+ IsIPAddress(address)));
+ // Since the device was rejected by the VPNProvider, DeviceInfo will
+ // remove the interface.
+ EXPECT_CALL(rtnl_handler_, RemoveInterface(kTestDeviceIndex)).Times(1);
+ EXPECT_FALSE(CreateDevice(
+ kTestDeviceName, "address", kTestDeviceIndex, Technology::kTunnel));
+}
+
+TEST_F(DeviceInfoTest, CreateDevicePPP) {
+ IPAddress address = CreateInterfaceAddress();
+
+ // A VPN device should be offered to VPNProvider.
+ StrictMock<MockVPNProvider> vpn_provider;
+ EXPECT_CALL(manager_, vpn_provider()).WillOnce(Return(&vpn_provider));
+ EXPECT_CALL(vpn_provider,
+ OnDeviceInfoAvailable(kTestDeviceName, kTestDeviceIndex))
+ .WillOnce(Return(false));
+ EXPECT_CALL(routing_table_, FlushRoutes(kTestDeviceIndex)).Times(1);
+ EXPECT_CALL(rtnl_handler_, RemoveInterfaceAddress(kTestDeviceIndex,
+ IsIPAddress(address)));
+ // We do not remove PPP interfaces even if the provider does not accept it.
+ EXPECT_CALL(rtnl_handler_, RemoveInterface(_)).Times(0);
+ EXPECT_FALSE(CreateDevice(
+ kTestDeviceName, "address", kTestDeviceIndex, Technology::kPPP));
+}
+
+TEST_F(DeviceInfoTest, CreateDeviceLoopback) {
+ // A loopback device should be brought up, and nothing else done to it.
+ EXPECT_CALL(routing_table_, FlushRoutes(_)).Times(0);
+ EXPECT_CALL(rtnl_handler_, RemoveInterfaceAddress(_, _)).Times(0);
+ EXPECT_CALL(rtnl_handler_,
+ SetInterfaceFlags(kTestDeviceIndex, IFF_UP, IFF_UP)).Times(1);
+ EXPECT_FALSE(CreateDevice(
+ kTestDeviceName, "address", kTestDeviceIndex, Technology::kLoopback));
+}
+
+TEST_F(DeviceInfoTest, CreateDeviceUnknown) {
+ IPAddress address = CreateInterfaceAddress();
+
+ // An unknown (blacklisted, unhandled, etc) device won't be flushed or
+ // registered.
+ EXPECT_CALL(routing_table_, FlushRoutes(_)).Times(0);
+ EXPECT_CALL(rtnl_handler_, RemoveInterfaceAddress(_, _)).Times(0);
+ EXPECT_TRUE(CreateDevice(
+ kTestDeviceName, "address", kTestDeviceIndex, Technology::kUnknown));
}
TEST_F(DeviceInfoTest, DeviceBlackList) {
device_info_.AddDeviceToBlackList(kTestDeviceName);
- device_info_.Start();
scoped_ptr<RTNLMessage> message(BuildLinkMessage(RTNLMessage::kModeAdd));
SendMessageToDeviceInfo(*message);
DeviceRefPtr device = device_info_.GetDevice(kTestDeviceIndex);
ASSERT_TRUE(device.get());
EXPECT_TRUE(device->TechnologyIs(Technology::kBlacklisted));
-
- device_info_.Stop();
}
TEST_F(DeviceInfoTest, DeviceAddressList) {
- device_info_.Start();
scoped_ptr<RTNLMessage> message(BuildLinkMessage(RTNLMessage::kModeAdd));
SendMessageToDeviceInfo(*message);
@@ -267,18 +445,16 @@
// Delete device
message.reset(BuildLinkMessage(RTNLMessage::kModeDelete));
+ EXPECT_CALL(manager_, DeregisterDevice(_)).Times(1);
SendMessageToDeviceInfo(*message);
// Should be able to handle message for interface that doesn't exist
message.reset(BuildAddressMessage(RTNLMessage::kModeAdd, ip_address0, 0, 0));
SendMessageToDeviceInfo(*message);
EXPECT_FALSE(device_info_.GetDevice(kTestDeviceIndex).get());
-
- device_info_.Stop();
}
TEST_F(DeviceInfoTest, FlushAddressList) {
- device_info_.Start();
scoped_ptr<RTNLMessage> message(BuildLinkMessage(RTNLMessage::kModeAdd));
SendMessageToDeviceInfo(*message);
@@ -319,11 +495,9 @@
EXPECT_CALL(rtnl_handler_, RemoveInterfaceAddress(kTestDeviceIndex,
IsIPAddress(address2)));
device_info_.FlushAddresses(kTestDeviceIndex);
- device_info_.Stop();
}
TEST_F(DeviceInfoTest, HasSubdir) {
- device_info_.Start();
ScopedTempDir temp_dir;
EXPECT_TRUE(temp_dir.CreateUniqueTempDir());
EXPECT_TRUE(file_util::CreateDirectory(temp_dir.path().Append("child1")));
diff --git a/device_unittest.cc b/device_unittest.cc
index ea9d5fa..07c9827 100644
--- a/device_unittest.cc
+++ b/device_unittest.cc
@@ -30,7 +30,6 @@
#include "shill/mock_manager.h"
#include "shill/mock_metrics.h"
#include "shill/mock_portal_detector.h"
-#include "shill/mock_routing_table.h"
#include "shill/mock_rtnl_handler.h"
#include "shill/mock_service.h"
#include "shill/mock_store.h"
@@ -95,7 +94,6 @@
virtual void SetUp() {
device_->metrics_ = &metrics_;
- device_->routing_table_ = &routing_table_;
device_->rtnl_handler_ = &rtnl_handler_;
}
@@ -120,7 +118,6 @@
DeviceRefPtr device_;
MockDeviceInfo device_info_;
MockMetrics metrics_;
- MockRoutingTable routing_table_;
StrictMock<MockRTNLHandler> rtnl_handler_;
};
@@ -303,8 +300,12 @@
}
TEST_F(DeviceTest, Start) {
- EXPECT_CALL(routing_table_, FlushRoutes(kDeviceInterfaceIndex));
+ EXPECT_FALSE(device_->running_);
+ EXPECT_FALSE(device_->enabled_);
+ EXPECT_FALSE(device_->enabled_pending_);
device_->SetEnabled(true);
+ EXPECT_TRUE(device_->running_);
+ EXPECT_TRUE(device_->enabled_pending_);
}
TEST_F(DeviceTest, Stop) {
diff --git a/manager.h b/manager.h
index b808e5e..7d2c904 100644
--- a/manager.h
+++ b/manager.h
@@ -73,8 +73,8 @@
const std::string &profile,
Error *error);
- void RegisterDevice(const DeviceRefPtr &to_manage);
- void DeregisterDevice(const DeviceRefPtr &to_forget);
+ virtual void RegisterDevice(const DeviceRefPtr &to_manage);
+ virtual void DeregisterDevice(const DeviceRefPtr &to_forget);
virtual bool HasService(const ServiceRefPtr &service);
// Register a Service with the Manager. Manager may choose to
@@ -171,8 +171,8 @@
}
virtual DeviceInfo *device_info() { return &device_info_; }
- ModemInfo *modem_info() { return &modem_info_; }
- VPNProvider *vpn_provider() { return &vpn_provider_; }
+ virtual ModemInfo *modem_info() { return &modem_info_; }
+ virtual VPNProvider *vpn_provider() { return &vpn_provider_; }
PropertyStore *mutable_store() { return &store_; }
virtual const PropertyStore &store() const { return store_; }
GLib *glib() const { return glib_; }
diff --git a/mock_manager.h b/mock_manager.h
index 8a86c94..3acd29e 100644
--- a/mock_manager.h
+++ b/mock_manager.h
@@ -21,10 +21,14 @@
virtual ~MockManager();
MOCK_METHOD0(device_info, DeviceInfo *());
+ MOCK_METHOD0(modem_info, ModemInfo *());
+ MOCK_METHOD0(vpn_provider, VPNProvider *());
MOCK_METHOD0(mutable_store, PropertyStore *());
MOCK_CONST_METHOD0(store, const PropertyStore &());
MOCK_CONST_METHOD0(run_path, const FilePath &());
MOCK_METHOD0(Start, void());
+ MOCK_METHOD1(RegisterDevice, void(const DeviceRefPtr &to_manage));
+ MOCK_METHOD1(DeregisterDevice, void(const DeviceRefPtr &to_forget));
MOCK_METHOD1(HasService, bool(const ServiceRefPtr &to_manage));
MOCK_METHOD1(RegisterService, void(const ServiceRefPtr &to_manage));
MOCK_METHOD1(UpdateService, void(const ServiceRefPtr &to_update));
diff --git a/mock_modem_info.cc b/mock_modem_info.cc
new file mode 100644
index 0000000..ba90c25
--- /dev/null
+++ b/mock_modem_info.cc
@@ -0,0 +1,13 @@
+// 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.
+
+#include "shill/mock_modem_info.h"
+
+namespace shill {
+
+MockModemInfo::MockModemInfo() : ModemInfo(NULL, NULL, NULL, NULL, NULL) {}
+
+MockModemInfo::~MockModemInfo() {}
+
+} // namespace shill
diff --git a/mock_modem_info.h b/mock_modem_info.h
new file mode 100644
index 0000000..0d4f42a
--- /dev/null
+++ b/mock_modem_info.h
@@ -0,0 +1,29 @@
+// 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.
+
+#ifndef SHILL_MOCK_MODEM_INFO_
+#define SHILL_MOCK_MODEM_INFO_
+
+#include <base/basictypes.h>
+#include <gmock/gmock.h>
+
+#include "shill/modem_info.h"
+
+namespace shill {
+
+class MockModemInfo : public ModemInfo {
+ public:
+ MockModemInfo();
+ virtual ~MockModemInfo();
+
+ MOCK_METHOD0(Start, void());
+ MOCK_METHOD0(Stop, void());
+ MOCK_METHOD1(OnDeviceInfoAvailable, void(const std::string &link_name));
+
+ DISALLOW_COPY_AND_ASSIGN(MockModemInfo);
+};
+
+} // namespace shill
+
+#endif // SHILL_MOCK_MODEM_INFO_
diff --git a/mock_rtnl_handler.h b/mock_rtnl_handler.h
index a5b14c4..eff4fc1 100644
--- a/mock_rtnl_handler.h
+++ b/mock_rtnl_handler.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.
@@ -31,6 +31,7 @@
const IPAddress &peer));
MOCK_METHOD2(RemoveInterfaceAddress, bool(int interface_index,
const IPAddress &local));
+ MOCK_METHOD1(RemoveInterface, bool(int interface_index));
MOCK_METHOD1(RequestDump, void(int request_flags));
MOCK_METHOD1(GetInterfaceIndex, int(const std::string &interface_name));
MOCK_METHOD1(SendMessage, bool(RTNLMessage *message));
diff --git a/mock_vpn_provider.cc b/mock_vpn_provider.cc
new file mode 100644
index 0000000..8b19e3a
--- /dev/null
+++ b/mock_vpn_provider.cc
@@ -0,0 +1,13 @@
+// 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.
+
+#include "shill/mock_vpn_provider.h"
+
+namespace shill {
+
+MockVPNProvider::MockVPNProvider() : VPNProvider(NULL, NULL, NULL, NULL) {}
+
+MockVPNProvider::~MockVPNProvider() {}
+
+} // namespace shill
diff --git a/mock_vpn_provider.h b/mock_vpn_provider.h
new file mode 100644
index 0000000..c914e5b
--- /dev/null
+++ b/mock_vpn_provider.h
@@ -0,0 +1,30 @@
+// 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.
+
+#ifndef SHILL_MOCK_VPN_PROVIDER_
+#define SHILL_MOCK_VPN_PROVIDER_
+
+#include <base/basictypes.h>
+#include <gmock/gmock.h>
+
+#include "shill/vpn_provider.h"
+
+namespace shill {
+
+class MockVPNProvider : public VPNProvider {
+ public:
+ MockVPNProvider();
+ virtual ~MockVPNProvider();
+
+ MOCK_METHOD0(Start, void());
+ MOCK_METHOD0(Stop, void());
+ MOCK_METHOD2(OnDeviceInfoAvailable, bool(const std::string &link_name,
+ int interface_index));
+
+ DISALLOW_COPY_AND_ASSIGN(MockVPNProvider);
+};
+
+} // namespace shill
+
+#endif // SHILL_MOCK_VPN_PROVIDER_
diff --git a/modem_info.h b/modem_info.h
index 1d98dea..18faeb3 100644
--- a/modem_info.h
+++ b/modem_info.h
@@ -29,12 +29,12 @@
Metrics *metrics,
Manager *manager,
GLib *glib);
- ~ModemInfo();
+ virtual ~ModemInfo();
- void Start();
- void Stop();
+ virtual void Start();
+ virtual void Stop();
- void OnDeviceInfoAvailable(const std::string &link_name);
+ virtual void OnDeviceInfoAvailable(const std::string &link_name);
private:
friend class ModemInfoTest;
diff --git a/rtnl_handler.cc b/rtnl_handler.cc
index af8c8ac..ecbbbb1 100644
--- a/rtnl_handler.cc
+++ b/rtnl_handler.cc
@@ -189,15 +189,15 @@
if (seq != last_dump_sequence_)
return;
- if ((request_flags_ & kRequestLink) != 0) {
- type = RTNLMessage::kTypeLink;
- flag = kRequestLink;
- } else if ((request_flags_ & kRequestAddr) != 0) {
+ if ((request_flags_ & kRequestAddr) != 0) {
type = RTNLMessage::kTypeAddress;
flag = kRequestAddr;
} else if ((request_flags_ & kRequestRoute) != 0) {
type = RTNLMessage::kTypeRoute;
flag = kRequestRoute;
+ } else if ((request_flags_ & kRequestLink) != 0) {
+ type = RTNLMessage::kTypeLink;
+ flag = kRequestLink;
} else {
SLOG(RTNL, 2) << "Done with requests";
in_request_ = false;
@@ -317,7 +317,7 @@
const IPAddress &peer) {
return AddressRequest(interface_index,
RTNLMessage::kModeAdd,
- NLM_F_CREATE | NLM_F_EXCL,
+ NLM_F_CREATE | NLM_F_EXCL | NLM_F_ECHO,
local,
broadcast,
peer);
@@ -327,7 +327,7 @@
const IPAddress &local) {
return AddressRequest(interface_index,
RTNLMessage::kModeDelete,
- 0,
+ NLM_F_ECHO,
local,
IPAddress(local.family()),
IPAddress(local.family()));
diff --git a/vpn_provider.h b/vpn_provider.h
index 3c8e070..610a31b 100644
--- a/vpn_provider.h
+++ b/vpn_provider.h
@@ -28,16 +28,17 @@
EventDispatcher *dispatcher,
Metrics *metrics,
Manager *manager);
- ~VPNProvider();
+ virtual ~VPNProvider();
- void Start();
- void Stop();
+ virtual void Start();
+ virtual void Stop();
VPNServiceRefPtr GetService(const KeyValueStore &args, Error *error);
// Offers an unclaimed interface to VPN services. Returns true if this
// device has been accepted by a service.
- bool OnDeviceInfoAvailable(const std::string &link_name, int interface_index);
+ virtual bool OnDeviceInfoAvailable(const std::string &link_name,
+ int interface_index);
// Clean up a VPN services that has been unloaded and will be deregistered.
// This removes the VPN provider's reference to this service in its
diff --git a/vpn_service_unittest.cc b/vpn_service_unittest.cc
index 834d8db..ca97843 100644
--- a/vpn_service_unittest.cc
+++ b/vpn_service_unittest.cc
@@ -14,6 +14,7 @@
#include "shill/mock_metrics.h"
#include "shill/mock_store.h"
#include "shill/mock_vpn_driver.h"
+#include "shill/mock_vpn_provider.h"
using testing::_;
using testing::NiceMock;
@@ -143,11 +144,13 @@
service_->set_save_credentials(true);
EXPECT_CALL(*driver_, Disconnect());
EXPECT_CALL(*driver_, UnloadCredentials());
- manager_.vpn_provider()->services_.push_back(service_);
+ MockVPNProvider provider;
+ EXPECT_CALL(manager_, vpn_provider()).WillRepeatedly(Return(&provider));
+ provider.services_.push_back(service_);
service_->Unload();
EXPECT_FALSE(service_->auto_connect());
EXPECT_FALSE(service_->save_credentials());
- EXPECT_TRUE(manager_.vpn_provider()->services_.empty());
+ EXPECT_TRUE(provider.services_.empty());
}
TEST_F(VPNServiceTest, InitPropertyStore) {