brillo: start AP service after AP interface is enumerated
For Brillo devices, it is required to use the AP mode interface
that's setup using the HAL API. So explicitly setup an AP mode
interface using the HAL API, and wait for the interface to
enumerated before starting the AP service.
This is achieved by handling the Start request asynchronously.
Also explicitly update the AP Config to use the newly created
AP interface, to avoid the Config from auto selecting a wrong
interface in the case when multiple AP-capable interfaces are
presented.
Bug: 25113165
TEST=Device setup works on both Brillo boards
Change-Id: Icdee32e8bab9cf41a467058aef5ea081562a3442
diff --git a/manager.cc b/manager.cc
index 9f51d07..2cebb74 100644
--- a/manager.cc
+++ b/manager.cc
@@ -169,6 +169,16 @@
shill_manager_.ReleaseInterface(interface_name);
}
+#if defined(__BRILLO__)
+bool Manager::SetupApModeInterface(string* interface_name) {
+ return shill_manager_.SetupApModeInterface(interface_name);
+}
+
+bool Manager::SetupStationModeInterface(string* interface_name) {
+ return shill_manager_.SetupStationModeInterface(interface_name);
+}
+#endif // __BRILLO__
+
void Manager::RequestDHCPPortAccess(const string& interface) {
firewall_manager_.RequestDHCPPortAccess(interface);
}
diff --git a/manager.h b/manager.h
index 63bee10..c30e60b 100644
--- a/manager.h
+++ b/manager.h
@@ -79,6 +79,14 @@
virtual void ClaimInterface(const std::string& interface_name);
// Release the given interface |interface_name| to shill.
virtual void ReleaseInterface(const std::string& interface_name);
+#if defined(__BRILLO__)
+ // Setup an AP mode interface. Returns true and sets |interface_name|
+ // on success, false otherwise.
+ virtual bool SetupApModeInterface(std::string* interface_name);
+ // Setup a station mode interface. Returns true and sets |interface_name|
+ // on success, false otherwise.
+ virtual bool SetupStationModeInterface(std::string* interface_name);
+#endif // __BRILLO__
// Request/release access to DHCP port for the specified interface.
virtual void RequestDHCPPortAccess(const std::string& interface);
diff --git a/mock_manager.h b/mock_manager.h
index 93ac36d..9335fdc 100644
--- a/mock_manager.h
+++ b/mock_manager.h
@@ -41,6 +41,10 @@
MOCK_METHOD1(ReleaseInterface, void(const std::string& interface_name));
MOCK_METHOD1(RequestDHCPPortAccess, void(const std::string& interface));
MOCK_METHOD1(ReleaseDHCPPortAccess, void(const std::string& interface));
+#if defined(__BRILLO__)
+ MOCK_METHOD1(SetupApModeInterface, bool(std::string* interface_name));
+ MOCK_METHOD1(SetupStationModeInterface, bool(std::string* interface_name));
+#endif // __BRILLO__
private:
DISALLOW_COPY_AND_ASSIGN(MockManager);
diff --git a/service.cc b/service.cc
index 99073fb..09af33a 100644
--- a/service.cc
+++ b/service.cc
@@ -27,6 +27,12 @@
#include "dbus/apmanager/dbus-constants.h"
#endif // __ANDROID__
+#if defined(__BRILLO__)
+#include <base/bind.h>
+
+#include "apmanager/event_dispatcher.h"
+#endif // __BRILLO__
+
#include "apmanager/manager.h"
using brillo::dbus_utils::AsyncEventSequencer;
@@ -52,6 +58,11 @@
"/data/misc/apmanager/hostapd/ctrl_iface";
#endif // __ANDROID__
+#if defined(__BRILLO__)
+const int Service::kAPInterfaceCheckIntervalMilliseconds = 200;
+const int Service::kAPInterfaceCheckMaxAttempts = 5;
+#endif // __BRILLO__
+
const int Service::kTerminationTimeoutSeconds = 2;
// static. Service state definitions.
@@ -78,6 +89,11 @@
// TODO(zqiu): come up with better server address management. This is good
// enough for now.
config_->SetServerAddressIndex(identifier_ & 0xFF);
+
+#if defined(__BRILLO__)
+ event_dispatcher_ = EventDispatcher::GetInstance();
+ start_in_progress_ = false;
+#endif
}
Service::~Service() {
@@ -171,7 +187,7 @@
if (!hostapd_monitor_) {
hostapd_monitor_.reset(
new HostapdMonitor(base::Bind(&Service::HostapdEventCallback,
- base::Unretained(this)),
+ weak_factory_.GetWeakPtr()),
config_->control_interface(),
config_->selected_interface()));
}
@@ -186,11 +202,38 @@
void Service::Start(std::unique_ptr<DBusMethodResponse<>> response) {
brillo::ErrorPtr error;
+#if !defined(__BRILLO__)
if (!StartInternal(&error)) {
response->ReplyWithError(error.get());
} else {
response->Return();
}
+#else
+ if (start_in_progress_) {
+ brillo::Error::AddTo(
+ &error, FROM_HERE, brillo::errors::dbus::kDomain, kServiceError,
+ "Start already in progress");
+ response->ReplyWithError(error.get());
+ return;
+ }
+
+ string interface_name;
+ if (!manager_->SetupApModeInterface(&interface_name)) {
+ brillo::Error::AddTo(
+ &error, FROM_HERE, brillo::errors::dbus::kDomain, kServiceError,
+ "Failed to setup AP mode interface");
+ response->ReplyWithError(error.get());
+ return;
+ }
+
+ event_dispatcher_->PostDelayedTask(
+ base::Bind(&Service::APInterfaceCheckTask,
+ weak_factory_.GetWeakPtr(),
+ interface_name,
+ 0, // Initial check count.
+ base::Passed(&response)),
+ kAPInterfaceCheckIntervalMilliseconds);
+#endif
}
bool Service::Stop(brillo::ErrorPtr* error) {
@@ -206,6 +249,55 @@
return true;
}
+#if defined(__BRILLO__)
+void Service::HandleStartFailure() {
+ // Restore station mode interface.
+ string station_mode_interface;
+ manager_->SetupStationModeInterface(&station_mode_interface);
+
+ // Reset state variables.
+ start_in_progress_ = false;
+}
+
+void Service::APInterfaceCheckTask(
+ const string& interface_name,
+ int check_count,
+ std::unique_ptr<DBusMethodResponse<>> response) {
+ brillo::ErrorPtr error;
+
+ // Check if the AP interface is enumerated.
+ if (manager_->GetDeviceFromInterfaceName(interface_name)) {
+ // Explicitly set the interface name to avoid picking other interface.
+ config_->SetInterfaceName(interface_name);
+ if (!StartInternal(&error)) {
+ HandleStartFailure();
+ response->ReplyWithError(error.get());
+ } else {
+ response->Return();
+ }
+ return;
+ }
+
+ check_count++;
+ if (check_count >= kAPInterfaceCheckMaxAttempts) {
+ brillo::Error::AddTo(
+ &error, FROM_HERE, brillo::errors::dbus::kDomain, kServiceError,
+ "Timeout waiting for AP interface to be enumerated");
+ HandleStartFailure();
+ response->ReplyWithError(error.get());
+ return;
+ }
+
+ event_dispatcher_->PostDelayedTask(
+ base::Bind(&Service::APInterfaceCheckTask,
+ weak_factory_.GetWeakPtr(),
+ interface_name,
+ check_count,
+ base::Passed(&response)),
+ kAPInterfaceCheckIntervalMilliseconds);
+}
+#endif // __BRILLO__
+
bool Service::IsHostapdRunning() {
return hostapd_process_ && hostapd_process_->pid() != 0 &&
brillo::Process::ProcessExists(hostapd_process_->pid());
@@ -235,6 +327,11 @@
dhcp_server_.reset();
config_->ReleaseDevice();
manager_->ReleaseDHCPPortAccess(config_->selected_interface());
+#if defined(__BRILLO__)
+ // Restore station mode interface.
+ string station_mode_interface;
+ manager_->SetupStationModeInterface(&station_mode_interface);
+#endif // __BRILLO__
}
void Service::HostapdEventCallback(HostapdMonitor::Event event,
diff --git a/service.h b/service.h
index e193943..1459229 100644
--- a/service.h
+++ b/service.h
@@ -20,6 +20,7 @@
#include <string>
#include <base/macros.h>
+#include <base/memory/weak_ptr.h>
#include <brillo/process.h>
#include "apmanager/config.h"
@@ -33,6 +34,10 @@
class Manager;
+#if defined(__BRILLO__)
+class EventDispatcher;
+#endif // __BRILLO__
+
class Service : public org::chromium::apmanager::ServiceAdaptor,
public org::chromium::apmanager::ServiceInterface {
public:
@@ -66,6 +71,21 @@
static const char kStateStarted[];
static const char kStateFailed[];
+#if defined(__BRILLO__)
+ static const int kAPInterfaceCheckIntervalMilliseconds;
+ static const int kAPInterfaceCheckMaxAttempts;
+
+ // Task to check enumeration status of the specified AP interface
+ // |interface_name|.
+ void APInterfaceCheckTask(
+ const std::string& interface_name,
+ int check_count,
+ std::unique_ptr<brillo::dbus_utils::DBusMethodResponse<>> response);
+
+ // Handle asynchronous service start failures.
+ void HandleStartFailure();
+#endif // __BRILLO__
+
bool StartInternal(brillo::ErrorPtr* error);
// Return true if hostapd process is currently running.
@@ -97,7 +117,12 @@
FileWriter* file_writer_;
ProcessFactory* process_factory_;
std::unique_ptr<HostapdMonitor> hostapd_monitor_;
+#if defined(__BRILLO__)
+ EventDispatcher* event_dispatcher_;
+ bool start_in_progress_;
+#endif // __BRILLO__
+ base::WeakPtrFactory<Service> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(Service);
};