diff mbox series

nl80211: Add support to get BIP failure counters

Message ID 1593498381-9337-1-git-send-email-vmaloo@codeaurora.org
State Under Review
Delegated to: Johannes Berg
Headers show
Series nl80211: Add support to get BIP failure counters | expand

Commit Message

Vinita S. Maloo June 30, 2020, 6:26 a.m. UTC
Add support to get number of MIC errors, missing MME incidents and
packet replay incidents observed while using IGTK/BIGTK keys when
PMF and/or beacon protection features are enabled.

Signed-off-by: Vinita S. Maloo <vmaloo@codeaurora.org>

Comments

Johannes Berg July 30, 2020, 11:41 a.m. UTC | #1
On Tue, 2020-06-30 at 11:56 +0530, Vinita S. Maloo wrote:
> Add support to get number of MIC errors, missing MME incidents and
> packet replay incidents observed while using IGTK/BIGTK keys when
> PMF and/or beacon protection features are enabled.

I can imagine you need this for WFA tests, but why are the debugfs
counters not enough for that?

I don't really see much functional reason (wpa_supplicant or so) to have
this, and thus why expose it in the nl80211 API?

johannes
Jouni Malinen July 31, 2020, 3:28 p.m. UTC | #2
On Thu, Jul 30, 2020 at 01:41:04PM +0200, Johannes Berg wrote:
> On Tue, 2020-06-30 at 11:56 +0530, Vinita S. Maloo wrote:
> > Add support to get number of MIC errors, missing MME incidents and
> > packet replay incidents observed while using IGTK/BIGTK keys when
> > PMF and/or beacon protection features are enabled.
> 
> I can imagine you need this for WFA tests, but why are the debugfs
> counters not enough for that?
> 
> I don't really see much functional reason (wpa_supplicant or so) to have
> this, and thus why expose it in the nl80211 API?

Do we have a policy or some kind of preference on how to address dot11
statistics counters that can be handy at debugging real production use
error cases? For this particular case, is there objection to adding
support for fetching dot11RSNAStatsCMACReplays and
dot11RSNAStatsCMACReplays counter values?
Johannes Berg Sept. 17, 2020, 10:10 a.m. UTC | #3
On Fri, 2020-07-31 at 18:28 +0300, Jouni Malinen wrote:
> On Thu, Jul 30, 2020 at 01:41:04PM +0200, Johannes Berg wrote:
> > On Tue, 2020-06-30 at 11:56 +0530, Vinita S. Maloo wrote:
> > > Add support to get number of MIC errors, missing MME incidents and
> > > packet replay incidents observed while using IGTK/BIGTK keys when
> > > PMF and/or beacon protection features are enabled.
> > 
> > I can imagine you need this for WFA tests, but why are the debugfs
> > counters not enough for that?
> > 
> > I don't really see much functional reason (wpa_supplicant or so) to have
> > this, and thus why expose it in the nl80211 API?
> 
> Do we have a policy or some kind of preference on how to address dot11
> statistics counters that can be handy at debugging real production use
> error cases? 

Not really ...

> For this particular case, is there objection to adding
> support for fetching dot11RSNAStatsCMACReplays and
> dot11RSNAStatsCMACReplays counter values?

Do we really want to add them that way, or should they be at a key
level? To me it always felt a bit like they should be per key, and we
expose some counters that way in debugfs I believe.

OTOH, you can argue that they are MIBs today, so they should be exposed
as such? And maybe firmware doesn't support per-key data, etc.

So I don't really have a strong opinion on it, but I feel that we have a
hodge-podge of things in this area, doing one thing with debugfs,
another thing with nl80211, etc.

johannes
diff mbox series

