In case of A-MSDU RX we should check flags for all
MSDU subframes to be sure we have correct FCS status.
Before we check attention flags only for first frame
where we didn't have correct info about FCS status in
case of A-MSDU. Next didn't mark RX_FLAG_FAILED_FCS_CRC
for skb. As a side efect we see big TP drop when TCP used.
This was easy to reproduce when AP interface was used
and added monitor interface run in promiscuous mode.
Reported-by: Denton Gentry <[email protected]>
Signed-off-by: Janusz Dziedzic <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 6c102b1..9f704fe 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -307,7 +307,8 @@ static void ath10k_htt_rx_free_msdu_chain(struct sk_buff *skb)
static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
u8 **fw_desc, int *fw_desc_len,
struct sk_buff **head_msdu,
- struct sk_buff **tail_msdu)
+ struct sk_buff **tail_msdu,
+ u32 *attention)
{
int msdu_len, msdu_chaining = 0;
struct sk_buff *msdu;
@@ -358,6 +359,11 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
break;
}
+ *attention |= __le32_to_cpu(rx_desc->attention.flags) &
+ (RX_ATTENTION_FLAGS_TKIP_MIC_ERR |
+ RX_ATTENTION_FLAGS_DECRYPT_ERR |
+ RX_ATTENTION_FLAGS_FCS_ERR |
+ RX_ATTENTION_FLAGS_MGMT_TYPE);
/*
* Copy the FW rx descriptor for this MSDU from the rx
* indication message into the MSDU's netbuf. HL uses the
@@ -1233,13 +1239,15 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
for (j = 0; j < mpdu_ranges[i].mpdu_count; j++) {
struct sk_buff *msdu_head, *msdu_tail;
+ attention = 0;
msdu_head = NULL;
msdu_tail = NULL;
ret = ath10k_htt_rx_amsdu_pop(htt,
&fw_desc,
&fw_desc_len,
&msdu_head,
- &msdu_tail);
+ &msdu_tail,
+ &attention);
if (ret < 0) {
ath10k_warn("failed to pop amsdu from htt rx ring %d\n",
@@ -1251,7 +1259,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
rxd = container_of((void *)msdu_head->data,
struct htt_rx_desc,
msdu_payload);
- attention = __le32_to_cpu(rxd->attention.flags);
if (!ath10k_htt_rx_amsdu_allowed(htt, msdu_head,
status,
@@ -1304,6 +1311,7 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
u8 *fw_desc;
int fw_desc_len, hdrlen, paramlen;
int trim;
+ u32 attention = 0;
fw_desc_len = __le16_to_cpu(frag->fw_rx_desc_bytes);
fw_desc = (u8 *)frag->fw_msdu_rx_desc;
@@ -1313,7 +1321,8 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
spin_lock_bh(&htt->rx_ring.lock);
ret = ath10k_htt_rx_amsdu_pop(htt, &fw_desc, &fw_desc_len,
- &msdu_head, &msdu_tail);
+ &msdu_head, &msdu_tail,
+ &attention);
spin_unlock_bh(&htt->rx_ring.lock);
ath10k_dbg(ATH10K_DBG_HTT_DUMP, "htt rx frag ahead\n");
@@ -1330,10 +1339,8 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
hdr = (struct ieee80211_hdr *)msdu_head->data;
rxd = (void *)msdu_head->data - sizeof(*rxd);
- tkip_mic_err = !!(__le32_to_cpu(rxd->attention.flags) &
- RX_ATTENTION_FLAGS_TKIP_MIC_ERR);
- decrypt_err = !!(__le32_to_cpu(rxd->attention.flags) &
- RX_ATTENTION_FLAGS_DECRYPT_ERR);
+ tkip_mic_err = !!(attention & RX_ATTENTION_FLAGS_TKIP_MIC_ERR);
+ decrypt_err = !!(attention & RX_ATTENTION_FLAGS_DECRYPT_ERR);
fmt = MS(__le32_to_cpu(rxd->msdu_start.info1),
RX_MSDU_START_INFO1_DECAP_FORMAT);
--
1.7.9.5
Janusz Dziedzic <[email protected]> writes:
> On 18 July 2014 14:50, Kalle Valo <[email protected]> wrote:
>> Janusz Dziedzic <[email protected]> writes:
>>
>>> In case of A-MSDU RX we should check flags for all
>>> MSDU subframes to be sure we have correct FCS status.
>>>
>>> Before we check attention flags only for first frame
>>> where we didn't have correct info about FCS status in
>>> case of A-MSDU. Next didn't mark RX_FLAG_FAILED_FCS_CRC
>>> for skb. As a side efect we see big TP drop when TCP used.
>>> This was easy to reproduce when AP interface was used
>>> and added monitor interface run in promiscuous mode.
>>>
>>> Reported-by: Denton Gentry <[email protected]>
>>> Signed-off-by: Janusz Dziedzic <[email protected]>
>>
>> I'm having a hard time to understand the commit log. I think it would
>> make it easier to read if you first clearly describe the bug you are
>> fixing and then, in a separate paragraph, tell how you fix it.
>
> Will change that.
Thanks. What you wrote in this email was really good, I understand the
issue now and I'm sure so do others. I think you should put the same in
the commit log, there's no max limit for that anyway :)
Also add verb "using" to the title: "ath10k: handle attention flags
correctly when using A-MSDU"
--
Kalle Valo
On 18 July 2014 14:50, Kalle Valo <[email protected]> wrote:
> Janusz Dziedzic <[email protected]> writes:
>
>> In case of A-MSDU RX we should check flags for all
>> MSDU subframes to be sure we have correct FCS status.
>>
>> Before we check attention flags only for first frame
>> where we didn't have correct info about FCS status in
>> case of A-MSDU. Next didn't mark RX_FLAG_FAILED_FCS_CRC
>> for skb. As a side efect we see big TP drop when TCP used.
>> This was easy to reproduce when AP interface was used
>> and added monitor interface run in promiscuous mode.
>>
>> Reported-by: Denton Gentry <[email protected]>
>> Signed-off-by: Janusz Dziedzic <[email protected]>
>
> I'm having a hard time to understand the commit log. I think it would
> make it easier to read if you first clearly describe the bug you are
> fixing and then, in a separate paragraph, tell how you fix it.
>
Will change that.
> So what is the bug Denton reported? "we see big TP drop when TCP used"
> implies that this happens with all TCP traffic, but that can't be right.
> Or did it happen with one of the reorder patches?
>
TP drop is seen with/without reordering patches when TCP STA --> AP and:
- used ath10k STA
- I force to use AMSDU (7 frames) on STA
- not using cables and other traffic on the channel (often FCS errors)
- monitor iface used on AP
Without a patch we report AMSDU packets with wrong FCS as a correct to
the stack, next there is a lot of DUP ACK packets from AP --> STA.
TP drop depends on FCS errors for AMSDU.
This is example what I see without patch (without reordering patch):
echo "1 64" > htt_max_amsdu_ampdu
[ ID] Interval Transfer Bandwidth
[ 3] 0.0- 5.0 sec 56.6 MBytes 95.0 Mbits/sec
[ 3] 5.0-10.0 sec 60.4 MBytes 101 Mbits/sec
[ 3] 10.0-15.0 sec 60.2 MBytes 101 Mbits/sec
[ 3] 15.0-20.0 sec 60.2 MBytes 101 Mbits/sec
[ 3] 20.0-25.0 sec 63.8 MBytes 107 Mbits/sec
[ 3] 25.0-30.0 sec 64.9 MBytes 109 Mbits/sec
echo "7 64" > htt_max_amsdu_ampdu // lot of DUP ACK from AP --> STA seen
[ 3] 30.0-35.0 sec 40.0 MBytes 67.1 Mbits/sec
[ 3] 35.0-40.0 sec 35.9 MBytes 60.2 Mbits/sec
[ 3] 40.0-45.0 sec 36.9 MBytes 61.9 Mbits/sec
[ 3] 45.0-50.0 sec 37.9 MBytes 63.5 Mbits/sec
[ 3] 50.0-55.0 sec 34.5 MBytes 57.9 Mbits/sec
[ 3] 55.0-60.0 sec 25.4 MBytes 42.6 Mbits/sec
[ 3] 60.0-65.0 sec 48.2 MBytes 81.0 Mbits/sec
[ 3] 65.0-70.0 sec 28.8 MBytes 48.2 Mbits/sec
[ 3] 70.0-75.0 sec 29.2 MBytes 49.1 Mbits/sec
[ 3] 75.0-80.0 sec 22.9 MBytes 38.4 Mbits/sec
[ 3] 80.0-85.0 sec 26.4 MBytes 44.2 Mbits/sec
[ 3] 85.0-90.0 sec 31.5 MBytes 52.8 Mbits/sec
With patch:
echo "1 64" > htt_max_amsdu_ampdu
[ 3] local 192.168.12.2 port 57512 connected with 192.168.12.1 port 5001
[ ID] Interval Transfer Bandwidth
[ 3] 0.0- 5.0 sec 60.8 MBytes 102 Mbits/sec
[ 3] 5.0-10.0 sec 62.2 MBytes 104 Mbits/sec
[ 3] 10.0-15.0 sec 60.9 MBytes 102 Mbits/sec
echo "7 64" > htt_max_amsdu_ampdu // none DUP ACK form AP --> STA seen
[ 3] 15.0-20.0 sec 68.1 MBytes 114 Mbits/sec
[ 3] 20.0-25.0 sec 80.5 MBytes 135 Mbits/sec
[ 3] 25.0-30.0 sec 83.0 MBytes 139 Mbits/sec
[ 3] 30.0-35.0 sec 79.1 MBytes 133 Mbits/sec
[ 3] 35.0-40.0 sec 77.1 MBytes 129 Mbits/sec
[ 3] 40.0-45.0 sec 77.4 MBytes 130 Mbits/sec
BR
Janusz
Janusz Dziedzic <[email protected]> writes:
> In case of A-MSDU RX we should check flags for all
> MSDU subframes to be sure we have correct FCS status.
>
> Before we check attention flags only for first frame
> where we didn't have correct info about FCS status in
> case of A-MSDU. Next didn't mark RX_FLAG_FAILED_FCS_CRC
> for skb. As a side efect we see big TP drop when TCP used.
> This was easy to reproduce when AP interface was used
> and added monitor interface run in promiscuous mode.
>
> Reported-by: Denton Gentry <[email protected]>
> Signed-off-by: Janusz Dziedzic <[email protected]>
I'm having a hard time to understand the commit log. I think it would
make it easier to read if you first clearly describe the bug you are
fixing and then, in a separate paragraph, tell how you fix it.
So what is the bug Denton reported? "we see big TP drop when TCP used"
implies that this happens with all TCP traffic, but that can't be right.
Or did it happen with one of the reorder patches?
--
Kalle Valo