RFC[1/2]mac80211: Add Statistic to debugfs

PATCH Saravana 3 participants 7 messages Nov 23, 2012 – Nov 26, 2012 mac80211

lore

Share & tools

lore

Saravana

author

2012-11-23 05:50 UTC #0 lore raw

Provide additional statistic support to the debugfs.

The statistic that are added are the

1. Number of rx packets dropped due to mic failure
- Currently the debugfs has support for number of rx packets dropped.
But it doesn't classify the reason for the drop. This statistic will count the number of rx packet dropped due to mic failure.

2. Rssi of the Last Ack Frame.
- This information is available as part of ieee80211_tx_info status
structure. Thought this information may be useful(in future).

Signed-off-by Saravana <[email protected]>

---
The diff file is generated from wireless-testing git tree.

net/mac80211/debugfs_sta.c | 3 +++

 net/mac80211/sta_info.h    |    4 ++++
 net/mac80211/status.c      |    2 ++
 net/mac80211/wpa.c         |    1 +
 4 files changed, 10 insertions(+), 0 deletions(-)

net/mac80211/debugfs_sta.c +0 -0

index 5ccec2c..d0508d6 100644
@@ -52,6 +52,7 @@ static const struct file_operations sta_ ##name##

_ops = { \
STA_FILE(aid, sta.aid, D);
STA_FILE(dev, sdata->name, S);
STA_FILE(last_signal, last_signal, D);
+STA_FILE(last_ack_signal, last_ack_signal, D);

static ssize_t sta_flags_read(struct file *file, char __user *userbuf,
size_t count, loff_t *ppos)
@@ -366,6 +367,7 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
DEBUGFS_ADD(dev);
DEBUGFS_ADD(last_signal);
DEBUGFS_ADD(ht_capa);
+ DEBUGFS_ADD(last_ack_signal);

DEBUGFS_ADD_COUNTER(rx_packets, rx_packets); DEBUGFS_ADD_COUNTER(tx_packets, tx_packets);
@@ -374,6 +376,7 @@ void ieee80211_sta_debugfs_add(struct sta_info *sta)
DEBUGFS_ADD_COUNTER(rx_duplicates, num_duplicates); DEBUGFS_ADD_COUNTER(rx_fragments, rx_fragments); DEBUGFS_ADD_COUNTER(rx_dropped, rx_dropped);
+ DEBUGFS_ADD_COUNTER(rx_dropped_mic_failure,rx_dropped_mic_failure);
DEBUGFS_ADD_COUNTER(tx_fragments, tx_fragments); DEBUGFS_ADD_COUNTER(tx_filtered, tx_filtered_count); DEBUGFS_ADD_COUNTER(tx_retry_failed, tx_retry_failed);

net/mac80211/sta_info.h +1 -0

index c88f161f..0905eda 100644
@@ -248,8 +248,10 @@ struct sta_ampdu_mlme {
  * @num_duplicates: number of duplicate frames received from this STA
  * @rx_fragments: number of received MPDUs
  * @rx_dropped: number of dropped MPDUs from this STA
+ * @rx_dropped_mic_failure: number of dropped MPDUs due to mic failure

from this STA
* @last_signal: signal of last received frame from this STA
* @avg_signal: moving average of signal of received frames from this STA
+ * @last_ack_signal: signal of last received Ack frame from this STA
* @last_seq_ctrl: last received seq/frag number from this STA (per RX
queue)
* @tx_filtered_count: number of frames the hardware filtered for this STA
* @tx_retry_failed: number of frames that failed retry
@@ -327,8 +329,10 @@ struct sta_info {
unsigned long num_duplicates;
unsigned long rx_fragments;
unsigned long rx_dropped;
+ unsigned long rx_dropped_mic_failure;
int last_signal;
struct ewma avg_signal;
+ int last_ack_signal;
/* Plus 1 for non-QoS frames */
__le16 last_seq_ctrl[NUM_RX_DATA_QUEUES + 1];

net/mac80211/status.c +0 -0

index ab63237..19baafc 100644
@@ -539,6 +539,8 @@ void ieee80211_tx_status(struct ieee80211_hw *hw,

struct sk_buff *skb)
sta->lost_packets = 0;
}
}
+ if(acked)
+ sta->last_ack_signal=info->status.ack_signal;
}

rcu_read_unlock();

net/mac80211/wpa.c +1 -0

index 8bd2f5c..87c0c3c 100644
@@ -166,6 +166,7 @@ mic_fail:
 	 * a driver that supports HW encryption. Send up the key idx only if
 	 * the key is set.
 	 */
+	 rx->sta->rx_dropped_mic_failure++;

Julian Calaby

2012-11-23 05:58 UTC #1 lore raw

Hi Saravana,

