Hi,
This patchset unifies rx path. Instead of
branching for A-MSDU and MSDU cases a single
codepath is introduced.
This yields:
text data bss dec hex filename
14068 0 0 14068 36f4 before/htt_rx.o
13308 3 0 13311 33ff after/htt_rx.o
I haven't measured any Rx performance loss in my
setup but I may be biased. In theory this could
reduce i-cache pressure and improve Rx throughput
on CPU contrained systems by a few mbps.
This patchset serves two purposes though:
- clean up and unify the Rx path a bit
- make it possible to reuse code more easily in
the future
Michal Kazior (7):
ath10k: start using sk_buff_head
ath10k: simplify Rx loop
ath10k: refactor htt->rx_confused
ath10k: unify rx undecapping
ath10k: remove unused function argument
ath10k: use rx descriptor for ppdu status extraction
ath10k: report rx rate and signal for fragmented Rx
drivers/net/wireless/ath/ath10k/htt_rx.c | 1063 ++++++++++++++++--------------
1 file changed, 577 insertions(+), 486 deletions(-)
--
1.8.5.3
On 17 November 2014 15:32, Kalle Valo <[email protected]> wrote:
> Michal Kazior <[email protected]> writes:
>
>> This creates a single, common path for MSDU,
>> A-MSDU and fragmented Rx.
>>
>> Hopefully this will make it easier to understand
>> Rx path and make it easier to work with.
>>
>> Signed-off-by: Michal Kazior <[email protected]>
>
> This patch had few checkpatch warnings. I fixed them with the folded
> patch and full patch here:
>
> https://github.com/kvalo/ath/commit/71fbd07d43e54f5f9f442bc5f2f4f9ef83aead63
Thanks!
[...]
> @@ -1132,7 +1133,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
> bool has_fcs_err;
> bool has_crypto_err;
> bool has_tkip_err;
> - bool has_peer_idx_invalid;
> + bool has_idx_invalid;
> bool is_decrypted;
I don't really like the has_idx_invalid. Perhaps has_peer_err conveys
a bit more of the original meaning?
As much as I'd like to leave the original var name I'd like to be
checkpatch warning free. Sigh..
Michał
Instead of using manual sk_buff linking via ->next
use sk_buff_head. It's more robust, cleaner and
there's plenty of helper functions in kernel
already to manage sk_buff_head.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 213 +++++++++++++------------------
1 file changed, 91 insertions(+), 122 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index a691fdf..85f11e6 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -303,27 +303,15 @@ static inline struct sk_buff *ath10k_htt_rx_netbuf_pop(struct ath10k_htt *htt)
return msdu;
}
-static void ath10k_htt_rx_free_msdu_chain(struct sk_buff *skb)
-{
- struct sk_buff *next;
-
- while (skb) {
- next = skb->next;
- dev_kfree_skb_any(skb);
- skb = next;
- }
-}
-
/* return: < 0 fatal error, 0 - non chained msdu, 1 chained msdu */
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_head *amsdu,
u32 *attention)
{
struct ath10k *ar = htt->ar;
int msdu_len, msdu_chaining = 0;
- struct sk_buff *msdu, *next;
+ struct sk_buff *msdu;
struct htt_rx_desc *rx_desc;
lockdep_assert_held(&htt->rx_ring.lock);
@@ -333,10 +321,19 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
return -1;
}
- msdu = *head_msdu = ath10k_htt_rx_netbuf_pop(htt);
- while (msdu) {
+ for (;;) {
int last_msdu, msdu_len_invalid, msdu_chained;
+ msdu = ath10k_htt_rx_netbuf_pop(htt);
+ if (!msdu) {
+ ath10k_err(ar, "failed to pop msdu\n");
+ __skb_queue_purge(amsdu);
+ htt->rx_confused = true;
+ break;
+ }
+
+ __skb_queue_tail(amsdu, msdu);
+
rx_desc = (struct htt_rx_desc *)msdu->data;
/* FIXME: we must report msdu payload since this is what caller
@@ -354,10 +351,8 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
*/
if (!(__le32_to_cpu(rx_desc->attention.flags)
& RX_ATTENTION_FLAGS_MSDU_DONE)) {
- ath10k_htt_rx_free_msdu_chain(*head_msdu);
- *head_msdu = NULL;
- msdu = NULL;
- ath10k_err(ar, "htt rx stopped. cannot recover\n");
+ ath10k_err(ar, "popped an incomplete msdu\n");
+ __skb_queue_purge(amsdu);
htt->rx_confused = true;
break;
}
@@ -423,25 +418,20 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
skb_put(msdu, min(msdu_len, HTT_RX_MSDU_SIZE));
msdu_len -= msdu->len;
- /* FIXME: Do chained buffers include htt_rx_desc or not? */
+ /* Note: Chained buffers do not contain rx descriptor */
while (msdu_chained--) {
- struct sk_buff *next = ath10k_htt_rx_netbuf_pop(htt);
-
- if (!next) {
+ msdu = ath10k_htt_rx_netbuf_pop(htt);
+ if (!msdu) {
ath10k_warn(ar, "failed to pop chained msdu\n");
- ath10k_htt_rx_free_msdu_chain(*head_msdu);
- *head_msdu = NULL;
- msdu = NULL;
+ __skb_queue_purge(amsdu);
htt->rx_confused = true;
break;
}
- skb_trim(next, 0);
- skb_put(next, min(msdu_len, HTT_RX_BUF_SIZE));
- msdu_len -= next->len;
-
- msdu->next = next;
- msdu = next;
+ __skb_queue_tail(amsdu, msdu);
+ skb_trim(msdu, 0);
+ skb_put(msdu, min(msdu_len, HTT_RX_BUF_SIZE));
+ msdu_len -= msdu->len;
msdu_chaining = 1;
}
@@ -450,18 +440,12 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
trace_ath10k_htt_rx_desc(ar, &rx_desc->attention,
sizeof(*rx_desc) - sizeof(u32));
- if (last_msdu) {
- msdu->next = NULL;
- break;
- }
- next = ath10k_htt_rx_netbuf_pop(htt);
- msdu->next = next;
- msdu = next;
+ if (last_msdu)
+ break;
}
- *tail_msdu = msdu;
- if (*head_msdu == NULL)
+ if (skb_queue_empty(amsdu))
msdu_chaining = -1;
/*
@@ -915,11 +899,11 @@ static int ath10k_htt_rx_nwifi_hdrlen(struct ieee80211_hdr *hdr)
static void ath10k_htt_rx_amsdu(struct ath10k_htt *htt,
struct ieee80211_rx_status *rx_status,
- struct sk_buff *skb_in)
+ struct sk_buff_head *amsdu)
{
struct ath10k *ar = htt->ar;
struct htt_rx_desc *rxd;
- struct sk_buff *skb = skb_in;
+ struct sk_buff *skb;
struct sk_buff *first;
enum rx_msdu_decap_format fmt;
enum htt_rx_mpdu_encrypt_type enctype;
@@ -927,7 +911,9 @@ static void ath10k_htt_rx_amsdu(struct ath10k_htt *htt,
u8 hdr_buf[64], da[ETH_ALEN], sa[ETH_ALEN], *qos;
unsigned int hdr_len;
- rxd = (void *)skb->data - sizeof(*rxd);
+ first = skb_peek(amsdu);
+
+ rxd = (void *)first->data - sizeof(*rxd);
enctype = MS(__le32_to_cpu(rxd->mpdu_start.info0),
RX_MPDU_START_INFO0_ENCRYPT_TYPE);
@@ -936,8 +922,7 @@ static void ath10k_htt_rx_amsdu(struct ath10k_htt *htt,
memcpy(hdr_buf, hdr, hdr_len);
hdr = (struct ieee80211_hdr *)hdr_buf;
- first = skb;
- while (skb) {
+ while ((skb = __skb_dequeue(amsdu))) {
void *decap_hdr;
int len;
@@ -1005,18 +990,15 @@ static void ath10k_htt_rx_amsdu(struct ath10k_htt *htt,
break;
}
- skb_in = skb;
- ath10k_htt_rx_h_protected(htt, rx_status, skb_in, enctype, fmt,
+ ath10k_htt_rx_h_protected(htt, rx_status, skb, enctype, fmt,
false);
- skb = skb->next;
- skb_in->next = NULL;
- if (skb)
- rx_status->flag |= RX_FLAG_AMSDU_MORE;
- else
+ if (skb_queue_empty(amsdu))
rx_status->flag &= ~RX_FLAG_AMSDU_MORE;
+ else
+ rx_status->flag |= RX_FLAG_AMSDU_MORE;
- ath10k_process_rx(htt->ar, rx_status, skb_in);
+ ath10k_process_rx(htt->ar, rx_status, skb);
}
/* FIXME: It might be nice to re-assemble the A-MSDU when there's a
@@ -1035,13 +1017,6 @@ static void ath10k_htt_rx_msdu(struct ath10k_htt *htt,
int hdr_len;
void *rfc1042;
- /* This shouldn't happen. If it does than it may be a FW bug. */
- if (skb->next) {
- ath10k_warn(ar, "htt rx received chained non A-MSDU frame\n");
- ath10k_htt_rx_free_msdu_chain(skb->next);
- skb->next = NULL;
- }
-
rxd = (void *)skb->data - sizeof(*rxd);
fmt = MS(__le32_to_cpu(rxd->msdu_start.info1),
RX_MSDU_START_INFO1_DECAP_FORMAT);
@@ -1127,10 +1102,9 @@ static int ath10k_htt_rx_get_csum_state(struct sk_buff *skb)
return CHECKSUM_UNNECESSARY;
}
-static int ath10k_unchain_msdu(struct sk_buff *msdu_head)
+static int ath10k_unchain_msdu(struct sk_buff_head *amsdu)
{
- struct sk_buff *next = msdu_head->next;
- struct sk_buff *to_free = next;
+ struct sk_buff *skb, *first;
int space;
int total_len = 0;
@@ -1141,40 +1115,33 @@ static int ath10k_unchain_msdu(struct sk_buff *msdu_head)
* skb?
*/
- msdu_head->next = NULL;
+ first = __skb_dequeue(amsdu);
/* Allocate total length all at once. */
- while (next) {
- total_len += next->len;
- next = next->next;
- }
+ skb_queue_walk(amsdu, skb)
+ total_len += skb->len;
- space = total_len - skb_tailroom(msdu_head);
+ space = total_len - skb_tailroom(first);
if ((space > 0) &&
- (pskb_expand_head(msdu_head, 0, space, GFP_ATOMIC) < 0)) {
+ (pskb_expand_head(first, 0, space, GFP_ATOMIC) < 0)) {
/* TODO: bump some rx-oom error stat */
/* put it back together so we can free the
* whole list at once.
*/
- msdu_head->next = to_free;
+ __skb_queue_head(amsdu, first);
return -1;
}
/* Walk list again, copying contents into
* msdu_head
*/
- next = to_free;
- while (next) {
- skb_copy_from_linear_data(next, skb_put(msdu_head, next->len),
- next->len);
- next = next->next;
+ while ((skb = __skb_dequeue(amsdu))) {
+ skb_copy_from_linear_data(skb, skb_put(first, skb->len),
+ skb->len);
+ dev_kfree_skb_any(skb);
}
- /* If here, we have consolidated skb. Free the
- * fragments and pass the main skb on up the
- * stack.
- */
- ath10k_htt_rx_free_msdu_chain(to_free);
+ __skb_queue_head(amsdu, first);
return 0;
}
@@ -1223,6 +1190,7 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
struct ath10k *ar = htt->ar;
struct ieee80211_rx_status *rx_status = &htt->rx_status;
struct htt_rx_indication_mpdu_range *mpdu_ranges;
+ struct sk_buff_head amsdu;
struct ieee80211_hdr *hdr;
int num_mpdu_ranges;
u32 attention;
@@ -1271,35 +1239,28 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
for (i = 0; i < num_mpdu_ranges; i++) {
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,
+ __skb_queue_head_init(&amsdu);
+ ret = ath10k_htt_rx_amsdu_pop(htt, &fw_desc,
+ &fw_desc_len, &amsdu,
&attention);
if (ret < 0) {
ath10k_warn(ar, "failed to pop amsdu from htt rx ring %d\n",
ret);
- ath10k_htt_rx_free_msdu_chain(msdu_head);
+ __skb_queue_purge(&amsdu);
continue;
}
- if (!ath10k_htt_rx_amsdu_allowed(htt, msdu_head,
+ if (!ath10k_htt_rx_amsdu_allowed(htt, skb_peek(&amsdu),
channel_set,
attention)) {
- ath10k_htt_rx_free_msdu_chain(msdu_head);
+ __skb_queue_purge(&amsdu);
continue;
}
- if (ret > 0 &&
- ath10k_unchain_msdu(msdu_head) < 0) {
- ath10k_htt_rx_free_msdu_chain(msdu_head);
+ if (ret > 0 && ath10k_unchain_msdu(&amsdu) < 0) {
+ __skb_queue_purge(&amsdu);
continue;
}
@@ -1313,12 +1274,13 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
else
rx_status->flag &= ~RX_FLAG_MMIC_ERROR;
- hdr = ath10k_htt_rx_skb_get_hdr(msdu_head);
+ hdr = ath10k_htt_rx_skb_get_hdr(skb_peek(&amsdu));
if (ath10k_htt_rx_hdr_is_amsdu(hdr))
- ath10k_htt_rx_amsdu(htt, rx_status, msdu_head);
+ ath10k_htt_rx_amsdu(htt, rx_status, &amsdu);
else
- ath10k_htt_rx_msdu(htt, rx_status, msdu_head);
+ ath10k_htt_rx_msdu(htt, rx_status,
+ __skb_dequeue(&amsdu));
}
}
@@ -1329,12 +1291,13 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
struct htt_rx_fragment_indication *frag)
{
struct ath10k *ar = htt->ar;
- struct sk_buff *msdu_head, *msdu_tail;
+ struct sk_buff *msdu;
enum htt_rx_mpdu_encrypt_type enctype;
struct htt_rx_desc *rxd;
enum rx_msdu_decap_format fmt;
struct ieee80211_rx_status *rx_status = &htt->rx_status;
struct ieee80211_hdr *hdr;
+ struct sk_buff_head amsdu;
int ret;
bool tkip_mic_err;
bool decrypt_err;
@@ -1346,13 +1309,11 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
fw_desc_len = __le16_to_cpu(frag->fw_rx_desc_bytes);
fw_desc = (u8 *)frag->fw_msdu_rx_desc;
- msdu_head = NULL;
- msdu_tail = NULL;
+ __skb_queue_head_init(&amsdu);
spin_lock_bh(&htt->rx_ring.lock);
ret = ath10k_htt_rx_amsdu_pop(htt, &fw_desc, &fw_desc_len,
- &msdu_head, &msdu_tail,
- &attention);
+ &amsdu, &attention);
spin_unlock_bh(&htt->rx_ring.lock);
tasklet_schedule(&htt->rx_replenish_task);
@@ -1362,15 +1323,23 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
if (ret) {
ath10k_warn(ar, "failed to pop amsdu from httr rx ring for fragmented rx %d\n",
ret);
- ath10k_htt_rx_free_msdu_chain(msdu_head);
+ __skb_queue_purge(&amsdu);
return;
}
+ if (skb_queue_len(&amsdu) != 1) {
+ ath10k_warn(ar, "failed to pop frag amsdu: too many msdus\n");
+ __skb_queue_purge(&amsdu);
+ return;
+ }
+
+ msdu = __skb_dequeue(&amsdu);
+
/* FIXME: implement signal strength */
rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL;
- hdr = (struct ieee80211_hdr *)msdu_head->data;
- rxd = (void *)msdu_head->data - sizeof(*rxd);
+ hdr = (struct ieee80211_hdr *)msdu->data;
+ rxd = (void *)msdu->data - sizeof(*rxd);
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),
@@ -1378,22 +1347,22 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
if (fmt != RX_MSDU_DECAP_RAW) {
ath10k_warn(ar, "we dont support non-raw fragmented rx yet\n");
- dev_kfree_skb_any(msdu_head);
+ dev_kfree_skb_any(msdu);
goto end;
}
enctype = MS(__le32_to_cpu(rxd->mpdu_start.info0),
RX_MPDU_START_INFO0_ENCRYPT_TYPE);
- ath10k_htt_rx_h_protected(htt, rx_status, msdu_head, enctype, fmt,
+ ath10k_htt_rx_h_protected(htt, rx_status, msdu, enctype, fmt,
true);
- msdu_head->ip_summed = ath10k_htt_rx_get_csum_state(msdu_head);
+ msdu->ip_summed = ath10k_htt_rx_get_csum_state(msdu);
if (tkip_mic_err)
ath10k_warn(ar, "tkip mic error\n");
if (decrypt_err) {
ath10k_warn(ar, "decryption err in fragmented rx\n");
- dev_kfree_skb_any(msdu_head);
+ dev_kfree_skb_any(msdu);
goto end;
}
@@ -1402,11 +1371,11 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
paramlen = ath10k_htt_rx_crypto_param_len(ar, enctype);
/* It is more efficient to move the header than the payload */
- memmove((void *)msdu_head->data + paramlen,
- (void *)msdu_head->data,
+ memmove((void *)msdu->data + paramlen,
+ (void *)msdu->data,
hdrlen);
- skb_pull(msdu_head, paramlen);
- hdr = (struct ieee80211_hdr *)msdu_head->data;
+ skb_pull(msdu, paramlen);
+ hdr = (struct ieee80211_hdr *)msdu->data;
}
/* remove trailing FCS */
@@ -1420,17 +1389,17 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
enctype == HTT_RX_MPDU_ENCRYPT_TKIP_WPA)
trim += MICHAEL_MIC_LEN;
- if (trim > msdu_head->len) {
+ if (trim > msdu->len) {
ath10k_warn(ar, "htt rx fragment: trailer longer than the frame itself? drop\n");
- dev_kfree_skb_any(msdu_head);
+ dev_kfree_skb_any(msdu);
goto end;
}
- skb_trim(msdu_head, msdu_head->len - trim);
+ skb_trim(msdu, msdu->len - trim);
ath10k_dbg_dump(ar, ATH10K_DBG_HTT_DUMP, NULL, "htt rx frag mpdu: ",
- msdu_head->data, msdu_head->len);
- ath10k_process_rx(htt->ar, rx_status, msdu_head);
+ msdu->data, msdu->len);
+ ath10k_process_rx(htt->ar, rx_status, msdu);
end:
if (fw_desc_len > 0) {
--
1.8.5.3
The original fix has been moved into a different
place in the code.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 6abfea7..55e4568 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -306,8 +306,7 @@ static inline struct sk_buff *ath10k_htt_rx_netbuf_pop(struct ath10k_htt *htt)
/* return: < 0 fatal error, 0 - non chained msdu, 1 chained msdu */
static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
u8 **fw_desc, int *fw_desc_len,
- struct sk_buff_head *amsdu,
- u32 *attention)
+ struct sk_buff_head *amsdu)
{
struct ath10k *ar = htt->ar;
int msdu_len, msdu_chaining = 0;
@@ -348,11 +347,6 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
return -EIO;
}
- *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
@@ -1362,7 +1356,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
struct htt_rx_indication_mpdu_range *mpdu_ranges;
struct sk_buff_head amsdu;
int num_mpdu_ranges;
- u32 attention;
int fw_desc_len;
u8 *fw_desc;
bool channel_set;
@@ -1412,11 +1405,9 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
mpdu_count += mpdu_ranges[i].mpdu_count;
while (mpdu_count--) {
- attention = 0;
__skb_queue_head_init(&amsdu);
ret = ath10k_htt_rx_amsdu_pop(htt, &fw_desc,
- &fw_desc_len, &amsdu,
- &attention);
+ &fw_desc_len, &amsdu);
if (ret < 0) {
ath10k_warn(ar, "rx ring became corrupted: %d\n", ret);
__skb_queue_purge(&amsdu);
@@ -1445,7 +1436,6 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
int ret;
u8 *fw_desc;
int fw_desc_len;
- u32 attention = 0;
fw_desc_len = __le16_to_cpu(frag->fw_rx_desc_bytes);
fw_desc = (u8 *)frag->fw_msdu_rx_desc;
@@ -1454,7 +1444,7 @@ 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,
- &amsdu, &attention);
+ &amsdu);
spin_unlock_bh(&htt->rx_ring.lock);
tasklet_schedule(&htt->rx_replenish_task);
--
1.8.5.3
Michal Kazior <[email protected]> writes:
> This creates a single, common path for MSDU,
> A-MSDU and fragmented Rx.
>
> Hopefully this will make it easier to understand
> Rx path and make it easier to work with.
>
> Signed-off-by: Michal Kazior <[email protected]>
This patch had few checkpatch warnings. I fixed them with the folded
patch and full patch here:
https://github.com/kvalo/ath/commit/71fbd07d43e54f5f9f442bc5f2f4f9ef83aead63
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 6abfea768173..08963439891b 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1066,7 +1066,8 @@ static void ath10k_htt_rx_h_undecap(struct ath10k *ar,
switch (decap) {
case RX_MSDU_DECAP_RAW:
- ath10k_htt_rx_h_undecap_raw(ar, msdu, status, enctype, is_decrypted);
+ ath10k_htt_rx_h_undecap_raw(ar, msdu, status, enctype,
+ is_decrypted);
break;
case RX_MSDU_DECAP_NATIVE_WIFI:
ath10k_htt_rx_h_undecap_nwifi(ar, msdu, status, first_hdr);
@@ -1132,7 +1133,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
bool has_fcs_err;
bool has_crypto_err;
bool has_tkip_err;
- bool has_peer_idx_invalid;
+ bool has_idx_invalid;
bool is_decrypted;
if (skb_queue_empty(amsdu))
@@ -1167,8 +1168,8 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
__cpu_to_le32(RX_ATTENTION_FLAGS_DECRYPT_ERR));
has_tkip_err = !!(rxd->attention.flags &
__cpu_to_le32(RX_ATTENTION_FLAGS_TKIP_MIC_ERR));
- has_peer_idx_invalid = !!(rxd->attention.flags &
- __cpu_to_le32(RX_ATTENTION_FLAGS_PEER_IDX_INVALID));
+ has_idx_invalid = !!(rxd->attention.flags &
+ __cpu_to_le32(RX_ATTENTION_FLAGS_PEER_IDX_INVALID));
/* Note: If hardware captures an encrypted frame that it can't decrypt,
* e.g. due to fcs error, missing peer or invalid key data it will
@@ -1177,7 +1178,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
is_decrypted = (enctype != HTT_RX_MPDU_ENCRYPT_NONE &&
!has_fcs_err &&
!has_crypto_err &&
- !has_peer_idx_invalid);
+ !has_idx_invalid);
/* Clear per-MPDU flags while leaving per-PPDU flags intact. */
status->flag &= ~(RX_FLAG_FAILED_FCS_CRC |
--
Kalle Valo
This makes it more in line with the new Rx path.
It also makes the code more reusable because Rx
descriptor is more accessible.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 137 ++++++++++++++++++++++---------
1 file changed, 96 insertions(+), 41 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 55e4568..9073a0e 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -631,23 +631,34 @@ static const u8 rx_legacy_rate_idx[] = {
};
static void ath10k_htt_rx_h_rates(struct ath10k *ar,
- enum ieee80211_band band,
- u8 info0, u32 info1, u32 info2,
- struct ieee80211_rx_status *status)
+ struct ieee80211_rx_status *status,
+ struct htt_rx_desc *rxd)
{
+ enum ieee80211_band band;
u8 cck, rate, rate_idx, bw, sgi, mcs, nss;
u8 preamble = 0;
+ u32 info1, info2, info3;
- /* Check if valid fields */
- if (!(info0 & HTT_RX_INDICATION_INFO0_START_VALID))
+ /* Band value can't be set as undefined but freq can be 0 - use that to
+ * determine whether band is provided.
+ *
+ * FIXME: Perhaps this can go away if CCK rate reporting is a little
+ * reworked?
+ */
+ if (!status->freq)
return;
- preamble = MS(info1, HTT_RX_INDICATION_INFO1_PREAMBLE_TYPE);
+ band = status->band;
+ info1 = __le32_to_cpu(rxd->ppdu_start.info1);
+ info2 = __le32_to_cpu(rxd->ppdu_start.info2);
+ info3 = __le32_to_cpu(rxd->ppdu_start.info3);
+
+ preamble = MS(info1, RX_PPDU_START_INFO1_PREAMBLE_TYPE);
switch (preamble) {
case HTT_RX_LEGACY:
- cck = info0 & HTT_RX_INDICATION_INFO0_LEGACY_RATE_CCK;
- rate = MS(info0, HTT_RX_INDICATION_INFO0_LEGACY_RATE);
+ cck = info1 & RX_PPDU_START_INFO1_L_SIG_RATE_SELECT;
+ rate = MS(info1, RX_PPDU_START_INFO1_L_SIG_RATE);
rate_idx = 0;
if (rate < 0x08 || rate > 0x0F)
@@ -674,11 +685,11 @@ static void ath10k_htt_rx_h_rates(struct ath10k *ar,
break;
case HTT_RX_HT:
case HTT_RX_HT_WITH_TXBF:
- /* HT-SIG - Table 20-11 in info1 and info2 */
- mcs = info1 & 0x1F;
+ /* HT-SIG - Table 20-11 in info2 and info3 */
+ mcs = info2 & 0x1F;
nss = mcs >> 3;
- bw = (info1 >> 7) & 1;
- sgi = (info2 >> 7) & 1;
+ bw = (info2 >> 7) & 1;
+ sgi = (info3 >> 7) & 1;
status->rate_idx = mcs;
status->flag |= RX_FLAG_HT;
@@ -689,12 +700,12 @@ static void ath10k_htt_rx_h_rates(struct ath10k *ar,
break;
case HTT_RX_VHT:
case HTT_RX_VHT_WITH_TXBF:
- /* VHT-SIG-A1 in info 1, VHT-SIG-A2 in info2
+ /* VHT-SIG-A1 in info2, VHT-SIG-A2 in info3
TODO check this */
- mcs = (info2 >> 4) & 0x0F;
- nss = ((info1 >> 10) & 0x07) + 1;
- bw = info1 & 3;
- sgi = info2 & 1;
+ mcs = (info3 >> 4) & 0x0F;
+ nss = ((info2 >> 10) & 0x07) + 1;
+ bw = info2 & 3;
+ sgi = info3 & 1;
status->rate_idx = mcs;
status->vht_nss = nss;
@@ -742,6 +753,73 @@ static bool ath10k_htt_rx_h_channel(struct ath10k *ar,
return true;
}
+static void ath10k_htt_rx_h_signal(struct ath10k *ar,
+ struct ieee80211_rx_status *status,
+ struct htt_rx_desc *rxd)
+{
+ /* FIXME: Get real NF */
+ status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
+ rxd->ppdu_start.rssi_comb;
+ status->flag &= ~RX_FLAG_NO_SIGNAL_VAL;
+}
+
+static void ath10k_htt_rx_h_mactime(struct ath10k *ar,
+ struct ieee80211_rx_status *status,
+ struct htt_rx_desc *rxd)
+{
+ /* FIXME: TSF is known only at the end of PPDU, in the last MPDU. This
+ * means all prior MSDUs in a PPDU are reported to mac80211 without the
+ * TSF. Is it worth holding frames until end of PPDU is known?
+ *
+ * FIXME: Can we get/compute 64bit TSF?
+ */
+ status->mactime = __le32_to_cpu(rxd->ppdu_end.tsf_timestamp);
+ status->flag |= RX_FLAG_MACTIME_END;
+}
+
+static void ath10k_htt_rx_h_ppdu(struct ath10k *ar,
+ struct sk_buff_head *amsdu,
+ struct ieee80211_rx_status *status)
+{
+ struct sk_buff *first;
+ struct htt_rx_desc *rxd;
+ bool is_first_ppdu;
+ bool is_last_ppdu;
+
+ if (skb_queue_empty(amsdu))
+ return;
+
+ first = skb_peek(amsdu);
+ rxd = (void *)first->data - sizeof(*rxd);
+
+ is_first_ppdu = !!(rxd->attention.flags &
+ __cpu_to_le32(RX_ATTENTION_FLAGS_FIRST_MPDU));
+ is_last_ppdu = !!(rxd->attention.flags &
+ __cpu_to_le32(RX_ATTENTION_FLAGS_LAST_MPDU));
+
+ if (is_first_ppdu) {
+ /* New PPDU starts so clear out the old per-PPDU status. */
+ status->freq = 0;
+ status->rate_idx = 0;
+ status->vht_nss = 0;
+ status->vht_flag &= ~RX_VHT_FLAG_80MHZ;
+ status->flag &= ~(RX_FLAG_HT |
+ RX_FLAG_VHT |
+ RX_FLAG_SHORT_GI |
+ RX_FLAG_40MHZ |
+ RX_FLAG_MACTIME_END);
+ status->flag |= RX_FLAG_NO_SIGNAL_VAL;
+
+ ath10k_htt_rx_h_signal(ar, status, rxd);
+ ath10k_htt_rx_h_channel(ar, status);
+ ath10k_htt_rx_h_rates(ar, status, rxd);
+ }
+
+ if (is_last_ppdu) {
+ ath10k_htt_rx_h_mactime(ar, status, rxd);
+ }
+}
+
static const char * const tid_to_ac[] = {
"BE",
"BK",
@@ -1358,7 +1436,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
int num_mpdu_ranges;
int fw_desc_len;
u8 *fw_desc;
- bool channel_set;
int i, ret, mpdu_count = 0;
lockdep_assert_held(&htt->rx_ring.lock);
@@ -1373,29 +1450,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
HTT_RX_INDICATION_INFO1_NUM_MPDU_RANGES);
mpdu_ranges = htt_rx_ind_get_mpdu_ranges(rx);
- /* Fill this once, while this is per-ppdu */
- if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_START_VALID) {
- memset(rx_status, 0, sizeof(*rx_status));
- rx_status->signal = ATH10K_DEFAULT_NOISE_FLOOR +
- rx->ppdu.combined_rssi;
- }
-
- if (rx->ppdu.info0 & HTT_RX_INDICATION_INFO0_END_VALID) {
- /* TSF available only in 32-bit */
- rx_status->mactime = __le32_to_cpu(rx->ppdu.tsf) & 0xffffffff;
- rx_status->flag |= RX_FLAG_MACTIME_END;
- }
-
- channel_set = ath10k_htt_rx_h_channel(htt->ar, rx_status);
-
- if (channel_set) {
- ath10k_htt_rx_h_rates(htt->ar, rx_status->band,
- rx->ppdu.info0,
- __le32_to_cpu(rx->ppdu.info1),
- __le32_to_cpu(rx->ppdu.info2),
- rx_status);
- }
-
ath10k_dbg_dump(ar, ATH10K_DBG_HTT_DUMP, NULL, "htt rx ind: ",
rx, sizeof(*rx) +
(sizeof(struct htt_rx_indication_mpdu_range) *
@@ -1418,6 +1472,7 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
break;
}
+ ath10k_htt_rx_h_ppdu(ar, &amsdu, rx_status);
ath10k_htt_rx_h_unchain(ar, &amsdu, ret > 0);
ath10k_htt_rx_h_filter(ar, &amsdu, rx_status);
ath10k_htt_rx_h_mpdu(ar, &amsdu, rx_status);
--
1.8.5.3
On 17 November 2014 16:11, Kalle Valo <[email protected]> wrote:
> Michal Kazior <[email protected]> writes:
>> On 17 November 2014 15:32, Kalle Valo <[email protected]> wrote:
[...]
>>> This patch had few checkpatch warnings. I fixed them with the folded
>>> patch and full patch here:
>>>
>>> https://github.com/kvalo/ath/commit/71fbd07d43e54f5f9f442bc5f2f4f9ef83aead63
[...]
>>> @@ -1132,7 +1133,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
>>> bool has_fcs_err;
>>> bool has_crypto_err;
>>> bool has_tkip_err;
>>> - bool has_peer_idx_invalid;
>>> + bool has_idx_invalid;
>>> bool is_decrypted;
>>
>> I don't really like the has_idx_invalid. Perhaps has_peer_err conveys
>> a bit more of the original meaning?
>
> What about just peer_idx_invalid? IMHO we really don't need the has_
> prefix in that relatively small function.
Good point but the assignment line with `peer_idx_invalid` will still
be over 80 chars long, no?
So perhaps instead of doing `rxd->attention.flags &
__cpu_to_le32(FLAG)` it could be `attention =
__le32_to_cpu(rxd->attention.flags)` and then `attention & FLAG` ?
This way it shouldn't exceed the 80 char limit and var names won't
need to be changed. Hopefully compiler will optimize it away.
Michał
Fragmented Rx wasn't reporting everything. With
the reworked Rx code it's very easy to fix it.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 9073a0e..a22e832 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1519,9 +1519,7 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
return;
}
- /* FIXME: implement signal strength */
- rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL;
-
+ ath10k_htt_rx_h_ppdu(ar, &amsdu, rx_status);
ath10k_htt_rx_h_filter(ar, &amsdu, rx_status);
ath10k_htt_rx_h_mpdu(ar, &amsdu, rx_status);
ath10k_htt_rx_h_deliver(ar, &amsdu, rx_status);
--
1.8.5.3
Since htt_rx_mpdu_status isn't used anymore
(instead attention flags are used) simplify the
loop.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 80 ++++++++++++++++----------------
1 file changed, 39 insertions(+), 41 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 85f11e6..cba2f3b 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1197,8 +1197,7 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
int fw_desc_len;
u8 *fw_desc;
bool channel_set;
- int i, j;
- int ret;
+ int i, ret, mpdu_count = 0;
lockdep_assert_held(&htt->rx_ring.lock);
@@ -1237,51 +1236,50 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
(sizeof(struct htt_rx_indication_mpdu_range) *
num_mpdu_ranges));
- for (i = 0; i < num_mpdu_ranges; i++) {
- for (j = 0; j < mpdu_ranges[i].mpdu_count; j++) {
- attention = 0;
- __skb_queue_head_init(&amsdu);
- ret = ath10k_htt_rx_amsdu_pop(htt, &fw_desc,
- &fw_desc_len, &amsdu,
- &attention);
-
- if (ret < 0) {
- ath10k_warn(ar, "failed to pop amsdu from htt rx ring %d\n",
- ret);
- __skb_queue_purge(&amsdu);
- continue;
- }
+ for (i = 0; i < num_mpdu_ranges; i++)
+ mpdu_count += mpdu_ranges[i].mpdu_count;
+
+ while (mpdu_count--) {
+ attention = 0;
+ __skb_queue_head_init(&amsdu);
+ ret = ath10k_htt_rx_amsdu_pop(htt, &fw_desc,
+ &fw_desc_len, &amsdu,
+ &attention);
+ if (ret < 0) {
+ ath10k_warn(ar, "failed to pop amsdu from htt rx ring %d\n",
+ ret);
+ __skb_queue_purge(&amsdu);
+ continue;
+ }
- if (!ath10k_htt_rx_amsdu_allowed(htt, skb_peek(&amsdu),
- channel_set,
- attention)) {
- __skb_queue_purge(&amsdu);
- continue;
- }
+ if (!ath10k_htt_rx_amsdu_allowed(htt, skb_peek(&amsdu),
+ channel_set, attention)) {
+ __skb_queue_purge(&amsdu);
+ continue;
+ }
- if (ret > 0 && ath10k_unchain_msdu(&amsdu) < 0) {
- __skb_queue_purge(&amsdu);
- continue;
- }
+ if (ret > 0 && ath10k_unchain_msdu(&amsdu) < 0) {
+ __skb_queue_purge(&amsdu);
+ continue;
+ }
- if (attention & RX_ATTENTION_FLAGS_FCS_ERR)
- rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
- else
- rx_status->flag &= ~RX_FLAG_FAILED_FCS_CRC;
+ if (attention & RX_ATTENTION_FLAGS_FCS_ERR)
+ rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
+ else
+ rx_status->flag &= ~RX_FLAG_FAILED_FCS_CRC;
- if (attention & RX_ATTENTION_FLAGS_TKIP_MIC_ERR)
- rx_status->flag |= RX_FLAG_MMIC_ERROR;
- else
- rx_status->flag &= ~RX_FLAG_MMIC_ERROR;
+ if (attention & RX_ATTENTION_FLAGS_TKIP_MIC_ERR)
+ rx_status->flag |= RX_FLAG_MMIC_ERROR;
+ else
+ rx_status->flag &= ~RX_FLAG_MMIC_ERROR;
- hdr = ath10k_htt_rx_skb_get_hdr(skb_peek(&amsdu));
+ hdr = ath10k_htt_rx_skb_get_hdr(skb_peek(&amsdu));
- if (ath10k_htt_rx_hdr_is_amsdu(hdr))
- ath10k_htt_rx_amsdu(htt, rx_status, &amsdu);
- else
- ath10k_htt_rx_msdu(htt, rx_status,
- __skb_dequeue(&amsdu));
- }
+ if (ath10k_htt_rx_hdr_is_amsdu(hdr))
+ ath10k_htt_rx_amsdu(htt, rx_status, &amsdu);
+ else
+ ath10k_htt_rx_msdu(htt, rx_status,
+ __skb_dequeue(&amsdu));
}
tasklet_schedule(&htt->rx_replenish_task);
--
1.8.5.3
Michal Kazior <[email protected]> writes:
> On 17 November 2014 15:32, Kalle Valo <[email protected]> wrote:
>> Michal Kazior <[email protected]> writes:
>>
>>> This creates a single, common path for MSDU,
>>> A-MSDU and fragmented Rx.
>>>
>>> Hopefully this will make it easier to understand
>>> Rx path and make it easier to work with.
>>>
>>> Signed-off-by: Michal Kazior <[email protected]>
>>
>> This patch had few checkpatch warnings. I fixed them with the folded
>> patch and full patch here:
>>
>> https://github.com/kvalo/ath/commit/71fbd07d43e54f5f9f442bc5f2f4f9ef83aead63
>
> Thanks!
>
> [...]
>> @@ -1132,7 +1133,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
>> bool has_fcs_err;
>> bool has_crypto_err;
>> bool has_tkip_err;
>> - bool has_peer_idx_invalid;
>> + bool has_idx_invalid;
>> bool is_decrypted;
>
> I don't really like the has_idx_invalid. Perhaps has_peer_err conveys
> a bit more of the original meaning?
What about just peer_idx_invalid? IMHO we really don't need the has_
prefix in that relatively small function.
> As much as I'd like to leave the original var name I'd like to be
> checkpatch warning free. Sigh..
Same here. The checkpatch is just so useful tool to keep the style
unified.
--
Kalle Valo
Make the rx_confused be handled by the rx
indication handlers instead of the buffer popping
function.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 29 ++++++++++++-----------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index cba2f3b..651028e 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -316,20 +316,13 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
lockdep_assert_held(&htt->rx_ring.lock);
- if (htt->rx_confused) {
- ath10k_warn(ar, "htt is confused. refusing rx\n");
- return -1;
- }
-
for (;;) {
int last_msdu, msdu_len_invalid, msdu_chained;
msdu = ath10k_htt_rx_netbuf_pop(htt);
if (!msdu) {
- ath10k_err(ar, "failed to pop msdu\n");
__skb_queue_purge(amsdu);
- htt->rx_confused = true;
- break;
+ return -ENOENT;
}
__skb_queue_tail(amsdu, msdu);
@@ -351,10 +344,8 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
*/
if (!(__le32_to_cpu(rx_desc->attention.flags)
& RX_ATTENTION_FLAGS_MSDU_DONE)) {
- ath10k_err(ar, "popped an incomplete msdu\n");
__skb_queue_purge(amsdu);
- htt->rx_confused = true;
- break;
+ return -EIO;
}
*attention |= __le32_to_cpu(rx_desc->attention.flags) &
@@ -422,10 +413,8 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
while (msdu_chained--) {
msdu = ath10k_htt_rx_netbuf_pop(htt);
if (!msdu) {
- ath10k_warn(ar, "failed to pop chained msdu\n");
__skb_queue_purge(amsdu);
- htt->rx_confused = true;
- break;
+ return -ENOENT;
}
__skb_queue_tail(amsdu, msdu);
@@ -1201,6 +1190,9 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
lockdep_assert_held(&htt->rx_ring.lock);
+ if (htt->rx_confused)
+ return;
+
fw_desc_len = __le16_to_cpu(rx->prefix.fw_rx_desc_bytes);
fw_desc = (u8 *)&rx->fw_desc;
@@ -1246,10 +1238,13 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
&fw_desc_len, &amsdu,
&attention);
if (ret < 0) {
- ath10k_warn(ar, "failed to pop amsdu from htt rx ring %d\n",
- ret);
+ ath10k_warn(ar, "rx ring became corrupted: %d\n", ret);
__skb_queue_purge(&amsdu);
- continue;
+ /* FIXME: It's probably a good idea to reboot the
+ * device instead of leaving it inoperable.
+ */
+ htt->rx_confused = true;
+ break;
}
if (!ath10k_htt_rx_amsdu_allowed(htt, skb_peek(&amsdu),
--
1.8.5.3
Michal Kazior <[email protected]> writes:
> On 17 November 2014 16:11, Kalle Valo <[email protected]> wrote:
>> Michal Kazior <[email protected]> writes:
>>> On 17 November 2014 15:32, Kalle Valo <[email protected]> wrote:
> [...]
>>>> This patch had few checkpatch warnings. I fixed them with the folded
>>>> patch and full patch here:
>>>>
>>>> https://github.com/kvalo/ath/commit/71fbd07d43e54f5f9f442bc5f2f4f9ef83aead63
> [...]
>>>> @@ -1132,7 +1133,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
>>>> bool has_fcs_err;
>>>> bool has_crypto_err;
>>>> bool has_tkip_err;
>>>> - bool has_peer_idx_invalid;
>>>> + bool has_idx_invalid;
>>>> bool is_decrypted;
>>>
>>> I don't really like the has_idx_invalid. Perhaps has_peer_err conveys
>>> a bit more of the original meaning?
>>
>> What about just peer_idx_invalid? IMHO we really don't need the has_
>> prefix in that relatively small function.
>
> Good point but the assignment line with `peer_idx_invalid` will still
> be over 80 chars long, no?
I actually test with 83 char limit nowadays. I should add the checkpatch
command line to the wiki.
> So perhaps instead of doing `rxd->attention.flags &
> __cpu_to_le32(FLAG)` it could be `attention =
> __le32_to_cpu(rxd->attention.flags)` and then `attention & FLAG` ?
> This way it shouldn't exceed the 80 char limit and var names won't
> need to be changed. Hopefully compiler will optimize it away.
That's much better and actually has a lot less calls to cpu_le32(). I
folded the patch below.
But aren't the '!!' operators useless? 'has_*' variables are booleans,
doesn't compiler handle that automatically?
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 6f58b7f912cd..6abb3b6a348f 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1133,8 +1133,9 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
bool has_fcs_err;
bool has_crypto_err;
bool has_tkip_err;
- bool has_idx_invalid;
+ bool has_peer_idx_invalid;
bool is_decrypted;
+ u32 attention;
if (skb_queue_empty(amsdu))
return;
@@ -1162,14 +1163,12 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
/* Some attention flags are valid only in the last MSDU. */
last = skb_peek_tail(amsdu);
rxd = (void *)last->data - sizeof(*rxd);
- has_fcs_err = !!(rxd->attention.flags &
- __cpu_to_le32(RX_ATTENTION_FLAGS_FCS_ERR));
- has_crypto_err = !!(rxd->attention.flags &
- __cpu_to_le32(RX_ATTENTION_FLAGS_DECRYPT_ERR));
- has_tkip_err = !!(rxd->attention.flags &
- __cpu_to_le32(RX_ATTENTION_FLAGS_TKIP_MIC_ERR));
- has_idx_invalid = !!(rxd->attention.flags &
- __cpu_to_le32(RX_ATTENTION_FLAGS_PEER_IDX_INVALID));
+ attention = __le32_to_cpu(rxd->attention.flags);
+
+ has_fcs_err = !!(attention & RX_ATTENTION_FLAGS_FCS_ERR);
+ has_crypto_err = !!(attention & RX_ATTENTION_FLAGS_DECRYPT_ERR);
+ has_tkip_err = !!(attention & RX_ATTENTION_FLAGS_TKIP_MIC_ERR);
+ has_peer_idx_invalid = !!(attention & RX_ATTENTION_FLAGS_PEER_IDX_INVALID);
/* Note: If hardware captures an encrypted frame that it can't decrypt,
* e.g. due to fcs error, missing peer or invalid key data it will
@@ -1178,7 +1177,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
is_decrypted = (enctype != HTT_RX_MPDU_ENCRYPT_NONE &&
!has_fcs_err &&
!has_crypto_err &&
- !has_idx_invalid);
+ !has_peer_idx_invalid);
/* Clear per-MPDU flags while leaving per-PPDU flags intact. */
status->flag &= ~(RX_FLAG_FAILED_FCS_CRC |
--
Kalle Valo
This creates a single, common path for MSDU,
A-MSDU and fragmented Rx.
Hopefully this will make it easier to understand
Rx path and make it easier to work with.
Signed-off-by: Michal Kazior <[email protected]>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 738 +++++++++++++++++--------------
1 file changed, 412 insertions(+), 326 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 651028e..6abfea7 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -603,35 +603,6 @@ static int ath10k_htt_rx_crypto_tail_len(struct ath10k *ar,
return 0;
}
-/* Applies for first msdu in chain, before altering it. */
-static struct ieee80211_hdr *ath10k_htt_rx_skb_get_hdr(struct sk_buff *skb)
-{
- struct htt_rx_desc *rxd;
- enum rx_msdu_decap_format fmt;
-
- rxd = (void *)skb->data - sizeof(*rxd);
- fmt = MS(__le32_to_cpu(rxd->msdu_start.info1),
- RX_MSDU_START_INFO1_DECAP_FORMAT);
-
- if (fmt == RX_MSDU_DECAP_RAW)
- return (void *)skb->data;
-
- return (void *)skb->data - RX_HTT_HDR_STATUS_LEN;
-}
-
-/* This function only applies for first msdu in an msdu chain */
-static bool ath10k_htt_rx_hdr_is_amsdu(struct ieee80211_hdr *hdr)
-{
- u8 *qc;
-
- if (ieee80211_is_data_qos(hdr->frame_control)) {
- qc = ieee80211_get_qos_ctl(hdr);
- if (qc[0] & 0x80)
- return true;
- }
- return false;
-}
-
struct rfc1042_hdr {
u8 llc_dsap;
u8 llc_ssap;
@@ -757,41 +728,6 @@ static void ath10k_htt_rx_h_rates(struct ath10k *ar,
}
}
-static void ath10k_htt_rx_h_protected(struct ath10k_htt *htt,
- struct ieee80211_rx_status *rx_status,
- struct sk_buff *skb,
- enum htt_rx_mpdu_encrypt_type enctype,
- enum rx_msdu_decap_format fmt,
- bool dot11frag)
-{
- struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
-
- rx_status->flag &= ~(RX_FLAG_DECRYPTED |
- RX_FLAG_IV_STRIPPED |
- RX_FLAG_MMIC_STRIPPED);
-
- if (enctype == HTT_RX_MPDU_ENCRYPT_NONE)
- return;
-
- /*
- * There's no explicit rx descriptor flag to indicate whether a given
- * frame has been decrypted or not. We're forced to use the decap
- * format as an implicit indication. However fragmentation rx is always
- * raw and it probably never reports undecrypted raws.
- *
- * This makes sure sniffed frames are reported as-is without stripping
- * the protected flag.
- */
- if (fmt == RX_MSDU_DECAP_RAW && !dot11frag)
- return;
-
- rx_status->flag |= RX_FLAG_DECRYPTED |
- RX_FLAG_IV_STRIPPED |
- RX_FLAG_MMIC_STRIPPED;
- hdr->frame_control = __cpu_to_le16(__le16_to_cpu(hdr->frame_control) &
- ~IEEE80211_FCTL_PROTECTED);
-}
-
static bool ath10k_htt_rx_h_channel(struct ath10k *ar,
struct ieee80211_rx_status *status)
{
@@ -886,178 +822,262 @@ static int ath10k_htt_rx_nwifi_hdrlen(struct ieee80211_hdr *hdr)
return round_up(ieee80211_hdrlen(hdr->frame_control), 4);
}
-static void ath10k_htt_rx_amsdu(struct ath10k_htt *htt,
- struct ieee80211_rx_status *rx_status,
- struct sk_buff_head *amsdu)
+static void ath10k_htt_rx_h_undecap_raw(struct ath10k *ar,
+ struct sk_buff *msdu,
+ struct ieee80211_rx_status *status,
+ enum htt_rx_mpdu_encrypt_type enctype,
+ bool is_decrypted)
{
- struct ath10k *ar = htt->ar;
+ struct ieee80211_hdr *hdr;
struct htt_rx_desc *rxd;
- struct sk_buff *skb;
- struct sk_buff *first;
- enum rx_msdu_decap_format fmt;
- enum htt_rx_mpdu_encrypt_type enctype;
+ size_t hdr_len;
+ size_t crypto_len;
+ bool is_first;
+ bool is_last;
+
+ rxd = (void *)msdu->data - sizeof(*rxd);
+ is_first = !!(rxd->msdu_end.info0 &
+ __cpu_to_le32(RX_MSDU_END_INFO0_FIRST_MSDU));
+ is_last = !!(rxd->msdu_end.info0 &
+ __cpu_to_le32(RX_MSDU_END_INFO0_LAST_MSDU));
+
+ /* Delivered decapped frame:
+ * [802.11 header]
+ * [crypto param] <-- can be trimmed if !fcs_err &&
+ * !decrypt_err && !peer_idx_invalid
+ * [amsdu header] <-- only if A-MSDU
+ * [rfc1042/llc]
+ * [payload]
+ * [FCS] <-- at end, needs to be trimmed
+ */
+
+ /* This probably shouldn't happen but warn just in case */
+ if (unlikely(WARN_ON_ONCE(!is_first)))
+ return;
+
+ /* This probably shouldn't happen but warn just in case */
+ if (unlikely(WARN_ON_ONCE(!(is_first && is_last))))
+ return;
+
+ skb_trim(msdu, msdu->len - FCS_LEN);
+
+ /* In most cases this will be true for sniffed frames. It makes sense
+ * to deliver them as-is without stripping the crypto param. This would
+ * also make sense for software based decryption (which is not
+ * implemented in ath10k).
+ *
+ * If there's no error then the frame is decrypted. At least that is
+ * the case for frames that come in via fragmented rx indication.
+ */
+ if (!is_decrypted)
+ return;
+
+ /* The payload is decrypted so strip crypto params. Start from tail
+ * since hdr is used to compute some stuff.
+ */
+
+ hdr = (void *)msdu->data;
+
+ /* Tail */
+ skb_trim(msdu, msdu->len - ath10k_htt_rx_crypto_tail_len(ar, enctype));
+
+ /* MMIC */
+ if (!ieee80211_has_morefrags(hdr->frame_control) &&
+ enctype == HTT_RX_MPDU_ENCRYPT_TKIP_WPA)
+ skb_trim(msdu, msdu->len - 8);
+
+ /* Head */
+ hdr_len = ieee80211_hdrlen(hdr->frame_control);
+ crypto_len = ath10k_htt_rx_crypto_param_len(ar, enctype);
+
+ memmove((void *)msdu->data + crypto_len,
+ (void *)msdu->data, hdr_len);
+ skb_pull(msdu, crypto_len);
+}
+
+static void ath10k_htt_rx_h_undecap_nwifi(struct ath10k *ar,
+ struct sk_buff *msdu,
+ struct ieee80211_rx_status *status,
+ const u8 first_hdr[64])
+{
struct ieee80211_hdr *hdr;
- u8 hdr_buf[64], da[ETH_ALEN], sa[ETH_ALEN], *qos;
- unsigned int hdr_len;
+ size_t hdr_len;
+ u8 da[ETH_ALEN];
+ u8 sa[ETH_ALEN];
- first = skb_peek(amsdu);
+ /* Delivered decapped frame:
+ * [nwifi 802.11 header] <-- replaced with 802.11 hdr
+ * [rfc1042/llc]
+ *
+ * Note: The nwifi header doesn't have QoS Control and is
+ * (always?) a 3addr frame.
+ *
+ * Note2: There's no A-MSDU subframe header. Even if it's part
+ * of an A-MSDU.
+ */
- rxd = (void *)first->data - sizeof(*rxd);
- enctype = MS(__le32_to_cpu(rxd->mpdu_start.info0),
- RX_MPDU_START_INFO0_ENCRYPT_TYPE);
+ /* pull decapped header and copy SA & DA */
+ hdr = (struct ieee80211_hdr *)msdu->data;
+ hdr_len = ath10k_htt_rx_nwifi_hdrlen(hdr);
+ ether_addr_copy(da, ieee80211_get_DA(hdr));
+ ether_addr_copy(sa, ieee80211_get_SA(hdr));
+ skb_pull(msdu, hdr_len);
- hdr = (struct ieee80211_hdr *)rxd->rx_hdr_status;
+ /* push original 802.11 header */
+ hdr = (struct ieee80211_hdr *)first_hdr;
hdr_len = ieee80211_hdrlen(hdr->frame_control);
- memcpy(hdr_buf, hdr, hdr_len);
- hdr = (struct ieee80211_hdr *)hdr_buf;
+ memcpy(skb_push(msdu, hdr_len), hdr, hdr_len);
- while ((skb = __skb_dequeue(amsdu))) {
- void *decap_hdr;
- int len;
-
- rxd = (void *)skb->data - sizeof(*rxd);
- fmt = MS(__le32_to_cpu(rxd->msdu_start.info1),
- RX_MSDU_START_INFO1_DECAP_FORMAT);
- decap_hdr = (void *)rxd->rx_hdr_status;
-
- skb->ip_summed = ath10k_htt_rx_get_csum_state(skb);
-
- /* First frame in an A-MSDU chain has more decapped data. */
- if (skb == first) {
- len = round_up(ieee80211_hdrlen(hdr->frame_control), 4);
- len += round_up(ath10k_htt_rx_crypto_param_len(ar,
- enctype), 4);
- decap_hdr += len;
- }
+ /* original 802.11 header has a different DA and in
+ * case of 4addr it may also have different SA
+ */
+ hdr = (struct ieee80211_hdr *)msdu->data;
+ ether_addr_copy(ieee80211_get_DA(hdr), da);
+ ether_addr_copy(ieee80211_get_SA(hdr), sa);
+}
- switch (fmt) {
- case RX_MSDU_DECAP_RAW:
- /* remove trailing FCS */
- skb_trim(skb, skb->len - FCS_LEN);
- break;
- case RX_MSDU_DECAP_NATIVE_WIFI:
- /* pull decapped header and copy SA & DA */
- hdr = (struct ieee80211_hdr *)skb->data;
- hdr_len = ath10k_htt_rx_nwifi_hdrlen(hdr);
- ether_addr_copy(da, ieee80211_get_DA(hdr));
- ether_addr_copy(sa, ieee80211_get_SA(hdr));
- skb_pull(skb, hdr_len);
-
- /* push original 802.11 header */
- hdr = (struct ieee80211_hdr *)hdr_buf;
- hdr_len = ieee80211_hdrlen(hdr->frame_control);
- memcpy(skb_push(skb, hdr_len), hdr, hdr_len);
-
- /* original A-MSDU header has the bit set but we're
- * not including A-MSDU subframe header */
- hdr = (struct ieee80211_hdr *)skb->data;
- qos = ieee80211_get_qos_ctl(hdr);
- qos[0] &= ~IEEE80211_QOS_CTL_A_MSDU_PRESENT;
-
- /* original 802.11 header has a different DA and in
- * case of 4addr it may also have different SA
- */
- ether_addr_copy(ieee80211_get_DA(hdr), da);
- ether_addr_copy(ieee80211_get_SA(hdr), sa);
- break;
- case RX_MSDU_DECAP_ETHERNET2_DIX:
- /* strip ethernet header and insert decapped 802.11
- * header, amsdu subframe header and rfc1042 header */
+static void *ath10k_htt_rx_h_find_rfc1042(struct ath10k *ar,
+ struct sk_buff *msdu,
+ enum htt_rx_mpdu_encrypt_type enctype)
+{
+ struct ieee80211_hdr *hdr;
+ struct htt_rx_desc *rxd;
+ size_t hdr_len, crypto_len;
+ void *rfc1042;
+ bool is_first, is_last, is_amsdu;
- len = 0;
- len += sizeof(struct rfc1042_hdr);
- len += sizeof(struct amsdu_subframe_hdr);
+ rxd = (void *)msdu->data - sizeof(*rxd);
+ hdr = (void *)rxd->rx_hdr_status;
- skb_pull(skb, sizeof(struct ethhdr));
- memcpy(skb_push(skb, len), decap_hdr, len);
- memcpy(skb_push(skb, hdr_len), hdr, hdr_len);
- break;
- case RX_MSDU_DECAP_8023_SNAP_LLC:
- /* insert decapped 802.11 header making a singly
- * A-MSDU */
- memcpy(skb_push(skb, hdr_len), hdr, hdr_len);
- break;
- }
+ is_first = !!(rxd->msdu_end.info0 &
+ __cpu_to_le32(RX_MSDU_END_INFO0_FIRST_MSDU));
+ is_last = !!(rxd->msdu_end.info0 &
+ __cpu_to_le32(RX_MSDU_END_INFO0_LAST_MSDU));
+ is_amsdu = !(is_first && is_last);
- ath10k_htt_rx_h_protected(htt, rx_status, skb, enctype, fmt,
- false);
+ rfc1042 = hdr;
- if (skb_queue_empty(amsdu))
- rx_status->flag &= ~RX_FLAG_AMSDU_MORE;
- else
- rx_status->flag |= RX_FLAG_AMSDU_MORE;
+ if (is_first) {
+ hdr_len = ieee80211_hdrlen(hdr->frame_control);
+ crypto_len = ath10k_htt_rx_crypto_param_len(ar, enctype);
- ath10k_process_rx(htt->ar, rx_status, skb);
+ rfc1042 += round_up(hdr_len, 4) +
+ round_up(crypto_len, 4);
}
- /* FIXME: It might be nice to re-assemble the A-MSDU when there's a
- * monitor interface active for sniffing purposes. */
+ if (is_amsdu)
+ rfc1042 += sizeof(struct amsdu_subframe_hdr);
+
+ return rfc1042;
}
-static void ath10k_htt_rx_msdu(struct ath10k_htt *htt,
- struct ieee80211_rx_status *rx_status,
- struct sk_buff *skb)
+static void ath10k_htt_rx_h_undecap_eth(struct ath10k *ar,
+ struct sk_buff *msdu,
+ struct ieee80211_rx_status *status,
+ const u8 first_hdr[64],
+ enum htt_rx_mpdu_encrypt_type enctype)
{
- struct ath10k *ar = htt->ar;
- struct htt_rx_desc *rxd;
struct ieee80211_hdr *hdr;
- enum rx_msdu_decap_format fmt;
- enum htt_rx_mpdu_encrypt_type enctype;
- int hdr_len;
+ struct ethhdr *eth;
+ size_t hdr_len;
void *rfc1042;
+ u8 da[ETH_ALEN];
+ u8 sa[ETH_ALEN];
- rxd = (void *)skb->data - sizeof(*rxd);
- fmt = MS(__le32_to_cpu(rxd->msdu_start.info1),
- RX_MSDU_START_INFO1_DECAP_FORMAT);
- enctype = MS(__le32_to_cpu(rxd->mpdu_start.info0),
- RX_MPDU_START_INFO0_ENCRYPT_TYPE);
- hdr = (struct ieee80211_hdr *)rxd->rx_hdr_status;
+ /* Delivered decapped frame:
+ * [eth header] <-- replaced with 802.11 hdr & rfc1042/llc
+ * [payload]
+ */
+
+ rfc1042 = ath10k_htt_rx_h_find_rfc1042(ar, msdu, enctype);
+ if (WARN_ON_ONCE(!rfc1042))
+ return;
+
+ /* pull decapped header and copy SA & DA */
+ eth = (struct ethhdr *)msdu->data;
+ ether_addr_copy(da, eth->h_dest);
+ ether_addr_copy(sa, eth->h_source);
+ skb_pull(msdu, sizeof(struct ethhdr));
+
+ /* push rfc1042/llc/snap */
+ memcpy(skb_push(msdu, sizeof(struct rfc1042_hdr)), rfc1042,
+ sizeof(struct rfc1042_hdr));
+
+ /* push original 802.11 header */
+ hdr = (struct ieee80211_hdr *)first_hdr;
+ hdr_len = ieee80211_hdrlen(hdr->frame_control);
+ memcpy(skb_push(msdu, hdr_len), hdr, hdr_len);
+
+ /* original 802.11 header has a different DA and in
+ * case of 4addr it may also have different SA
+ */
+ hdr = (struct ieee80211_hdr *)msdu->data;
+ ether_addr_copy(ieee80211_get_DA(hdr), da);
+ ether_addr_copy(ieee80211_get_SA(hdr), sa);
+}
+
+static void ath10k_htt_rx_h_undecap_snap(struct ath10k *ar,
+ struct sk_buff *msdu,
+ struct ieee80211_rx_status *status,
+ const u8 first_hdr[64])
+{
+ struct ieee80211_hdr *hdr;
+ size_t hdr_len;
+
+ /* Delivered decapped frame:
+ * [amsdu header] <-- replaced with 802.11 hdr
+ * [rfc1042/llc]
+ * [payload]
+ */
+
+ skb_pull(msdu, sizeof(struct amsdu_subframe_hdr));
+
+ hdr = (struct ieee80211_hdr *)first_hdr;
hdr_len = ieee80211_hdrlen(hdr->frame_control);
+ memcpy(skb_push(msdu, hdr_len), hdr, hdr_len);
+}
- skb->ip_summed = ath10k_htt_rx_get_csum_state(skb);
+static void ath10k_htt_rx_h_undecap(struct ath10k *ar,
+ struct sk_buff *msdu,
+ struct ieee80211_rx_status *status,
+ u8 first_hdr[64],
+ enum htt_rx_mpdu_encrypt_type enctype,
+ bool is_decrypted)
+{
+ struct htt_rx_desc *rxd;
+ enum rx_msdu_decap_format decap;
+ struct ieee80211_hdr *hdr;
- switch (fmt) {
+ /* First msdu's decapped header:
+ * [802.11 header] <-- padded to 4 bytes long
+ * [crypto param] <-- padded to 4 bytes long
+ * [amsdu header] <-- only if A-MSDU
+ * [rfc1042/llc]
+ *
+ * Other (2nd, 3rd, ..) msdu's decapped header:
+ * [amsdu header] <-- only if A-MSDU
+ * [rfc1042/llc]
+ */
+
+ rxd = (void *)msdu->data - sizeof(*rxd);
+ hdr = (void *)rxd->rx_hdr_status;
+ decap = MS(__le32_to_cpu(rxd->msdu_start.info1),
+ RX_MSDU_START_INFO1_DECAP_FORMAT);
+
+ switch (decap) {
case RX_MSDU_DECAP_RAW:
- /* remove trailing FCS */
- skb_trim(skb, skb->len - FCS_LEN);
+ ath10k_htt_rx_h_undecap_raw(ar, msdu, status, enctype, is_decrypted);
break;
case RX_MSDU_DECAP_NATIVE_WIFI:
- /* Pull decapped header */
- hdr = (struct ieee80211_hdr *)skb->data;
- hdr_len = ath10k_htt_rx_nwifi_hdrlen(hdr);
- skb_pull(skb, hdr_len);
-
- /* Push original header */
- hdr = (struct ieee80211_hdr *)rxd->rx_hdr_status;
- hdr_len = ieee80211_hdrlen(hdr->frame_control);
- memcpy(skb_push(skb, hdr_len), hdr, hdr_len);
+ ath10k_htt_rx_h_undecap_nwifi(ar, msdu, status, first_hdr);
break;
case RX_MSDU_DECAP_ETHERNET2_DIX:
- /* strip ethernet header and insert decapped 802.11 header and
- * rfc1042 header */
-
- rfc1042 = hdr;
- rfc1042 += roundup(hdr_len, 4);
- rfc1042 += roundup(ath10k_htt_rx_crypto_param_len(ar,
- enctype), 4);
-
- skb_pull(skb, sizeof(struct ethhdr));
- memcpy(skb_push(skb, sizeof(struct rfc1042_hdr)),
- rfc1042, sizeof(struct rfc1042_hdr));
- memcpy(skb_push(skb, hdr_len), hdr, hdr_len);
+ ath10k_htt_rx_h_undecap_eth(ar, msdu, status, first_hdr, enctype);
break;
case RX_MSDU_DECAP_8023_SNAP_LLC:
- /* remove A-MSDU subframe header and insert
- * decapped 802.11 header. rfc1042 header is already there */
-
- skb_pull(skb, sizeof(struct amsdu_subframe_hdr));
- memcpy(skb_push(skb, hdr_len), hdr, hdr_len);
+ ath10k_htt_rx_h_undecap_snap(ar, msdu, status, first_hdr);
break;
}
-
- ath10k_htt_rx_h_protected(htt, rx_status, skb, enctype, fmt, false);
-
- ath10k_process_rx(htt->ar, rx_status, skb);
}
static int ath10k_htt_rx_get_csum_state(struct sk_buff *skb)
@@ -1091,6 +1111,126 @@ static int ath10k_htt_rx_get_csum_state(struct sk_buff *skb)
return CHECKSUM_UNNECESSARY;
}
+static void ath10k_htt_rx_h_csum_offload(struct sk_buff *msdu)
+{
+ msdu->ip_summed = ath10k_htt_rx_get_csum_state(msdu);
+}
+
+static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
+ struct sk_buff_head *amsdu,
+ struct ieee80211_rx_status *status)
+{
+ struct sk_buff *first;
+ struct sk_buff *last;
+ struct sk_buff *msdu;
+ struct htt_rx_desc *rxd;
+ struct ieee80211_hdr *hdr;
+ enum htt_rx_mpdu_encrypt_type enctype;
+ u8 first_hdr[64];
+ u8 *qos;
+ size_t hdr_len;
+ bool has_fcs_err;
+ bool has_crypto_err;
+ bool has_tkip_err;
+ bool has_peer_idx_invalid;
+ bool is_decrypted;
+
+ if (skb_queue_empty(amsdu))
+ return;
+
+ first = skb_peek(amsdu);
+ rxd = (void *)first->data - sizeof(*rxd);
+
+ enctype = MS(__le32_to_cpu(rxd->mpdu_start.info0),
+ RX_MPDU_START_INFO0_ENCRYPT_TYPE);
+
+ /* First MSDU's Rx descriptor in an A-MSDU contains full 802.11
+ * decapped header. It'll be used for undecapping of each MSDU.
+ */
+ hdr = (void *)rxd->rx_hdr_status;
+ hdr_len = ieee80211_hdrlen(hdr->frame_control);
+ memcpy(first_hdr, hdr, hdr_len);
+
+ /* Each A-MSDU subframe will use the original header as the base and be
+ * reported as a separate MSDU so strip the A-MSDU bit from QoS Ctl.
+ */
+ hdr = (void *)first_hdr;
+ qos = ieee80211_get_qos_ctl(hdr);
+ qos[0] &= ~IEEE80211_QOS_CTL_A_MSDU_PRESENT;
+
+ /* Some attention flags are valid only in the last MSDU. */
+ last = skb_peek_tail(amsdu);
+ rxd = (void *)last->data - sizeof(*rxd);
+ has_fcs_err = !!(rxd->attention.flags &
+ __cpu_to_le32(RX_ATTENTION_FLAGS_FCS_ERR));
+ has_crypto_err = !!(rxd->attention.flags &
+ __cpu_to_le32(RX_ATTENTION_FLAGS_DECRYPT_ERR));
+ has_tkip_err = !!(rxd->attention.flags &
+ __cpu_to_le32(RX_ATTENTION_FLAGS_TKIP_MIC_ERR));
+ has_peer_idx_invalid = !!(rxd->attention.flags &
+ __cpu_to_le32(RX_ATTENTION_FLAGS_PEER_IDX_INVALID));
+
+ /* Note: If hardware captures an encrypted frame that it can't decrypt,
+ * e.g. due to fcs error, missing peer or invalid key data it will
+ * report the frame as raw.
+ */
+ is_decrypted = (enctype != HTT_RX_MPDU_ENCRYPT_NONE &&
+ !has_fcs_err &&
+ !has_crypto_err &&
+ !has_peer_idx_invalid);
+
+ /* Clear per-MPDU flags while leaving per-PPDU flags intact. */
+ status->flag &= ~(RX_FLAG_FAILED_FCS_CRC |
+ RX_FLAG_MMIC_ERROR |
+ RX_FLAG_DECRYPTED |
+ RX_FLAG_IV_STRIPPED |
+ RX_FLAG_MMIC_STRIPPED);
+
+ if (has_fcs_err)
+ status->flag |= RX_FLAG_FAILED_FCS_CRC;
+
+ if (has_tkip_err)
+ status->flag |= RX_FLAG_MMIC_ERROR;
+
+ if (is_decrypted)
+ status->flag |= RX_FLAG_DECRYPTED |
+ RX_FLAG_IV_STRIPPED |
+ RX_FLAG_MMIC_STRIPPED;
+
+ skb_queue_walk(amsdu, msdu) {
+ ath10k_htt_rx_h_csum_offload(msdu);
+ ath10k_htt_rx_h_undecap(ar, msdu, status, first_hdr, enctype,
+ is_decrypted);
+
+ /* Undecapping involves copying the original 802.11 header back
+ * to sk_buff. If frame is protected and hardware has decrypted
+ * it then remove the protected bit.
+ */
+ if (!is_decrypted)
+ continue;
+
+ hdr = (void *)msdu->data;
+ hdr->frame_control &= ~__cpu_to_le16(IEEE80211_FCTL_PROTECTED);
+ }
+}
+
+static void ath10k_htt_rx_h_deliver(struct ath10k *ar,
+ struct sk_buff_head *amsdu,
+ struct ieee80211_rx_status *status)
+{
+ struct sk_buff *msdu;
+
+ while ((msdu = __skb_dequeue(amsdu))) {
+ /* Setup per-MSDU flags */
+ if (skb_queue_empty(amsdu))
+ status->flag &= ~RX_FLAG_AMSDU_MORE;
+ else
+ status->flag |= RX_FLAG_AMSDU_MORE;
+
+ ath10k_process_rx(ar, status, msdu);
+ }
+}
+
static int ath10k_unchain_msdu(struct sk_buff_head *amsdu)
{
struct sk_buff *skb, *first;
@@ -1134,45 +1274,86 @@ static int ath10k_unchain_msdu(struct sk_buff_head *amsdu)
return 0;
}
-static bool ath10k_htt_rx_amsdu_allowed(struct ath10k_htt *htt,
- struct sk_buff *head,
- bool channel_set,
- u32 attention)
+static void ath10k_htt_rx_h_unchain(struct ath10k *ar,
+ struct sk_buff_head *amsdu,
+ bool chained)
{
- struct ath10k *ar = htt->ar;
+ struct sk_buff *first;
+ struct htt_rx_desc *rxd;
+ enum rx_msdu_decap_format decap;
- if (head->len == 0) {
- ath10k_dbg(ar, ATH10K_DBG_HTT,
- "htt rx dropping due to zero-len\n");
- return false;
- }
+ first = skb_peek(amsdu);
+ rxd = (void *)first->data - sizeof(*rxd);
+ decap = MS(__le32_to_cpu(rxd->msdu_start.info1),
+ RX_MSDU_START_INFO1_DECAP_FORMAT);
- if (attention & RX_ATTENTION_FLAGS_DECRYPT_ERR) {
- ath10k_dbg(ar, ATH10K_DBG_HTT,
- "htt rx dropping due to decrypt-err\n");
- return false;
+ if (!chained)
+ return;
+
+ /* FIXME: Current unchaining logic can only handle simple case of raw
+ * msdu chaining. If decapping is other than raw the chaining may be
+ * more complex and this isn't handled by the current code. Don't even
+ * try re-constructing such frames - it'll be pretty much garbage.
+ */
+ if (decap != RX_MSDU_DECAP_RAW ||
+ skb_queue_len(amsdu) != 1 + rxd->frag_info.ring2_more_count) {
+ __skb_queue_purge(amsdu);
+ return;
}
- if (!channel_set) {
- ath10k_warn(ar, "no channel configured; ignoring frame!\n");
+ ath10k_unchain_msdu(amsdu);
+}
+
+static bool ath10k_htt_rx_amsdu_allowed(struct ath10k *ar,
+ struct sk_buff_head *amsdu,
+ struct ieee80211_rx_status *rx_status)
+{
+ struct sk_buff *msdu;
+ struct htt_rx_desc *rxd;
+
+ msdu = skb_peek(amsdu);
+ rxd = (void *)msdu->data - sizeof(*rxd);
+
+ /* FIXME: It might be a good idea to do some fuzzy-testing to drop
+ * invalid/dangerous frames.
+ */
+
+ if (!rx_status->freq) {
+ ath10k_warn(ar, "no channel configured; ignoring frame(s)!\n");
return false;
}
- /* Skip mgmt frames while we handle this in WMI */
- if (attention & RX_ATTENTION_FLAGS_MGMT_TYPE) {
+ /* Management frames are handled via WMI events. The pros of such
+ * approach is that channel is explicitly provided in WMI events
+ * whereas HTT doesn't provide channel information for Rxed frames.
+ */
+ if (rxd->attention.flags &
+ __cpu_to_le32(RX_ATTENTION_FLAGS_MGMT_TYPE)) {
ath10k_dbg(ar, ATH10K_DBG_HTT, "htt rx mgmt ctrl\n");
return false;
}
- if (test_bit(ATH10K_CAC_RUNNING, &htt->ar->dev_flags)) {
- ath10k_dbg(ar, ATH10K_DBG_HTT,
- "htt rx CAC running\n");
+ if (test_bit(ATH10K_CAC_RUNNING, &ar->dev_flags)) {
+ ath10k_dbg(ar, ATH10K_DBG_HTT, "htt rx cac running\n");
return false;
}
return true;
}
+static void ath10k_htt_rx_h_filter(struct ath10k *ar,
+ struct sk_buff_head *amsdu,
+ struct ieee80211_rx_status *rx_status)
+{
+ if (skb_queue_empty(amsdu))
+ return;
+
+ if (ath10k_htt_rx_amsdu_allowed(ar, amsdu, rx_status))
+ return;
+
+ __skb_queue_purge(amsdu);
+}
+
static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
struct htt_rx_indication *rx)
{
@@ -1180,7 +1361,6 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
struct ieee80211_rx_status *rx_status = &htt->rx_status;
struct htt_rx_indication_mpdu_range *mpdu_ranges;
struct sk_buff_head amsdu;
- struct ieee80211_hdr *hdr;
int num_mpdu_ranges;
u32 attention;
int fw_desc_len;
@@ -1247,34 +1427,10 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
break;
}
- if (!ath10k_htt_rx_amsdu_allowed(htt, skb_peek(&amsdu),
- channel_set, attention)) {
- __skb_queue_purge(&amsdu);
- continue;
- }
-
- if (ret > 0 && ath10k_unchain_msdu(&amsdu) < 0) {
- __skb_queue_purge(&amsdu);
- continue;
- }
-
- if (attention & RX_ATTENTION_FLAGS_FCS_ERR)
- rx_status->flag |= RX_FLAG_FAILED_FCS_CRC;
- else
- rx_status->flag &= ~RX_FLAG_FAILED_FCS_CRC;
-
- if (attention & RX_ATTENTION_FLAGS_TKIP_MIC_ERR)
- rx_status->flag |= RX_FLAG_MMIC_ERROR;
- else
- rx_status->flag &= ~RX_FLAG_MMIC_ERROR;
-
- hdr = ath10k_htt_rx_skb_get_hdr(skb_peek(&amsdu));
-
- if (ath10k_htt_rx_hdr_is_amsdu(hdr))
- ath10k_htt_rx_amsdu(htt, rx_status, &amsdu);
- else
- ath10k_htt_rx_msdu(htt, rx_status,
- __skb_dequeue(&amsdu));
+ ath10k_htt_rx_h_unchain(ar, &amsdu, ret > 0);
+ ath10k_htt_rx_h_filter(ar, &amsdu, rx_status);
+ ath10k_htt_rx_h_mpdu(ar, &amsdu, rx_status);
+ ath10k_htt_rx_h_deliver(ar, &amsdu, rx_status);
}
tasklet_schedule(&htt->rx_replenish_task);
@@ -1284,19 +1440,11 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
struct htt_rx_fragment_indication *frag)
{
struct ath10k *ar = htt->ar;
- struct sk_buff *msdu;
- enum htt_rx_mpdu_encrypt_type enctype;
- struct htt_rx_desc *rxd;
- enum rx_msdu_decap_format fmt;
struct ieee80211_rx_status *rx_status = &htt->rx_status;
- struct ieee80211_hdr *hdr;
struct sk_buff_head amsdu;
int ret;
- bool tkip_mic_err;
- bool decrypt_err;
u8 *fw_desc;
- int fw_desc_len, hdrlen, paramlen;
- int trim;
+ int fw_desc_len;
u32 attention = 0;
fw_desc_len = __le16_to_cpu(frag->fw_rx_desc_bytes);
@@ -1326,75 +1474,13 @@ static void ath10k_htt_rx_frag_handler(struct ath10k_htt *htt,
return;
}
- msdu = __skb_dequeue(&amsdu);
-
/* FIXME: implement signal strength */
rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL;
- hdr = (struct ieee80211_hdr *)msdu->data;
- rxd = (void *)msdu->data - sizeof(*rxd);
- 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);
-
- if (fmt != RX_MSDU_DECAP_RAW) {
- ath10k_warn(ar, "we dont support non-raw fragmented rx yet\n");
- dev_kfree_skb_any(msdu);
- goto end;
- }
-
- enctype = MS(__le32_to_cpu(rxd->mpdu_start.info0),
- RX_MPDU_START_INFO0_ENCRYPT_TYPE);
- ath10k_htt_rx_h_protected(htt, rx_status, msdu, enctype, fmt,
- true);
- msdu->ip_summed = ath10k_htt_rx_get_csum_state(msdu);
-
- if (tkip_mic_err)
- ath10k_warn(ar, "tkip mic error\n");
-
- if (decrypt_err) {
- ath10k_warn(ar, "decryption err in fragmented rx\n");
- dev_kfree_skb_any(msdu);
- goto end;
- }
-
- if (enctype != HTT_RX_MPDU_ENCRYPT_NONE) {
- hdrlen = ieee80211_hdrlen(hdr->frame_control);
- paramlen = ath10k_htt_rx_crypto_param_len(ar, enctype);
-
- /* It is more efficient to move the header than the payload */
- memmove((void *)msdu->data + paramlen,
- (void *)msdu->data,
- hdrlen);
- skb_pull(msdu, paramlen);
- hdr = (struct ieee80211_hdr *)msdu->data;
- }
-
- /* remove trailing FCS */
- trim = 4;
-
- /* remove crypto trailer */
- trim += ath10k_htt_rx_crypto_tail_len(ar, enctype);
-
- /* last fragment of TKIP frags has MIC */
- if (!ieee80211_has_morefrags(hdr->frame_control) &&
- enctype == HTT_RX_MPDU_ENCRYPT_TKIP_WPA)
- trim += MICHAEL_MIC_LEN;
-
- if (trim > msdu->len) {
- ath10k_warn(ar, "htt rx fragment: trailer longer than the frame itself? drop\n");
- dev_kfree_skb_any(msdu);
- goto end;
- }
-
- skb_trim(msdu, msdu->len - trim);
-
- ath10k_dbg_dump(ar, ATH10K_DBG_HTT_DUMP, NULL, "htt rx frag mpdu: ",
- msdu->data, msdu->len);
- ath10k_process_rx(htt->ar, rx_status, msdu);
+ ath10k_htt_rx_h_filter(ar, &amsdu, rx_status);
+ ath10k_htt_rx_h_mpdu(ar, &amsdu, rx_status);
+ ath10k_htt_rx_h_deliver(ar, &amsdu, rx_status);
-end:
if (fw_desc_len > 0) {
ath10k_dbg(ar, ATH10K_DBG_HTT,
"expecting more fragmented rx in one indication %d\n",
--
1.8.5.3
Michal Kazior <[email protected]> writes:
> This makes it more in line with the new Rx path.
> It also makes the code more reusable because Rx
> descriptor is more accessible.
>
> Signed-off-by: Michal Kazior <[email protected]>
This patch also had a checkpatch warning. Full patch here:
https://github.com/kvalo/ath/commit/d01090f45a5891f811bfaadb9a15c7e47ef0ef26
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 41dce044e3e8..bf82cb756548 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -815,9 +815,8 @@ static void ath10k_htt_rx_h_ppdu(struct ath10k *ar,
ath10k_htt_rx_h_rates(ar, status, rxd);
}
- if (is_last_ppdu) {
+ if (is_last_ppdu)
ath10k_htt_rx_h_mactime(ar, status, rxd);
- }
}
static const char * const tid_to_ac[] = {
--
Kalle Valo
Michal Kazior <[email protected]> writes:
> This patchset unifies rx path. Instead of
> branching for A-MSDU and MSDU cases a single
> codepath is introduced.
>
> This yields:
>
> text data bss dec hex filename
> 14068 0 0 14068 36f4 before/htt_rx.o
> 13308 3 0 13311 33ff after/htt_rx.o
>
> I haven't measured any Rx performance loss in my
> setup but I may be biased. In theory this could
> reduce i-cache pressure and improve Rx throughput
> on CPU contrained systems by a few mbps.
>
> This patchset serves two purposes though:
> - clean up and unify the Rx path a bit
> - make it possible to reuse code more easily in
> the future
>
>
> Michal Kazior (7):
> ath10k: start using sk_buff_head
> ath10k: simplify Rx loop
> ath10k: refactor htt->rx_confused
> ath10k: unify rx undecapping
> ath10k: remove unused function argument
> ath10k: use rx descriptor for ppdu status extraction
> ath10k: report rx rate and signal for fragmented Rx
Thanks, all seven applied.
--
Kalle Valo