On 2018-01-30 12:30, Arend van Spriel wrote:
On 1/30/2018 10:09 AM, Rafał Miłecki wrote:
From: Rafał Miłecki <rafal@xxxxxxxxxx>
When using 4366b1 and 4366c0 chipsets with more recent firmwares
1) 10.10 (TOB) (r663589)
2) 10.10.122.20 (r683106)
respectively, it is impossible to use brcmfmac with interface in AP
mode. With the AP interface bridged and multicast used, no STA will be
able to associate; the STA will be immediately disassociated when
attempting to associate.
Debugging revealed this to be caused by a "faked" packet (generated by
firmware), that is passed to the networking subsystem and then back to
the firmware. Fortunately this packet is easily identified and can be
detected and ignored as a workaround for misbehaving firmware.
Signed-off-by: Rafał Miłecki <rafal@xxxxxxxxxx>
---
.../wireless/broadcom/brcm80211/brcmfmac/core.c | 46
++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 930e423f83a8..a98ba9bbc7fe 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -323,8 +323,54 @@ void brcmf_txflowblock_if(struct brcmf_if *ifp,
spin_unlock_irqrestore(&ifp->netif_stop_lock, flags);
}
+/**
+ * brcmf_is_valid_skb - validates skb received from the hardware
+ *
+ * @skb: skb to check
+ *
+ * Sometimes firmware/hardware can generate broken packets that
aren't real or
+ * valid and their skb-s shouldn't be passed up to the networking
subsystem.
+ *
+ * Firmwares for 43602a1, 4366b1 and 4366c0 are known to *generate* a
faked 6 B
+ * packet whenever a STA associates. The purpose of this fake packet
remains
+ * unknown but it is clearly not data coming from a station. As such
it
+ * shouldn't be passed to the networking subsystem.
+ *
+ * Normally such a packet would simply be ignored, but this is not
the case with
+ * more recent 4366b1 and 4366c0 firmwares. These firmwares seem to
explicitly
+ * check for this packet and will reject (disassociate) the station,
making it
+ * impossible to connect to the AP at all. This can happen when using
a bridged
+ * interface with multicasting. Such a scenario apparently isn't
tested (or
+ * supported) by Broadcom's internal team.
+ */
+static bool brcmf_is_valid_skb(struct sk_buff *skb)
+{
+ const u8 fw_faked_packet[6] __aligned(2) = {
+ 0x00, 0x01, 0xaf, 0x81, 0x01, 0x00,
+ };
+#if !defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+ const u16 *a = (const u16 *)skb->data;
+ const u16 *b = (const u16 *)fw_faked_packet;
+#endif
+
+ if (skb->len != 6)
+ return true;
+
+#if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
+ return !!(((*(const u32 *)skb->data) ^ (*(const u32
*)fw_faked_packet)) |
+ ((*(const u16 *)(skb->data + 4)) ^ (*(const u16
*)(fw_faked_packet + 4))));
+#else
+ return !!((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2]));
+#endif
+}
The code above does look very much like ether_addr_equal(). Why not
use that instead of reinventing it.
You're right about ether_addr_equal(), I wasn't sure if that is
acceptable to use it for comparing any 6 bytes data.
I know it'd work, it's just not what it was designed for.
Kalle?
void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff *skb)
{
+ if (!brcmf_is_valid_skb(skb)) {
+ brcmu_pkt_buf_free_skb(skb);
Maybe we should add a driver stat for this although I better have a
look into the root cause of this.
It seems there are following stats fields we can use:
1) rx_errors
2) rx_dropped
3) rx_length_errors
4) rx_over_errors
5) rx_crc_errors
6) rx_frame_errors
7) rx_fifo_errors
8) rx_missed_errors
Which one do you think may fit this case the best?
