shill: DeviceInfo: Decide between WiFi interface types
When a WiFi device appears, use the nl80211 "get interface"
call to retrieve the interface type, deferring creation of
the Device instance until a reply to this message is received.
Only create a WiFi device if the interface is in "station"
mode.
BUG=chrome-os-partner:18698
TEST=Unit tests; run on system with kernel-next patches for
chromeos-partner:18698 comment #5 applied, and
https://gerrit.chromium.org/gerrit/47863 reverted, ensure
only one WiFi device appears in list-devices (mlan0)
Change-Id: Ia559e0931a8bd4aaa067d71aae5d1bd1bf1ceedc
Reviewed-on: https://gerrit.chromium.org/gerrit/48250
Commit-Queue: Paul Stewart <pstew@chromium.org>
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/device_info.cc b/device_info.cc
index 2c1b269..f298ddc 100644
--- a/device_info.cc
+++ b/device_info.cc
@@ -34,6 +34,9 @@
#include "shill/ethernet.h"
#include "shill/logging.h"
#include "shill/manager.h"
+#include "shill/netlink_attribute.h"
+#include "shill/netlink_manager.h"
+#include "shill/nl80211_message.h"
#include "shill/routing_table.h"
#include "shill/rtnl_handler.h"
#include "shill/rtnl_listener.h"
@@ -93,6 +96,7 @@
device_info_root_(kDeviceInfoRoot),
routing_table_(RoutingTable::GetInstance()),
rtnl_handler_(RTNLHandler::GetInstance()),
+ netlink_manager_(NetlinkManager::GetInstance()),
sockets_(new Sockets()) {
}
@@ -201,7 +205,7 @@
Technology::Identifier DeviceInfo::GetDeviceTechnology(
const string &iface_name) {
string type_string;
- int arp_type = ARPHRD_VOID;;
+ int arp_type = ARPHRD_VOID;
if (GetDeviceInfoContents(iface_name, kInterfaceType, &type_string) &&
TrimString(type_string, "\n", &type_string) &&
!base::StringToInt(type_string, &arp_type)) {
@@ -414,9 +418,9 @@
device->EnableIPv6Privacy();
break;
case Technology::kWifi:
- device = new WiFi(control_interface_, dispatcher_, metrics_, manager_,
- link_name, address, interface_index);
- device->EnableIPv6Privacy();
+ // Defer creating this device until we get information about the
+ // type of WiFi interface.
+ GetWiFiInterfaceInfo(interface_index);
break;
case Technology::kWiMax:
// WiMax devices are managed by WiMaxProvider.
@@ -866,4 +870,77 @@
kRequestLinkStatisticsIntervalMilliseconds);
}
+void DeviceInfo::GetWiFiInterfaceInfo(int interface_index) {
+ GetInterfaceMessage msg;
+ if (!msg.attributes()->SetU32AttributeValue(NL80211_ATTR_IFINDEX,
+ interface_index)) {
+ LOG(ERROR) << "Unable to set interface index attribute for "
+ "GetInterface message. Interface type cannot be "
+ "determined!";
+ return;
+ }
+ netlink_manager_->SendMessage(&msg,
+ Bind(&DeviceInfo::OnWiFiInterfaceInfoReceived,
+ AsWeakPtr()));
+}
+
+void DeviceInfo::OnWiFiInterfaceInfoReceived(
+ const NetlinkMessage &raw_message) {
+ if (raw_message.message_type() != Nl80211Message::GetMessageType()) {
+ LOG(ERROR) << "Unknown message type received: "
+ << raw_message.message_type();
+ return;
+ }
+ // Due to the test above, this is guaranteed to be an NL80211Message type.
+ const Nl80211Message *msg =
+ reinterpret_cast<const Nl80211Message *>(&raw_message);
+
+ if (msg->command() != NL80211_CMD_NEW_INTERFACE) {
+ LOG(ERROR) << "Message is not a new interface response";
+ return;
+ }
+
+ uint32_t interface_index;
+ if (!msg->const_attributes()->GetU32AttributeValue(NL80211_ATTR_IFINDEX,
+ &interface_index)) {
+ LOG(ERROR) << "Message contains no interface index";
+ return;
+ }
+ uint32_t interface_type;
+ if (!msg->const_attributes()->GetU32AttributeValue(NL80211_ATTR_IFTYPE,
+ &interface_type)) {
+ LOG(ERROR) << "Message contains no interface type";
+ return;
+ }
+ const Info *info = GetInfo(interface_index);
+ if (!info) {
+ LOG(ERROR) << "Could not find device info for interface index "
+ << interface_index;
+ return;
+ }
+ if (info->device) {
+ LOG(ERROR) << "Device already created for interface index "
+ << interface_index;
+ return;
+ }
+ if (interface_type != NL80211_IFTYPE_STATION) {
+ LOG(INFO) << "Ignoring WiFi device "
+ << info->name
+ << " at interface index "
+ << interface_index
+ << " since it is not in station mode.";
+ return;
+ }
+ LOG(INFO) << "Creating WiFi device for station mode interface "
+ << info->name
+ << " at interface index "
+ << interface_index;
+ string address = StringToLowerASCII(info->mac_address.HexEncode());
+ DeviceRefPtr device =
+ new WiFi(control_interface_, dispatcher_, metrics_, manager_,
+ info->name, address, interface_index);
+ device->EnableIPv6Privacy();
+ RegisterDevice(device);
+}
+
} // namespace shill
diff --git a/device_info.h b/device_info.h
index 90fac40..90ab907 100644
--- a/device_info.h
+++ b/device_info.h
@@ -28,6 +28,8 @@
class Manager;
class Metrics;
+class NetlinkManager;
+class NetlinkMessage;
class RoutingTable;
class RTNLHandler;
class RTNLMessage;
@@ -216,6 +218,10 @@
void RetrieveLinkStatistics(int interface_index, const RTNLMessage &msg);
void RequestLinkStatistics();
+ // Use nl80211 to get information on |interface_index|.
+ void GetWiFiInterfaceInfo(int interface_index);
+ void OnWiFiInterfaceInfoReceived(const NetlinkMessage &raw_message);
+
void set_sockets(Sockets* sockets) { sockets_.reset(sockets); }
ControlInterface *control_interface_;
@@ -243,6 +249,7 @@
// Cache copy of singleton pointers.
RoutingTable *routing_table_;
RTNLHandler *rtnl_handler_;
+ NetlinkManager *netlink_manager_;
// A member of the class so that a mock can be injected for testing.
scoped_ptr<Sockets> sockets_;
diff --git a/device_info_unittest.cc b/device_info_unittest.cc
index 055cfd7..fbdbd1d 100644
--- a/device_info_unittest.cc
+++ b/device_info_unittest.cc
@@ -13,6 +13,7 @@
#include <linux/sockios.h>
#include <net/if_arp.h>
+#include <base/bind.h>
#include <base/file_util.h>
#include <base/files/scoped_temp_dir.h>
#include <base/memory/ref_counted.h>
@@ -28,14 +29,18 @@
#include "shill/mock_control.h"
#include "shill/mock_device.h"
#include "shill/mock_glib.h"
+#include "shill/mock_log.h"
#include "shill/mock_manager.h"
#include "shill/mock_metrics.h"
#include "shill/mock_modem_info.h"
+#include "shill/mock_netlink_manager.h"
#include "shill/mock_routing_table.h"
#include "shill/mock_rtnl_handler.h"
#include "shill/mock_sockets.h"
#include "shill/mock_vpn_provider.h"
#include "shill/mock_wimax_provider.h"
+#include "shill/netlink_attribute.h"
+#include "shill/nl80211_message.h"
#include "shill/rtnl_message.h"
#include "shill/wimax.h"
@@ -46,9 +51,11 @@
using std::string;
using std::vector;
using testing::_;
+using testing::AnyNumber;
using testing::ContainerEq;
using testing::DoAll;
using testing::ElementsAreArray;
+using testing::HasSubstr;
using testing::Mock;
using testing::NotNull;
using testing::Return;
@@ -81,6 +88,7 @@
virtual void SetUp() {
device_info_.rtnl_handler_ = &rtnl_handler_;
device_info_.routing_table_ = &routing_table_;
+ device_info_.netlink_manager_ = &netlink_manager_;
}
IPAddress CreateInterfaceAddress() {
@@ -153,6 +161,7 @@
DeviceInfo device_info_;
TestEventDispatcherForDeviceInfo dispatcher_;
MockRoutingTable routing_table_;
+ MockNetlinkManager netlink_manager_;
StrictMock<MockRTNLHandler> rtnl_handler_;
MockSockets *mock_sockets_; // Owned by DeviceInfo.
};
@@ -442,14 +451,39 @@
device = NULL;
}
+MATCHER_P(IsGetInterfaceMessage, index, "") {
+ if (arg->message_type() != Nl80211Message::GetMessageType()) {
+ return false;
+ }
+ const Nl80211Message *msg = reinterpret_cast<const Nl80211Message *>(arg);
+ if (msg->command() != NL80211_CMD_GET_INTERFACE) {
+ return false;
+ }
+ uint32_t interface_index;
+ if (!msg->const_attributes()->GetU32AttributeValue(NL80211_ATTR_IFINDEX,
+ &interface_index)) {
+ return false;
+ }
+ // kInterfaceIndex is signed, but the attribute as handed from the kernel
+ // is unsigned. We're silently casting it away with this assignment.
+ uint32_t test_interface_index = index;
+ return interface_index == test_interface_index;
+}
+
TEST_F(DeviceInfoTest, CreateDeviceWiFi) {
IPAddress address = CreateInterfaceAddress();
// WiFi looks a lot like Ethernet too.
- EXPECT_CALL(routing_table_, FlushRoutes(kTestDeviceIndex)).Times(1);
+ EXPECT_CALL(routing_table_, FlushRoutes(kTestDeviceIndex));
EXPECT_CALL(rtnl_handler_, RemoveInterfaceAddress(kTestDeviceIndex,
IsIPAddress(address)));
- EXPECT_TRUE(CreateDevice(
+
+ // Set the nl80211 message type to some non-default value.
+ Nl80211Message::SetMessageType(1234);
+
+ EXPECT_CALL(netlink_manager_,
+ SendMessage(IsGetInterfaceMessage(kTestDeviceIndex), _));
+ EXPECT_FALSE(CreateDevice(
kTestDeviceName, "address", kTestDeviceIndex, Technology::kWifi));
}
@@ -1064,6 +1098,10 @@
GetDelayedDevices().insert(kTestDeviceIndex);
}
+ void TriggerOnWiFiInterfaceInfoReceived(const NetlinkMessage &message) {
+ test_device_info_.OnWiFiInterfaceInfoReceived(message);
+ }
+
protected:
DeviceInfoForDelayedCreationTest test_device_info_;
};
@@ -1096,4 +1134,73 @@
EXPECT_TRUE(GetDelayedDevices().empty());
}
+TEST_F(DeviceInfoDelayedCreationTest, WiFiDevice) {
+ ScopedMockLog log;
+ EXPECT_CALL(manager_, RegisterDevice(_)).Times(0);
+ EXPECT_CALL(log, Log(logging::LOG_ERROR, _,
+ HasSubstr("Unknown message type received")));
+ GenericNetlinkMessage non_nl80211_message(0, 0, "foo");
+ TriggerOnWiFiInterfaceInfoReceived(non_nl80211_message);
+ Mock::VerifyAndClearExpectations(&log);
+
+ EXPECT_CALL(log, Log(logging::LOG_ERROR, _,
+ HasSubstr("Message is not a new interface response")));
+ GetInterfaceMessage non_interface_response_message;
+ TriggerOnWiFiInterfaceInfoReceived(non_interface_response_message);
+ Mock::VerifyAndClearExpectations(&log);
+
+ EXPECT_CALL(log, Log(logging::LOG_ERROR, _,
+ HasSubstr("Message contains no interface index")));
+ NewInterfaceMessage message;
+ TriggerOnWiFiInterfaceInfoReceived(message);
+ Mock::VerifyAndClearExpectations(&log);
+
+ message.attributes()->CreateAttribute(
+ NL80211_ATTR_IFINDEX, base::Bind(
+ &NetlinkAttribute::NewNl80211AttributeFromId));
+ message.attributes()->SetU32AttributeValue(NL80211_ATTR_IFINDEX,
+ kTestDeviceIndex);
+ EXPECT_CALL(log, Log(logging::LOG_ERROR, _,
+ HasSubstr("Message contains no interface type")));
+ TriggerOnWiFiInterfaceInfoReceived(message);
+ Mock::VerifyAndClearExpectations(&log);
+
+ message.attributes()->CreateAttribute(
+ NL80211_ATTR_IFTYPE, base::Bind(
+ &NetlinkAttribute::NewNl80211AttributeFromId));
+ message.attributes()->SetU32AttributeValue(NL80211_ATTR_IFTYPE,
+ NL80211_IFTYPE_AP);
+ EXPECT_CALL(log, Log(logging::LOG_ERROR, _,
+ HasSubstr("Could not find device info for interface")));
+ TriggerOnWiFiInterfaceInfoReceived(message);
+ Mock::VerifyAndClearExpectations(&log);
+
+ // Use the AddDelayedDevice() method to create a device info entry with no
+ // associated device.
+ AddDelayedDevice();
+
+ EXPECT_CALL(log, Log(logging::LOG_INFO, _,
+ HasSubstr("it is not in station mode")));
+ TriggerOnWiFiInterfaceInfoReceived(message);
+ Mock::VerifyAndClearExpectations(&log);
+ Mock::VerifyAndClearExpectations(&manager_);
+
+ message.attributes()->SetU32AttributeValue(NL80211_ATTR_IFTYPE,
+ NL80211_IFTYPE_STATION);
+ EXPECT_CALL(manager_, RegisterDevice(_));
+ EXPECT_CALL(manager_, device_info())
+ .WillRepeatedly(Return(&test_device_info_));
+ EXPECT_CALL(log, Log(_, _, _)).Times(AnyNumber());
+ EXPECT_CALL(log, Log(logging::LOG_INFO, _,
+ HasSubstr("Creating WiFi device")));
+ TriggerOnWiFiInterfaceInfoReceived(message);
+ Mock::VerifyAndClearExpectations(&log);
+ Mock::VerifyAndClearExpectations(&manager_);
+
+ EXPECT_CALL(manager_, RegisterDevice(_)).Times(0);
+ EXPECT_CALL(log, Log(logging::LOG_ERROR, _,
+ HasSubstr("Device already created for interface")));
+ TriggerOnWiFiInterfaceInfoReceived(message);
+}
+
} // namespace shill
diff --git a/mock_netlink_manager.h b/mock_netlink_manager.h
index a271931..1e2cf7a 100644
--- a/mock_netlink_manager.h
+++ b/mock_netlink_manager.h
@@ -15,6 +15,9 @@
public:
MockNetlinkManager() {}
~MockNetlinkManager() {}
+
+ MOCK_METHOD2(SendMessage, bool(NetlinkMessage *message,
+ const NetlinkMessageHandler &message_handler));
};
} // namespace shill
diff --git a/netlink_attribute.cc b/netlink_attribute.cc
index 80bace7..44f6eb0 100644
--- a/netlink_attribute.cc
+++ b/netlink_attribute.cc
@@ -62,6 +62,9 @@
case NL80211_ATTR_IFINDEX:
attr.reset(new Nl80211AttributeIfindex());
break;
+ case NL80211_ATTR_IFTYPE:
+ attr.reset(new Nl80211AttributeIftype());
+ break;
case NL80211_ATTR_KEY_IDX:
attr.reset(new Nl80211AttributeKeyIdx());
break;
diff --git a/netlink_manager.h b/netlink_manager.h
index 66cffb0..a0072c0 100644
--- a/netlink_manager.h
+++ b/netlink_manager.h
@@ -165,8 +165,8 @@
// installing a handler to deal with the kernel's response to the message.
// TODO(wdg): Eventually, this should also include a timeout and a callback
// to call in case of timeout.
- bool SendMessage(NetlinkMessage *message, const NetlinkMessageHandler
- &message_handler);
+ virtual bool SendMessage(NetlinkMessage *message,
+ const NetlinkMessageHandler &message_handler);
// Uninstall the handler for a specific netlink message.
bool RemoveMessageHandler(const NetlinkMessage &message);
diff --git a/nl80211_attribute.cc b/nl80211_attribute.cc
index 8ff93fa..873a65d 100644
--- a/nl80211_attribute.cc
+++ b/nl80211_attribute.cc
@@ -57,6 +57,9 @@
const int Nl80211AttributeIfindex::kName = NL80211_ATTR_IFINDEX;
const char Nl80211AttributeIfindex::kNameString[] = "NL80211_ATTR_IFINDEX";
+const int Nl80211AttributeIftype::kName = NL80211_ATTR_IFTYPE;
+const char Nl80211AttributeIftype::kNameString[] = "NL80211_ATTR_IFTYPE";
+
const int Nl80211AttributeKeyIdx::kName = NL80211_ATTR_KEY_IDX;
const char Nl80211AttributeKeyIdx::kNameString[] = "NL80211_ATTR_KEY_IDX";
diff --git a/nl80211_attribute.h b/nl80211_attribute.h
index 8952ae7..c10c1fd 100644
--- a/nl80211_attribute.h
+++ b/nl80211_attribute.h
@@ -91,6 +91,16 @@
DISALLOW_COPY_AND_ASSIGN(Nl80211AttributeIfindex);
};
+class Nl80211AttributeIftype : public NetlinkU32Attribute {
+ public:
+ static const int kName;
+ static const char kNameString[];
+ Nl80211AttributeIftype() : NetlinkU32Attribute(kName, kNameString) {}
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(Nl80211AttributeIftype);
+};
+
class Nl80211AttributeKeyType : public NetlinkU32Attribute {
public:
static const int kName;
diff --git a/nl80211_message.cc b/nl80211_message.cc
index 9b7e982..9b0fbb9 100644
--- a/nl80211_message.cc
+++ b/nl80211_message.cc
@@ -66,6 +66,11 @@
uint16_t Nl80211Message::nl80211_message_type_ = kIllegalMessageType;
// static
+uint16_t Nl80211Message::GetMessageType() {
+ return nl80211_message_type_;
+}
+
+// static
void Nl80211Message::SetMessageType(uint16_t message_type) {
if (message_type == NetlinkMessage::kIllegalMessageType) {
LOG(FATAL) << "Absolutely need a legal message type for Nl80211 messages.";
@@ -615,6 +620,18 @@
const char UnprotDisassociateMessage::kCommandString[] =
"NL80211_CMD_UNPROT_DISASSOCIATE";
+GetInterfaceMessage::GetInterfaceMessage()
+ : Nl80211Message(kCommand, kCommandString) {
+ attributes()->CreateAttribute(
+ NL80211_ATTR_IFINDEX, Bind(&NetlinkAttribute::NewNl80211AttributeFromId));
+}
+
+const uint8_t GetInterfaceMessage::kCommand = NL80211_CMD_GET_INTERFACE;
+const char GetInterfaceMessage::kCommandString[] = "NL80211_CMD_GET_INTERFACE";
+
+const uint8_t NewInterfaceMessage::kCommand = NL80211_CMD_NEW_INTERFACE;
+const char NewInterfaceMessage::kCommandString[] = "NL80211_CMD_NEW_INTERFACE";
+
// static
NetlinkMessage *Nl80211Message::CreateMessage(const nlmsghdr *const_msg) {
if (!const_msg) {
@@ -647,12 +664,16 @@
return new DisconnectMessage();
case FrameTxStatusMessage::kCommand:
return new FrameTxStatusMessage();
+ case GetInterfaceMessage::kCommand:
+ return new GetInterfaceMessage();
case GetRegMessage::kCommand:
return new GetRegMessage();
case JoinIbssMessage::kCommand:
return new JoinIbssMessage();
case MichaelMicFailureMessage::kCommand:
return new MichaelMicFailureMessage();
+ case NewInterfaceMessage::kCommand:
+ return new NewInterfaceMessage();
case NewScanResultsMessage::kCommand:
return new NewScanResultsMessage();
case NewStationMessage::kCommand:
diff --git a/nl80211_message.h b/nl80211_message.h
index 6d5c8ac..e3c6944 100644
--- a/nl80211_message.h
+++ b/nl80211_message.h
@@ -30,6 +30,9 @@
: GenericNetlinkMessage(nl80211_message_type_, command, command_string) {}
virtual ~Nl80211Message() {}
+ // Gets the family_id / message_type for all Nl80211 messages.
+ static uint16_t GetMessageType();
+
// Sets the family_id / message_type for all Nl80211 messages.
static void SetMessageType(uint16_t message_type);
@@ -428,6 +431,29 @@
DISALLOW_COPY_AND_ASSIGN(UnprotDisassociateMessage);
};
+
+class GetInterfaceMessage : public Nl80211Message {
+ public:
+ static const uint8_t kCommand;
+ static const char kCommandString[];
+
+ GetInterfaceMessage();
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(GetInterfaceMessage);
+};
+
+class NewInterfaceMessage : public Nl80211Message {
+ public:
+ static const uint8_t kCommand;
+ static const char kCommandString[];
+
+ NewInterfaceMessage() : Nl80211Message(kCommand, kCommandString) {}
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(NewInterfaceMessage);
+};
+
// Nl80211MessageDataCollector - this class is used to collect data to be
// used for unit tests. It is only invoked in this case.