Re: [PATCH] mac80211: Fix wrong channel bandwidths reported for aggregates

2022-07-19 Thread Linus Lüssing 

On 19/07/2022 17:03, Adrian Chadd wrote:

Hi!

It's not a hardware bug. Dating back to the original AR5416 11n chip,
most flags aren't valid for subframes in an aggregate. Only the final
frame has valid flags. This was explicitly covered internally way back
when.


Ah, thanks for the clarification! I see it in the datasheet for the 
QCA9531, too, now. And thanks for the confirmation, that what we are 
doing so far is not correct for ath9k.


Words 0+2 are valid for all RX descriptors, 0+2+11 valid for the last RX 
descriptor of each packet and 0-11 for the last RX descriptor of an 
aggregate or last RX descriptor of a stand-alone packet. Or in other 
words, word 4, which contains the 20 vs. 40 MHz indicator, is invalid 
for any aggregate sub-frame other than the last one. I can rename that 
in the commit message.



Another approach that also came to my mind was introducing more explicit 
flags in cfg80211.h's "struct rate_info", like a RATE_INFO_BW_UNKNOWN in 
"enum rate_info_bw" and/or RATE_INFO_FLAGS_UNKNOWN in "enum 
rate_info_flags". And setting those flags in ath9k_cmn_process_rate().


The current approach is smaller though, as it simply uses the already 
existing flags. If anyone has any preferences, please let me know.


Regards, Linus

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH] mac80211: Fix wrong channel bandwidths reported for aggregates

2022-07-18 Thread Linus Lüssing 
From: Linus Lüssing 

AR9003 based wifi chips have a hardware bug, they always report a
channel bandwidth of HT40 for any sub-frame of an aggregate which is
not the last one. Only the last sub-frame has correct channel bandwidth
information.

This can be easily reproduced by setting an ath9k based wifi to HT20 and
running an iperf test. Then "iw dev wlan0 station dump" will occasionally,
wrongly show something like:

  rx bitrate: 121.5 MBit/s MCS 6 40MHz

Debug output in ath9k_hw_process_rxdesc_edma() confirmed that it is
always frames with (rxs->rs_isaggr && !rxs->rs_moreaggr) and no others
which report RATE_INFO_BW_40.

Unfortunately we cannot easily fix this within ath9k as in ath9k we
cannot peek at the rate/bandwidth info of the last aggregate
sub-frame and there is no queueing within ath9k after receiving the
frame from the wifi chip, it is directly handed over to mac80211.

Therefore fixing this within mac80211: For an aggergated AMPDU only
update the RX "last_rate" variable from the last sub-frame of an
aggregate. In theory, without hardware bugs, rate/bandwidth info
should be the same for all sub-frames of an aggregate anyway.

This change only affects ath9k, ath9k-htc and ath10k as these are
currently the only drivers implementing the RX_FLAG_AMPDU_LAST_KNOWN
flag.

Tested-on: 8devices Lima board, QCA9531 WiFi

Cc: Sven Eckelmann 
Cc: Simon Wunderlich 
Cc: Linus Lüssing 
Signed-off-by: Linus Lüssing 
---
 net/mac80211/rx.c   |  8 
 net/mac80211/sta_info.h | 16 +---
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 304b9909f025..988dbf058489 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1723,6 +1723,7 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
struct sk_buff *skb = rx->skb;
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+   u32 *last_rate = >deflink.rx_stats.last_rate;
int i;
 
if (!sta)
@@ -1744,8 +1745,7 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
sta->deflink.rx_stats.last_rx = jiffies;
if (ieee80211_is_data(hdr->frame_control) &&
!is_multicast_ether_addr(hdr->addr1))
-   sta->deflink.rx_stats.last_rate =
-   sta_stats_encode_rate(status);
+   sta_stats_encode_rate(status, last_rate);
}
} else if (rx->sdata->vif.type == NL80211_IFTYPE_OCB) {
sta->deflink.rx_stats.last_rx = jiffies;
@@ -1757,7 +1757,7 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
 */
sta->deflink.rx_stats.last_rx = jiffies;
if (ieee80211_is_data(hdr->frame_control))
-   sta->deflink.rx_stats.last_rate = 
sta_stats_encode_rate(status);
+   sta_stats_encode_rate(status, last_rate);
}
 
