2015-11-30 12:57:11

by Grzegorz Bajorski

[permalink] [raw]
Subject: [PATCH] ath10k: deliver mgmt frames from htt to monitor vifs only

Until now only WMI originating mgmt frames were
reported to mac80211. Management frames on HTT
were basically dropped (except frames which looked
like management but had FCS error).

To allow sniffing all frames (including offloaded
frames) without interfering with mac80211
operation and states a new rx_flag was introduced
and is not being used to distinguish frames and
classify them for mac80211.

Signed-off-by: Grzegorz Bajorski <[email protected]>
---
depends on:
mac80211: allow drivers to report (non-)monitor frames

drivers/net/wireless/ath/ath10k/htt_rx.c | 70 ++++++++++++++++----------------
drivers/net/wireless/ath/ath10k/wmi.c | 6 +++
2 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 396645b..898eff0 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1076,20 +1076,25 @@ static void ath10k_htt_rx_h_undecap_raw(struct ath10k *ar,
hdr = (void *)msdu->data;

/* Tail */
- skb_trim(msdu, msdu->len - ath10k_htt_rx_crypto_tail_len(ar, enctype));
+ if (status->flag & RX_FLAG_IV_STRIPPED)
+ skb_trim(msdu, msdu->len -
+ ath10k_htt_rx_crypto_tail_len(ar, enctype));

/* MMIC */
- if (!ieee80211_has_morefrags(hdr->frame_control) &&
+ if ((status->flag & RX_FLAG_MMIC_STRIPPED) &&
+ !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);
+ if (status->flag & RX_FLAG_IV_STRIPPED) {
+ 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);
+ 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,
@@ -1330,6 +1335,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
bool has_tkip_err;
bool has_peer_idx_invalid;
bool is_decrypted;
+ bool is_mgmt;
u32 attention;

if (skb_queue_empty(amsdu))
@@ -1338,6 +1344,9 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
first = skb_peek(amsdu);
rxd = (void *)first->data - sizeof(*rxd);

+ is_mgmt = !!(rxd->attention.flags &
+ __cpu_to_le32(RX_ATTENTION_FLAGS_MGMT_TYPE));
+
enctype = MS(__le32_to_cpu(rxd->mpdu_start.info0),
RX_MPDU_START_INFO0_ENCRYPT_TYPE);

@@ -1379,6 +1388,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
RX_FLAG_MMIC_ERROR |
RX_FLAG_DECRYPTED |
RX_FLAG_IV_STRIPPED |
+ RX_FLAG_ONLY_MONITOR |
RX_FLAG_MMIC_STRIPPED);

if (has_fcs_err)
@@ -1387,10 +1397,21 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
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;
+ /* Firmware reports all necessary management frames via WMI already.
+ * They are not reported to monitor interfaces at all so pass the ones
+ * coming via HTT to monitor interfaces instead. This simplifies
+ * matters a lot.
+ */
+ if (is_mgmt)
+ status->flag |= RX_FLAG_ONLY_MONITOR;
+
+ if (is_decrypted) {
+ status->flag |= RX_FLAG_DECRYPTED;
+
+ if (likely(!is_mgmt))
+ status->flag |= RX_FLAG_IV_STRIPPED |
+ RX_FLAG_MMIC_STRIPPED;
+}

skb_queue_walk(amsdu, msdu) {
ath10k_htt_rx_h_csum_offload(msdu);
@@ -1403,6 +1424,8 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
*/
if (!is_decrypted)
continue;
+ if (is_mgmt)
+ continue;

hdr = (void *)msdu->data;
hdr->frame_control &= ~__cpu_to_le16(IEEE80211_FCTL_PROTECTED);
@@ -1503,14 +1526,6 @@ 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;
- bool is_mgmt;
- bool has_fcs_err;
-
- 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.
*/
@@ -1520,23 +1535,6 @@ static bool ath10k_htt_rx_amsdu_allowed(struct ath10k *ar,
return false;
}

- is_mgmt = !!(rxd->attention.flags &
- __cpu_to_le32(RX_ATTENTION_FLAGS_MGMT_TYPE));
- has_fcs_err = !!(rxd->attention.flags &
- __cpu_to_le32(RX_ATTENTION_FLAGS_FCS_ERR));
-
- /* 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.
- *
- * However some firmware revisions don't report corrupted frames via
- * WMI so don't drop them.
- */
- if (is_mgmt && !has_fcs_err) {
- ath10k_dbg(ar, ATH10K_DBG_HTT, "htt rx mgmt ctrl\n");
- return false;
- }
-
if (test_bit(ATH10K_CAC_RUNNING, &ar->dev_flags)) {
ath10k_dbg(ar, ATH10K_DBG_HTT, "htt rx cac running\n");
return false;
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 9021079..847f91a 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -2298,6 +2298,12 @@ int ath10k_wmi_event_mgmt_rx(struct ath10k *ar, struct sk_buff *skb)
hdr = (struct ieee80211_hdr *)skb->data;
fc = le16_to_cpu(hdr->frame_control);

+ /* Firmware is guaranteed to report all essential management frames via
+ * WMI while it can deliver some extra via HTT. Since there can be
+ * duplicates split the reporting wrt monitor/sniffing.
+ */
+ status->flag |= RX_FLAG_SKIP_MONITOR;
+
ath10k_wmi_handle_wep_reauth(ar, skb, status);

/* FW delivers WEP Shared Auth frame with Protected Bit set and
--
2.3.7



2015-11-30 19:47:07

by Peter Oh

[permalink] [raw]
Subject: Re: [PATCH] ath10k: deliver mgmt frames from htt to monitor vifs only


On 11/30/2015 04:56 AM, Grzegorz Bajorski wrote:
> Until now only WMI originating mgmt frames were
> reported to mac80211. Management frames on HTT
> were basically dropped (except frames which looked
> like management but had FCS error).
>
> To allow sniffing all frames (including offloaded
> frames) without interfering with mac80211
> operation and states a new rx_flag was introduced
> and is not being used to distinguish frames and
> classify them for mac80211.
Does this change valid for all the firmware 10.1, 10.2, 10.4, and etc.?
>
> Signed-off-by: Grzegorz Bajorski <[email protected]>
> ---
> depends on:
> mac80211: allow drivers to report (non-)monitor frames
>
> drivers/net/wireless/ath/ath10k/htt_rx.c | 70
> ++++++++++++++++----------------
> drivers/net/wireless/ath/ath10k/wmi.c | 6 +++
> 2 files changed, 40 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
> b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 396645b..898eff0 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -1076,20 +1076,25 @@ static void ath10k_htt_rx_h_undecap_raw(struct
> ath10k *ar,
> hdr = (void *)msdu->data;
>
> /* Tail */
> - skb_trim(msdu, msdu->len - ath10k_htt_rx_crypto_tail_len(ar,
> enctype));
> + if (status->flag & RX_FLAG_IV_STRIPPED)
> + skb_trim(msdu, msdu->len -
> + ath10k_htt_rx_crypto_tail_len(ar, enctype));
>
> /* MMIC */
> - if (!ieee80211_has_morefrags(hdr->frame_control) &&
> + if ((status->flag & RX_FLAG_MMIC_STRIPPED) &&
> + !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);
> + if (status->flag & RX_FLAG_IV_STRIPPED) {
> + 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);
> + 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,
> @@ -1330,6 +1335,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
> bool has_tkip_err;
> bool has_peer_idx_invalid;
> bool is_decrypted;
> + bool is_mgmt;
> u32 attention;
>
> if (skb_queue_empty(amsdu))
> @@ -1338,6 +1344,9 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
> first = skb_peek(amsdu);
> rxd = (void *)first->data - sizeof(*rxd);
>
> + is_mgmt = !!(rxd->attention.flags &
> + __cpu_to_le32(RX_ATTENTION_FLAGS_MGMT_TYPE));
> +
> enctype = MS(__le32_to_cpu(rxd->mpdu_start.info0),
> RX_MPDU_START_INFO0_ENCRYPT_TYPE);
>
> @@ -1379,6 +1388,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
> RX_FLAG_MMIC_ERROR |
> RX_FLAG_DECRYPTED |
> RX_FLAG_IV_STRIPPED |
> + RX_FLAG_ONLY_MONITOR |
> RX_FLAG_MMIC_STRIPPED);
>
> if (has_fcs_err)
> @@ -1387,10 +1397,21 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k
> *ar,
> 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;
> + /* Firmware reports all necessary management frames via WMI
> already.
> + * They are not reported to monitor interfaces at all so pass the
> ones
> + * coming via HTT to monitor interfaces instead. This simplifies
> + * matters a lot.
> + */
> + if (is_mgmt)
> + status->flag |= RX_FLAG_ONLY_MONITOR;
> +
> + if (is_decrypted) {
> + status->flag |= RX_FLAG_DECRYPTED;
> +
> + if (likely(!is_mgmt))
> + status->flag |= RX_FLAG_IV_STRIPPED |
> + RX_FLAG_MMIC_STRIPPED;
Management frames are encrypted in MFP condition.
This change seems breaking MFP frame from working.
> +}
>
> skb_queue_walk(amsdu, msdu) {
> ath10k_htt_rx_h_csum_offload(msdu);
> @@ -1403,6 +1424,8 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
> */
> if (!is_decrypted)
> continue;
> + if (is_mgmt)
> + continue;
Ditto.
Could you provide test results in MFP case?
>
> hdr = (void *)msdu->data;
> hdr->frame_control &=
> ~__cpu_to_le16(IEEE80211_FCTL_PROTECTED);
> @@ -1503,14 +1526,6 @@ 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;
> - bool is_mgmt;
> - bool has_fcs_err;
> -
> - 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.
> */
> @@ -1520,23 +1535,6 @@ static bool ath10k_htt_rx_amsdu_allowed(struct
> ath10k *ar,
> return false;
> }
>
> - is_mgmt = !!(rxd->attention.flags &
> - __cpu_to_le32(RX_ATTENTION_FLAGS_MGMT_TYPE));
> - has_fcs_err = !!(rxd->attention.flags &
> - __cpu_to_le32(RX_ATTENTION_FLAGS_FCS_ERR));
> -
> - /* 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.
> - *
> - * However some firmware revisions don't report corrupted frames
> via
> - * WMI so don't drop them.
> - */
> - if (is_mgmt && !has_fcs_err) {
> - ath10k_dbg(ar, ATH10K_DBG_HTT, "htt rx mgmt ctrl\n");
> - return false;
> - }
> -
> if (test_bit(ATH10K_CAC_RUNNING, &ar->dev_flags)) {
> ath10k_dbg(ar, ATH10K_DBG_HTT, "htt rx cac running\n");
> return false;
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c
> b/drivers/net/wireless/ath/ath10k/wmi.c
> index 9021079..847f91a 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -2298,6 +2298,12 @@ int ath10k_wmi_event_mgmt_rx(struct ath10k *ar,
> struct sk_buff *skb)
> hdr = (struct ieee80211_hdr *)skb->data;
> fc = le16_to_cpu(hdr->frame_control);
>
> + /* Firmware is guaranteed to report all essential management
> frames via
> + * WMI while it can deliver some extra via HTT. Since there can be
> + * duplicates split the reporting wrt monitor/sniffing.
> + */
> + status->flag |= RX_FLAG_SKIP_MONITOR;
> +
> ath10k_wmi_handle_wep_reauth(ar, skb, status);
>
> /* FW delivers WEP Shared Auth frame with Protected Bit set and


2015-11-30 15:45:04

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] ath10k: deliver mgmt frames from htt to monitor vifs only

Hi Grzegorz,

[auto build test ERROR on: net-next/master]
[also build test ERROR on: v4.4-rc3 next-20151127]

url: https://github.com/0day-ci/linux/commits/Grzegorz-Bajorski/ath10k-deliver-mgmt-frames-from-htt-to-monitor-vifs-only/20151130-205850
config: i386-allmodconfig (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386

All errors (new ones prefixed by >>):

drivers/net/wireless/ath/ath10k/htt_rx.c: In function 'ath10k_htt_rx_h_mpdu':
>> drivers/net/wireless/ath/ath10k/htt_rx.c:1391:6: error: 'RX_FLAG_ONLY_MONITOR' undeclared (first use in this function)
RX_FLAG_ONLY_MONITOR |
^
drivers/net/wireless/ath/ath10k/htt_rx.c:1391:6: note: each undeclared identifier is reported only once for each function it appears in
--
drivers/net/wireless/ath/ath10k/wmi.c: In function 'ath10k_wmi_event_mgmt_rx':
>> drivers/net/wireless/ath/ath10k/wmi.c:2263:18: error: 'RX_FLAG_SKIP_MONITOR' undeclared (first use in this function)
status->flag |= RX_FLAG_SKIP_MONITOR;
^
drivers/net/wireless/ath/ath10k/wmi.c:2263:18: note: each undeclared identifier is reported only once for each function it appears in

vim +/RX_FLAG_ONLY_MONITOR +1391 drivers/net/wireless/ath/ath10k/htt_rx.c

1385
1386 /* Clear per-MPDU flags while leaving per-PPDU flags intact. */
1387 status->flag &= ~(RX_FLAG_FAILED_FCS_CRC |
1388 RX_FLAG_MMIC_ERROR |
1389 RX_FLAG_DECRYPTED |
1390 RX_FLAG_IV_STRIPPED |
> 1391 RX_FLAG_ONLY_MONITOR |
1392 RX_FLAG_MMIC_STRIPPED);
1393
1394 if (has_fcs_err)

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (1.75 kB)
.config.gz (51.38 kB)
Download all attachments

2015-12-01 09:43:00

by Grzegorz Bajorski

[permalink] [raw]
Subject: Re: [PATCH] ath10k: deliver mgmt frames from htt to monitor vifs only

2015-11-30 20:44 GMT+01:00 Peter Oh <[email protected]>:
>
> On 11/30/2015 04:56 AM, Grzegorz Bajorski wrote:
>>
>> Until now only WMI originating mgmt frames were
>> reported to mac80211. Management frames on HTT
>> were basically dropped (except frames which looked
>> like management but had FCS error).
>>
>> To allow sniffing all frames (including offloaded
>> frames) without interfering with mac80211
>> operation and states a new rx_flag was introduced
>> and is not being used to distinguish frames and
>> classify them for mac80211.
>
> Does this change valid for all the firmware 10.1, 10.2, 10.4, and etc.?

Yes.

>
>>
>> Signed-off-by: Grzegorz Bajorski <[email protected]>
>> ---
>> depends on:
>> mac80211: allow drivers to report (non-)monitor frames
>>
>> drivers/net/wireless/ath/ath10k/htt_rx.c | 70
>> ++++++++++++++++----------------
>> drivers/net/wireless/ath/ath10k/wmi.c | 6 +++
>> 2 files changed, 40 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> index 396645b..898eff0 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> @@ -1076,20 +1076,25 @@ static void ath10k_htt_rx_h_undecap_raw(struct
>> ath10k *ar,
>> hdr = (void *)msdu->data;
>> /* Tail */
>> - skb_trim(msdu, msdu->len - ath10k_htt_rx_crypto_tail_len(ar,
>> enctype));
>> + if (status->flag & RX_FLAG_IV_STRIPPED)
>> + skb_trim(msdu, msdu->len -
>> + ath10k_htt_rx_crypto_tail_len(ar, enctype));
>> /* MMIC */
>> - if (!ieee80211_has_morefrags(hdr->frame_control) &&
>> + if ((status->flag & RX_FLAG_MMIC_STRIPPED) &&
>> + !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);
>> + if (status->flag & RX_FLAG_IV_STRIPPED) {
>> + 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);
>> + 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,
>> @@ -1330,6 +1335,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
>> bool has_tkip_err;
>> bool has_peer_idx_invalid;
>> bool is_decrypted;
>> + bool is_mgmt;
>> u32 attention;
>> if (skb_queue_empty(amsdu))
>> @@ -1338,6 +1344,9 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
>> first = skb_peek(amsdu);
>> rxd = (void *)first->data - sizeof(*rxd);
>> + is_mgmt = !!(rxd->attention.flags &
>> + __cpu_to_le32(RX_ATTENTION_FLAGS_MGMT_TYPE));
>> +
>> enctype = MS(__le32_to_cpu(rxd->mpdu_start.info0),
>> RX_MPDU_START_INFO0_ENCRYPT_TYPE);
>> @@ -1379,6 +1388,7 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k
>> *ar,
>> RX_FLAG_MMIC_ERROR |
>> RX_FLAG_DECRYPTED |
>> RX_FLAG_IV_STRIPPED |
>> + RX_FLAG_ONLY_MONITOR |
>> RX_FLAG_MMIC_STRIPPED);
>> if (has_fcs_err)
>> @@ -1387,10 +1397,21 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k
>> *ar,
>> 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;
>> + /* Firmware reports all necessary management frames via WMI
>> already.
>> + * They are not reported to monitor interfaces at all so pass the
>> ones
>> + * coming via HTT to monitor interfaces instead. This simplifies
>> + * matters a lot.
>> + */
>> + if (is_mgmt)
>> + status->flag |= RX_FLAG_ONLY_MONITOR;
>> +
>> + if (is_decrypted) {
>> + status->flag |= RX_FLAG_DECRYPTED;
>> +
>> + if (likely(!is_mgmt))
>> + status->flag |= RX_FLAG_IV_STRIPPED |
>> + RX_FLAG_MMIC_STRIPPED;
>
> Management frames are encrypted in MFP condition.
> This change seems breaking MFP frame from working.

This patch don't affect processing PMF frames by mac80211 and higher layers.
With this change, mgmt frames from HTT instead of being dropped can be
sent to mac80211 _only_ to monitor vif's. Frames from wmi_mgmt_rx are
send to mac80211 to all vif _except_ monitor.

Behavior does not change - we take PMF frames from WMI path. The only
diffrent is that frames from WMI never go to moniotr vif - we take
them form HTT path.

>>
>> +}
>> skb_queue_walk(amsdu, msdu) {
>> ath10k_htt_rx_h_csum_offload(msdu);
>> @@ -1403,6 +1424,8 @@ static void ath10k_htt_rx_h_mpdu(struct ath10k *ar,
>> */
>> if (!is_decrypted)
>> continue;
>> + if (is_mgmt)
>> + continue;
>
> Ditto.
> Could you provide test results in MFP case?
>
>> hdr = (void *)msdu->data;
>> hdr->frame_control &=
>> ~__cpu_to_le16(IEEE80211_FCTL_PROTECTED);
>> @@ -1503,14 +1526,6 @@ 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;
>> - bool is_mgmt;
>> - bool has_fcs_err;
>> -
>> - 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.
>> */
>> @@ -1520,23 +1535,6 @@ static bool ath10k_htt_rx_amsdu_allowed(struct
>> ath10k *ar,
>> return false;
>> }
>> - is_mgmt = !!(rxd->attention.flags &
>> - __cpu_to_le32(RX_ATTENTION_FLAGS_MGMT_TYPE));
>> - has_fcs_err = !!(rxd->attention.flags &
>> - __cpu_to_le32(RX_ATTENTION_FLAGS_FCS_ERR));
>> -
>> - /* 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.
>> - *
>> - * However some firmware revisions don't report corrupted frames
>> via
>> - * WMI so don't drop them.
>> - */
>> - if (is_mgmt && !has_fcs_err) {
>> - ath10k_dbg(ar, ATH10K_DBG_HTT, "htt rx mgmt ctrl\n");
>> - return false;
>> - }
>> -
>> if (test_bit(ATH10K_CAC_RUNNING, &ar->dev_flags)) {
>> ath10k_dbg(ar, ATH10K_DBG_HTT, "htt rx cac running\n");
>> return false;
>> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c
>> b/drivers/net/wireless/ath/ath10k/wmi.c
>> index 9021079..847f91a 100644
>> --- a/drivers/net/wireless/ath/ath10k/wmi.c
>> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
>> @@ -2298,6 +2298,12 @@ int ath10k_wmi_event_mgmt_rx(struct ath10k *ar,
>> struct sk_buff *skb)
>> hdr = (struct ieee80211_hdr *)skb->data;
>> fc = le16_to_cpu(hdr->frame_control);
>> + /* Firmware is guaranteed to report all essential management
>> frames via
>> + * WMI while it can deliver some extra via HTT. Since there can be
>> + * duplicates split the reporting wrt monitor/sniffing.
>> + */
>> + status->flag |= RX_FLAG_SKIP_MONITOR;
>> +
>> ath10k_wmi_handle_wep_reauth(ar, skb, status);
>> /* FW delivers WEP Shared Auth frame with Protected Bit set and
>
>



--
Pozdrawiam / Best Regards
Grzegorz Bajorski

2016-03-11 12:12:43

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath10k: deliver mgmt frames from htt to monitor vifs only

Grzegorz Bajorski <[email protected]> writes:

> Until now only WMI originating mgmt frames were
> reported to mac80211. Management frames on HTT
> were basically dropped (except frames which looked
> like management but had FCS error).
>
> To allow sniffing all frames (including offloaded
> frames) without interfering with mac80211
> operation and states a new rx_flag was introduced
> and is not being used to distinguish frames and
> classify them for mac80211.
>
> Signed-off-by: Grzegorz Bajorski <[email protected]>
> ---
> depends on:
> mac80211: allow drivers to report (non-)monitor frames

Applied, thanks.

--
Kalle Valo

2016-03-04 08:47:40

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] ath10k: deliver mgmt frames from htt to monitor vifs only

Grzegorz Bajorski <[email protected]> writes:

> Until now only WMI originating mgmt frames were
> reported to mac80211. Management frames on HTT
> were basically dropped (except frames which looked
> like management but had FCS error).
>
> To allow sniffing all frames (including offloaded
> frames) without interfering with mac80211
> operation and states a new rx_flag was introduced
> and is not being used to distinguish frames and
> classify them for mac80211.
>
> Signed-off-by: Grzegorz Bajorski <[email protected]>
> ---
> depends on:
> mac80211: allow drivers to report (non-)monitor frames

Note to patchwork:

178830481eee mac80211: allow drivers to report (non-)monitor frames

Is in net-next at the moment.

--
Kalle Valo