shill: Device: Add LinkMonitor
Start link monitoring for technologies for which it is enabled.
Add facilities in the manager and default profile to determine
and persist a list of technologies for which link monitoring
is enabled. Provide a means for the Device to report the current
rolling average LinkMonitor response time.
BUG=chromium-os:32600
TEST=Unit tests
Change-Id: I39dcc8ce2332d7be3c95d9953b4ae7d7172d7df1
Reviewed-on: https://gerrit.chromium.org/gerrit/29731
Commit-Ready: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
diff --git a/default_profile.cc b/default_profile.cc
index 0b7bdbd..0f89638 100644
--- a/default_profile.cc
+++ b/default_profile.cc
@@ -13,6 +13,7 @@
#include "shill/adaptor_interfaces.h"
#include "shill/control_interface.h"
+#include "shill/link_monitor.h"
#include "shill/manager.h"
#include "shill/portal_detector.h"
#include "shill/resolver.h"
@@ -33,6 +34,9 @@
// static
const char DefaultProfile::kStorageHostName[] = "HostName";
// static
+const char DefaultProfile::kStorageLinkMonitorTechnologies[] =
+ "LinkMonitorTechnologies";
+// static
const char DefaultProfile::kStorageName[] = "Name";
// static
const char DefaultProfile::kStorageOfflineMode[] = "OfflineMode";
@@ -61,6 +65,8 @@
&manager_props.check_portal_list);
store->RegisterConstString(flimflam::kCountryProperty,
&manager_props.country);
+ store->RegisterConstString(shill::kLinkMonitorTechnologiesProperty,
+ &manager_props.link_monitor_technologies);
store->RegisterConstBool(flimflam::kOfflineModeProperty,
&manager_props.offline_mode);
store->RegisterConstString(flimflam::kPortalURLProperty,
@@ -84,6 +90,12 @@
&manager_props->check_portal_list)) {
manager_props->check_portal_list = PortalDetector::kDefaultCheckPortalList;
}
+ if (!storage()->GetString(kStorageId,
+ kStorageLinkMonitorTechnologies,
+ &manager_props->link_monitor_technologies)) {
+ manager_props->link_monitor_technologies =
+ LinkMonitor::kDefaultLinkMonitorTechnologies;
+ }
if (!storage()->GetString(kStorageId, kStoragePortalURL,
&manager_props->portal_url)) {
manager_props->portal_url = PortalDetector::kDefaultURL;
@@ -129,6 +141,9 @@
kStorageCheckPortalList,
props_.check_portal_list);
storage()->SetString(kStorageId,
+ kStorageLinkMonitorTechnologies,
+ props_.link_monitor_technologies);
+ storage()->SetString(kStorageId,
kStoragePortalURL,
props_.portal_url);
storage()->SetString(kStorageId,
diff --git a/default_profile.h b/default_profile.h
index 894c06c..4ed7fc2 100644
--- a/default_profile.h
+++ b/default_profile.h
@@ -67,6 +67,7 @@
static const char kStorageArpGateway[];
static const char kStorageCheckPortalList[];
static const char kStorageHostName[];
+ static const char kStorageLinkMonitorTechnologies[];
static const char kStorageName[];
static const char kStorageOfflineMode[];
static const char kStoragePortalCheckInterval[];
diff --git a/default_profile_unittest.cc b/default_profile_unittest.cc
index 4271401..ed47db3 100644
--- a/default_profile_unittest.cc
+++ b/default_profile_unittest.cc
@@ -15,6 +15,7 @@
#include "shill/key_file_store.h"
#include "shill/glib.h"
+#include "shill/link_monitor.h"
#include "shill/manager.h"
#include "shill/mock_control.h"
#include "shill/mock_device.h"
@@ -123,6 +124,11 @@
DefaultProfile::kStorageCheckPortalList,
""))
.WillOnce(Return(true));
+ EXPECT_CALL(*storage.get(),
+ SetString(DefaultProfile::kStorageId,
+ DefaultProfile::kStorageLinkMonitorTechnologies,
+ ""))
+ .WillOnce(Return(true));
EXPECT_CALL(*storage.get(), SetString(DefaultProfile::kStorageId,
DefaultProfile::kStoragePortalURL,
""))
@@ -166,6 +172,11 @@
DefaultProfile::kStorageCheckPortalList,
&manager_props.check_portal_list))
.WillOnce(Return(false));
+ EXPECT_CALL(*storage.get(),
+ GetString(DefaultProfile::kStorageId,
+ DefaultProfile::kStorageLinkMonitorTechnologies,
+ _))
+ .WillOnce(Return(false));
EXPECT_CALL(*storage.get(), GetString(DefaultProfile::kStorageId,
DefaultProfile::kStoragePortalURL,
&manager_props.portal_url))
@@ -180,6 +191,7 @@
DefaultProfile::kStorageShortDNSTimeoutTechnologies,
_))
.WillOnce(Return(false));
+
profile_->set_storage(storage.release());
ASSERT_TRUE(profile_->LoadManagerProperties(&manager_props));
@@ -188,6 +200,8 @@
EXPECT_FALSE(manager_props.offline_mode);
EXPECT_EQ(PortalDetector::kDefaultCheckPortalList,
manager_props.check_portal_list);
+ EXPECT_EQ(LinkMonitor::kDefaultLinkMonitorTechnologies,
+ manager_props.link_monitor_technologies);
EXPECT_EQ(PortalDetector::kDefaultURL, manager_props.portal_url);
EXPECT_EQ(PortalDetector::kDefaultCheckIntervalSeconds,
manager_props.portal_check_interval_seconds);
@@ -215,6 +229,13 @@
DefaultProfile::kStorageCheckPortalList,
_))
.WillOnce(DoAll(SetArgumentPointee<2>(portal_list), Return(true)));
+ const string link_monitor_technologies("ethernet,wimax");
+ EXPECT_CALL(*storage.get(),
+ GetString(DefaultProfile::kStorageId,
+ DefaultProfile::kStorageLinkMonitorTechnologies,
+ _))
+ .WillOnce(DoAll(SetArgumentPointee<2>(link_monitor_technologies),
+ Return(true)));
const string portal_url("http://www.chromium.org");
EXPECT_CALL(*storage.get(), GetString(DefaultProfile::kStorageId,
DefaultProfile::kStoragePortalURL,
@@ -243,6 +264,8 @@
EXPECT_EQ(host_name, manager_props.host_name);
EXPECT_TRUE(manager_props.offline_mode);
EXPECT_EQ(portal_list, manager_props.check_portal_list);
+ EXPECT_EQ(link_monitor_technologies,
+ manager_props.link_monitor_technologies);
EXPECT_EQ(portal_url, manager_props.portal_url);
EXPECT_EQ(portal_check_interval_int,
manager_props.portal_check_interval_seconds);
diff --git a/device.cc b/device.cc
index 9bfd3c8..6086045 100644
--- a/device.cc
+++ b/device.cc
@@ -26,6 +26,7 @@
#include "shill/error.h"
#include "shill/event_dispatcher.h"
#include "shill/http_proxy.h"
+#include "shill/link_monitor.h"
#include "shill/logging.h"
#include "shill/manager.h"
#include "shill/metrics.h"
@@ -135,6 +136,8 @@
HelpRegisterDerivedString(flimflam::kTypeProperty,
&Device::GetTechnologyString,
NULL);
+ HelpRegisterConstDerivedUint64(shill::kLinkMonitorResponseTimeProperty,
+ &Device::GetLinkMonitorResponseTime);
// TODO(cmasone): Chrome doesn't use this...does anyone?
// store_.RegisterConstBool(flimflam::kReconnectProperty, &reconnect_);
@@ -442,6 +445,7 @@
// to the Connected state because this call may immediately transition
// to the Online state.
StartPortalDetection();
+ StartLinkMonitor();
} else {
// TODO(pstew): This logic gets yet more complex when multiple
// IPConfig types are run in parallel (e.g. DHCP and DHCP6)
@@ -493,6 +497,7 @@
void Device::DestroyConnection() {
SLOG(Device, 2) << __func__;
StopPortalDetection();
+ StopLinkMonitor();
if (selected_service_.get()) {
selected_service_->SetConnection(NULL);
}
@@ -666,6 +671,40 @@
portal_detector_.reset();
}
+void Device::set_link_monitor(LinkMonitor *link_monitor) {
+ link_monitor_.reset(link_monitor);
+}
+
+bool Device::StartLinkMonitor() {
+ if (!manager_->IsTechnologyLinkMonitorEnabled(technology())) {
+ SLOG(Device, 2) << "Device " << FriendlyName()
+ << ": Link Monitoring is disabled.";
+ return false;
+ }
+
+ if (!link_monitor()) {
+ set_link_monitor(
+ new LinkMonitor(
+ connection_, dispatcher_, metrics(), manager_->device_info(),
+ Bind(&Device::OnLinkMonitorFailure, weak_ptr_factory_.GetWeakPtr())));
+ }
+
+ SLOG(Device, 2) << "Device " << FriendlyName()
+ << ": Link Monitor starting.";
+ return link_monitor_->Start();
+}
+
+void Device::StopLinkMonitor() {
+ SLOG(Device, 2) << "Device " << FriendlyName()
+ << ": Link Monitor stopping.";
+ link_monitor_.reset();
+}
+
+void Device::OnLinkMonitorFailure() {
+ LOG(ERROR) << "Device " << FriendlyName()
+ << ": Link Monitor indicates failure.";
+}
+
void Device::SetServiceConnectedState(Service::ConnectState state) {
DCHECK(selected_service_.get());
@@ -761,6 +800,17 @@
return adaptor_->GetRpcConnectionIdentifier();
}
+uint64 Device::GetLinkMonitorResponseTime(Error *error) {
+ if (!link_monitor_.get()) {
+ // It is not strictly an error that the link monitor does not
+ // exist, but returning an error here allows the GetProperties
+ // call in our Adaptor to omit this parameter.
+ error->Populate(Error::kNotFound, "Device is not running LinkMonitor");
+ return 0;
+ }
+ return link_monitor_->GetResponseTimeMilliseconds();
+}
+
uint64 Device::GetReceiveByteCount(Error */*error*/) {
uint64 rx_byte_count = 0, tx_byte_count = 0;
manager_->device_info()->GetByteCounts(
diff --git a/device.h b/device.h
index 1c57cf4..48ed2b2 100644
--- a/device.h
+++ b/device.h
@@ -33,6 +33,7 @@
class Endpoint;
class Error;
class EventDispatcher;
+class LinkMonitor;
class Manager;
class Metrics;
class RTNLHandler;
@@ -296,6 +297,15 @@
// Stop portal detection if it is running.
void StopPortalDetection();
+ // Initiate link monitoring, if enabled for this device type.
+ bool StartLinkMonitor();
+
+ // Stop link monitoring if it is running.
+ void StopLinkMonitor();
+
+ // Respond to a LinkMonitor failure in a Device-specific manner.
+ virtual void OnLinkMonitorFailure();
+
// Set the state of the selected service, with checks to make sure
// the service is already in a connected state before doing so.
void SetServiceConnectedState(Service::ConnectState state);
@@ -325,6 +335,8 @@
Metrics *metrics() const { return metrics_; }
Manager *manager() const { return manager_; }
bool running() const { return running_; }
+ const LinkMonitor *link_monitor() const { return link_monitor_.get(); }
+ void set_link_monitor(LinkMonitor *link_monitor);
private:
friend class DeviceAdaptorInterface;
@@ -370,6 +382,9 @@
std::vector<std::string> AvailableIPConfigs(Error *error);
std::string GetRpcConnectionIdentifier();
+ // Get the LinkMonitor's average response time.
+ uint64 GetLinkMonitorResponseTime(Error *error);
+
// Get receive and transmit byte counters.
uint64 GetReceiveByteCount(Error *error);
uint64 GetTransmitByteCount(Error *error);
@@ -420,6 +435,7 @@
base::WeakPtrFactory<Device> weak_ptr_factory_;
scoped_ptr<DeviceAdaptorInterface> adaptor_;
scoped_ptr<PortalDetector> portal_detector_;
+ scoped_ptr<LinkMonitor> link_monitor_;
base::Callback<void(const PortalDetector::Result &)>
portal_detector_callback_;
Technology::Identifier technology_;
diff --git a/device_unittest.cc b/device_unittest.cc
index 86ac95d..42ee525 100644
--- a/device_unittest.cc
+++ b/device_unittest.cc
@@ -29,6 +29,7 @@
#include "shill/mock_dhcp_provider.h"
#include "shill/mock_glib.h"
#include "shill/mock_ipconfig.h"
+#include "shill/mock_link_monitor.h"
#include "shill/mock_manager.h"
#include "shill/mock_metrics.h"
#include "shill/mock_portal_detector.h"
@@ -120,6 +121,26 @@
device_->connection_ = connection;
}
+ void SetLinkMonitor(LinkMonitor *link_monitor) {
+ device_->set_link_monitor(link_monitor); // Passes ownership.
+ }
+
+ bool StartLinkMonitor() {
+ return device_->StartLinkMonitor();
+ }
+
+ void StopLinkMonitor() {
+ device_->StopLinkMonitor();
+ }
+
+ uint64 GetLinkMonitorResponseTime(Error *error) {
+ return device_->GetLinkMonitorResponseTime(error);
+ }
+
+ void SetManager(Manager *manager) {
+ device_->manager_ = manager;
+ }
+
MockControl control_interface_;
DeviceRefPtr device_;
MockDeviceInfo device_info_;
@@ -338,7 +359,7 @@
metrics(),
glib());
EXPECT_CALL(manager, UpdateDevice(_));
- device_->manager_ = &manager;
+ SetManager(&manager);
Error error;
device_->SetEnabledPersistent(true, &error, ResultCallback());
EXPECT_TRUE(device_->enabled_persistent_);
@@ -396,6 +417,51 @@
device_->OnAfterResume();
}
+TEST_F(DeviceTest, LinkMonitor) {
+ scoped_refptr<MockConnection> connection(
+ new StrictMock<MockConnection>(&device_info_));
+ MockManager manager(control_interface(),
+ dispatcher(),
+ metrics(),
+ glib());
+ scoped_refptr<MockService> service(
+ new StrictMock<MockService>(control_interface(),
+ dispatcher(),
+ metrics(),
+ &manager));
+ SelectService(service);
+ SetConnection(connection.get());
+ MockLinkMonitor *link_monitor = new StrictMock<MockLinkMonitor>();
+ SetLinkMonitor(link_monitor); // Passes ownership.
+ SetManager(&manager);
+ EXPECT_CALL(*link_monitor, Start()).Times(0);
+ EXPECT_CALL(manager, IsTechnologyLinkMonitorEnabled(Technology::kUnknown))
+ .WillOnce(Return(false))
+ .WillRepeatedly(Return(true));
+ EXPECT_FALSE(StartLinkMonitor());
+
+ EXPECT_CALL(*link_monitor, Start())
+ .WillOnce(Return(false))
+ .WillOnce(Return(true));
+ EXPECT_FALSE(StartLinkMonitor());
+ EXPECT_TRUE(StartLinkMonitor());
+
+ unsigned int kResponseTime = 123;
+ EXPECT_CALL(*link_monitor, GetResponseTimeMilliseconds())
+ .WillOnce(Return(kResponseTime));
+ {
+ Error error;
+ EXPECT_EQ(kResponseTime, GetLinkMonitorResponseTime(&error));
+ EXPECT_TRUE(error.IsSuccess());
+ }
+ StopLinkMonitor();
+ {
+ Error error;
+ EXPECT_EQ(0, GetLinkMonitorResponseTime(&error));
+ EXPECT_FALSE(error.IsSuccess());
+ }
+}
+
class DevicePortalDetectionTest : public DeviceTest {
public:
DevicePortalDetectionTest()
@@ -415,7 +481,7 @@
SelectService(service_);
SetConnection(connection_.get());
device_->portal_detector_.reset(portal_detector_); // Passes ownership.
- device_->manager_ = &manager_;
+ SetManager(&manager_);
}
protected:
diff --git a/doc/device-api.txt b/doc/device-api.txt
index 4843899..bd83c5a 100644
--- a/doc/device-api.txt
+++ b/doc/device-api.txt
@@ -414,6 +414,13 @@
the currently used IPConfig. In the future, use
the "IPConfig" property of the Service.
+ int32 LinkMonitorResponseTime [readonly]
+
+ Latest running average of the link monitor response
+ time for this device in milliseconds, if the link
+ monitor is active. This property is only visible
+ if a link monitor is active on this device.
+
string Name [readonly]
The device name (for example "Wireless" etc.)
diff --git a/doc/manager-api.txt b/doc/manager-api.txt
index 1e003bc..7691c62 100644
--- a/doc/manager-api.txt
+++ b/doc/manager-api.txt
@@ -329,6 +329,13 @@
The default for this name is empty, which
means that the system will not report a hostname.
+ string LinkMonitorTechnologies [readwrite]
+
+ The list of technologies for which thie Link
+ Monitor will be enabled. The Link monitor
+ periodically checks for connectivity to the
+ default gateway using ARP requests.
+
boolean OfflineMode [readwrite]
The offline mode indicates the global setting for
diff --git a/link_monitor.cc b/link_monitor.cc
index 26cb0dd..6bf1208 100644
--- a/link_monitor.cc
+++ b/link_monitor.cc
@@ -28,6 +28,7 @@
namespace shill {
const unsigned int LinkMonitor::kTestPeriodMilliseconds = 5000;
+const char LinkMonitor::kDefaultLinkMonitorTechnologies[] = "wifi";
const unsigned int LinkMonitor::kFailureThreshold = 5;
const unsigned int LinkMonitor::kMaxResponseSampleFilterDepth = 5;
diff --git a/link_monitor.h b/link_monitor.h
index 3a420cf..6182670 100644
--- a/link_monitor.h
+++ b/link_monitor.h
@@ -40,6 +40,9 @@
// The number of milliseconds between ARP requests. Needed by Metrics.
static const unsigned int kTestPeriodMilliseconds;
+ // The default list of technologies for which link monitoring is enabled.
+ static const char kDefaultLinkMonitorTechnologies[];
+
LinkMonitor(const ConnectionRefPtr &connection,
EventDispatcher *dispatcher,
Metrics *metrics,
diff --git a/manager.cc b/manager.cc
index f37ef57..2095e7f 100644
--- a/manager.cc
+++ b/manager.cc
@@ -111,6 +111,8 @@
HelpRegisterDerivedStrings(flimflam::kEnabledTechnologiesProperty,
&Manager::EnabledTechnologies,
NULL);
+ store_.RegisterString(shill::kLinkMonitorTechnologiesProperty,
+ &props_.link_monitor_technologies);
store_.RegisterBool(flimflam::kOfflineModeProperty, &props_.offline_mode);
store_.RegisterString(flimflam::kPortalURLProperty, &props_.portal_url);
store_.RegisterInt32(kPortalCheckIntervalProperty,
@@ -521,14 +523,19 @@
return default_service ? default_service->GetRpcIdentifier() : "/";
}
-bool Manager::IsPortalDetectionEnabled(Technology::Identifier tech) {
+bool Manager::IsTechnologyInList(const string &technology_list,
+ Technology::Identifier tech) const {
Error error;
- vector<Technology::Identifier> portal_technologies;
- return Technology::GetTechnologyVectorFromString(GetCheckPortalList(NULL),
- &portal_technologies,
+ vector<Technology::Identifier> technologies;
+ return Technology::GetTechnologyVectorFromString(technology_list,
+ &technologies,
&error) &&
- std::find(portal_technologies.begin(), portal_technologies.end(),
- tech) != portal_technologies.end();
+ std::find(technologies.begin(), technologies.end(), tech) !=
+ technologies.end();
+}
+
+bool Manager::IsPortalDetectionEnabled(Technology::Identifier tech) {
+ return IsTechnologyInList(GetCheckPortalList(NULL), tech);
}
void Manager::SetStartupPortalList(const string &portal_list) {
@@ -541,15 +548,13 @@
}
bool Manager::IsTechnologyShortDNSTimeoutEnabled(
- Technology::Identifier tech) const {
- Error error;
- vector<Technology::Identifier> short_dns_technologies;
- return Technology::GetTechnologyVectorFromString(
- props_.short_dns_timeout_technologies,
- &short_dns_technologies,
- &error) &&
- std::find(short_dns_technologies.begin(), short_dns_technologies.end(),
- tech) != short_dns_technologies.end();
+ Technology::Identifier technology) const {
+ return IsTechnologyInList(props_.short_dns_timeout_technologies, technology);
+}
+
+bool Manager::IsTechnologyLinkMonitorEnabled(
+ Technology::Identifier technology) const {
+ return IsTechnologyInList(props_.link_monitor_technologies, technology);
}
const ProfileRefPtr &Manager::ActiveProfile() const {
diff --git a/manager.h b/manager.h
index dc90c7a..5077637 100644
--- a/manager.h
+++ b/manager.h
@@ -60,6 +60,9 @@
// Comma separated list of technologies on which to use a short DNS
// timeout to improve performance.
std::string short_dns_timeout_technologies;
+ // Comma-separated list of technologies for which link-monitoring is
+ // enabled.
+ std::string link_monitor_technologies;
};
Manager(ControlInterface *control_interface,
@@ -181,6 +184,10 @@
// Return whether a technology is enabled for using short DNS timeouts.
bool IsTechnologyShortDNSTimeoutEnabled(Technology::Identifier tech) const;
+ // Return whether a technology is enabled for link monitoring.
+ virtual bool IsTechnologyLinkMonitorEnabled(
+ Technology::Identifier technology) const;
+
std::string CalculateState(Error *error);
virtual int GetPortalCheckInterval() const {
@@ -254,6 +261,7 @@
FRIEND_TEST(ManagerTest, EnableTechnology);
FRIEND_TEST(ManagerTest, EnumerateProfiles);
FRIEND_TEST(ManagerTest, HandleProfileEntryDeletionWithUnload);
+ FRIEND_TEST(ManagerTest, LinkMonitorEnabled);
FRIEND_TEST(ManagerTest, NotifyDefaultServiceChanged);
FRIEND_TEST(ManagerTest, PopProfileWithUnload);
FRIEND_TEST(ManagerTest, SortServices);
@@ -281,6 +289,8 @@
ServiceRefPtr GetServiceInner(const KeyValueStore &args, Error *error);
void SetCheckPortalList(const std::string &portal_list, Error *error);
void EmitDefaultService();
+ bool IsTechnologyInList(const std::string &technology_list,
+ Technology::Identifier tech) const;
void EmitDeviceProperties();
// Unload a service while iterating through |services_|. Returns true if
diff --git a/manager_unittest.cc b/manager_unittest.cc
index 92f2da0..1a36418 100644
--- a/manager_unittest.cc
+++ b/manager_unittest.cc
@@ -2389,6 +2389,14 @@
EXPECT_TRUE(manager()->IsPortalDetectionEnabled(Technology::kPPP));
}
+TEST_F(ManagerTest, LinkMonitorEnabled) {
+ const string kEnabledTechnologies("wifi,vpn");
+ manager()->props_.link_monitor_technologies = kEnabledTechnologies;
+ EXPECT_TRUE(manager()->IsTechnologyLinkMonitorEnabled(Technology::kWifi));
+ EXPECT_FALSE(
+ manager()->IsTechnologyLinkMonitorEnabled(Technology::kCellular));
+}
+
TEST_F(ManagerTest, EnableTechnology) {
Error error(Error::kOperationInitiated);
ResultCallback callback;
diff --git a/mock_manager.h b/mock_manager.h
index 6070887..21eddba 100644
--- a/mock_manager.h
+++ b/mock_manager.h
@@ -47,6 +47,8 @@
MOCK_METHOD1(IsPortalDetectionEnabled, bool(Technology::Identifier tech));
MOCK_CONST_METHOD1(IsServiceEphemeral,
bool(const ServiceConstRefPtr &service));
+ MOCK_CONST_METHOD1(IsTechnologyLinkMonitorEnabled,
+ bool(Technology::Identifier tech));
MOCK_CONST_METHOD0(GetPortalCheckURL, const std::string &());
MOCK_CONST_METHOD0(GetPortalCheckInterval, int());