shill: Fixes disconnect UMA stat
To determine the kernel's disconnect notification behaviour for these
UMA stats, I originally reverse-engineered the disconnect netlink
messages that we get from the kernel (should have looked at the kernel
code which, eventually, I did). I found, erroneously, that all of the
disconnect reasons were found in the disassociate message. It turns out
that the disconnect reasons are from the disassociate message only for
(some!) disconnections instigated by the station. When the AP is the
source of the disconnect, the reason is stored in a disconnect message.
This code, then, extracts the reason code from whatever message type
(disconnect or disassociate) that contains one. The message will still
be a little noisy since some station-started disconnections don't
provide a disassociate message and I've noticed that some AP-started
disconnections seem to send 2 messages. I haven't tracked any of this
down.
BUG=chromium:215808
TEST=unittest (new) and manual - I test using the AP and description
found in chromium:294315.
CQ-DEPEND=Icf79aa729b1ed125743686c4536fe1b59183fed2
Change-Id: If648d530c613f485c074acf58ddb0bca4de22084
Reviewed-on: https://chromium-review.googlesource.com/170926
Reviewed-by: Wade Guthrie <wdg@chromium.org>
Commit-Queue: Wade Guthrie <wdg@chromium.org>
Tested-by: Wade Guthrie <wdg@chromium.org>
diff --git a/callback80211_metrics.cc b/callback80211_metrics.cc
index dedf2ab..7ce42ed 100644
--- a/callback80211_metrics.cc
+++ b/callback80211_metrics.cc
@@ -12,38 +12,80 @@
namespace shill {
-Callback80211Metrics::Callback80211Metrics(
- const NetlinkManager &netlink_manager, Metrics *metrics)
+Callback80211Metrics::Callback80211Metrics(Metrics *metrics)
: metrics_(metrics) {}
+IEEE_80211::WiFiReasonCode Callback80211Metrics::WiFiReasonCodeFromUint16(
+ uint16_t reason) const {
+ IEEE_80211::WiFiReasonCode reason_enum = IEEE_80211::kReasonCodeInvalid;
+ if (reason == IEEE_80211::kReasonCodeReserved0 ||
+ reason == IEEE_80211::kReasonCodeReserved12 ||
+ (reason >= IEEE_80211::kReasonCodeReservedBegin25 &&
+ reason <= IEEE_80211::kReasonCodeReservedEnd31) ||
+ (reason >= IEEE_80211::kReasonCodeReservedBegin40 &&
+ reason <= IEEE_80211::kReasonCodeReservedEnd44) ||
+ reason >= IEEE_80211::kReasonCodeMax) {
+ SLOG(WiFi, 1) << "Invalid reason code in disconnect message";
+ reason_enum = IEEE_80211::kReasonCodeInvalid;
+ } else {
+ reason_enum = static_cast<IEEE_80211::WiFiReasonCode>(reason);
+ }
+ return reason_enum;
+}
+
void Callback80211Metrics::CollectDisconnectStatistics(
const NetlinkMessage &netlink_message) {
- // We only handle deauthenticate messages, which are nl80211 messages.
+ if (!metrics_) {
+ return;
+ }
+ // We only handle disconnect and deauthenticate messages, both of which are
+ // nl80211 messages.
if (netlink_message.message_type() != Nl80211Message::GetMessageType()) {
return;
}
const Nl80211Message &message =
* reinterpret_cast<const Nl80211Message *>(&netlink_message);
- if (metrics_ &&
- message.command() == DeauthenticateMessage::kCommand) {
+ // Station-instigated disconnects provide their information in the
+ // deauthenticate message but AP-instigated disconnects provide it in the
+ // disconnect message.
+ uint16_t reason = IEEE_80211::kReasonCodeUnspecified;
+ if (message.command() == DeauthenticateMessage::kCommand) {
SLOG(WiFi, 3) << "Handling Deauthenticate Message";
- Metrics::WiFiDisconnectByWhom by_whom =
- message.const_attributes()->IsFlagAttributeTrue(
- NL80211_ATTR_DISCONNECTED_BY_AP) ?
- Metrics::kDisconnectedByAp : Metrics::kDisconnectedNotByAp;
- uint16_t reason = static_cast<uint16_t>(
- IEEE_80211::kReasonCodeInvalid);
+ message.Print(3, 3);
+ // If there's no frame, this is probably an AP-caused disconnect and
+ // there'll be a disconnect message to tell us about that.
ByteString rawdata;
- if (message.const_attributes()->GetRawAttributeValue(NL80211_ATTR_FRAME,
+ if (!message.const_attributes()->GetRawAttributeValue(NL80211_ATTR_FRAME,
&rawdata)) {
- Nl80211Frame frame(rawdata);
- reason = frame.reason();
+ SLOG(WiFi, 5) << "No frame in deauthenticate message, ignoring";
+ return;
}
- IEEE_80211::WiFiReasonCode reason_enum =
- static_cast<IEEE_80211::WiFiReasonCode>(reason);
- metrics_->Notify80211Disconnect(by_whom, reason_enum);
+ Nl80211Frame frame(rawdata);
+ reason = frame.reason();
+ } else if (message.command() == DisconnectMessage::kCommand) {
+ SLOG(WiFi, 3) << "Handling Disconnect Message";
+ message.Print(3, 3);
+ // If there's no reason code, this is probably a STA-caused disconnect and
+ // there was be a disconnect message to tell us about that.
+ if (!message.const_attributes()->GetU16AttributeValue(
+ NL80211_ATTR_REASON_CODE, &reason)) {
+ SLOG(WiFi, 5) << "No reason code in disconnect message, ignoring";
+ return;
+ }
+ } else {
+ return;
}
+
+ IEEE_80211::WiFiReasonCode reason_enum = WiFiReasonCodeFromUint16(reason);
+
+ Metrics::WiFiDisconnectByWhom by_whom =
+ message.const_attributes()->IsFlagAttributeTrue(
+ NL80211_ATTR_DISCONNECTED_BY_AP) ? Metrics::kDisconnectedByAp :
+ Metrics::kDisconnectedNotByAp;
+ SLOG(WiFi, 1) << "Notify80211Disconnect by " << (by_whom ? "station" : "AP")
+ << " because:" << reason_enum;
+ metrics_->Notify80211Disconnect(by_whom, reason_enum);
}
} // namespace shill.