Re: [PATCH V2 2/3] brcmfmac: handle monitor mode marked msgbuf packets

2018-06-11 Thread Arend van Spriel

On 5/30/2018 10:13 PM, Rafał Miłecki wrote:

From: Rafał Miłecki 

New Broadcom firmwares mark monitor mode packets using a newly defined
bit in the flags field. Use it to filter them out and pass to the
monitor interface. These defines were found in bcmmsgbuf.h from SDK.

As not every firmware generates radiotap header this commit introduces
BRCMF_FEAT_MON_FMT_RADIOTAP that has to be set per firmware version. If
not present brcmf_netif_mon_rx() assumed packet being a raw 802.11 frame
and prepends it with an empty radiotap header.

It's limited to the msgbuf protocol. Adding support for SDIO/USB devices
will require some extra research.

Signed-off-by: Rafał Miłecki 
---
V2: Use cpu_to_le16 when setting it_len
---
  .../wireless/broadcom/brcm80211/brcmfmac/core.c| 24 ++
  .../wireless/broadcom/brcm80211/brcmfmac/core.h|  2 ++
  .../wireless/broadcom/brcm80211/brcmfmac/feature.h |  6 +-
  .../wireless/broadcom/brcm80211/brcmfmac/msgbuf.c  | 17 +++
  4 files changed, 48 insertions(+), 1 deletion(-)


[snip]


diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
index d1193825e559..6e417d104b7f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
@@ -33,6 +33,8 @@
   * MFP: 802.11w Management Frame Protection.
   * GSCAN: enhanced scan offload feature.
   * FWSUP: Firmware supplicant.
+ * MON_802_11_FLAG: monitor packets flagged as 802.11
+ * MON_FMT_RADIOTAP: monitor packets include radiotap header
   */
  #define BRCMF_FEAT_LIST \
BRCMF_FEAT_DEF(MBSS) \
@@ -48,7 +50,9 @@
BRCMF_FEAT_DEF(WOWL_ARP_ND) \
BRCMF_FEAT_DEF(MFP) \
BRCMF_FEAT_DEF(GSCAN) \
-   BRCMF_FEAT_DEF(FWSUP)
+   BRCMF_FEAT_DEF(FWSUP) \
+   BRCMF_FEAT_DEF(MON_802_11_FLAG) \


No sure if I want to expose such detail. I would want a feature flag to 
indicate monitor mode is present, but leave out details on how protocol 
layer like msgbuf passes them to the driver.



+   BRCMF_FEAT_DEF(MON_FMT_RADIOTAP)


For firmware not passing the packets with radiotap there is some info 
passed, ie. rx status, which the driver can use to fill specific 
radiotap fields. We need to look into that.


Regards,
Arend


Re: [PATCH V2 2/3] brcmfmac: handle monitor mode marked msgbuf packets

2018-05-30 Thread Arend van Spriel

On 5/30/2018 10:13 PM, Rafał Miłecki wrote:

From: Rafał Miłecki 

New Broadcom firmwares mark monitor mode packets using a newly defined
bit in the flags field. Use it to filter them out and pass to the
monitor interface. These defines were found in bcmmsgbuf.h from SDK.

As not every firmware generates radiotap header this commit introduces
BRCMF_FEAT_MON_FMT_RADIOTAP that has to be set per firmware version. If
not present brcmf_netif_mon_rx() assumed packet being a raw 802.11 frame
and prepends it with an empty radiotap header.

It's limited to the msgbuf protocol. Adding support for SDIO/USB devices
will require some extra research.


No just extra research but actual firmware change.


Signed-off-by: Rafał Miłecki 
---
V2: Use cpu_to_le16 when setting it_len
---
  .../wireless/broadcom/brcm80211/brcmfmac/core.c| 24 ++
  .../wireless/broadcom/brcm80211/brcmfmac/core.h|  2 ++
  .../wireless/broadcom/brcm80211/brcmfmac/feature.h |  6 +-
  .../wireless/broadcom/brcm80211/brcmfmac/msgbuf.c  | 17 +++
  4 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 72954fd6df3b..c9e1f6fcc57b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -21,6 +21,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -404,6 +405,29 @@ void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff 
*skb)
netif_rx_ni(skb);
  }

+void brcmf_netif_mon_rx(struct brcmf_if *ifp, struct sk_buff *skb)
+{
+   if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MON_FMT_RADIOTAP)) {
+   /* Do nothing */
+   } else {
+   struct ieee80211_radiotap_header *radiotap;
+
+   radiotap = skb_push(skb, sizeof(*radiotap));
+   memset(radiotap, 0, sizeof(*radiotap));
+   radiotap->it_len = cpu_to_le16(sizeof(*radiotap));
+
+   /* TODO: what are these extra 4 bytes? */
+   skb->len -= 4;


This could be dongle memory location holding receive status needed to 
build radiotap header on the host. Will look into this.



+   }


[snip]


diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
index d1193825e559..6e417d104b7f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
@@ -33,6 +33,8 @@
   * MFP: 802.11w Management Frame Protection.
   * GSCAN: enhanced scan offload feature.
   * FWSUP: Firmware supplicant.
+ * MON_802_11_FLAG: monitor packets flagged as 802.11
+ * MON_FMT_RADIOTAP: monitor packets include radiotap header
   */
  #define BRCMF_FEAT_LIST \
BRCMF_FEAT_DEF(MBSS) \
@@ -48,7 +50,9 @@
BRCMF_FEAT_DEF(WOWL_ARP_ND) \
BRCMF_FEAT_DEF(MFP) \
BRCMF_FEAT_DEF(GSCAN) \
-   BRCMF_FEAT_DEF(FWSUP)
+   BRCMF_FEAT_DEF(FWSUP) \
+   BRCMF_FEAT_DEF(MON_802_11_FLAG) \


On branch I created for 4366c0 release firmware includes 'monitor' in 
the 'cap' iovar.



+   BRCMF_FEAT_DEF(MON_FMT_RADIOTAP)


I intend to add this to the 'cap' iovar as well for 4366c0 release if I 
get green light for it. Either 'rtap' or just 'radiotap'.


As it turns out the 'cap' iovar returns worst case (or best if you are a 
sucker for features) a string of 566 characters, but brcmfmac uses 512 
bytes right now. Better increase that to 768 or so.


Regards,
Arend