Shill: Wait until cfg80211/mac80211 are up before trying to configure.
BUG=chromium-os:33837
TEST=Updated unit tests.
Change-Id: I69711d0a3c366297b0568bca770dd50c2a44f722
Reviewed-on: https://gerrit.chromium.org/gerrit/31237
Commit-Ready: Wade Guthrie <wdg@chromium.org>
Reviewed-by: Wade Guthrie <wdg@chromium.org>
Tested-by: Wade Guthrie <wdg@chromium.org>
diff --git a/config80211.cc b/config80211.cc
index 2343da5..64a7b84 100644
--- a/config80211.cc
+++ b/config80211.cc
@@ -13,6 +13,7 @@
#include <string>
#include <base/memory/weak_ptr.h>
+#include <base/stl_util.h>
#include <base/stringprintf.h>
#include "shill/io_handler.h"
@@ -24,7 +25,6 @@
using base::Bind;
using base::LazyInstance;
using base::StringAppendF;
-using std::map;
using std::string;
namespace shill {
@@ -33,10 +33,11 @@
LazyInstance<Config80211> g_config80211 = LAZY_INSTANCE_INITIALIZER;
} // namespace
-map<Config80211::EventType, std::string> *Config80211::event_types_ = NULL;
+Config80211::EventTypeStrings *Config80211::event_types_ = NULL;
Config80211::Config80211()
- : dispatcher_(NULL),
+ : wifi_state_(kWifiDown),
+ dispatcher_(NULL),
weak_ptr_factory_(this),
dispatcher_callback_(Bind(&Config80211::HandleIncomingEvents,
weak_ptr_factory_.GetWeakPtr())),
@@ -54,6 +55,11 @@
return g_config80211.Pointer();
}
+void Config80211::Reset() {
+ wifi_state_ = kWifiDown;
+ subscribed_events_.clear();
+}
+
bool Config80211::Init(EventDispatcher *dispatcher) {
if (!sock_) {
sock_ = new Nl80211Socket;
@@ -68,7 +74,7 @@
}
if (!event_types_) {
- event_types_ = new std::map<EventType, std::string>;
+ event_types_ = new EventTypeStrings;
(*event_types_)[Config80211::kEventTypeConfig] = "config";
(*event_types_)[Config80211::kEventTypeScan] = "scan";
(*event_types_)[Config80211::kEventTypeRegulatory] = "regulatory";
@@ -98,7 +104,7 @@
return false;
}
- map<EventType, string>::iterator match = (*event_types_).find(type);
+ EventTypeStrings::iterator match = (*event_types_).find(type);
if (match == (*event_types_).end()) {
LOG(ERROR) << "Event type " << type << " not found";
return false;
@@ -107,8 +113,44 @@
return true;
}
+void Config80211::SetWifiState(WifiState new_state) {
+ if (wifi_state_ == new_state) {
+ return;
+ }
+
+ // If we're newly-up, subscribe to all the event types that have been
+ // requested.
+ if (new_state == kWifiUp) {
+ SubscribedEvents::const_iterator i;
+ for (i = subscribed_events_.begin(); i != subscribed_events_.end(); ++i) {
+ string event_type_string;
+ GetEventTypeString(*i, &event_type_string);
+ ActuallySubscribeToEvents(*i);
+ }
+ }
+ wifi_state_ = new_state;
+}
+
bool Config80211::SubscribeToEvents(EventType type) {
+ string event_type_string;
+ GetEventTypeString(type, &event_type_string);
+ bool it_worked = true;
+ if (!ContainsKey(subscribed_events_, type)) {
+ if (wifi_state_ == kWifiUp) {
+ it_worked = ActuallySubscribeToEvents(type);
+ }
+ // |subscribed_events_| is a list of events to which we want to subscribe
+ // when wifi comes up (including when it comes _back_ up after it goes
+ // down sometime in the future).
+ subscribed_events_.insert(type);
+ }
+ return it_worked;
+}
+
+bool Config80211::ActuallySubscribeToEvents(EventType type) {
string group_name;
+ string event_type_string;
+ GetEventTypeString(type, &event_type_string);
if (!GetEventTypeString(type, &group_name)) {
return false;
@@ -116,7 +158,6 @@
if (!sock_->AddGroupMembership(group_name)) {
return false;
}
-
// No sequence checking for multicast messages.
if (!sock_->DisableSequenceChecking()) {
return false;
diff --git a/config80211.h b/config80211.h
index b88b229..cf00efc 100644
--- a/config80211.h
+++ b/config80211.h
@@ -63,6 +63,7 @@
#include <iomanip>
#include <map>
+#include <set>
#include <string>
#include <base/basictypes.h>
@@ -97,6 +98,12 @@
kEventTypeCount
};
+ // This represents whether the cfg80211/mac80211 are installed in the kernel.
+ enum WifiState {
+ kWifiUp=10,
+ kWifiDown=20
+ };
+
virtual ~Config80211();
// This is a singleton -- use Config80211::GetInstance()->Foo()
@@ -127,9 +134,14 @@
// Return a string corresponding to the passed-in EventType.
static bool GetEventTypeString(EventType type, std::string *value);
- // Sign-up to receive and log multicast events of a specific type.
+ // Sign-up to receive and log multicast events of a specific type (once wifi
+ // is up).
bool SubscribeToEvents(EventType type);
+ // Indicate that the mac80211 driver is up and, ostensibly, accepting event
+ // subscription requests or down.
+ void SetWifiState(WifiState new_state);
+
protected:
friend struct base::DefaultLazyInstanceTraits<Config80211>;
@@ -138,6 +150,10 @@
private:
friend class Config80211Test;
+ // Sign-up to receive and log multicast events of a specific type (assumes
+ // wifi is up).
+ bool ActuallySubscribeToEvents(EventType type);
+
// EventDispatcher calls this when data is available on our socket. This
// callback reads data from the driver, parses that data, and logs it.
void HandleIncomingEvents(int fd);
@@ -147,6 +163,10 @@
// them.
static int OnNlMessageReceived(struct nl_msg *msg, void *arg);
+ // Just for tests, this method turns off WiFi and clears the subscribed
+ // events list.
+ void Reset();
+
// Config80211 Callback, OnNlMessageReceived invokes this User-supplied
// callback object when _it_ gets called to read libnl data.
Callback default_callback_;
@@ -154,7 +174,13 @@
// TODO(wdg): implement the following.
// std::map<uint32_t, Callback> message_callback_;
- static std::map<EventType, std::string> *event_types_;
+ typedef std::map<EventType, std::string> EventTypeStrings;
+ static EventTypeStrings *event_types_;
+
+ WifiState wifi_state_;
+
+ typedef std::set<EventType> SubscribedEvents;
+ SubscribedEvents subscribed_events_;
// Hooks needed to be called by shill's EventDispatcher.
EventDispatcher *dispatcher_;
diff --git a/config80211_unittest.cc b/config80211_unittest.cc
index 39922e0..153542e 100644
--- a/config80211_unittest.cc
+++ b/config80211_unittest.cc
@@ -344,6 +344,7 @@
EXPECT_NE(config80211_, reinterpret_cast<Config80211 *>(NULL));
config80211_->sock_ = &socket_;
EXPECT_TRUE(config80211_->Init(reinterpret_cast<EventDispatcher *>(NULL)));
+ config80211_->Reset();
}
Config80211 *config80211_;
@@ -382,31 +383,57 @@
// Create a default callback.
TestCallbackObject callback_object;
- // Install the callback and subscribe to events using it.
+ // Install the callback and subscribe to events using it, wifi down
+ // (shouldn't actually send the subscription request).
+ EXPECT_CALL(socket_, AddGroupMembership(_)).Times(0);
+ EXPECT_CALL(socket_, DisableSequenceChecking()).Times(0);
+ EXPECT_CALL(socket_, SetNetlinkCallback(_,_)).Times(0);
+
config80211_->SetDefaultCallback(callback_object.GetCallback());
- Config80211::EventType event_type = Config80211::kEventTypeScan;
- string event_type_string;
- EXPECT_TRUE(Config80211::GetEventTypeString(event_type, &event_type_string));
- EXPECT_CALL(socket_, AddGroupMembership(event_type_string))
+ Config80211::EventType scan_event = Config80211::kEventTypeScan;
+ string scan_event_string;
+ EXPECT_TRUE(Config80211::GetEventTypeString(scan_event, &scan_event_string));
+ EXPECT_TRUE(config80211_->SubscribeToEvents(scan_event));
+
+ // Wifi up, should subscribe to events.
+ EXPECT_CALL(socket_, AddGroupMembership(scan_event_string))
.WillOnce(Return(true));
EXPECT_CALL(socket_, DisableSequenceChecking())
.WillOnce(Return(true));
EXPECT_CALL(socket_, SetNetlinkCallback(
_, IsEqualToCallback(&callback_object.GetCallback())))
.WillOnce(Return(true));
+ config80211_->SetWifiState(Config80211::kWifiUp);
- EXPECT_TRUE(config80211_->SubscribeToEvents(event_type));
+ // Second subscribe, same event (should do nothing).
+ EXPECT_CALL(socket_, AddGroupMembership(_)).Times(0);
+ EXPECT_CALL(socket_, DisableSequenceChecking()).Times(0);
+ EXPECT_CALL(socket_, SetNetlinkCallback(_,_)).Times(0);
+ EXPECT_TRUE(config80211_->SubscribeToEvents(scan_event));
- // Deinstall the callback.
- config80211_->UnsetDefaultCallback();
- EXPECT_TRUE(Config80211::GetEventTypeString(event_type, &event_type_string));
- EXPECT_CALL(socket_, AddGroupMembership(event_type_string))
+ // Bring the wifi back down.
+ config80211_->SetWifiState(Config80211::kWifiDown);
+
+ // Subscribe to a new event with the wifi down (should still do nothing).
+ Config80211::EventType mlme_event = Config80211::kEventTypeMlme;
+ string mlme_event_string;
+ EXPECT_TRUE(Config80211::GetEventTypeString(mlme_event, &mlme_event_string));
+ EXPECT_TRUE(config80211_->SubscribeToEvents(mlme_event));
+
+ // Wifi up (again), should subscribe to the original scan event and the new
+ // mlme event.
+ EXPECT_CALL(socket_, AddGroupMembership(scan_event_string))
.WillOnce(Return(true));
- EXPECT_CALL(socket_, DisableSequenceChecking()).WillOnce(Return(true));
- EXPECT_CALL(socket_, SetNetlinkCallback(_,
- IsEqualToCallback(NULL)
- )).WillOnce(Return(true));
- EXPECT_TRUE(config80211_->SubscribeToEvents(event_type));
+ EXPECT_CALL(socket_, AddGroupMembership(mlme_event_string))
+ .WillOnce(Return(true));
+ EXPECT_CALL(socket_, DisableSequenceChecking())
+ .Times(2)
+ .WillRepeatedly(Return(true));
+ EXPECT_CALL(socket_, SetNetlinkCallback(
+ _, IsEqualToCallback(&callback_object.GetCallback())))
+ .Times(2)
+ .WillRepeatedly(Return(true));
+ config80211_->SetWifiState(Config80211::kWifiUp);
}
TEST_F(Config80211Test, NL80211_CMD_TRIGGER_SCAN) {
diff --git a/nl80211_socket.cc b/nl80211_socket.cc
index 7c3ea34..1f9409c 100644
--- a/nl80211_socket.cc
+++ b/nl80211_socket.cc
@@ -65,6 +65,7 @@
return false;
}
+ LOG(INFO) << "Nl80211Socket initialized successfully";
return true;
}
@@ -80,6 +81,7 @@
return false;
}
}
+ LOG(INFO) << " Group " << group_name << " added successfully";
return true;
}
diff --git a/user_bound_nlmessage.cc b/user_bound_nlmessage.cc
index 9d40f27..ab4d406 100644
--- a/user_bound_nlmessage.cc
+++ b/user_bound_nlmessage.cc
@@ -1851,11 +1851,6 @@
return NULL;
}
-//#if 0 // If we're collecting data for unit tests, uncommnet this.
- UserBoundNlMessageDataCollector::GetInstance()->CollectDebugData(*message,
- msg);
-//#endif // 0
-
return message.release();
}
@@ -1897,20 +1892,20 @@
doit = node->second;
if (doit) {
- LOG(ERROR) << "@@const unsigned char "
+ LOG(INFO) << "@@const unsigned char "
<< "k" << message.GetMessageTypeString()
- << "[] = { ";
+ << "[] = {";
int payload_bytes = nlmsg_len(msg);
size_t bytes = nlmsg_total_size(payload_bytes);
unsigned char *rawdata = reinterpret_cast<unsigned char *>(msg);
for (size_t i=0; i<bytes; ++i) {
- LOG(ERROR) << " 0x"
+ LOG(INFO) << " 0x"
<< std::hex << std::setfill('0') << std::setw(2)
<< + rawdata[i] << ",";
}
- LOG(ERROR) << "};";
+ LOG(INFO) << "};";
need_to_print[message.GetMessageType()] = false;
}
}
diff --git a/wifi.cc b/wifi.cc
index bb83755..cc23b77 100644
--- a/wifi.cc
+++ b/wifi.cc
@@ -22,6 +22,7 @@
#include <chromeos/dbus/service_constants.h>
#include <glib.h>
+#include "shill/config80211.h"
#include "shill/control_interface.h"
#include "shill/dbus_adaptor.h"
#include "shill/device.h"
@@ -171,6 +172,10 @@
// Connect to WPA supplicant if it's already present. If not, we'll connect to
// it when it appears.
ConnectToSupplicant();
+ Config80211 *config80211 = Config80211::GetInstance();
+ if (config80211) {
+ config80211->SetWifiState(Config80211::kWifiUp);
+ }
}
void WiFi::Stop(Error *error, const EnabledStateChangedCallback &callback) {