Patch

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index e2dbc9c0..ade5ade 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1556,6 +1556,16 @@  struct cfg80211_tid_stats {
  *	an FCS error. This counter should be incremented only when TA of the
  *	received packet with an FCS error matches the peer MAC address.
  * @airtime_link_metric: mesh airtime link metric.
+ * @bip_mic_error_count: number of group addressed robust mgmt. frames received
+ *	from this station with invalid MIC or missing MME when PMF is enabled.
+ * @bip_replay_count: number of group addressed robust mgmt. frames received
+ *	from this station with packet number less than or equal to the last
+ *	received packet number (Replay packets) when PMF is enabled.
+ * @beacon_mic_error_count: number of beacons received from this station with
+ *	invalid MIC or missing MME when beacon protection is enabled.
+ * @beacon_replay_count: number of beacons received from this station with
+ *	packet number less than or equal to the last received packet number
+ *	(Replay packets) when beacon protection is enabled.
  */
 struct station_info {
 	u64 filled;
@@ -1613,6 +1623,11 @@  struct station_info {
 	u32 fcs_err_count;
 
 	u32 airtime_link_metric;
+
+	u32 bip_mic_error_count;
+	u32 bip_replay_count;
+	u32 beacon_mic_error_count;
+	u32 beacon_replay_count;
 };
 
 #if IS_ENABLED(CONFIG_CFG80211)
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index c14666b..a06b2c4 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -3365,6 +3365,18 @@  enum nl80211_sta_bss_param {
  * @NL80211_STA_INFO_AIRTIME_LINK_METRIC: airtime link metric for mesh station
  * @NL80211_STA_INFO_ASSOC_AT_BOOTTIME: Timestamp (CLOCK_BOOTTIME, nanoseconds)
  *	of STA's association
+ * @NL80211_STA_INFO_BIP_MIC_ERROR_COUNT: number of group addressed robust mgmt.
+ *	frames received from this station with invalid MIC or missing MME when
+ *	PMF is enabled (u32).
+ * @NL80211_STA_INFO_BIP_REPLAY_COUNT: number of group addressed robust mgmt.
+ *	frames received from this station with packet number less than or equal
+ *	to the last received packet number when PMF is enabled (u32)
+ * @NL80211_STA_INFO_BEACON_MIC_ERROR_COUNT: number of beacons received from
+ *	this station with invalid MIC or missing MME when beacon protection is
+ *	enabled (u32)
+ * @NL80211_STA_INFO_BEACON_REPLAY_COUNT: number of beacons received from this
+ *	station with packet number less than or equal to the last received
+ *	packet number when beacon protection is enabled (u32)
  * @__NL80211_STA_INFO_AFTER_LAST: internal
  * @NL80211_STA_INFO_MAX: highest possible station info attribute
  */
@@ -3412,6 +3424,10 @@  enum nl80211_sta_info {
 	NL80211_STA_INFO_AIRTIME_WEIGHT,
 	NL80211_STA_INFO_AIRTIME_LINK_METRIC,
 	NL80211_STA_INFO_ASSOC_AT_BOOTTIME,
+	NL80211_STA_INFO_BIP_MIC_ERROR_COUNT,
+	NL80211_STA_INFO_BIP_REPLAY_COUNT,
+	NL80211_STA_INFO_BEACON_MIC_ERROR_COUNT,
+	NL80211_STA_INFO_BEACON_REPLAY_COUNT,
 
 	/* keep last */
 	__NL80211_STA_INFO_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 22c4d13..bc6767c 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5409,6 +5409,10 @@  static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
 	PUT_SINFO(BEACON_SIGNAL_AVG, rx_beacon_signal_avg, u8);
 	PUT_SINFO(RX_MPDUS, rx_mpdu_count, u32);
 	PUT_SINFO(FCS_ERROR_COUNT, fcs_err_count, u32);
+	PUT_SINFO(BIP_MIC_ERROR_COUNT, bip_mic_error_count, u32);
+	PUT_SINFO(BIP_REPLAY_COUNT, bip_replay_count, u32);
+	PUT_SINFO(BEACON_MIC_ERROR_COUNT, beacon_mic_error_count, u32);
+	PUT_SINFO(BEACON_REPLAY_COUNT, beacon_replay_count, u32);
 	if (wiphy_ext_feature_isset(&rdev->wiphy,
 				    NL80211_EXT_FEATURE_ACK_SIGNAL_SUPPORT)) {
 		PUT_SINFO(ACK_SIGNAL, ack_signal, u8);