shill: Connect Ethernet Device to DHCPConfig.
Most of the implementation is in the base Device class.
BUG=chromium-os:16794
TEST=unit test
Change-Id: I583761f7e54c88b043ce4343cb43f8298aaedf8b
Reviewed-on: http://gerrit.chromium.org/gerrit/2949
Reviewed-by: Darin Petkov <petkov@chromium.org>
Tested-by: Darin Petkov <petkov@chromium.org>
diff --git a/Makefile b/Makefile
index 7027005..65742c4 100644
--- a/Makefile
+++ b/Makefile
@@ -78,6 +78,7 @@
TEST_OBJS = \
dbus_adaptor_unittest.o \
device_info_unittest.o \
+ device_unittest.o \
dhcp_config_unittest.o \
dhcp_provider_unittest.o \
ipconfig_unittest.o \
diff --git a/device.cc b/device.cc
index 3c07bcb..e7103d3 100644
--- a/device.cc
+++ b/device.cc
@@ -14,6 +14,7 @@
#include "shill/control_interface.h"
#include "shill/device.h"
#include "shill/device_dbus_adaptor.h"
+#include "shill/dhcp_provider.h"
#include "shill/error.h"
#include "shill/manager.h"
#include "shill/shill_event.h"
@@ -25,10 +26,10 @@
Device::Device(ControlInterface *control_interface,
EventDispatcher *dispatcher,
Manager *manager,
- const string& link_name,
+ const string &link_name,
int interface_index)
- : link_name_(link_name),
- manager_(manager),
+ : manager_(manager),
+ link_name_(link_name),
adaptor_(control_interface->CreateDeviceAdaptor(this)),
interface_index_(interface_index),
running_(false) {
@@ -51,6 +52,10 @@
adaptor_->UpdateEnabled();
}
+bool Device::TechnologyIs(const Technology type) {
+ return false;
+}
+
void Device::LinkEvent(unsigned flags, unsigned change) {
VLOG(2) << "Device " << link_name_ << " flags " << flags << " changed "
<< change;
@@ -108,7 +113,28 @@
const string& Device::UniqueName() const {
// TODO(pstew): link_name is only run-time unique and won't persist
- return link_name_;
+ return link_name();
+}
+
+void Device::DestroyIPConfig() {
+ if (ipconfig_.get()) {
+ ipconfig_->ReleaseIP();
+ ipconfig_ = NULL;
+ }
+}
+
+bool Device::AcquireDHCPConfig() {
+ DestroyIPConfig();
+ ipconfig_ = DHCPProvider::GetInstance()->CreateConfig(link_name());
+ ipconfig_->RegisterUpdateCallback(
+ NewCallback(this, &Device::IPConfigUpdatedCallback));
+ return ipconfig_->RequestIP();
+}
+
+void Device::IPConfigUpdatedCallback(IPConfigRefPtr ipconfig, bool success) {
+ // TODO(petkov): Use DeviceInfo to configure IP, etc. -- maybe through
+ // ConfigIP? Also, maybe allow forwarding the callback to interested listeners
+ // (e.g., the Manager).
}
} // namespace shill
diff --git a/device.h b/device.h
index e258517..4fd96d6 100644
--- a/device.h
+++ b/device.h
@@ -11,8 +11,10 @@
#include <base/basictypes.h>
#include <base/memory/ref_counted.h>
#include <base/memory/scoped_ptr.h>
+#include <gtest/gtest_prod.h> // for FRIEND_TEST
#include "shill/device_config_interface.h"
+#include "shill/ipconfig.h"
#include "shill/property_store_interface.h"
#include "shill/service.h"
#include "shill/shill_event.h"
@@ -48,14 +50,16 @@
Device(ControlInterface *control_interface,
EventDispatcher *dispatcher,
Manager *manager,
- const std::string& link_name,
+ const std::string &link_name,
int interface_index);
virtual ~Device();
virtual void Start();
virtual void Stop();
- virtual bool TechnologyIs(const Technology type) = 0;
+ // Base method always returns false.
+ virtual bool TechnologyIs(const Technology type);
+
virtual void LinkEvent(unsigned flags, unsigned change);
virtual void Scan();
@@ -72,20 +76,42 @@
bool SetUint16Property(const std::string& name, uint16 value, Error *error);
bool SetUint32Property(const std::string& name, uint32 value, Error *error);
- // Returns a string that is guaranteed to uniquely identify this
- // Device instance.
- const std::string& UniqueName() const;
+ const std::string &link_name() const { return link_name_; }
+
+ // Returns a string that is guaranteed to uniquely identify this Device
+ // instance.
+ const std::string &UniqueName() const;
protected:
+ FRIEND_TEST(DeviceTest, AcquireDHCPConfig);
+ FRIEND_TEST(DeviceTest, DestroyIPConfig);
+ FRIEND_TEST(DeviceTest, DestroyIPConfigNULL);
+
+ // If there's an IP configuration in |ipconfig_|, releases the IP address and
+ // destroys the configuration instance.
+ void DestroyIPConfig();
+
+ // Creates a new DHCP IP configuration instance, stores it in |ipconfig_| and
+ // requests a new IP configuration. Registers a callback to
+ // IPConfigUpdatedCallback on IP configuration changes. Returns true if the IP
+ // request was successfully sent.
+ bool AcquireDHCPConfig();
+
std::vector<ServiceRefPtr> services_;
- std::string link_name_;
int interface_index_;
bool running_;
Manager *manager_;
+ IPConfigRefPtr ipconfig_;
private:
- scoped_ptr<DeviceAdaptorInterface> adaptor_;
friend class DeviceAdaptorInterface;
+
+ // Callback invoked on every IP configuration update.
+ void IPConfigUpdatedCallback(IPConfigRefPtr ipconfig, bool success);
+
+ const std::string link_name_;
+ scoped_ptr<DeviceAdaptorInterface> adaptor_;
+
DISALLOW_COPY_AND_ASSIGN(Device);
};
diff --git a/device_info_unittest.cc b/device_info_unittest.cc
index e66cd30..3f1a40f 100644
--- a/device_info_unittest.cc
+++ b/device_info_unittest.cc
@@ -11,8 +11,10 @@
#include <gmock/gmock.h>
#include "shill/device_info.h"
+#include "shill/dhcp_provider.h"
#include "shill/manager.h"
#include "shill/mock_control.h"
+#include "shill/mock_glib.h"
#include "shill/rtnl_handler.h"
namespace shill {
@@ -26,11 +28,13 @@
: manager_(&control_interface_, &dispatcher_),
device_info_(&control_interface_, &dispatcher_, &manager_),
factory_(this) {
+ DHCPProvider::GetInstance()->glib_ = &glib_;
}
base::hash_map<int, DeviceRefPtr> *DeviceInfoDevices() {
return &device_info_.devices_;
}
protected:
+ MockGLib glib_;
MockControl control_interface_;
Manager manager_;
DeviceInfo device_info_;
diff --git a/device_unittest.cc b/device_unittest.cc
new file mode 100644
index 0000000..0b81954
--- /dev/null
+++ b/device_unittest.cc
@@ -0,0 +1,62 @@
+// 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 <gtest/gtest.h>
+
+#include "shill/device.h"
+#include "shill/dhcp_provider.h"
+#include "shill/mock_control.h"
+#include "shill/mock_glib.h"
+
+using testing::_;
+using testing::Return;
+using testing::Test;
+
+namespace shill {
+
+namespace {
+const char kDeviceName[] = "testdevice";
+} // namespace {}
+
+class DeviceTest : public Test {
+ public:
+ DeviceTest()
+ : device_(new Device(&control_interface_, NULL, NULL, kDeviceName, 0)) {
+ DHCPProvider::GetInstance()->glib_ = &glib_;
+ }
+
+ protected:
+ MockGLib glib_;
+ MockControl control_interface_;
+ DeviceRefPtr device_;
+};
+
+TEST_F(DeviceTest, TechnologyIs) {
+ EXPECT_FALSE(device_->TechnologyIs(Device::kEthernet));
+}
+
+TEST_F(DeviceTest, DestroyIPConfig) {
+ ASSERT_FALSE(device_->ipconfig_.get());
+ device_->ipconfig_ = new IPConfig(kDeviceName);
+ device_->DestroyIPConfig();
+ ASSERT_FALSE(device_->ipconfig_.get());
+}
+
+TEST_F(DeviceTest, DestroyIPConfigNULL) {
+ ASSERT_FALSE(device_->ipconfig_.get());
+ device_->DestroyIPConfig();
+ ASSERT_FALSE(device_->ipconfig_.get());
+}
+
+TEST_F(DeviceTest, AcquireDHCPConfig) {
+ device_->ipconfig_ = new IPConfig("randomname");
+ EXPECT_CALL(glib_, SpawnAsync(_, _, _, _, _, _, _, _))
+ .WillOnce(Return(false));
+ EXPECT_FALSE(device_->AcquireDHCPConfig());
+ ASSERT_TRUE(device_->ipconfig_.get());
+ EXPECT_EQ(kDeviceName, device_->ipconfig_->device_name());
+ EXPECT_TRUE(device_->ipconfig_->update_callback_.get());
+}
+
+} // namespace shill
diff --git a/dhcp_provider.h b/dhcp_provider.h
index a0f3914..add55b1 100644
--- a/dhcp_provider.h
+++ b/dhcp_provider.h
@@ -55,6 +55,8 @@
private:
friend struct DefaultSingletonTraits<DHCPProvider>;
friend class DHCPProviderTest;
+ friend class DeviceInfoTest;
+ friend class DeviceTest;
FRIEND_TEST(DHCPProviderTest, CreateConfig);
typedef std::map<int, DHCPConfigRefPtr> PIDConfigMap;
@@ -64,7 +66,7 @@
virtual ~DHCPProvider();
// A single listener is used to catch signals from all DHCP clients and
- // dispatch them to the appropriate proxy.
+ // dispatch them to the appropriate DHCP configuration instance.
scoped_ptr<DHCPListenerInterface> listener_;
// A map that binds PIDs to DHCP configuration instances.
diff --git a/ethernet.cc b/ethernet.cc
index 0d10ee7..2ae07c8 100644
--- a/ethernet.cc
+++ b/ethernet.cc
@@ -24,10 +24,11 @@
using std::string;
namespace shill {
+
Ethernet::Ethernet(ControlInterface *control_interface,
EventDispatcher *dispatcher,
Manager *manager,
- const std::string& link_name,
+ const string &link_name,
int interface_index)
: Device(control_interface,
dispatcher,
@@ -40,7 +41,7 @@
"service-" + link_name)),
link_up_(false),
service_registered_(false) {
- VLOG(2) << "Ethernet device " << link_name_ << " initialized.";
+ VLOG(2) << "Ethernet device " << link_name << " initialized.";
}
Ethernet::~Ethernet() {
@@ -54,7 +55,8 @@
}
void Ethernet::Stop() {
- manager_->DeregisterService(service_.get());
+ manager_->DeregisterService(service_);
+ DestroyIPConfig();
Device::Stop();
RTNLHandler::GetInstance()->SetInterfaceFlags(interface_index_, 0, IFF_UP);
}
@@ -63,15 +65,19 @@
return type == Device::kEthernet;
}
-void Ethernet::LinkEvent(unsigned flags, unsigned change) {
+void Ethernet::LinkEvent(unsigned int flags, unsigned int change) {
Device::LinkEvent(flags, change);
if ((flags & IFF_LOWER_UP) != 0 && !link_up_) {
- LOG(INFO) << link_name_ << " is up; should start L3!";
+ LOG(INFO) << link_name() << " is up; should start L3!";
link_up_ = true;
- manager_->RegisterService(service_.get());
+ manager_->RegisterService(service_);
+ if (service_->auto_connect()) {
+ LOG_IF(ERROR, !AcquireDHCPConfig()) << "Unable to acquire DHCP config.";
+ }
} else if ((flags & IFF_LOWER_UP) == 0 && link_up_) {
link_up_ = false;
- manager_->DeregisterService(service_.get());
+ manager_->DeregisterService(service_);
+ DestroyIPConfig();
}
}
diff --git a/ethernet.h b/ethernet.h
index 09e0ee5..fbcacd9 100644
--- a/ethernet.h
+++ b/ethernet.h
@@ -15,21 +15,23 @@
class Ethernet : public Device {
public:
- explicit Ethernet(ControlInterface *control_interface,
- EventDispatcher *dispatcher,
- Manager *manager,
- const std::string& link_name,
- int interface_index);
+ Ethernet(ControlInterface *control_interface,
+ EventDispatcher *dispatcher,
+ Manager *manager,
+ const std::string& link_name,
+ int interface_index);
~Ethernet();
+
void Start();
void Stop();
bool TechnologyIs(Device::Technology type);
- void LinkEvent(unsigned flags, unsigned change);
- bool link_up_;
+ void LinkEvent(unsigned int flags, unsigned int change);
private:
bool service_registered_;
ServiceRefPtr service_;
+ bool link_up_;
+
DISALLOW_COPY_AND_ASSIGN(Ethernet);
};
diff --git a/ethernet_service.cc b/ethernet_service.cc
index 44a7080..b3e31f7 100644
--- a/ethernet_service.cc
+++ b/ethernet_service.cc
@@ -2,6 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include "shill/ethernet_service.h"
+
#include <time.h>
#include <stdio.h>
#include <netinet/ether.h>
@@ -14,21 +16,21 @@
#include "shill/control_interface.h"
#include "shill/device.h"
#include "shill/device_info.h"
+#include "shill/ethernet.h"
#include "shill/manager.h"
#include "shill/shill_event.h"
-#include "shill/ethernet.h"
-#include "shill/ethernet_service.h"
-
using std::string;
namespace shill {
+
EthernetService::EthernetService(ControlInterface *control_interface,
EventDispatcher *dispatcher,
Ethernet *device,
- const std::string& name)
+ const string &name)
: Service(control_interface, dispatcher, device, name),
ethernet_(device) {
+ set_auto_connect(true);
}
EthernetService::~EthernetService() { }
diff --git a/ipconfig.h b/ipconfig.h
index c29971a..1a10083 100644
--- a/ipconfig.h
+++ b/ipconfig.h
@@ -64,6 +64,8 @@
void UpdateProperties(const Properties &properties, bool success);
private:
+ FRIEND_TEST(DeviceTest, AcquireDHCPConfig);
+ FRIEND_TEST(DeviceTest, DestroyIPConfig);
FRIEND_TEST(IPConfigTest, UpdateCallback);
FRIEND_TEST(IPConfigTest, UpdateProperties);
diff --git a/service.h b/service.h
index 61e2efc..0cd0107 100644
--- a/service.h
+++ b/service.h
@@ -76,7 +76,10 @@
// Returns a string that is guaranteed to uniquely identify this
// Service instance.
- virtual const std::string& UniqueName() { return name_; }
+ virtual const std::string &UniqueName() { return name_; }
+
+ bool auto_connect() const { return auto_connect_; }
+ void set_auto_connect(bool connect) { auto_connect_ = connect; }
protected:
EventDispatcher *dispatcher_;
diff --git a/wifi.cc b/wifi.cc
index cec2d98..a42dce4 100644
--- a/wifi.cc
+++ b/wifi.cc
@@ -131,7 +131,7 @@
dispatcher_(dispatcher),
dbus_(DBus::Connection::SystemBus()),
scan_pending_(false) {
- VLOG(2) << "WiFi device " << link_name_ << " initialized.";
+ VLOG(2) << "WiFi device " << link_name << " initialized.";
}
WiFi::~WiFi() {
@@ -144,7 +144,7 @@
try {
std::map<string, DBus::Variant> create_interface_args;
create_interface_args["Ifname"].writer().
- append_string(link_name_.c_str());
+ append_string(link_name().c_str());
create_interface_args["Driver"].writer().
append_string(kSupplicantWiFiDriver);
// TODO(quiche) create_interface_args["ConfigFile"].writer().append_string
@@ -154,7 +154,7 @@
} catch (const DBus::Error e) { // NOLINT
if (!strcmp(e.name(), kSupplicantErrorInterfaceExists)) {
interface_path =
- supplicant_process_proxy_->GetInterface(link_name_);
+ supplicant_process_proxy_->GetInterface(link_name());
// XXX crash here, if device missing?
} else {
// XXX