* [PATCH 1/7] ath10k: start using sk_buff_head
2014-11-04 14:22 [PATCH 0/7] ath10k: rework rx path Michal Kazior
@ 2014-11-04 14:22 ` Michal Kazior
2014-11-04 14:22 ` [PATCH 2/7] ath10k: simplify Rx loop Michal Kazior
` (6 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Michal Kazior @ 2014-11-04 14:22 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
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 <michal.kazior@tieto.com>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/7] ath10k: simplify Rx loop
2014-11-04 14:22 [PATCH 0/7] ath10k: rework rx path Michal Kazior
2014-11-04 14:22 ` [PATCH 1/7] ath10k: start using sk_buff_head Michal Kazior
@ 2014-11-04 14:22 ` Michal Kazior
2014-11-04 14:22 ` [PATCH 3/7] ath10k: refactor htt->rx_confused Michal Kazior
` (5 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Michal Kazior @ 2014-11-04 14:22 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
Since htt_rx_mpdu_status isn't used anymore
(instead attention flags are used) simplify the
loop.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/7] ath10k: refactor htt->rx_confused
2014-11-04 14:22 [PATCH 0/7] ath10k: rework rx path Michal Kazior
2014-11-04 14:22 ` [PATCH 1/7] ath10k: start using sk_buff_head Michal Kazior
2014-11-04 14:22 ` [PATCH 2/7] ath10k: simplify Rx loop Michal Kazior
@ 2014-11-04 14:22 ` Michal Kazior
2014-11-04 14:22 ` [PATCH 4/7] ath10k: unify rx undecapping Michal Kazior
` (4 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Michal Kazior @ 2014-11-04 14:22 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
Make the rx_confused be handled by the rx
indication handlers instead of the buffer popping
function.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/7] ath10k: unify rx undecapping
2014-11-04 14:22 [PATCH 0/7] ath10k: rework rx path Michal Kazior
` (2 preceding siblings ...)
2014-11-04 14:22 ` [PATCH 3/7] ath10k: refactor htt->rx_confused Michal Kazior
@ 2014-11-04 14:22 ` Michal Kazior
2014-11-17 14:32 ` Kalle Valo
2014-11-04 14:22 ` [PATCH 5/7] ath10k: remove unused function argument Michal Kazior
` (3 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Michal Kazior @ 2014-11-04 14:22 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
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 <michal.kazior@tieto.com>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 4/7] ath10k: unify rx undecapping
2014-11-04 14:22 ` [PATCH 4/7] ath10k: unify rx undecapping Michal Kazior
@ 2014-11-17 14:32 ` Kalle Valo
2014-11-17 14:54 ` Michal Kazior
0 siblings, 1 reply; 15+ messages in thread
From: Kalle Valo @ 2014-11-17 14:32 UTC (permalink / raw)
To: Michal Kazior; +Cc: ath10k, linux-wireless
Michal Kazior <michal.kazior@tieto.com> 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 <michal.kazior@tieto.com>
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 4/7] ath10k: unify rx undecapping
2014-11-17 14:32 ` Kalle Valo
@ 2014-11-17 14:54 ` Michal Kazior
2014-11-17 15:11 ` Kalle Valo
0 siblings, 1 reply; 15+ messages in thread
From: Michal Kazior @ 2014-11-17 14:54 UTC (permalink / raw)
To: Kalle Valo; +Cc: ath10k@lists.infradead.org, linux-wireless
On 17 November 2014 15:32, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> 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 <michal.kazior@tieto.com>
>
> 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ł
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/7] ath10k: unify rx undecapping
2014-11-17 14:54 ` Michal Kazior
@ 2014-11-17 15:11 ` Kalle Valo
2014-11-18 6:46 ` Michal Kazior
0 siblings, 1 reply; 15+ messages in thread
From: Kalle Valo @ 2014-11-17 15:11 UTC (permalink / raw)
To: Michal Kazior; +Cc: ath10k@lists.infradead.org, linux-wireless
Michal Kazior <michal.kazior@tieto.com> writes:
> On 17 November 2014 15:32, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> 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 <michal.kazior@tieto.com>
>>
>> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/7] ath10k: unify rx undecapping
2014-11-17 15:11 ` Kalle Valo
@ 2014-11-18 6:46 ` Michal Kazior
2014-11-18 13:58 ` Kalle Valo
0 siblings, 1 reply; 15+ messages in thread
From: Michal Kazior @ 2014-11-18 6:46 UTC (permalink / raw)
To: Kalle Valo; +Cc: ath10k@lists.infradead.org, linux-wireless
On 17 November 2014 16:11, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>> On 17 November 2014 15:32, Kalle Valo <kvalo@qca.qualcomm.com> 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ł
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 4/7] ath10k: unify rx undecapping
2014-11-18 6:46 ` Michal Kazior
@ 2014-11-18 13:58 ` Kalle Valo
0 siblings, 0 replies; 15+ messages in thread
From: Kalle Valo @ 2014-11-18 13:58 UTC (permalink / raw)
To: Michal Kazior; +Cc: ath10k@lists.infradead.org, linux-wireless
Michal Kazior <michal.kazior@tieto.com> writes:
> On 17 November 2014 16:11, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>> On 17 November 2014 15:32, Kalle Valo <kvalo@qca.qualcomm.com> 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 5/7] ath10k: remove unused function argument
2014-11-04 14:22 [PATCH 0/7] ath10k: rework rx path Michal Kazior
` (3 preceding siblings ...)
2014-11-04 14:22 ` [PATCH 4/7] ath10k: unify rx undecapping Michal Kazior
@ 2014-11-04 14:22 ` Michal Kazior
2014-11-04 14:22 ` [PATCH 6/7] ath10k: use rx descriptor for ppdu status extraction Michal Kazior
` (2 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Michal Kazior @ 2014-11-04 14:22 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
The original fix has been moved into a different
place in the code.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 6/7] ath10k: use rx descriptor for ppdu status extraction
2014-11-04 14:22 [PATCH 0/7] ath10k: rework rx path Michal Kazior
` (4 preceding siblings ...)
2014-11-04 14:22 ` [PATCH 5/7] ath10k: remove unused function argument Michal Kazior
@ 2014-11-04 14:22 ` Michal Kazior
2014-11-17 14:33 ` Kalle Valo
2014-11-04 14:22 ` [PATCH 7/7] ath10k: report rx rate and signal for fragmented Rx Michal Kazior
2014-11-21 17:01 ` [PATCH 0/7] ath10k: rework rx path Kalle Valo
7 siblings, 1 reply; 15+ messages in thread
From: Michal Kazior @ 2014-11-04 14:22 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
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 <michal.kazior@tieto.com>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 7/7] ath10k: report rx rate and signal for fragmented Rx
2014-11-04 14:22 [PATCH 0/7] ath10k: rework rx path Michal Kazior
` (5 preceding siblings ...)
2014-11-04 14:22 ` [PATCH 6/7] ath10k: use rx descriptor for ppdu status extraction Michal Kazior
@ 2014-11-04 14:22 ` Michal Kazior
2014-11-21 17:01 ` [PATCH 0/7] ath10k: rework rx path Kalle Valo
7 siblings, 0 replies; 15+ messages in thread
From: Michal Kazior @ 2014-11-04 14:22 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless, Michal Kazior
Fragmented Rx wasn't reporting everything. With
the reworked Rx code it's very easy to fix it.
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 0/7] ath10k: rework rx path
2014-11-04 14:22 [PATCH 0/7] ath10k: rework rx path Michal Kazior
` (6 preceding siblings ...)
2014-11-04 14:22 ` [PATCH 7/7] ath10k: report rx rate and signal for fragmented Rx Michal Kazior
@ 2014-11-21 17:01 ` Kalle Valo
7 siblings, 0 replies; 15+ messages in thread
From: Kalle Valo @ 2014-11-21 17:01 UTC (permalink / raw)
To: Michal Kazior; +Cc: ath10k, linux-wireless
Michal Kazior <michal.kazior@tieto.com> 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
^ permalink raw reply [flat|nested] 15+ messages in thread