[ath9k-devel] [PATCH v2] ath9k: collect statistics about Rx-Dup and Rx-STBC packets
Collect statistics about recived duplicate and STBC packets. This information should help see if STBC is actually working. Tested on ar9285; Should work for all chips after ar9280. Changes: - v2. test for stbc vield only on ar9280 and later. reanme rx_gi to rx_short_gi Signed-off-by: Oleksij Rempel li...@rempel-privat.de --- drivers/net/wireless/ath/ath9k/debug.c | 20 +++- drivers/net/wireless/ath/ath9k/debug.h | 4 drivers/net/wireless/ath/ath9k/mac.c | 7 +++ drivers/net/wireless/ath/ath9k/mac.h | 13 ++--- 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c index e6307b8..fec68f3 100644 --- a/drivers/net/wireless/ath/ath9k/debug.c +++ b/drivers/net/wireless/ath/ath9k/debug.c @@ -848,7 +848,7 @@ static ssize_t read_file_recv(struct file *file, char __user *user_buf, struct ath_softc *sc = file-private_data; char *buf; - unsigned int len = 0, size = 1600; + unsigned int len = 0, size = 2048; ssize_t retval = 0; buf = kzalloc(size, GFP_KERNEL); @@ -900,6 +900,11 @@ static ssize_t read_file_recv(struct file *file, char __user *user_buf, RXS_ERR(RX-Frags, rx_frags); RXS_ERR(RX-Spectral, rx_spectral); + RXS_ERR(RX-ShortGI, rx_short_gi); + RXS_ERR(RX-HT40, rx_ht40); + RXS_ERR(RX-Duplicate, rx_duplicate); + RXS_ERR(RX-STBC, rx_stbc); + if (len size) len = size; @@ -939,6 +944,14 @@ void ath_debug_stat_rx(struct ath_softc *sc, struct ath_rx_status *rs) if (rs-rs_phyerr ATH9K_PHYERR_MAX) RX_PHY_ERR_INC(rs-rs_phyerr); } + if (rs-rs_flags ATH9K_RX_GI) + RX_STAT_INC(rx_short_gi); + if (rs-rs_flags ATH9K_RX_2040) + RX_STAT_INC(rx_ht40); + if (rs-rs_flags_2 ATH9K_RX_DUP) + RX_STAT_INC(rx_duplicate); + if (rs-rs_flags_2 ATH9K_RX_STBC) + RX_STAT_INC(rx_stbc); #ifdef CONFIG_ATH9K_MAC_DEBUG spin_lock(sc-debug.samp_lock); @@ -1993,6 +2006,11 @@ void ath9k_get_et_stats(struct ieee80211_hw *hw, AWDATA(data_underrun); AWDATA(delim_underrun); + AWDATA_RX(rx_short_gi); + AWDATA_RX(rx_ht40); + AWDATA_RX(rx_duplicate); + AWDATA_RX(rx_stbc); + AWDATA_RX(crc_err); AWDATA_RX(decrypt_crc_err); AWDATA_RX(phy_err); diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h index 794a7ec..639dae9 100644 --- a/drivers/net/wireless/ath/ath9k/debug.h +++ b/drivers/net/wireless/ath/ath9k/debug.h @@ -241,6 +241,10 @@ struct ath_rx_stats { u32 rx_beacons; u32 rx_frags; u32 rx_spectral; + u32 rx_short_gi; + u32 rx_ht40; + u32 rx_duplicate; + u32 rx_stbc; }; struct ath_stats { diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c index 498fee0..2064d45 100644 --- a/drivers/net/wireless/ath/ath9k/mac.c +++ b/drivers/net/wireless/ath/ath9k/mac.c @@ -547,6 +547,7 @@ int ath9k_hw_rxprocdesc(struct ath_hw *ah, struct ath_desc *ds, rs-rs_status = 0; rs-rs_flags = 0; + rs-rs_flags_2 = 0; rs-rs_datalen = ads.ds_rxstatus1 AR_DataLen; rs-rs_tstamp = ads.AR_RcvTimestamp; @@ -590,6 +591,12 @@ int ath9k_hw_rxprocdesc(struct ath_hw *ah, struct ath_desc *ds, (ads.ds_rxstatus3 AR_GI) ? ATH9K_RX_GI : 0; rs-rs_flags |= (ads.ds_rxstatus3 AR_2040) ? ATH9K_RX_2040 : 0; + rs-rs_flags_2 |= + (ads.ds_rxstatus3 AR_RXST_DUP) ? ATH9K_RX_DUP : 0; + + if (AR_SREV_9280_20_OR_LATER(ah)) + rs-rs_flags_2 |= + (ads.ds_rxstatus3 AR_RXST_STBC) ? ATH9K_RX_STBC : 0; if (ads.ds_rxstatus8 AR_PreDelimCRCErr) rs-rs_flags |= ATH9K_RX_DELIM_CRC_PRE; diff --git a/drivers/net/wireless/ath/ath9k/mac.h b/drivers/net/wireless/ath/ath9k/mac.h index 5865f92..5e5a3af 100644 --- a/drivers/net/wireless/ath/ath9k/mac.h +++ b/drivers/net/wireless/ath/ath9k/mac.h @@ -143,6 +143,7 @@ struct ath_rx_status { u8 rs_moreaggr; u8 rs_num_delims; u8 rs_flags; + u8 rs_flags_2; bool is_mybeacon; u32 evm0; u32 evm1; @@ -185,6 +186,7 @@ struct ath_htc_rx_status { #define ATH9K_RXERR_KEYMISS 0x20 #define ATH9K_RXERR_CORRUPT_DESC 0x40 +/* rs_flags */ #define ATH9K_RX_MORE 0x01 #define ATH9K_RX_MORE_AGGR0x02 #define ATH9K_RX_GI 0x04 @@ -193,6 +195,10 @@ struct ath_htc_rx_status { #define ATH9K_RX_DELIM_CRC_POST 0x20 #define ATH9K_RX_DECRYPT_BUSY 0x40 +/* rs_flags_2 */ +#define ATH9K_RX_DUP 0x01 +#define ATH9K_RX_STBC 0x02 + #define ATH9K_RXKEYIX_INVALID ((u8)-1) #define ATH9K_TXKEYIX_INVALID ((u8)-1) @@ -529,11
Re: [ath9k-devel] [PATCH v2] ath9k: collect statistics about Rx-Dup and Rx-STBC packets
Hiya, Why not just bump rs_flags to be a u16, rather than a u8? then you don't need an rs_flags_2. (And then go and re-align things inside that struct so you don't waste space.) Adrian ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH v2] ath9k: collect statistics about Rx-Dup and Rx-STBC packets
Am 27.04.2013 20:51, schrieb Adrian Chadd: Hiya, Why not just bump rs_flags to be a u16, rather than a u8? then you don't need an rs_flags_2. ok (And then go and re-align things inside that struct so you don't waste space.) hmm.. what do you mean here? -- Regards, Oleksij ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH v2] ath9k: collect statistics about Rx-Dup and Rx-STBC packets
On 27 April 2013 11:53, Oleksij Rempel li...@rempel-privat.de wrote: (And then go and re-align things inside that struct so you don't waste space.) hmm.. what do you mean here? Structure alignment? Well, you typically want to have everything be dword aligned (32 bits) or word (16 bits) aligned. Otherwise the compiler may insert extra padding between fields in order to meet alignment requirements on platforms that need it (MIPS, older ARM) or platforms that perform slower (newer ARM.) Eg: u32 a; u16 b; u8 c; u8 d; .. that's fine - the u32 is dword aligned, the u16 is word aligned, the u8's don't need aligning. But, considder: u32 a; u8 b; u16 c; u8 d; .. u32 is dword aligned, u8 b is fine as it's a a byte and doesn't need aligning, but 'u16 c' isn't dword aligned! So the compiler will insert a byte padding between 'b' and 'c'. same deal with: u32 a; u16 b; u32 c; .. 'a' is fine; 'b' is fine, but 'c' starts at a word boundary, not a dword boundary. Hence why things like IP/TCP headers and such look the way they do. :-) Now, i don't know what 'bool' is, whether it's a byte, word or dword. That is_mybeacon field should probably be just another flag in rx_status, then just extend 'rs_flags' to 16 bits and include it. That way the alignment is easy to see - all the fields in rx_status and the htc rx_status structs have explicit sizes. :-) Felix, what do you think? Adrian ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH v2] ath9k: collect statistics about Rx-Dup and Rx-STBC packets
Am 27.04.2013 21:06, schrieb Adrian Chadd: On 27 April 2013 11:53, Oleksij Rempel li...@rempel-privat.de wrote: (And then go and re-align things inside that struct so you don't waste space.) hmm.. what do you mean here? Structure alignment? Well, you typically want to have everything be dword aligned (32 bits) or word (16 bits) aligned. Otherwise the compiler may insert extra padding between fields in order to meet alignment requirements on platforms that need it (MIPS, older ARM) or platforms that perform slower (newer ARM.) Eg: u32 a; u16 b; u8 c; u8 d; .. that's fine - the u32 is dword aligned, the u16 is word aligned, the u8's don't need aligning. But, considder: u32 a; u8 b; u16 c; u8 d; .. u32 is dword aligned, u8 b is fine as it's a a byte and doesn't need aligning, but 'u16 c' isn't dword aligned! So the compiler will insert a byte padding between 'b' and 'c'. same deal with: u32 a; u16 b; u32 c; .. 'a' is fine; 'b' is fine, but 'c' starts at a word boundary, not a dword boundary. Hence why things like IP/TCP headers and such look the way they do. :-) Now, i don't know what 'bool' is, whether it's a byte, word or dword. That is_mybeacon field should probably be just another flag in rx_status, then just extend 'rs_flags' to 16 bits and include it. That way the alignment is easy to see - all the fields in rx_status and the htc rx_status structs have explicit sizes. :-) ok, i was not sure if you mean what i think :) I would prefer to do this in separate patch. -- Regards, Oleksij ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel
Re: [ath9k-devel] [PATCH v2] ath9k: collect statistics about Rx-Dup and Rx-STBC packets
On 2013-04-27 9:06 PM, Adrian Chadd wrote: On 27 April 2013 11:53, Oleksij Rempel li...@rempel-privat.de wrote: (And then go and re-align things inside that struct so you don't waste space.) hmm.. what do you mean here? Structure alignment? Well, you typically want to have everything be dword aligned (32 bits) or word (16 bits) aligned. Otherwise the compiler may insert extra padding between fields in order to meet alignment requirements on platforms that need it (MIPS, older ARM) or platforms that perform slower (newer ARM.) I think in struct ath_rx_status alignment does not matter much, it's only kept on the stack anyway. But yes, in other cases it makes sense to pay attention to padding to keep structs small. I also agree that making rs_flags u16 is a good idea. Now, i don't know what 'bool' is, whether it's a byte, word or dword. bool is a byte. That is_mybeacon field should probably be just another flag in rx_status, then just extend 'rs_flags' to 16 bits and include it. That way the alignment is easy to see - all the fields in rx_status and the htc rx_status structs have explicit sizes. :-) Felix, what do you think? Sounds good :) - Felix ___ ath9k-devel mailing list ath9k-devel@lists.ath9k.org https://lists.ath9k.org/mailman/listinfo/ath9k-devel