sta->deflink.rx_stats.fragments++;
@@ -4502,7 +4502,7 @@ static void ieee80211_rx_8023(struct ieee80211_rx_data 
*rx,
/* end of statistics */
 
stats->last_rx = jiffies;
-   stats->last_rate = sta_stats_encode_rate(status);
+   sta_stats_encode_rate(status, >last_rate);
 
stats->fragments++;
stats->packets++;
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 70ee55ec5518..67f9c1647567 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -941,10 +941,19 @@ enum sta_stats_type {
 
 #define STA_STATS_RATE_INVALID 0
 
-static inline u32 sta_stats_encode_rate(struct ieee80211_rx_status *s)
+static inline void
+sta_stats_encode_rate(struct ieee80211_rx_status *s, u32 *rate)
 {
u32 r;
 
+   /* some drivers (notably ath9k) only report a valid bandwidth
+* in the last subframe of an aggregate, skip the others
+* in that case
+*/
+   if (s->flag & RX_FLAG_AMPDU_LAST_KNOWN &&
+   !(s->flag & RX_FLAG_AMPDU_IS_LAST))
+   return;
+
r = STA_STATS_FIELD(BW, s->bw);
 
if (s->enc_flags & RX_ENC_FLAG_SHORT_GI)
@@ -975,10 +984,11 @@ static inline u32 sta_stats_encode_rate(struct 
ieee80211_rx_status *s)
break;
default:
WARN_ON(1);
-   return STA_STATS_RATE_INVALID;
+   *rate = STA_STATS_RATE_INVALID;
+   return;
}
 
-   return r;
+   *rate = r;
 }
 
 #endif /* STA_INFO_H */
-- 
2.36.1


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH] ath10k: increase rx buffer size to 2048

2020-02-05 Thread Linus Lüssing 
From: Linus Lüssing 

Before, only frames with a maximum size of 1528 bytes could be
transmitted between two 802.11s nodes.

For batman-adv for instance, which adds its own header to each frame,
we typically need an MTU of at least 1532 bytes to be able to transmit
without fragmentation.

This patch now increases the maxmimum frame size from 1528 to 1656
bytes.

Tested with two ath10k devices in 802.11s mode, as well as with
batman-adv on top of 802.11s with forwarding disabled.

Fix originally found and developed by Ben Greear.

Link: https://github.com/greearb/ath10k-ct/issues/89
Link: 
https://github.com/greearb/ath10k-ct/commit/9e5ab25027e0971fa24ccf93373324c08c4e992d
Cc: Ben Greear 
Signed-off-by: Linus Lüssing 
---
 drivers/net/wireless/ath/ath10k/htt.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt.h 