Quoted 9 lines · Saravana

Provide additional statistic support to the debugfs.
The statistic that are added are the
1. Number of rx packets dropped due to mic failure
- Currently the debugfs has support for number of rx packets
dropped. But it doesn't classify the reason for the drop. This statistic will count the number of rx packet dropped due to mic failure.

Quoted 5 lines

2. Rssi of the Last Ack Frame.
- This information is available as part of ieee80211_tx_info status
structure. Thought this information may be useful(in future).
Signed-off-by Saravana <[email protected]>

Firstly, these two changes should be in separate patches.

Also, as you're adding new statistics which are only used when debugfs is enabled, you might want to wrap them in #ifdefs so that they're only compiled when debugfs support is compiled.

Quoted 14 lines

---
The diff file is generated from wireless-testing git tree.
net/mac80211/debugfs_sta.c | 3 +++
 net/mac80211/sta_info.h    |    4 ++++
 net/mac80211/status.c      |    2 ++
 net/mac80211/wpa.c         |    1 +
 4 files changed, 10 insertions(+), 0 deletions(-)
net/mac80211/status.c +0 -0
index ab63237..19baafc 100644

net/mac80211/status.c void ieee80211_tx_status(struct ieee80211_hw *hw, · line 544

Quoted 6 lines

net/mac80211/status.c +2 -0

@@ -539,6 +539,8 @@ void ieee80211_tx_status(struct ieee80211_hw *hw,
 struct sk_buff *skb)
                                sta->lost_packets = 0;
                        }
                }
+               if(acked)
+               sta->last_ack_signal=info->status.ack_signal;

Either the indentation here is wrong or your mailer mangled the patch. Either way it should get fixed up when you resubmit.

Quoted 3 lines

net/mac80211/status.c +0 -0

        rcu_read_unlock();

Thanks,

Signature · 5 lines

Julian Calaby

Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

Saravana

author

2012-11-23 06:39 UTC #2 lore raw

Hi Julian,

Thanks for the comments. I will post individual statistics as individual patch. Please find my comments inline.

Quoted 9 lines · Julian Calaby

Hi Saravana,
Quoted 2 lines · Saravana
Provide additional statistic support to the debugfs.
Also, as you're adding new statistics which are only used when debugfs is enabled, you might want to wrap them in #ifdefs so that they're only compiled when debugfs support is compiled.

The function ieee80211_sta_debugfs_add(struct sta_info *sta) which adds the debugfs will try to create the directory first.This debugfs_create_dir(mac, stations_dir) will be return a valid dentry only when CONFIG_DEBUG_FS is defined.if CONFIG_DEBUG_FS is not defined then dentry will be invalid and hence the ieee80211_sta_debugfs_add() will return without adding the files. Hence we don't need to wrap this up in #ifdef as it is already taken care during the directory creation.

Quoted 9 lines

Quoted 9 lines
---
The diff file is generated from wireless-testing git tree.
net/mac80211/debugfs_sta.c | 3 +++
  net/mac80211/sta_info.h    |    4 ++++
  net/mac80211/status.c      |    2 ++
  net/mac80211/wpa.c         |    1 +
  4 files changed, 10 insertions(+), 0 deletions(-)

Quoted 15 lines

net/mac80211/status.c void ieee80211_tx_status(struct ieee80211_hw *hw, · line 544
Quoted 11 lines
net/mac80211/status.c +2 -0
index ab63237..19baafc 100644
@@ -539,6 +539,8 @@ void ieee80211_tx_status(struct ieee80211_hw *hw,
 struct sk_buff *skb)
                                 sta->lost_packets = 0;
                         }
                 }
+               if(acked)
+               sta->last_ack_signal=info->status.ack_signal;
Either the indentation here is wrong or your mailer mangled the patch. Either way it should get fixed up when you resubmit.

+ if (acked)
+ sta->last_ack_signal=info->status.ack_signal;
Please find the corrected indentation.

Quoted 6 lines

Quoted 3 lines
net/mac80211/status.c +0 -0
         }
 
         rcu_read_unlock();
Thanks,

Saravana

author

2012-11-23 06:53 UTC #3 lore raw

Quoted 17 lines · Saravana

Hi Julian,
Thanks for the comments. I will post individual statistics as individual patch.
Please find my comments inline.
Quoted 9 lines · Julian Calaby
Hi Saravana,
Quoted 2 lines · Saravana
Provide additional statistic support to the debugfs.
Also, as you're adding new statistics which are only used when debugfs is enabled, you might want to wrap them in #ifdefs so that they're only compiled when debugfs support is compiled.

Quoted 8 lines

