ath9k: Add more recv stats.
PATCH 2/2 ath9k: Add more recv stats.
From: Ben Greear <[email protected]>
This adds counters in various places that can drop packets on rx without otherwise incrementing a counter. It also counts some non-error cases, such as becons and fragments received.
Should help with figuring out where packets are (and are not) dropped.
---
:100644 100644 9078279... 244723a... M drivers/net/wireless/ath/ath9k/debug.c
:100644 100644 d1c132f... 56a7053... M drivers/net/wireless/ath/ath9k/debug.h
:100644 100644 1c4583c... ad2b2b6... M drivers/net/wireless/ath/ath9k/recv.c
drivers/net/wireless/ath/ath9k/debug.c | 23 +++++++++++++++++++- drivers/net/wireless/ath/ath9k/debug.h | 18 ++++++++++++++++ drivers/net/wireless/ath/ath9k/recv.c | 35 +++++++++++++++++++++++-------- 3 files changed, 65 insertions(+), 11 deletions(-)
drivers/net/wireless/ath/ath9k/debug.c +21 -2
@@ -916,6 +916,21 @@ static ssize_t read_file_recv(struct file *file, char __user *user_buf, len +=snprintf (buf + len, size - len,"%22s : %10u\n" ,"DECRYPT BUSY ERR" , sc->debug.stats.rxstats.decrypt_busy_err); + len +=snprintf (buf + len, size - len, +"%22s : %10u\n" ,"RX-LENGTH-ERR" , + sc->debug.stats.rxstats.rx_len_err); + len +=snprintf (buf + len, size - len, +"%22s : %10u\n" ,"RX-OOM-ERR" , + sc->debug.stats.rxstats.rx_oom_err); + len +=snprintf (buf + len, size - len, +"%22s : %10u\n" ,"RX-RATE-ERR" , + sc->debug.stats.rxstats.rx_rate_err); + len +=snprintf (buf + len, size - len, +"%22s : %10u\n" ,"RX-DROP-RXFLUSH" , + sc->debug.stats.rxstats.rx_drop_rxflush); + len +=snprintf (buf + len, size - len, +"%22s : %10u\n" ,"RX-TOO-MANY-FRAGS" , + sc->debug.stats.rxstats.rx_too_many_frags_err);PHY_ERR ("UNDERRUN ERR" ,ATH9K_PHYERR_UNDERRUN );PHY_ERR ("TIMING ERR" ,ATH9K_PHYERR_TIMING );
@@ -950,6 +965,12 @@ static ssize_t read_file_recv(struct file *file, char __user *user_buf, len +=snprintf (buf + len, size - len,"%22s : %10u\n" ,"RX-Bytes-All" , sc->debug.stats.rxstats.rx_bytes_all); + len +=snprintf (buf + len, size - len, +"%22s : %10u\n" ,"RX-Beacons" , + sc->debug.stats.rxstats.rx_beacons); + len +=snprintf (buf + len, size - len, +"%22s : %10u\n" ,"RX-Frags" , + sc->debug.stats.rxstats.rx_frags);if (len > size) len = size;
@@ -964,7 +985,6 @@ static ssize_t read_file_recv(struct file *file, char __user *user_buf,void ath_debug_stat_rx (struct ath_softc *sc,struct ath_rx_status *rs) { -#define RX_STAT_INC (c) sc->debug.stats.rxstats.c++#define RX_PHY_ERR_INC (c) sc->debug.stats.rxstats.phy_err_stats[c]++#define RX_SAMP_DBG (c) (sc->debug.bb_mac_samp[sc->debug.sampidx].rs\ [sc->debug.rsidx].c)
@@ -1010,7 +1030,6 @@ void ath_debug_stat_rx(struct ath_softc *sc, struct ath_rx_status *rs)#endif -#undef RX_STAT_INC#undef RX_PHY_ERR_INC#undef RX_SAMP_DBG }
drivers/net/wireless/ath/ath9k/debug.h +18 -0
@@ -139,6 +139,8 @@ struct ath_tx_stats {u32 txfailed; }; +#define RX_STAT_INC (c) (sc->debug.stats.rxstats.c++) +/** * struct ath_rx_stats - RX Statistics * @rx_pkts_all: No. of total frames received, including ones that
@@ -155,6 +157,13 @@ struct ath_tx_stats {* @post_delim_crc_err: Post-Frame delimiter CRC error detections * @decrypt_busy_err: Decryption interruptions counter * @phy_err_stats: Individual PHY error statistics +* @rx_len_err: No. of frames discarded due to bad length. +* @rx_oom_err: No. of frames dropped due to OOM issues. +* @rx_rate_err: No. of frames dropped due to rate errors. +* @rx_too_many_frags_err: Frames dropped due to too-many-frags received. +* @rx_drop_rxflush: No. of frames dropped due to RX-FLUSH. +* @rx_beacons: No. of beacons received. +* @rx_frags: No. of rx-fragements received. */ struct ath_rx_stats {u32 rx_pkts_all;
@@ -167,6 +176,13 @@ struct ath_rx_stats {u32 post_delim_crc_err;u32 decrypt_busy_err;u32 phy_err_stats[ATH9K_PHYERR_MAX ]; +u32 rx_len_err; +u32 rx_oom_err; +u32 rx_rate_err; +u32 rx_too_many_frags_err; +u32 rx_drop_rxflush; +u32 rx_beacons; +u32 rx_frags; };enum ath_reset_type {
@@ -249,6 +265,8 @@ void ath_debug_stat_rx(struct ath_softc *sc, struct ath_rx_status *rs);#else +#define RX_STAT_INC (c)/* NOP */ +static inline int ath9k_init_debug (struct ath_hw *ah) {return 0 ;
drivers/net/wireless/ath/ath9k/recv.c +26 -9
@@ -824,15 +824,20 @@ static bool ath9k_rx_accept(struct ath_common *common,if (rx_stats->rs_keyix ==ATH9K_RXKEYIX_INVALID ) rx_stats->rs_status &= ~ATH9K_RXERR_KEYMISS ; -if (!rx_stats->rs_datalen) +if (!rx_stats->rs_datalen) { +RX_STAT_INC (rx_len_err);return false; + } +/* * rs_status follows rs_datalen so if rs_datalen is too large * we can take a hint that hardware corrupted it, so ignore * those frames. */ -if (rx_stats->rs_datalen > (common->rx_bufsize - rx_status_len)) +if (rx_stats->rs_datalen > (common->rx_bufsize - rx_status_len)) { +RX_STAT_INC (rx_len_err);return false; + }/* Only use error bits from the last fragment */ if (rx_stats->rs_more)
@@ -902,6 +907,7 @@ static int ath9k_process_rate(struct ath_common *common,struct ieee80211_supported_band *sband;enum ieee80211_band band;unsigned int i =0 ; +struct ath_softc *sc = (struct ath_softc *) common->priv; band = hw->conf.channel->band; sband = hw->wiphy->bands[band];
@@ -936,7 +942,7 @@ static int ath9k_process_rate(struct ath_common *common,ath_dbg (common,ANY ,"unsupported hw bitrate detected 0x%02x using 1 Mbit\n" , rx_stats->rs_rate); - +RX_STAT_INC (rx_rate_err);return -EINVAL ; }
@@ -1823,10 +1829,14 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp) hdr = (struct ieee80211_hdr *) (hdr_skb->data + rx_status_len); rxs =IEEE80211_SKB_RXCB (hdr_skb); -if (ieee80211_is_beacon (hdr->frame_control) && - !is_zero_ether_addr (common->curbssid) && - !compare_ether_addr (hdr->addr3, common->curbssid)) - rs.is_mybeacon = true; +if (ieee80211_is_beacon (hdr->frame_control)) { +RX_STAT_INC (rx_beacons); +if (!is_zero_ether_addr (common->curbssid) && + !compare_ether_addr (hdr->addr3, common->curbssid)) + rs.is_mybeacon = true; +else + rs.is_mybeacon = false; + }else rs.is_mybeacon = false;
@@ -1836,8 +1846,10 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)* If we're asked to flush receive queue, directly * chain it back at the queue without processing it. */ -if (sc->sc_flags &SC_OP_RXFLUSH ) +if (sc->sc_flags &SC_OP_RXFLUSH ) { +RX_STAT_INC (rx_drop_rxflush); goto requeue_drop_frag; + }memset (rxs,0 ,sizeof (struct ieee80211_rx_status ));
@@ -1863,8 +1875,10 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)* tell hardware it can give us a new frame using the old * skb and put it at the tail of the sc->rx.rxbuf list for * processing. */ -if (!requeue_skb) +if (!requeue_skb) { +RX_STAT_INC (rx_oom_err); goto requeue_drop_frag; + }/* Unmap the frame */ dma_unmap_single(sc->dev, bf->bf_buf_addr,
@@ -1895,6 +1909,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp) }if (rs.rs_more) { +RX_STAT_INC (rx_frags);/* * rs_more indicates chained descriptors which can be * used to link buffers together for a sort of
@@ -1904,6 +1919,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)/* too many fragments - cannot handle frame */ dev_kfree_skb_any (sc->rx.frag);dev_kfree_skb_any (skb); +RX_STAT_INC (rx_too_many_frags_err); skb =NULL ; } sc->rx.frag = skb;
@@ -1915,6 +1931,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)if (pskb_expand_head (hdr_skb,0 , space,GFP_ATOMIC ) <0 ) {dev_kfree_skb (skb); +RX_STAT_INC (rx_oom_err); goto requeue_drop_frag; }
Signature · 1 line
1.7.3.4
PATCH 2/2 ath9k: Add more recv stats.
Quoted 88 lines · On 2012-04-03 6:18 PM, [email protected] wrote:
From: Ben Greear <[email protected]>
This adds counters in various places that can drop packets on rx without otherwise incrementing a counter. It also counts some non-error cases, such as becons and fragments received.
Should help with figuring out where packets are (and are not) dropped.
Signed-off-by Ben Greear <[email protected]>---
:100644 100644 9078279... 244723a... Mdrivers/net/wireless/ath/ath9k/debug.c
:100644 100644 d1c132f... 56a7053... Mdrivers/net/wireless/ath/ath9k/debug.h
:100644 100644 1c4583c... ad2b2b6... Mdrivers/net/wireless/ath/ath9k/recv.cdrivers/net/wireless/ath/ath9k/debug.c | 23 +++++++++++++++++++- drivers/net/wireless/ath/ath9k/debug.h | 18 ++++++++++++++++ drivers/net/wireless/ath/ath9k/recv.c | 35 +++++++++++++++++++++++-------- 3 files changed, 65 insertions(+), 11 deletions(-)drivers/net/wireless/ath/ath9k/debug.c +21
-2@@ -916,6 +916,21 @@ static ssize_t read_file_recv(struct file *file, char __user *user_buf, len +=snprintf (buf + len, size - len,"%22s : %10u\n" ,"DECRYPT BUSY ERR" , sc->debug.stats.rxstats.decrypt_busy_err); + len +=snprintf (buf + len, size - len, +"%22s : %10u\n" ,"RX-LENGTH-ERR" , + sc->debug.stats.rxstats.rx_len_err); + len +=snprintf (buf + len, size - len, +"%22s : %10u\n" ,"RX-OOM-ERR" , + sc->debug.stats.rxstats.rx_oom_err); + len +=snprintf (buf + len, size - len, +"%22s : %10u\n" ,"RX-RATE-ERR" , + sc->debug.stats.rxstats.rx_rate_err); + len +=snprintf (buf + len, size - len, +"%22s : %10u\n" ,"RX-DROP-RXFLUSH" , + sc->debug.stats.rxstats.rx_drop_rxflush); + len +=snprintf (buf + len, size - len, +"%22s : %10u\n" ,"RX-TOO-MANY-FRAGS" , + sc->debug.stats.rxstats.rx_too_many_frags_err);PHY_ERR ("UNDERRUN ERR" ,ATH9K_PHYERR_UNDERRUN );PHY_ERR ("TIMING ERR" ,ATH9K_PHYERR_TIMING );@@ -950,6 +965,12 @@ static ssize_t read_file_recv(struct file *file, char __user *user_buf, len +=snprintf (buf + len, size - len,"%22s : %10u\n" ,"RX-Bytes-All" , sc->debug.stats.rxstats.rx_bytes_all); + len +=snprintf (buf + len, size - len, +"%22s : %10u\n" ,"RX-Beacons" , + sc->debug.stats.rxstats.rx_beacons); + len +=snprintf (buf + len, size - len, +"%22s : %10u\n" ,"RX-Frags" , + sc->debug.stats.rxstats.rx_frags);if (len > size) len = size;@@ -964,7 +985,6 @@ static ssize_t read_file_recv(struct file *file, char __user *user_buf,void ath_debug_stat_rx (struct ath_softc *sc,struct ath_rx_status *rs) { -#define RX_STAT_INC (c) sc->debug.stats.rxstats.c++#define RX_PHY_ERR_INC (c) sc->debug.stats.rxstats.phy_err_stats[c]++#define RX_SAMP_DBG (c) (sc->debug.bb_mac_samp[sc->debug.sampidx].rs\ [sc->debug.rsidx].c)@@ -1010,7 +1030,6 @@ void ath_debug_stat_rx(struct ath_softc *sc, struct ath_rx_status *rs)#endif -#undef RX_STAT_INC#undef RX_PHY_ERR_INC#undef RX_SAMP_DBG }drivers/net/wireless/ath/ath9k/debug.h +2
-0@@ -139,6 +139,8 @@ struct ath_tx_stats {u32 txfailed; }; +#define RX_STAT_INC (c) (sc->debug.stats.rxstats.c++) +/** * struct ath_rx_stats - RX Statistics * @rx_pkts_all: No. of total frames received, including ones that
drivers/net/wireless/ath/ath9k/debug.h struct ath_tx_stats { · line 166
Quoted 10 lines
drivers/net/wireless/ath/ath9k/debug.h +7
-0@@ -155,6 +157,13 @@ struct ath_tx_stats {* @post_delim_crc_err: Post-Frame delimiter CRC error detections * @decrypt_busy_err: Decryption interruptions counter * @phy_err_stats: Individual PHY error statistics +* @rx_len_err: No. of frames discarded due to bad length. +* @rx_oom_err: No. of frames dropped due to OOM issues. +* @rx_rate_err: No. of frames dropped due to rate errors. +* @rx_too_many_frags_err: Frames dropped due to too-many-frags received. +* @rx_drop_rxflush: No. of frames dropped due to RX-FLUSH. +* @rx_beacons: No. of beacons received. +* @rx_frags: No. of rx-fragements received.
Why should the driver keep track of those last two?
- Felix
PATCH 2/2 ath9k: Add more recv stats.
Quoted 3 lines · Felix Fietkau
Quoted 2 lines
+ * @rx_beacons: No. of beacons received.
+ * @rx_frags: No. of rx-fragements received.Why should the driver keep track of those last two?
Well, for instance, I often see around 5Mbps of total rx bytes (as counted by the NIC), but much less transmitted. And this is with stations sending to stations.
I was trying to figure out where the extra packets come from. It seems beacons is a lot of it so it seemed worth counting. And maybe fragments count towards that too since there would be more overhead???
As for rx-frags, just seemed useful to know how many frags were received. My understanding is that this is like receiving 1/2 of a packet at a time...so if we wanted to know how many real packets the NIC received, we'd need to take frags v/s non-frags into account. The current logic that counts 'all-packets' for the rx path is counting individual fragments, I think.
At least for beacons, as long as they are always passed up the stack, I can count them in the mac80211 layer instead if you prefer.
Thanks,
Ben
Quoted 2 lines
- Felix
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
PATCH 2/2 ath9k: Add more recv stats.
Quoted 21 lines · Ben Greear
Quoted 3 lines · Felix Fietkau
Quoted 2 lines
+ * @rx_beacons: No. of beacons received.
+ * @rx_frags: No. of rx-fragements received.Why should the driver keep track of those last two?
Well, for instance, I often see around 5Mbps of total rx bytes (as counted by the NIC), but much less transmitted. And this is with stations sending to stations.
I was trying to figure out where the extra packets come from. It seems beacons is a lot of it so it seemed worth counting. And maybe fragments count towards that too since there would be more overhead???
As for rx-frags, just seemed useful to know how many frags were received. My understanding is that this is like receiving 1/2 of a packet at a time...so if we wanted to know how many real packets the NIC received, we'd need to take frags v/s non-frags into account. The current logic that counts 'all-packets' for the rx path is counting individual fragments, I think.
Quoted 3 lines
At least for beacons, as long as they are always passed up the stack, I can count them in the mac80211 layer instead if you prefer.
When you only need the packet type based counters for debugging stuff, why not just create a monitor mode interface and count in user space? I think that would make more sense, since these counters are irrelevant for most users, and avoiding unnecessary work in the rx path is useful for performance reasons.
- Felix
PATCH 2/2 ath9k: Add more recv stats.
Quoted 22 lines · Felix Fietkau
Quoted 21 lines · Ben Greear
Quoted 3 lines · Felix Fietkau
Quoted 2 lines
+ * @rx_beacons: No. of beacons received.
+ * @rx_frags: No. of rx-fragements received.Why should the driver keep track of those last two?
Well, for instance, I often see around 5Mbps of total rx bytes (as counted by the NIC), but much less transmitted. And this is with stations sending to stations.
I was trying to figure out where the extra packets come from. It seems beacons is a lot of it so it seemed worth counting. And maybe fragments count towards that too since there would be more overhead???
As for rx-frags, just seemed useful to know how many frags were received. My understanding is that this is like receiving 1/2 of a packet at a time...so if we wanted to know how many real packets the NIC received, we'd need to take frags v/s non-frags into account. The current logic that counts 'all-packets' for the rx path is counting individual fragments, I think.
Quoted 8 lines
Quoted 3 lines
At least for beacons, as long as they are always passed up the stack, I can count them in the mac80211 layer instead if you prefer.
When you only need the packet type based counters for debugging stuff, why not just create a monitor mode interface and count in user space? I think that would make more sense, since these counters are irrelevant for most users, and avoiding unnecessary work in the rx path is useful for performance reasons.
I always want to be calculating stats. I will poll many of them in user-space every few seconds to generate reports. It is way easier and more efficient to get some numbers out of ethtool (or have a user cat a debugfs file)
than it is to set up a sniffer and parse it's output.
If you want me to create another compile-time option for the extra stats, I can do that?
Thanks,
Ben
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
PATCH 2/2 ath9k: Add more recv stats.
Hi,
I've thought about doing exactly that with the BSD ath/hal statistics code.
It's useful having all those fine grained counters, but I do think some users will want to compile them out for whatever the performance reason is. Especially in the net80211 case where it may have to do a hashtable lookup just to add some error counters.
I'd do it with some kind of macro which does the statistics incrementing, then compile that macro to a null statement if stats aren't being kept.
Adrian