2012-11-23 05:50:15

by Saravana

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

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(-)

diff --git a/net/mac80211/debugfs_sta.c b/net/mac80211/debugfs_sta.c
index 5ccec2c..d0508d6 100644
--- a/net/mac80211/debugfs_sta.c
+++ b/net/mac80211/debugfs_sta.c
@@ -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);
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index c88f161f..0905eda 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -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];

diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index ab63237..19baafc 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -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();
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index 8bd2f5c..87c0c3c 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -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++;




2012-11-23 06:39:31

by Saravana

[permalink] [raw]
Subject: Re: RFC[1/2]mac80211: Add Statistic to debugfs

Hi Julian,

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

On 11/23/2012 11:28 AM, Julian Calaby wrote:
> Hi Saravana,
>
> On Fri, Nov 23, 2012 at 4:50 PM, Saravana <[email protected]> wrote:
>> 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.
>> ---
>> 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(-)
>>
>> diff --git a/net/mac80211/status.c b/net/mac80211/status.c
>> index ab63237..19baafc 100644
>> --- a/net/mac80211/status.c
>> +++ b/net/mac80211/status.c
>> @@ -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.
>> }
>>
>> rcu_read_unlock();
>
> Thanks,
>


2012-11-26 09:12:50

by Johannes Berg

[permalink] [raw]
Subject: Re: RFC[1/2]mac80211: Add Statistic to debugfs

On Fri, 2012-11-23 at 15:26 +0530, Saravana wrote:
> Hi Johannes,
> On 11/23/2012 12:41 PM, Johannes Berg wrote:
> > On Fri, 2012-11-23 at 12:09 +0530, Saravana wrote:
> >
> >>> 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.
> >
> > [...]
> >> 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.

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


2012-11-23 05:58:43

by Julian Calaby

[permalink] [raw]
Subject: Re: RFC[1/2]mac80211: Add Statistic to debugfs

Hi Saravana,

On Fri, Nov 23, 2012 at 4:50 PM, Saravana <[email protected]> wrote:
> 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]>

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.

> ---
> 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(-)
>
> diff --git a/net/mac80211/status.c b/net/mac80211/status.c
> index ab63237..19baafc 100644
> --- a/net/mac80211/status.c
> +++ b/net/mac80211/status.c
> @@ -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.

> }
>
> rcu_read_unlock();

Thanks,

--
Julian Calaby

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

2012-11-23 07:11:28

by Johannes Berg

[permalink] [raw]
Subject: Re: RFC[1/2]mac80211: Add Statistic to debugfs

On Fri, 2012-11-23 at 12:09 +0530, Saravana wrote:

> > 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.

[...]
> 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


2012-11-23 09:56:25

by Saravana

[permalink] [raw]
Subject: Re: RFC[1/2]mac80211: Add Statistic to debugfs

Hi Johannes,
On 11/23/2012 12:41 PM, Johannes Berg wrote:
> On Fri, 2012-11-23 at 12:09 +0530, Saravana wrote:
>
>>> 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.
>
> [...]
>> 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.
> johannes
>
>


2012-11-23 06:53:54

by Saravana

[permalink] [raw]
Subject: Re: RFC[1/2]mac80211: Add Statistic to debugfs

On 11/23/2012 12:09 PM, Saravana wrote:
> Hi Julian,
>
> Thanks for the comments. I will post individual statistics as individual
> patch.
> Please find my comments inline.
>
> On 11/23/2012 11:28 AM, Julian Calaby wrote:
>> Hi Saravana,
>>
>> On Fri, Nov 23, 2012 at 4:50 PM, Saravana <[email protected]> wrote:
>>> 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.
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.
>>> ---
>>> 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(-)
>>>
>>> diff --git a/net/mac80211/status.c b/net/mac80211/status.c
>>> index ab63237..19baafc 100644
>>> --- a/net/mac80211/status.c
>>> +++ b/net/mac80211/status.c
>>> @@ -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.
>>> }
>>>
>>> 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
>
>