The function ieee80211_sta_debugfs_add(struct sta_info *sta) which adds the debugfs will try to create the directory first.This debugfs_create_dir(mac, stations_dir) will be return a valid dentry only when CONFIG_DEBUG_FS is defined.if CONFIG_DEBUG_FS is not defined then dentry will be invalid and hence the ieee80211_sta_debugfs_add() will return without adding the files.
Hence we don't need to wrap this up in #ifdef as it is already taken care during the directory creation.

Just One another thing. DEBUGFS_ADD() will internally use debugfs_create_file(). This debugfs_create_file() will return be a dummy function (returning invalid) when CONFIG_DEBUG_FS is not defined. Hence the predefined debugfs macros are taking care of this.

Quoted 41 lines

Quoted 24 lines
Quoted 14 lines
---
The diff file is generated from wireless-testing git tree.
net/mac80211/debugfs_sta.c | 3 +++
  net/mac80211/sta_info.h    |    4 ++++
  net/mac80211/status.c      |    2 ++
  net/mac80211/wpa.c         |    1 +
  4 files changed, 10 insertions(+), 0 deletions(-)
net/mac80211/status.c +0 -0
index ab63237..19baafc 100644
net/mac80211/status.c void ieee80211_tx_status(struct ieee80211_hw *hw, · line 544
Quoted 6 lines
net/mac80211/status.c +2 -0
@@ -539,6 +539,8 @@ void ieee80211_tx_status(struct ieee80211_hw *hw,
 struct sk_buff *skb)
                                 sta->lost_packets = 0;
                         }
                 }
+               if(acked)
+               sta->last_ack_signal=info->status.ack_signal;
Either the indentation here is wrong or your mailer mangled the patch. Either way it should get fixed up when you resubmit.
(quoted diff) +2 -0
+        if (acked)
+            sta->last_ack_signal=info->status.ack_signal;
Please find the corrected indentation.
Quoted 6 lines
Quoted 3 lines
net/mac80211/status.c +0 -0
         }
 
         rcu_read_unlock();
Thanks,
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Johannes Berg

maintainer

2012-11-23 07:11 UTC #4 lore raw

In reply to Saravana

Quoted 3 lines · Saravana

Quoted 3 lines
Also, as you're adding new statistics which are only used when debugfs is enabled, you might want to wrap them in #ifdefs so that they're only compiled when debugfs support is compiled.

[...]

Quoted 2 lines

Hence we don't need to wrap this up in #ifdef as it is already taken care during the directory creation.

I think Julian is arguing that you should have the *counters* themselves be under ifdef, and I tend to agree.

johannes

Saravana

author

2012-11-23 09:56 UTC #5 lore raw

Hi Johannes,

Quoted 6 lines · Johannes Berg

Quoted 3 lines · Saravana
Quoted 3 lines
Also, as you're adding new statistics which are only used when debugfs is enabled, you might want to wrap them in #ifdefs so that they're only compiled when debugfs support is compiled.

Quoted 7 lines

[...]
Quoted 2 lines
Hence we don't need to wrap this up in #ifdef as it is already taken care during the directory creation.
I think Julian is arguing that you should have the *counters* themselves be under ifdef, and I tend to agree.

When we went through the make file of mac80211, it is seen that the only when CONFIG_MAC80211_DEBUGFS macro is enabled, the files debugfs.c, debugfs_sta.c, debugfs_netdev.c, debugfs_key.c are compiled in.

So it may not be required to wrap up any piece of code in these files are they will not be compiled in the first place when the macro CONFIG_MAC80211_DEBUGFS is disabled. Let me know your thoughts on this.

Quoted 3 lines

johannes

Johannes Berg

maintainer

2012-11-26 09:12 UTC #6 lore raw

Quoted 15 lines · Saravana

Hi Johannes,
Quoted 6 lines · Johannes Berg
Quoted 3 lines · Saravana
Quoted 3 lines
Also, as you're adding new statistics which are only used when debugfs is enabled, you might want to wrap them in #ifdefs so that they're only compiled when debugfs support is compiled.
Quoted 7 lines
[...]
Quoted 2 lines
Hence we don't need to wrap this up in #ifdef as it is already taken care during the directory creation.
I think Julian is arguing that you should have the *counters* themselves be under ifdef, and I tend to agree.

Quoted 7 lines

When we went through the make file of mac80211, it is seen that the only when CONFIG_MAC80211_DEBUGFS macro is enabled, the files debugfs.c, debugfs_sta.c, debugfs_netdev.c, debugfs_key.c are compiled in.
So it may not be required to wrap up any piece of code in these files are they will not be compiled in the first place when the macro CONFIG_MAC80211_DEBUGFS is disabled.

You're thinking of the wrong code -- Julian was saying the *counters* themselves could be ifdef'ed.

I'd say you don't want to ifdef every single counter, but check which ones could be expensive to keep etc.

johannes