shill: Add ArpGateway and network-based leases
Add two arguments to the DHCP client:
- Turn on ArpGateway (ARP for default gateway in order to test validity
of lease) by default, and use the same manager flag as flimflam did.
- Use per-network lease files for Ethernet and WiFi. This means that
multiple leases can be held in parallel for different Ethernet devices
and WiFi SSIDs.
Bonus changes: Fix DHCP lease filename template which was broken in flimflam
and ported with full fidelity to shill. Make removal of old lease files
conditional on whether the lease file was non-default.
BUG=chromium-os:25717,chromium-os:16885
TEST=New unit tests + manual: Ensure dhcpcd runs with correct arguments ("-R"
added when ArpGateway is enabled on the manager, no "-R" otherwise), and that
the "set_arpgw" command in crosh works correctly. Monitor dhcpcd command line
for new lease suffix parameter, and ensure that leases are being written out
to those files, and that the files are not being removed on program exit.
CQ-DEPEND=Iac282c1686695239a790bbcc0d110c6a69bf45e0
Change-Id: I68bb3cbd18c95f01003eaf049fa60aad446f8116
Reviewed-on: https://gerrit.chromium.org/gerrit/22065
Commit-Ready: Paul Stewart <pstew@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/cellular_unittest.cc b/cellular_unittest.cc
index 25d444a..0c69539 100644
--- a/cellular_unittest.cc
+++ b/cellular_unittest.cc
@@ -116,11 +116,7 @@
: manager_(&control_interface_, &dispatcher_, &metrics_, &glib_),
device_info_(&control_interface_, &dispatcher_, &metrics_, &manager_),
dhcp_config_(new MockDHCPConfig(&control_interface_,
- &dispatcher_,
- &dhcp_provider_,
- kTestDeviceName,
- "",
- &glib_)),
+ kTestDeviceName)),
device_(new Cellular(&control_interface_,
&dispatcher_,
&metrics_,
@@ -491,7 +487,7 @@
device_->set_modem_state(Cellular::kModemStateConnected);
GetCapabilityClassic()->meid_ = kMEID;
ExpectCdmaStartModem(flimflam::kNetworkTechnologyEvdo);
- EXPECT_CALL(dhcp_provider_, CreateConfig(kTestDeviceName, _))
+ EXPECT_CALL(dhcp_provider_, CreateConfig(kTestDeviceName, _, _, _))
.WillOnce(Return(dhcp_config_));
EXPECT_CALL(*dhcp_config_, RequestIP()).WillOnce(Return(true));
EXPECT_CALL(manager_, UpdateService(_)).Times(2);
diff --git a/default_profile.cc b/default_profile.cc
index 640f27c..1f7ce71 100644
--- a/default_profile.cc
+++ b/default_profile.cc
@@ -26,6 +26,8 @@
// static
const char DefaultProfile::kStorageId[] = "global";
// static
+const char DefaultProfile::kStorageArpGateway[] = "ArpGateway";
+// static
const char DefaultProfile::kStorageCheckPortalList[] = "CheckPortalList";
// static
const char DefaultProfile::kStorageHostName[] = "HostName";
@@ -49,6 +51,8 @@
profile_id_(profile_id),
props_(manager_props) {
PropertyStore *store = this->mutable_store();
+ store->RegisterConstBool(flimflam::kArpGatewayProperty,
+ &manager_props.arp_gateway);
store->RegisterConstString(flimflam::kCheckPortalListProperty,
&manager_props.check_portal_list);
store->RegisterConstString(flimflam::kCountryProperty,
@@ -64,6 +68,8 @@
DefaultProfile::~DefaultProfile() {}
bool DefaultProfile::LoadManagerProperties(Manager::Properties *manager_props) {
+ storage()->GetBool(kStorageId, kStorageArpGateway,
+ &manager_props->arp_gateway);
storage()->GetString(kStorageId, kStorageHostName, &manager_props->host_name);
storage()->GetBool(kStorageId, kStorageOfflineMode,
&manager_props->offline_mode);
@@ -103,6 +109,7 @@
}
bool DefaultProfile::Save() {
+ storage()->SetBool(kStorageId, kStorageArpGateway, props_.arp_gateway);
storage()->SetString(kStorageId, kStorageHostName, props_.host_name);
storage()->SetString(kStorageId, kStorageName, GetFriendlyName());
storage()->SetBool(kStorageId, kStorageOfflineMode, props_.offline_mode);
diff --git a/default_profile.h b/default_profile.h
index d28c312..d87450a 100644
--- a/default_profile.h
+++ b/default_profile.h
@@ -61,6 +61,7 @@
FRIEND_TEST(DefaultProfileTest, Save);
static const char kStorageId[];
+ static const char kStorageArpGateway[];
static const char kStorageCheckPortalList[];
static const char kStorageHostName[];
static const char kStorageName[];
diff --git a/default_profile_unittest.cc b/default_profile_unittest.cc
index ac4edb0..d3319f1 100644
--- a/default_profile_unittest.cc
+++ b/default_profile_unittest.cc
@@ -102,6 +102,10 @@
TEST_F(DefaultProfileTest, Save) {
scoped_ptr<MockStore> storage(new MockStore);
+ EXPECT_CALL(*storage.get(), SetBool(DefaultProfile::kStorageId,
+ DefaultProfile::kStorageArpGateway,
+ true))
+ .WillOnce(Return(true));
EXPECT_CALL(*storage.get(), SetString(DefaultProfile::kStorageId,
DefaultProfile::kStorageName,
DefaultProfile::kDefaultId))
@@ -139,21 +143,26 @@
TEST_F(DefaultProfileTest, LoadManagerDefaultProperties) {
scoped_ptr<MockStore> storage(new MockStore);
+ Manager::Properties manager_props;
+ EXPECT_CALL(*storage.get(), GetBool(DefaultProfile::kStorageId,
+ DefaultProfile::kStorageArpGateway,
+ &manager_props.arp_gateway))
+ .WillOnce(Return(false));
EXPECT_CALL(*storage.get(), GetString(DefaultProfile::kStorageId,
DefaultProfile::kStorageHostName,
- _))
+ &manager_props.host_name))
.WillOnce(Return(false));
EXPECT_CALL(*storage.get(), GetBool(DefaultProfile::kStorageId,
DefaultProfile::kStorageOfflineMode,
- _))
+ &manager_props.offline_mode))
.WillOnce(Return(false));
EXPECT_CALL(*storage.get(), GetString(DefaultProfile::kStorageId,
DefaultProfile::kStorageCheckPortalList,
- _))
+ &manager_props.check_portal_list))
.WillOnce(Return(false));
EXPECT_CALL(*storage.get(), GetString(DefaultProfile::kStorageId,
DefaultProfile::kStoragePortalURL,
- _))
+ &manager_props.portal_url))
.WillOnce(Return(false));
EXPECT_CALL(*storage.get(),
GetString(DefaultProfile::kStorageId,
@@ -162,8 +171,8 @@
.WillOnce(Return(false));
profile_->set_storage(storage.release());
- Manager::Properties manager_props;
ASSERT_TRUE(profile_->LoadManagerProperties(&manager_props));
+ EXPECT_TRUE(manager_props.arp_gateway);
EXPECT_EQ("", manager_props.host_name);
EXPECT_FALSE(manager_props.offline_mode);
EXPECT_EQ(PortalDetector::kDefaultCheckPortalList,
@@ -176,6 +185,10 @@
TEST_F(DefaultProfileTest, LoadManagerProperties) {
scoped_ptr<MockStore> storage(new MockStore);
const string host_name("hostname");
+ EXPECT_CALL(*storage.get(), GetBool(DefaultProfile::kStorageId,
+ DefaultProfile::kStorageArpGateway,
+ _))
+ .WillOnce(DoAll(SetArgumentPointee<2>(false), Return(true)));
EXPECT_CALL(*storage.get(), GetString(DefaultProfile::kStorageId,
DefaultProfile::kStorageHostName,
_))
@@ -206,6 +219,7 @@
Manager::Properties manager_props;
ASSERT_TRUE(profile_->LoadManagerProperties(&manager_props));
+ EXPECT_FALSE(manager_props.arp_gateway);
EXPECT_EQ(host_name, manager_props.host_name);
EXPECT_TRUE(manager_props.offline_mode);
EXPECT_EQ(portal_list, manager_props.check_portal_list);
diff --git a/device.cc b/device.cc
index 5ade55d..540a538 100644
--- a/device.cc
+++ b/device.cc
@@ -300,9 +300,16 @@
}
bool Device::AcquireIPConfig() {
+ return AcquireIPConfigWithLeaseName(string());
+}
+
+bool Device::AcquireIPConfigWithLeaseName(const string &lease_name) {
DestroyIPConfig();
EnableIPv6();
- ipconfig_ = dhcp_provider_->CreateConfig(link_name_, manager_->GetHostName());
+ ipconfig_ = dhcp_provider_->CreateConfig(link_name_,
+ manager_->GetHostName(),
+ lease_name,
+ manager_->GetArpGateway());
ipconfig_->RegisterUpdateCallback(Bind(&Device::OnIPConfigUpdated,
weak_ptr_factory_.GetWeakPtr()));
dispatcher_->PostTask(Bind(&Device::ConfigureStaticIPTask,
diff --git a/device.h b/device.h
index e0e539a..85bf946 100644
--- a/device.h
+++ b/device.h
@@ -219,11 +219,19 @@
void DestroyIPConfig();
// Creates a new DHCP IP configuration instance, stores it in |ipconfig_| and
- // requests a new IP configuration. Registers a callback to
+ // requests a new IP configuration. Saves the DHCP lease to the generic
+ // lease filename based on the interface name. Registers a callback to
// IPConfigUpdatedCallback on IP configuration changes. Returns true if the IP
// request was successfully sent.
bool AcquireIPConfig();
+ // Creates a new DHCP IP configuration instance, stores it in |ipconfig_| and
+ // requests a new IP configuration. Saves the DHCP lease to a filename
+ // based on the passed-in |lease_name|. Registers a callback to
+ // IPConfigUpdatedCallback on IP configuration changes. Returns true if the IP
+ // request was successfully sent.
+ bool AcquireIPConfigWithLeaseName(const std::string &lease_name);
+
// Callback invoked on every IP configuration update.
void OnIPConfigUpdated(const IPConfigRefPtr &ipconfig, bool success);
diff --git a/dhcp_config.cc b/dhcp_config.cc
index 8708f12..18bfcc0 100644
--- a/dhcp_config.cc
+++ b/dhcp_config.cc
@@ -37,7 +37,8 @@
const int DHCPConfig::kDHCPCDExitPollMilliseconds = 50;
const int DHCPConfig::kDHCPCDExitWaitMilliseconds = 3000;
const char DHCPConfig::kDHCPCDPath[] = "/sbin/dhcpcd";
-const char DHCPConfig::kDHCPCDPathFormatLease[] = "var/run/dhcpcd-%s.lease";
+const char DHCPConfig::kDHCPCDPathFormatLease[] =
+ "var/lib/dhcpcd/dhcpcd-%s.lease";
const char DHCPConfig::kDHCPCDPathFormatPID[] = "var/run/dhcpcd-%s.pid";
const int DHCPConfig::kMinMTU = 576;
const char DHCPConfig::kReasonBound[] = "BOUND";
@@ -54,17 +55,24 @@
DHCPProvider *provider,
const string &device_name,
const string &request_hostname,
+ const string &lease_file_suffix,
+ bool arp_gateway,
GLib *glib)
: IPConfig(control_interface, device_name, kType),
proxy_factory_(ProxyFactory::GetInstance()),
provider_(provider),
request_hostname_(request_hostname),
+ lease_file_suffix_(lease_file_suffix),
+ arp_gateway_(arp_gateway),
pid_(0),
child_watch_tag_(0),
root_("/"),
dispatcher_(dispatcher),
glib_(glib) {
SLOG(DHCP, 2) << __func__ << ": " << device_name;
+ if (lease_file_suffix_.empty()) {
+ lease_file_suffix_ = device_name;
+ }
}
DHCPConfig::~DHCPConfig() {
@@ -142,12 +150,20 @@
vector<char *> args;
args.push_back(const_cast<char *>(kDHCPCDPath));
- args.push_back(const_cast<char *>("-B")); // foreground
- args.push_back(const_cast<char *>(device_name().c_str()));
+ args.push_back(const_cast<char *>("-B")); // Run in foreground.
if (!request_hostname_.empty()) {
- args.push_back(const_cast<char *>("-h")); // request hostname
+ args.push_back(const_cast<char *>("-h")); // Request hostname from server.
args.push_back(const_cast<char *>(request_hostname_.c_str()));
}
+ if (arp_gateway_) {
+ args.push_back(const_cast<char *>("-R")); // ARP for default gateway.
+ }
+ string interface_arg(device_name());
+ if (lease_file_suffix_ != device_name()) {
+ interface_arg = base::StringPrintf("%s=%s", device_name().c_str(),
+ lease_file_suffix_.c_str());
+ }
+ args.push_back(const_cast<char *>(interface_arg.c_str()));
args.push_back(NULL);
char *envp[1] = { NULL };
@@ -297,12 +313,14 @@
pid_ = 0;
}
proxy_.reset();
- file_util::Delete(root_.Append(base::StringPrintf(kDHCPCDPathFormatLease,
- device_name().c_str())),
- false);
- file_util::Delete(root_.Append(base::StringPrintf(kDHCPCDPathFormatPID,
- device_name().c_str())),
- false);
+ if (lease_file_suffix_ == device_name()) {
+ // If the lease file suffix was left as default, clean it up at exit.
+ file_util::Delete(root_.Append(
+ base::StringPrintf(kDHCPCDPathFormatLease,
+ device_name().c_str())), false);
+ }
+ file_util::Delete(root_.Append(
+ base::StringPrintf(kDHCPCDPathFormatPID, device_name().c_str())), false);
}
} // namespace shill
diff --git a/dhcp_config.h b/dhcp_config.h
index 2afa200..429e15e 100644
--- a/dhcp_config.h
+++ b/dhcp_config.h
@@ -22,6 +22,16 @@
class GLib;
class ProxyFactory;
+// This class provides a DHCP client instance for the device |device_name|.
+// If |request_hostname| is non-empty, it asks the DHCP server to register
+// this hostname on our behalf, for purposes of administration or creating
+// a dynamic DNS entry.
+//
+// The DHPCConfig instance asks the DHCP client to create a lease file
+// containing the name |lease_file_suffix|. If this suffix is the same as
+// |device_name|, the lease is considered to be ephemeral, and the lease
+// file is removed whenever this DHCPConfig instance is no longer needed.
+// Otherwise, the lease file persists and will be re-used in future attempts.
class DHCPConfig : public IPConfig {
public:
typedef std::map<std::string, DBus::Variant> Configuration;
@@ -33,6 +43,8 @@
DHCPProvider *provider,
const std::string &device_name,
const std::string &request_hostname,
+ const std::string &lease_file_suffix,
+ bool arp_gateway,
GLib *glib);
virtual ~DHCPConfig();
@@ -64,8 +76,9 @@
FRIEND_TEST(DHCPConfigTest, RestartNoClient);
FRIEND_TEST(DHCPConfigTest, StartFail);
FRIEND_TEST(DHCPConfigTest, StartWithHostname);
+ FRIEND_TEST(DHCPConfigTest, StartWithoutArpGateway);
FRIEND_TEST(DHCPConfigTest, StartWithoutHostname);
- FRIEND_TEST(DHCPConfigTest, StartSuccess);
+ FRIEND_TEST(DHCPConfigTest, StartWithoutLeaseSuffix);
FRIEND_TEST(DHCPConfigTest, Stop);
FRIEND_TEST(DHCPProviderTest, CreateConfig);
@@ -127,6 +140,14 @@
// server in the request.
std::string request_hostname_;
+ // DHCP lease file suffix, used to differentiate the lease of one interface
+ // or network from another.
+ std::string lease_file_suffix_;
+
+ // Specifies whether to supply an argument to the DHCP client to validate
+ // the acquired IP address using an ARP request to the gateway IP address.
+ bool arp_gateway_;
+
// The PID of the spawned DHCP client. May be 0 if no client has been spawned
// yet or the client has died.
int pid_;
diff --git a/dhcp_config_unittest.cc b/dhcp_config_unittest.cc
index 115b0f7..38f11bb 100644
--- a/dhcp_config_unittest.cc
+++ b/dhcp_config_unittest.cc
@@ -34,6 +34,8 @@
namespace {
const char kDeviceName[] = "eth0";
const char kHostName[] = "hostname";
+const char kLeaseFileSuffix[] = "leasefilesuffix";
+const bool kArpGateway = true;
} // namespace {}
class DHCPConfigTest : public PropertyStoreTest {
@@ -46,6 +48,8 @@
DHCPProvider::GetInstance(),
kDeviceName,
kHostName,
+ kLeaseFileSuffix,
+ kArpGateway,
glib())) {}
virtual void SetUp() {
@@ -56,6 +60,12 @@
config_->proxy_factory_ = NULL;
}
+ DHCPConfigRefPtr CreateRunningConfig(const string &hostname,
+ const string &lease_suffix,
+ bool arp_gateway);
+ void StopRunningConfigAndExpect(DHCPConfigRefPtr config,
+ bool lease_file_exists);
+
protected:
class TestProxyFactory : public ProxyFactory {
public:
@@ -69,12 +79,67 @@
DHCPConfigTest *test_;
};
+ static const int kPID;
+ static const unsigned int kTag;
+
+ FilePath lease_file_;
+ FilePath pid_file_;
+ ScopedTempDir temp_dir_;
scoped_ptr<MockDHCPProxy> proxy_;
TestProxyFactory proxy_factory_;
MockControl control_;
DHCPConfigRefPtr config_;
};
+const int DHCPConfigTest::kPID = 123456;
+const unsigned int DHCPConfigTest::kTag = 77;
+
+DHCPConfigRefPtr DHCPConfigTest::CreateRunningConfig(const string &hostname,
+ const string &lease_suffix,
+ bool arp_gateway) {
+ DHCPConfigRefPtr config(new DHCPConfig(&control_,
+ dispatcher(),
+ DHCPProvider::GetInstance(),
+ kDeviceName,
+ hostname,
+ lease_suffix,
+ arp_gateway,
+ glib()));
+ EXPECT_CALL(*glib(), SpawnAsync(_, _, _, _, _, _, _, _))
+ .WillOnce(DoAll(SetArgumentPointee<6>(kPID), Return(true)));
+ EXPECT_CALL(*glib(), ChildWatchAdd(kPID, _, _)).WillOnce(Return(kTag));
+ EXPECT_TRUE(config->Start());
+ EXPECT_EQ(kPID, config->pid_);
+ EXPECT_EQ(config.get(), DHCPProvider::GetInstance()->GetConfig(kPID).get());
+ EXPECT_EQ(kTag, config->child_watch_tag_);
+
+ EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
+ config->root_ = temp_dir_.path();
+ FilePath varrun = temp_dir_.path().Append("var/run");
+ EXPECT_TRUE(file_util::CreateDirectory(varrun));
+ pid_file_ = varrun.Append(base::StringPrintf("dhcpcd-%s.pid", kDeviceName));
+ FilePath varlib = temp_dir_.path().Append("var/lib/dhcpcd");
+ EXPECT_TRUE(file_util::CreateDirectory(varlib));
+ lease_file_ =
+ varlib.Append(base::StringPrintf("dhcpcd-%s.lease", kDeviceName));
+ EXPECT_EQ(0, file_util::WriteFile(pid_file_, "", 0));
+ EXPECT_EQ(0, file_util::WriteFile(lease_file_, "", 0));
+ EXPECT_TRUE(file_util::PathExists(pid_file_));
+ EXPECT_TRUE(file_util::PathExists(lease_file_));
+
+ return config;
+}
+
+void DHCPConfigTest::StopRunningConfigAndExpect(DHCPConfigRefPtr config,
+ bool lease_file_exists) {
+ EXPECT_CALL(*glib(), SpawnClosePID(kPID)).Times(1);
+ DHCPConfig::ChildWatchCallback(kPID, 0, config.get());
+ EXPECT_EQ(NULL, DHCPProvider::GetInstance()->GetConfig(kPID).get());
+
+ EXPECT_FALSE(file_util::PathExists(pid_file_));
+ EXPECT_EQ(lease_file_exists, file_util::PathExists(lease_file_));
+}
+
TEST_F(DHCPConfigTest, GetIPv4AddressString) {
EXPECT_EQ("255.255.255.255", config_->GetIPv4AddressString(0xffffffff));
EXPECT_EQ("0.0.0.0", config_->GetIPv4AddressString(0));
@@ -152,30 +217,35 @@
EXPECT_EQ(0, config_->pid_);
}
-MATCHER_P(IsDHCPCDArgs, has_hostname, "") {
+MATCHER_P3(IsDHCPCDArgs, has_hostname, has_arp_gateway, has_lease_suffix, "") {
if (string(arg[0]) != "/sbin/dhcpcd" ||
- string(arg[1]) != "-B" ||
- string(arg[2]) != kDeviceName) {
+ string(arg[1]) != "-B") {
return false;
}
+ int end_offset = 2;
if (has_hostname) {
- if (string(arg[3]) != "-h" ||
- string(arg[4]) != kHostName ||
- arg[5] != NULL) {
+ if (string(arg[end_offset]) != "-h" ||
+ string(arg[end_offset + 1]) != kHostName) {
return false;
}
- } else {
- if (arg[3] != NULL) {
- return false;
- }
+ end_offset += 2;
}
- return true;
+ if (has_arp_gateway) {
+ if (string(arg[end_offset]) != "-R")
+ return false;
+ ++end_offset;
+ }
+
+ string device_arg = has_lease_suffix ?
+ string(kDeviceName) + "=" + string(kLeaseFileSuffix) : kDeviceName;
+ return string(arg[end_offset]) == device_arg && arg[end_offset + 1] == NULL;
}
TEST_F(DHCPConfigTest, StartWithHostname) {
- EXPECT_CALL(*glib(), SpawnAsync(_, IsDHCPCDArgs(true), _, _, _, _, _, _))
+ EXPECT_CALL(*glib(),
+ SpawnAsync(_, IsDHCPCDArgs(true, true, true), _, _, _, _, _, _))
.WillOnce(Return(false));
EXPECT_FALSE(config_->Start());
}
@@ -186,9 +256,44 @@
DHCPProvider::GetInstance(),
kDeviceName,
"",
+ kLeaseFileSuffix,
+ kArpGateway,
glib()));
- EXPECT_CALL(*glib(), SpawnAsync(_, IsDHCPCDArgs(false), _, _, _, _, _, _))
+ EXPECT_CALL(*glib(),
+ SpawnAsync(_, IsDHCPCDArgs(false, true, true), _, _, _, _, _, _))
+ .WillOnce(Return(false));
+ EXPECT_FALSE(config->Start());
+}
+
+TEST_F(DHCPConfigTest, StartWithoutArpGateway) {
+ DHCPConfigRefPtr config(new DHCPConfig(&control_,
+ dispatcher(),
+ DHCPProvider::GetInstance(),
+ kDeviceName,
+ kHostName,
+ kLeaseFileSuffix,
+ false,
+ glib()));
+
+ EXPECT_CALL(*glib(),
+ SpawnAsync(_, IsDHCPCDArgs(true, false, true), _, _, _, _, _, _))
+ .WillOnce(Return(false));
+ EXPECT_FALSE(config->Start());
+}
+
+TEST_F(DHCPConfigTest, StartWithoutLeaseSuffix) {
+ DHCPConfigRefPtr config(new DHCPConfig(&control_,
+ dispatcher(),
+ DHCPProvider::GetInstance(),
+ kDeviceName,
+ kHostName,
+ kDeviceName,
+ kArpGateway,
+ glib()));
+
+ EXPECT_CALL(*glib(),
+ SpawnAsync(_, IsDHCPCDArgs(true, true, false), _, _, _, _, _, _))
.WillOnce(Return(false));
EXPECT_FALSE(config->Start());
}
@@ -332,35 +437,16 @@
config_->child_watch_tag_ = 0;
}
-TEST_F(DHCPConfigTest, StartSuccess) {
- const int kPID = 123456;
- const unsigned int kTag = 55;
- EXPECT_CALL(*glib(), SpawnAsync(_, _, _, _, _, _, _, _))
- .WillOnce(DoAll(SetArgumentPointee<6>(kPID), Return(true)));
- EXPECT_CALL(*glib(), ChildWatchAdd(kPID, _, _)).WillOnce(Return(kTag));
- EXPECT_TRUE(config_->Start());
- EXPECT_EQ(kPID, config_->pid_);
- EXPECT_EQ(config_.get(), DHCPProvider::GetInstance()->GetConfig(kPID).get());
- EXPECT_EQ(kTag, config_->child_watch_tag_);
+TEST_F(DHCPConfigTest, StartSuccessEphemeral) {
+ DHCPConfigRefPtr config =
+ CreateRunningConfig(kHostName, kDeviceName, kArpGateway);
+ StopRunningConfigAndExpect(config, false);
+}
- ScopedTempDir temp_dir;
- config_->root_ = temp_dir.path();
- FilePath varrun = temp_dir.path().Append("var/run");
- EXPECT_TRUE(file_util::CreateDirectory(varrun));
- FilePath pid_file =
- varrun.Append(base::StringPrintf("dhcpcd-%s.pid", kDeviceName));
- FilePath lease_file =
- varrun.Append(base::StringPrintf("dhcpcd-%s.lease", kDeviceName));
- EXPECT_EQ(0, file_util::WriteFile(pid_file, "", 0));
- EXPECT_EQ(0, file_util::WriteFile(lease_file, "", 0));
- ASSERT_TRUE(file_util::PathExists(pid_file));
- ASSERT_TRUE(file_util::PathExists(lease_file));
-
- EXPECT_CALL(*glib(), SpawnClosePID(kPID)).Times(1);
- DHCPConfig::ChildWatchCallback(kPID, 0, config_.get());
- EXPECT_EQ(NULL, DHCPProvider::GetInstance()->GetConfig(kPID).get());
- EXPECT_FALSE(file_util::PathExists(pid_file));
- EXPECT_FALSE(file_util::PathExists(lease_file));
+TEST_F(DHCPConfigTest, StartSuccessPersistent) {
+ DHCPConfigRefPtr config =
+ CreateRunningConfig(kHostName, kLeaseFileSuffix, kArpGateway);
+ StopRunningConfigAndExpect(config, true);
}
TEST_F(DHCPConfigTest, Stop) {
diff --git a/dhcp_provider.cc b/dhcp_provider.cc
index 611fe51..e712378 100644
--- a/dhcp_provider.cc
+++ b/dhcp_provider.cc
@@ -47,10 +47,18 @@
}
DHCPConfigRefPtr DHCPProvider::CreateConfig(const string &device_name,
- const string &host_name) {
+ const string &host_name,
+ const string &lease_file_suffix,
+ bool arp_gateway) {
SLOG(DHCP, 2) << __func__ << " device: " << device_name;
- return new DHCPConfig(
- control_interface_, dispatcher_, this, device_name, host_name, glib_);
+ return new DHCPConfig(control_interface_,
+ dispatcher_,
+ this,
+ device_name,
+ host_name,
+ lease_file_suffix,
+ arp_gateway,
+ glib_);
}
DHCPConfigRefPtr DHCPProvider::GetConfig(int pid) {
diff --git a/dhcp_provider.h b/dhcp_provider.h
index 6a66c4f..bad79dd 100644
--- a/dhcp_provider.h
+++ b/dhcp_provider.h
@@ -27,7 +27,10 @@
// configurations for devices can be obtained through its CreateConfig
// method. For example, a single DHCP configuration request can be initiated as:
//
-// DHCPProvider::GetInstance()->CreateConfig(device_name, host_name)->Request();
+// DHCPProvider::GetInstance()->CreateConfig(device_name,
+// host_name,
+// lease_file_suffix,
+// arp_gateway)->Request();
class DHCPProvider {
public:
virtual ~DHCPProvider();
@@ -44,11 +47,17 @@
// Creates a new DHCPConfig for |device_name|. The DHCP configuration for the
// device can then be initiated through DHCPConfig::Request and
- // DHCPConfig::Renew. If |hostname| is not-empty, it is placed in the DHCP
+ // DHCPConfig::Renew. If |host_name| is not-empty, it is placed in the DHCP
// request to allow the server to map the request to a specific user-named
- // origin.
+ // origin. The DHCP lease file will contain the suffix supplied
+ // in |lease_file_suffix| if non-empty, otherwise |device_name|. If
+ // |arp_gateway| is true, the DHCP client will ARP for the gateway IP
+ // address as an additional safeguard against the issued IP address being
+ // in-use by another station.
virtual DHCPConfigRefPtr CreateConfig(const std::string &device_name,
- const std::string &host_name);
+ const std::string &host_name,
+ const std::string &lease_file_suffix,
+ bool arp_gateway);
// Returns the DHCP configuration associated with DHCP client |pid|. Return
// NULL if |pid| is not bound to a configuration.
diff --git a/dhcp_provider_unittest.cc b/dhcp_provider_unittest.cc
index d028935..042cea5 100644
--- a/dhcp_provider_unittest.cc
+++ b/dhcp_provider_unittest.cc
@@ -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.
@@ -15,6 +15,8 @@
namespace {
const char kDeviceName[] = "testdevicename";
const char kHostName[] = "testhostname";
+const char kStorageIdentifier[] = "teststorageidentifier";
+const bool kArpGateway = false;
} // namespace {}
class DHCPProviderTest : public Test {
@@ -31,7 +33,10 @@
};
TEST_F(DHCPProviderTest, CreateConfig) {
- DHCPConfigRefPtr config = provider_->CreateConfig(kDeviceName, kHostName);
+ DHCPConfigRefPtr config = provider_->CreateConfig(kDeviceName,
+ kHostName,
+ kStorageIdentifier,
+ kArpGateway);
EXPECT_TRUE(config.get());
EXPECT_EQ(&glib_, config->glib_);
EXPECT_EQ(kDeviceName, config->device_name());
diff --git a/doc/manager-api.txt b/doc/manager-api.txt
index 93c2132..3ce8db4 100644
--- a/doc/manager-api.txt
+++ b/doc/manager-api.txt
@@ -273,6 +273,15 @@
Object path of the current active profile.
+ boolean ArpGateway [readwrite]
+
+ Specifies whether the DHCP client should be
+ configured to perform the extra step of performing
+ an ARP of the gateway IP address. This provides
+ a level of assurance that the issued IP address is
+ valid and not blocked in some manner unknown by the
+ DHCP server.
+
array{string} AvailableTechnologies [readonly]
The list of available technologies. The strings
diff --git a/ethernet.cc b/ethernet.cc
index 65997a3..bdb7635 100644
--- a/ethernet.cc
+++ b/ethernet.cc
@@ -88,7 +88,7 @@
if (service_) {
manager()->RegisterService(service_);
if (service_->auto_connect()) {
- if (AcquireIPConfig()) {
+ if (AcquireIPConfigWithLeaseName(service_->GetStorageIdentifier())) {
SelectService(service_);
SetServiceState(Service::kStateConfiguring);
} else {
diff --git a/manager.cc b/manager.cc
index 828e709..520e64e 100644
--- a/manager.cc
+++ b/manager.cc
@@ -82,6 +82,7 @@
HelpRegisterDerivedString(flimflam::kActiveProfileProperty,
&Manager::GetActiveProfileRpcIdentifier,
NULL);
+ store_.RegisterBool(flimflam::kArpGatewayProperty, &props_.arp_gateway);
HelpRegisterDerivedStrings(flimflam::kAvailableTechnologiesProperty,
&Manager::AvailableTechnologies,
NULL);
diff --git a/manager.h b/manager.h
index 7d2c904..2b752ef 100644
--- a/manager.h
+++ b/manager.h
@@ -38,13 +38,18 @@
struct Properties {
public:
Properties()
- : offline_mode(false), portal_check_interval_seconds(0) {}
+ : offline_mode(false),
+ portal_check_interval_seconds(0),
+ arp_gateway(true) {}
bool offline_mode;
std::string check_portal_list;
std::string country;
int32 portal_check_interval_seconds;
std::string portal_url;
std::string host_name;
+ // Whether to ARP for the default gateway in the DHCP client after
+ // acquiring a lease.
+ bool arp_gateway;
};
Manager(ControlInterface *control_interface,
@@ -185,7 +190,8 @@
void set_startup_profiles(const std::vector<std::string> &startup_profiles) {
startup_profiles_ = startup_profiles;
}
- virtual const std::string &GetHostName() { return props_.host_name; }
+ bool GetArpGateway() const { return props_.arp_gateway; }
+ const std::string &GetHostName() const { return props_.host_name; }
virtual void UpdateEnabledTechnologies();
PowerManager *power_manager() const { return power_manager_.get(); }
diff --git a/mock_dhcp_config.cc b/mock_dhcp_config.cc
index 4d2cf4d..94648e5 100644
--- a/mock_dhcp_config.cc
+++ b/mock_dhcp_config.cc
@@ -1,23 +1,23 @@
-// 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.
#include "shill/mock_dhcp_config.h"
+using std::string;
+
namespace shill {
MockDHCPConfig::MockDHCPConfig(ControlInterface *control_interface,
- EventDispatcher *dispatcher,
- DHCPProvider *provider,
- const std::string &device_name,
- const std::string &request_host_name,
- GLib *glib)
+ const string &device_name)
: DHCPConfig(control_interface,
- dispatcher,
- provider,
+ NULL,
+ NULL,
device_name,
- request_host_name,
- glib) {}
+ string(),
+ string(),
+ false,
+ NULL) {}
MockDHCPConfig::~MockDHCPConfig() {}
diff --git a/mock_dhcp_config.h b/mock_dhcp_config.h
index 9e4fe48..f74ccc6 100644
--- a/mock_dhcp_config.h
+++ b/mock_dhcp_config.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.
@@ -17,11 +17,7 @@
class MockDHCPConfig : public DHCPConfig {
public:
MockDHCPConfig(ControlInterface *control_interface,
- EventDispatcher *dispatcher,
- DHCPProvider *provider,
- const std::string &device_name,
- const std::string &request_host_name,
- GLib *glib);
+ const std::string &device_name);
virtual ~MockDHCPConfig();
MOCK_METHOD0(RequestIP, bool());
diff --git a/mock_dhcp_provider.h b/mock_dhcp_provider.h
index 0f5b040..b0b8179 100644
--- a/mock_dhcp_provider.h
+++ b/mock_dhcp_provider.h
@@ -22,8 +22,11 @@
virtual ~MockDHCPProvider();
MOCK_METHOD3(Init, void(ControlInterface *, EventDispatcher *, GLib *));
- MOCK_METHOD2(CreateConfig, DHCPConfigRefPtr(const std::string &device_name,
- const std::string &host_name));
+ MOCK_METHOD4(CreateConfig,
+ DHCPConfigRefPtr(const std::string &device_name,
+ const std::string &host_name,
+ const std::string &storage_identifier,
+ bool arp_gateway));
private:
DISALLOW_COPY_AND_ASSIGN(MockDHCPProvider);
diff --git a/wifi.cc b/wifi.cc
index bdf9880..0845d36 100644
--- a/wifi.cc
+++ b/wifi.cc
@@ -1050,8 +1050,9 @@
if (new_state == wpa_supplicant::kInterfaceStateCompleted &&
!affected_service->IsConnected()) {
- if (AcquireIPConfig()) {
- LOG(INFO) << link_name() << " is up; should start L3!";
+ if (AcquireIPConfigWithLeaseName(
+ affected_service->GetStorageIdentifier())) {
+ LOG(INFO) << link_name() << " is up; started L3 configuration.";
affected_service->SetState(Service::kStateConfiguring);
} else {
LOG(ERROR) << "Unable to acquire DHCP config.";
diff --git a/wifi_unittest.cc b/wifi_unittest.cc
index 4d70296..a6f20f7 100644
--- a/wifi_unittest.cc
+++ b/wifi_unittest.cc
@@ -176,11 +176,7 @@
supplicant_bss_proxy_(
new NiceMock<MockSupplicantBSSProxy>()),
dhcp_config_(new MockDHCPConfig(&control_interface_,
- &dispatcher_,
- &dhcp_provider_,
- kDeviceName,
- kHostName,
- &glib_)),
+ kDeviceName)),
proxy_factory_(this),
power_manager_(new MockPowerManager(&proxy_factory_)) {
::testing::DefaultValue< ::DBus::Path>::Set("/default/path");
@@ -191,7 +187,7 @@
ON_CALL(manager_, HasService(_)).
WillByDefault(Return(true));
- ON_CALL(dhcp_provider_, CreateConfig(_, _)).
+ ON_CALL(dhcp_provider_, CreateConfig(_, _, _, _)).
WillByDefault(Return(dhcp_config_));
ON_CALL(*dhcp_config_.get(), RequestIP()).
WillByDefault(Return(true));
@@ -490,7 +486,6 @@
protected:
static const char kDeviceName[];
static const char kDeviceAddress[];
- static const char kHostName[];
static const char kNetworkModeAdHoc[];
static const char kNetworkModeInfrastructure[];
@@ -508,7 +503,6 @@
const char WiFiMainTest::kDeviceName[] = "wlan0";
const char WiFiMainTest::kDeviceAddress[] = "000102030405";
-const char WiFiMainTest::kHostName[] = "hostname";
const char WiFiMainTest::kNetworkModeAdHoc[] = "ad-hoc";
const char WiFiMainTest::kNetworkModeInfrastructure[] = "infrastructure";
@@ -895,7 +889,7 @@
TEST_F(WiFiMainTest, DisconnectPendingServiceWithCurrent) {
EXPECT_CALL(*manager(), RegisterService(_)).Times(AnyNumber());
- EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _)).Times(AnyNumber());
+ EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _, _, _)).Times(AnyNumber());
EXPECT_CALL(*dhcp_config_.get(), RequestIP()).Times(AnyNumber());
EXPECT_CALL(*manager(), UpdateService(_)).Times(AnyNumber());
MockSupplicantInterfaceProxy &supplicant_interface_proxy =
@@ -926,7 +920,7 @@
TEST_F(WiFiMainTest, DisconnectCurrentService) {
EXPECT_CALL(*manager(), RegisterService(_)).Times(AnyNumber());
- EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _)).Times(AnyNumber());
+ EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _, _, _)).Times(AnyNumber());
EXPECT_CALL(*dhcp_config_.get(), RequestIP()).Times(AnyNumber());
EXPECT_CALL(*manager(), UpdateService(_)).Times(AnyNumber());
MockSupplicantInterfaceProxy &supplicant_interface_proxy =
@@ -950,7 +944,7 @@
TEST_F(WiFiMainTest, DisconnectCurrentServiceWithPending) {
EXPECT_CALL(*manager(), RegisterService(_)).Times(AnyNumber());
- EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _)).Times(AnyNumber());
+ EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _, _, _)).Times(AnyNumber());
EXPECT_CALL(*dhcp_config_.get(), RequestIP()).Times(AnyNumber());
EXPECT_CALL(*manager(), UpdateService(_)).Times(AnyNumber());
MockSupplicantInterfaceProxy &supplicant_interface_proxy =
@@ -992,7 +986,7 @@
TEST_F(WiFiMainTest, DisconnectCurrentServiceFailure) {
EXPECT_CALL(*manager(), RegisterService(_)).Times(AnyNumber());
- EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _)).Times(AnyNumber());
+ EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _, _, _)).Times(AnyNumber());
EXPECT_CALL(*dhcp_config_.get(), RequestIP()).Times(AnyNumber());
EXPECT_CALL(*manager(), UpdateService(_)).Times(AnyNumber());
MockSupplicantInterfaceProxy &supplicant_interface_proxy =
@@ -1388,7 +1382,7 @@
TEST_F(WiFiMainTest, StateChangeBackwardsWithService) {
// Some backwards transitions should not trigger a Service state change.
// Supplicant state should still be updated, however.
- EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _)).Times(AnyNumber());
+ EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _, _, _)).Times(AnyNumber());
EXPECT_CALL(*dhcp_config_.get(), RequestIP()).Times(AnyNumber());
StartWiFi();
dispatcher_.DispatchPendingEvents();
@@ -1467,7 +1461,7 @@
TEST_F(WiFiMainTest, CurrentBSSChangeConnectedToDisconnected) {
EXPECT_CALL(*manager(), HasService(_)).Times(AnyNumber());
- EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _)).Times(AnyNumber());
+ EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _, _, _)).Times(AnyNumber());
EXPECT_CALL(*dhcp_config_.get(), RequestIP()).Times(AnyNumber());
EXPECT_CALL(*manager(), UpdateService(_)).Times(AnyNumber());
WiFiEndpointRefPtr ap = MakeEndpoint("an_ssid", "00:01:02:03:04:05");
@@ -1498,7 +1492,7 @@
TEST_F(WiFiMainTest, CurrentBSSChangeConnectedToConnectedNewService) {
EXPECT_CALL(*manager(), HasService(_)).Times(AnyNumber());
- EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _)).Times(AnyNumber());
+ EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _, _, _)).Times(AnyNumber());
EXPECT_CALL(*dhcp_config_.get(), RequestIP()).Times(AnyNumber());
EXPECT_CALL(*manager(), UpdateService(_)).Times(AnyNumber());
WiFiEndpointRefPtr ap1 = MakeEndpoint("an_ssid", "00:01:02:03:04:05");
@@ -1534,7 +1528,7 @@
TEST_F(WiFiMainTest, CurrentBSSChangeDisconnectedToConnected) {
EXPECT_CALL(*manager(), HasService(_)).Times(AnyNumber());
EXPECT_CALL(*manager(), UpdateService(_)).Times(AnyNumber());
- EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _)).Times(AnyNumber());
+ EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _, _, _)).Times(AnyNumber());
EXPECT_CALL(*dhcp_config_.get(), RequestIP()).Times(AnyNumber());
WiFiEndpointRefPtr ap = MakeEndpoint("an_ssid", "00:01:02:03:04:05");
WiFiServiceRefPtr service = CreateServiceForEndpoint(*ap);
@@ -1653,7 +1647,7 @@
TEST_F(WiFiMainTest, SupplicantCompleted) {
EXPECT_CALL(*manager(), UpdateService(_)).Times(AnyNumber());
- EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _)).Times(AnyNumber());
+ EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _, _, _)).Times(AnyNumber());
EXPECT_CALL(*manager(), HasService(_)).Times(AnyNumber());
WiFiEndpointRefPtr ap = MakeEndpoint("an_ssid", "00:01:02:03:04:05");
WiFiServiceRefPtr service = CreateServiceForEndpoint(*ap);
@@ -1674,7 +1668,7 @@
EXPECT_CALL(*manager(), UpdateService(_)).Times(AnyNumber());
EXPECT_CALL(*device_info(), FlushAddresses(_)).Times(AnyNumber());
EXPECT_CALL(*manager(), device_info()).Times(AnyNumber());
- EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _)).Times(AnyNumber());
+ EXPECT_CALL(*dhcp_provider(), CreateConfig(_, _, _, _)).Times(AnyNumber());
EXPECT_CALL(*manager(), HasService(_)).Times(AnyNumber());
EXPECT_CALL(*manager(), IsPortalDetectionEnabled(_)).Times(AnyNumber());
WiFiEndpointRefPtr ap = MakeEndpoint("an_ssid", "00:01:02:03:04:05");