shill: Cleanup of the config80211 code.
BUG=None.
TEST=Manual and unit tests.
Change-Id: Iceba3473b8d7476b5a7340d4a918d7aa6354278c
Reviewed-on: https://gerrit.chromium.org/gerrit/37576
Reviewed-by: Christopher Wiley <wiley@chromium.org>
Commit-Ready: Wade Guthrie <wdg@chromium.org>
Tested-by: Wade Guthrie <wdg@chromium.org>
diff --git a/callback80211_object.cc b/callback80211_object.cc
index 3133100..3d45bc7 100644
--- a/callback80211_object.cc
+++ b/callback80211_object.cc
@@ -13,7 +13,6 @@
#include "shill/ieee80211.h"
#include "shill/link_monitor.h"
#include "shill/logging.h"
-#include "shill/metrics.h"
#include "shill/scope_logger.h"
#include "shill/user_bound_nlmessage.h"
diff --git a/callback80211_object.h b/callback80211_object.h
index 85a7875..baf2159 100644
--- a/callback80211_object.h
+++ b/callback80211_object.h
@@ -35,14 +35,7 @@
// Deinstall ourselves as a callback.
bool DeinstallAsCallback();
- // TODO(wdg): remove debug code:
- void SetName(std::string name) { name_ = name;}
- const std::string &GetName() { return name_; }
-
private:
- // TODO(wdg): remove debug code:
- std::string name_;
-
// When installed, this is the method Config80211 will call when it gets a
// message from the mac80211 drivers.
virtual void Config80211MessageCallback(const UserBoundNlMessage &msg);
diff --git a/config80211.cc b/config80211.cc
index fa47642..14217fb 100644
--- a/config80211.cc
+++ b/config80211.cc
@@ -15,6 +15,7 @@
#include <base/stl_util.h>
#include "shill/io_handler.h"
+#include "shill/kernel_bound_nlmessage.h"
#include "shill/logging.h"
#include "shill/nl80211_socket.h"
#include "shill/scope_logger.h"
@@ -70,6 +71,15 @@
if (!sock_->Init()) {
return false;
}
+
+ // Install the global NetLink Callback.
+ sock_->SetNetlinkCallback(OnRawNlMessageReceived,
+ static_cast<void *>(this));
+
+ // Don't make libnl do the sequence checking (because it'll not like
+ // the broadcast messages for which we'll eventually ask). We'll do all the
+ // sequence checking we need.
+ sock_->DisableSequenceChecking();
}
if (!event_types_) {
@@ -136,6 +146,20 @@
broadcast_callbacks_.clear();
}
+bool Config80211::SendMessage(KernelBoundNlMessage *message,
+ const Callback &callback) {
+ if (!message) {
+ LOG(ERROR) << "Message is NULL.";
+ return false;
+ }
+ if (!SetMessageCallback(*message, callback)) {
+ return false;
+ }
+
+ message->Send(sock_);
+ return true;
+}
+
bool Config80211::SetMessageCallback(const KernelBoundNlMessage &message,
const Callback &callback) {
LOG(INFO) << "Setting callback for message " << message.GetId();
@@ -193,9 +217,17 @@
return;
}
+ if (!sock_) {
+ LOG(ERROR) << "Config80211::Init needs to be called before this";
+ return;
+ }
+
// If we're newly-up, subscribe to all the event types that have been
// requested.
if (new_state == kWifiUp) {
+ // Install the global NetLink Callback.
+ sock_->SetNetlinkCallback(OnRawNlMessageReceived,
+ static_cast<void *>(this));
SubscribedEvents::const_iterator i;
for (i = subscribed_events_.begin(); i != subscribed_events_.end(); ++i) {
ActuallySubscribeToEvents(*i);
@@ -227,17 +259,6 @@
if (!sock_->AddGroupMembership(group_name)) {
return false;
}
- // No sequence checking for multicast messages.
- if (!sock_->DisableSequenceChecking()) {
- return false;
- }
-
- // Install the global NetLink Callback for messages along with a parameter.
- // The Netlink Callback's parameter is passed to 'C' as a 'void *'.
- if (!sock_->SetNetlinkCallback(OnRawNlMessageReceived,
- static_cast<void *>(this))) {
- return false;
- }
return true;
}
diff --git a/config80211.h b/config80211.h
index f32be1a..ac07b05 100644
--- a/config80211.h
+++ b/config80211.h
@@ -134,10 +134,17 @@
// Uninstall all Config80211 broadcast Callbacks.
void ClearBroadcastCallbacks();
+ // Sends a kernel-bound message using the Config80211 socket after
+ // installing a callback to handle it.
+ // TODO(wdg): Eventually, this should also include a timeout and a callback
+ // to call in case of timeout.
+ bool SendMessage(KernelBoundNlMessage *message, const Callback &callback);
+
// Install a Config80211 Callback to handle the response to a specific
// message.
// TODO(wdg): Eventually, this should also include a timeout and a callback
// to call in case of timeout.
+ // TODO(wdg): maybe this should be private and only accessed by tests.
bool SetMessageCallback(const KernelBoundNlMessage &message,
const Callback &callback);
diff --git a/config80211_unittest.cc b/config80211_unittest.cc
index 9bae303..c128793 100644
--- a/config80211_unittest.cc
+++ b/config80211_unittest.cc
@@ -395,7 +395,6 @@
// Install the callback and subscribe to events using it, wifi down
// (shouldn't actually send the subscription request until wifi comes up).
EXPECT_CALL(socket_, AddGroupMembership(_)).Times(0);
- EXPECT_CALL(socket_, DisableSequenceChecking()).Times(0);
EXPECT_CALL(socket_, SetNetlinkCallback(_, _)).Times(0);
EXPECT_CALL(socket_, GetSequenceNumber())
.WillRepeatedly(Invoke(&MockNl80211Socket::GetNextNumber));
@@ -410,8 +409,6 @@
// 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(
_, ContainsCallback(callback_object.GetCallback())))
.WillOnce(Return(true));
@@ -419,7 +416,6 @@
// 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));
@@ -438,12 +434,9 @@
.WillOnce(Return(true));
EXPECT_CALL(socket_, AddGroupMembership(mlme_event_string))
.WillOnce(Return(true));
- EXPECT_CALL(socket_, DisableSequenceChecking())
- .Times(2)
- .WillRepeatedly(Return(true));
EXPECT_CALL(socket_, SetNetlinkCallback(
_, ContainsCallback(callback_object.GetCallback())))
- .Times(2)
+ .Times(1)
.WillRepeatedly(Return(true));
config80211_->SetWifiState(Config80211::kWifiUp);
}
diff --git a/kernel_bound_nlmessage.cc b/kernel_bound_nlmessage.cc
index ae8d8ad..4e97abc 100644
--- a/kernel_bound_nlmessage.cc
+++ b/kernel_bound_nlmessage.cc
@@ -104,22 +104,10 @@
LOG(ERROR) << "NULL |socket| parameter";
return false;
}
- if (!message_) {
- LOG(ERROR) << "NULL |message_|.";
- return -1;
- }
-
- // Complete AND SEND a message.
- int result = nl_send_auto_complete(
- const_cast<struct nl_sock *>(socket->GetConstNlSock()), message_);
SLOG(WiFi, 6) << "NL Message " << GetId() << " ===>";
- if (result < 0) {
- LOG(ERROR) << "Failed call to 'nl_send_auto_complete': " << result;
- return false;
- }
- return true;
+ return socket->Send(message_);
}
} // namespace shill.
diff --git a/kernel_bound_nlmessage.h b/kernel_bound_nlmessage.h
index d3469f2..b51d465 100644
--- a/kernel_bound_nlmessage.h
+++ b/kernel_bound_nlmessage.h
@@ -25,8 +25,6 @@
#ifndef SHILL_KERNEL_BOUND_NLMESSAGE_H_
#define SHILL_KERNEL_BOUND_NLMESSAGE_H_
-#include "shill/kernel_bound_nlmessage.h"
-
#include <base/basictypes.h>
#include <base/bind.h>
diff --git a/mock_nl80211_socket.h b/mock_nl80211_socket.h
index 6c1b0f9..20a9d70 100644
--- a/mock_nl80211_socket.h
+++ b/mock_nl80211_socket.h
@@ -22,8 +22,6 @@
MockNl80211Socket() {}
MOCK_METHOD0(Init, bool());
MOCK_METHOD1(AddGroupMembership, bool(const std::string &group_name));
- using Nl80211Socket::DisableSequenceChecking;
- MOCK_METHOD0(DisableSequenceChecking, bool());
using Nl80211Socket::GetMessages;
MOCK_METHOD0(GetMessages, bool());
using Nl80211Socket::SetNetlinkCallback;
diff --git a/netlink_socket.cc b/netlink_socket.cc
index 15fa99c..6b08920 100644
--- a/netlink_socket.cc
+++ b/netlink_socket.cc
@@ -40,7 +40,6 @@
#include <base/logging.h>
-#include "shill/kernel_bound_nlmessage.h"
#include "shill/scope_logger.h"
namespace shill {
@@ -114,6 +113,26 @@
return true;
}
+bool NetlinkSocket::Send(struct nl_msg *message) {
+ if (!message) {
+ LOG(ERROR) << "NULL |message|.";
+ return false;
+ }
+
+ if (!nl_sock_) {
+ LOG(ERROR) << "Need to initialize the socket first.";
+ return false;
+ }
+
+ int result = nl_send_auto_complete(nl_sock_, message);
+ if (result < 0) {
+ LOG(ERROR) << "Failed call to 'nl_send_auto_complete': " << result;
+ return false;
+ }
+ return true;
+}
+
+
bool NetlinkSocket::DisableSequenceChecking() {
if (!nl_sock_) {
LOG(ERROR) << "NULL socket";
diff --git a/netlink_socket.h b/netlink_socket.h
index 6f0d60e..a698db3 100644
--- a/netlink_socket.h
+++ b/netlink_socket.h
@@ -37,8 +37,6 @@
#include <base/bind.h>
#include <base/logging.h>
-#include "shill/kernel_bound_nlmessage.h"
-
enum nl_cb_kind;
enum nl_cb_type;
struct nl_msg;
@@ -100,6 +98,9 @@
// Non-trivial initialization.
bool Init();
+ // Send a message.
+ virtual bool Send(struct nl_msg *message);
+
// Disables sequence checking on the message stream.
virtual bool DisableSequenceChecking();
@@ -129,8 +130,6 @@
// Returns the family name of the socket created by this type of object.
virtual std::string GetSocketFamilyName() const = 0;
- const struct nl_sock *GetConstNlSock() const { return nl_sock_; }
-
protected:
struct nl_sock *GetNlSock() { return nl_sock_; }