b/drivers/net/wireless/ath/ath10k/htt.h
index 4a12564fc30e..6a2b5e10e568 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -2220,7 +2220,7 @@ struct htt_rx_chan_info {
  * Should be: sizeof(struct htt_host_rx_desc) + max rx MSDU size,
  * rounded up to a cache line size.
  */
-#define HTT_RX_BUF_SIZE 1920
+#define HTT_RX_BUF_SIZE 2048
 #define HTT_RX_MSDU_SIZE (HTT_RX_BUF_SIZE - (int)sizeof(struct htt_rx_desc))
 
 /* Refill a bunch of RX buffers for each refill round so that FW/HW can handle
-- 
2.25.0


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH net-next v2] ath10k: fix RX of frames with broken FCS in monitor mode

2019-11-15 Thread Linus Lüssing 
From: Linus Lüssing 

So far, frames were forwarded regardless of the FCS correctness leading
to userspace applications listening on the monitor mode interface to
receive potentially broken frames, even with the "fcsfail" flag unset.

By default, with the "fcsfail" flag of a monitor mode interface
unset, frames with FCS errors should be dropped. With this patch, the
fcsfail flag is taken into account correctly.

Cc: Simon Wunderlich 
Signed-off-by: Linus Lüssing 
---
This was tested on an Open Mesh A41 device, featuring a QCA4019. And
with this firmware:

https://www.candelatech.com/downloads/ath10k-4019-10-4b/firmware-5-ct-full-community-12.bin-lede.011

But from looking at the code it seems that the vanilla ath10k has the
same issue, therefore submitting it here.

Changelog v2:

* removed the spinlock as only a 32 bit statistics counter is
  incremented

Changelog RFC->v1:

* removed "ar->monitor" check
* added a debug counter

---

 drivers/net/wireless/ath/ath10k/core.h   | 1 +
 drivers/net/wireless/ath/ath10k/debug.c  | 2 ++
 drivers/net/wireless/ath/ath10k/htt_rx.c | 7 +++
 3 files changed, 10 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index af68eb5d0776..d445482fa945 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -1180,6 +1180,7 @@ struct ath10k {
 
struct {
/* protected by data_lock */
+   u32 rx_crc_err_drop;
u32 fw_crash_counter;
u32 fw_warm_reset_counter;
u32 fw_cold_reset_counter;
diff --git a/drivers/net/wireless/ath/ath10k/debug.c 
b/drivers/net/wireless/ath/ath10k/debug.c
index bd2b5628f850..5e4cd2966e6f 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -1094,6 +1094,7 @@ static const char 
ath10k_gstrings_stats[][ETH_GSTRING_LEN] = {
"d_rts_good",
"d_tx_power", /* in .5 dbM I think */
"d_rx_crc_err", /* fcs_bad */
+   "d_rx_crc_err_drop", /* frame with FCS error, dropped late in kernel */
"d_no_beacon",
"d_tx_mpdus_queued",
"d_tx_msdu_queued",
@@ -1193,6 +1194,7 @@ void ath10k_debug_get_et_stats(struct ieee80211_hw *hw,
data[i++] = pdev_stats->rts_good;
data[i++] = pdev_stats->chan_tx_power;
data[i++] = pdev_stats->fcs_bad;
+   data[i++] = ar->stats.rx_crc_err_drop;
data[i++] = pdev_stats->no_beacons;
data[i++] = pdev_stats->mpdu_enqued;
data[i++] = pdev_stats->msdu_enqued;
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 9f0e7b4943ec..8139c9cea1d8 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1285,6 +1285,13 @@ static void ath10k_process_rx(struct ath10k *ar, struct 
sk_buff *skb)
 
status = IEEE80211_SKB_RXCB(skb);
 
+   if (!(ar->filter_flags & FIF_FCSFAIL) &&
+   status->flag & RX_FLAG_FAILED_FCS_CRC) {
+   ar->stats.rx_crc_err_drop++;
+   dev_kfree_skb_any(skb);
+   return;
+   }
+
ath10k_dbg(ar, ATH10K_DBG_DATA,
   "rx skb %pK len %u peer %pM %s %s sn %u %s%s%s%s%s%s 
%srate_idx %u vht_nss %u freq %u band %u flag 0x%x fcs-err %i mic-err %i 
amsdu-more %i\n",
   skb,
-- 
2.24.0.rc2


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [PATCH net-next] ath10k: fix RX of frames with broken FCS in monitor mode

2019-11-07 Thread Linus Lüssing 
On Tue, Nov 05, 2019 at 09:19:20AM -0800, Ben Greear wrote:
> Thanks for adding the counter.  Since it us u32, I doubt you need the spin 
> lock
> below?

Ok, I can remove the spin-lock.

Just for clarification though, if I recall correctly then an increment operator
is not guaranteed to work atomically. But you think it's unlikely
to race with a concurrent ++ and therefore it's fine for just a debug counter?
(and if it were racing, it'd just be a missed +1)

Or is there another mechanism that avoids concurrency in the
ath10k RX path?


> 
> --Ben
> 
> > +   if (!(ar->filter_flags & FIF_FCSFAIL) &&
> > +   status->flag & RX_FLAG_FAILED_FCS_CRC) {
> > +   spin_lock_bh(>data_lock);
> > +   ar->stats.rx_crc_err_drop++;
> > +   spin_unlock_bh(>data_lock);
> > +
> > +   dev_kfree_skb_any(skb);
> > +   return;
> > +   }
> > +
> > ath10k_dbg(ar, ATH10K_DBG_DATA,
> >"rx skb %pK len %u peer %pM %s %s sn %u %s%s%s%s%s%s 
> > %srate_idx %u vht_nss %u freq %u band %u flag 0x%x fcs-err %i mic-err %i 
> > amsdu-more %i\n",
> >skb,
> > 
> 
> 
> -- 
> Ben Greear 
> Candela Technologies Inc  http://www.candelatech.com
> 

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[PATCH net-next] ath10k: fix RX of frames with broken FCS in monitor mode

2019-11-05 Thread Linus Lüssing 
From: Linus Lüssing 

So far, frames were forwarded regardless of the FCS correctness leading
to userspace applications listening on the monitor mode interface to
receive potentially broken frames, even with the "fcsfail" flag unset.

By default, with the "fcsfail" flag of a monitor mode interface
unset, frames with FCS errors should be dropped. With this patch, the
fcsfail flag is taken into account correctly.

Cc: Simon Wunderlich 
Signed-off-by: Linus Lüssing 
---
This was tested on an Open Mesh A41 device, featuring a QCA4019. And
with this firmware:

https://www.candelatech.com/downloads/ath10k-4019-10-4b/firmware-5-ct-full-community-12.bin-lede.011

But from looking at the code it seems that the vanilla ath10k has the
same issue, therefore submitting it here.

Changelog RFC->v1:

* removed "ar->monitor" check
* added a debug counter

---
 drivers/net/wireless/ath/ath10k/core.h   |  1 +
 drivers/net/wireless/ath/ath10k/debug.c  |  2 ++
 drivers/net/wireless/ath/ath10k/htt_rx.c | 10 ++
 3 files changed, 13 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.h 
b/drivers/net/wireless/ath/ath10k/core.h
index 4d7db07db6ba..787065a9eb3f 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -1171,6 +1171,7 @@ struct ath10k {
 
struct {
/* protected by data_lock */
+   u32 rx_crc_err_drop;
u32 fw_crash_counter;
u32 fw_warm_reset_counter;
u32 fw_cold_reset_counter;
diff --git a/drivers/net/wireless/ath/ath10k/debug.c 
b/drivers/net/wireless/ath/ath10k/debug.c
index bd2b5628f850..5e4cd2966e6f 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -1094,6 +1094,7 @@ static const char 
ath10k_gstrings_stats[][ETH_GSTRING_LEN] = {
"d_rts_good",
"d_tx_power", /* in .5 dbM I think */
"d_rx_crc_err", /* fcs_bad */
+   "d_rx_crc_err_drop", /* frame with FCS error, dropped late in kernel */
"d_no_beacon",
"d_tx_mpdus_queued",
"d_tx_msdu_queued",
@@ -1193,6 +1194,7 @@ void ath10k_debug_get_et_stats(struct ieee80211_hw *hw,
data[i++] = pdev_stats->rts_good;
data[i++] = pdev_stats->chan_tx_power;
data[i++] = pdev_stats->fcs_bad;
+   data[i++] = ar->stats.rx_crc_err_drop;
data[i++] = pdev_stats->no_beacons;
data[i++] = pdev_stats->mpdu_enqued;
data[i++] = pdev_stats->msdu_enqued;
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 53f1095de8ff..149728588e80 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1285,6 +1285,16 @@ static void ath10k_process_rx(struct ath10k *ar, struct 
sk_buff *skb)
 
status = IEEE80211_SKB_RXCB(skb);
 
+   if (!(ar->filter_flags & FIF_FCSFAIL) &&
+   status->flag & RX_FLAG_FAILED_FCS_CRC) {
+   spin_lock_bh(>data_lock);
+   ar->stats.rx_crc_err_drop++;
+   spin_unlock_bh(>data_lock);
+
+   dev_kfree_skb_any(skb);
+   return;
+   }
+
ath10k_dbg(ar, ATH10K_DBG_DATA,
   "rx skb %pK len %u peer %pM %s %s sn %u %s%s%s%s%s%s 
%srate_idx %u vht_nss %u freq %u band %u flag 0x%x fcs-err %i mic-err %i 
amsdu-more %i\n",
   skb,
-- 
2.24.0.rc2


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Re: [RFC PATCH] ath10k: fix RX of frames with broken FCS in monitor mode

2019-11-05 Thread Linus Lüssing 
On Fri, Nov 01, 2019 at 08:46:53AM -0700, Ben Greear wrote:
> > diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
> > b/drivers/net/wireless/ath/ath10k/htt_rx.c
> > index 53f1095de8ff..ce0a16ebb8bb 100644
> > --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> > +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> > @@ -1285,6 +1285,12 @@ static void ath10k_process_rx(struct ath10k *ar, 
> > struct sk_buff *skb)
> > status = IEEE80211_SKB_RXCB(skb);
> > +   if (ar->monitor && !(ar->filter_flags & FIF_FCSFAIL) &&
> > +   status->flag & RX_FLAG_FAILED_FCS_CRC) {
> > +   dev_kfree_skb_any(skb);
> > +   return;
> > +   }
> 
> Maybe worth adding a counter like 'rx_drop_crc' to the ath10k_debug struct 
> and increment it here
> and also show in debugfs and/or ethtool stats?

Ok.

> 
> And, maybe no check for ar->monitor, in case somehow the frame is still 
> received
> with bad CRC even without monitor mode?

I think ath9k is also checking for "ah->is_monitoring" here:

https://elixir.bootlin.com/linux/v5.3.8/source/drivers/net/wireless/ath/ath9k/common.c#L96

And I didn't want to divert from this. Should I remove it anyway?

___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

[RFC PATCH] ath10k: fix RX of frames with broken FCS in monitor mode

2019-11-01 Thread Linus Lüssing 
From: Linus Lüssing 

So far, frames were forwarded regardless of the FCS correctness leading
to userspace applications listening on the monitor mode interface to
receive potentially broken frames, even with the "fcsfail" flag unset.

By default, with the "fcsfail" flag of a monitor mode interface
unset, frames with FCS errors should be dropped. With this patch, the
fcsfail flag is taken into account correctly.

Signed-off-by: Linus Lüssing 
---
This was tested on an Open Mesh A41 device, featuring a QCA4019. And
with this firmware:

https://www.candelatech.com/downloads/ath10k-4019-10-4b/firmware-5-ct-full-community-12.bin-lede.011

But from looking at the code it seems that the vanilla ath10k has the
same issue, therefore submitting it here.

I'm also not that familiar with the ath10k code yet. So not 100% sure if
it's the right place for this check. Therefore sending as RFC.
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 53f1095de8ff..ce0a16ebb8bb 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1285,6 +1285,12 @@ static void ath10k_process_rx(struct ath10k *ar, struct 
sk_buff *skb)
 
status = IEEE80211_SKB_RXCB(skb);
 
+   if (ar->monitor && !(ar->filter_flags & FIF_FCSFAIL) &&
+   status->flag & RX_FLAG_FAILED_FCS_CRC) {
+   dev_kfree_skb_any(skb);
+   return;
+   }
+
ath10k_dbg(ar, ATH10K_DBG_DATA,
   "rx skb %pK len %u peer %pM %s %s sn %u %s%s%s%s%s%s 
%srate_idx %u vht_nss %u freq %u band %u flag 0x%x fcs-err %i mic-err %i 
amsdu-more %i\n",
   skb,
-- 
2.24.0.rc2


___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k