This series has a number of fixes for 3.10 functionality and a patch
for a smatch warning that was reported to us.
The series is intended for v3.10 kernel and applies to the master branch
of the wireless-next repository.
Arend van Spriel (10):
brcmfmac: reinitialize dequeue mask per node
brcmfmac: check memory allocation in brcmf_add_if()
brcmfmac: remove error message upon allocation failure
brcmutil: simplify brcmu_pkt_free_skb()
brcmfmac: destination mac closed when interface is closed
brcmfmac: schedule dequeue upon firmware-signal reception
brcmfmac: use lock in brcmf_fws_del_interface()
brcmfmac: finalize transmit upon any rollback failure
brcmfmac: change return type for brcmf_rollback_toq() to void
brcmfmac: stop dequeue upon sk_buff commit failure
.../net/wireless/brcm80211/brcmfmac/dhd_linux.c | 6 +-
drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c | 76 +++++++++++++-------
drivers/net/wireless/brcm80211/brcmutil/utils.c | 12 +---
3 files changed, 56 insertions(+), 38 deletions(-)
--
1.7.10.4
Firmware signals a destination is closed as well as an interface. A
destination is associated with an interface. When an interface is
closed consequently the destination should be considered closed as
well.
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
index 9dae8fc..0d2ff60 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
@@ -714,11 +714,21 @@ done:
return entry;
}
-static bool brcmf_fws_mac_desc_closed(struct brcmf_fws_mac_descriptor *entry,
+static bool brcmf_fws_mac_desc_closed(struct brcmf_fws_info *fws,
+ struct brcmf_fws_mac_descriptor *entry,
int fifo)
{
+ struct brcmf_fws_mac_descriptor *if_entry;
bool closed;
+ /* for unique destination entries the related interface
+ * may be closed.
+ */
+ if (entry->mac_handle) {
+ if_entry = &fws->desc.iface[entry->interface_id];
+ if (if_entry->state == BRCMF_FWS_STATE_CLOSE)
+ return true;
+ }
/* an entry is closed when the state is closed and
* the firmware did not request anything.
*/
@@ -1079,7 +1089,8 @@ static struct sk_buff *brcmf_fws_deq(struct brcmf_fws_info *fws, int fifo)
for (i = 0; i < num_nodes; i++) {
entry = &table[(node_pos + i) % num_nodes];
- if (!entry->occupied || brcmf_fws_mac_desc_closed(entry, fifo))
+ if (!entry->occupied ||
+ brcmf_fws_mac_desc_closed(fws, entry, fifo))
continue;
if (entry->suppressed)
@@ -1753,7 +1764,7 @@ int brcmf_fws_process_skb(struct brcmf_if *ifp, struct sk_buff *skb)
brcmf_fws_lock(drvr, flags);
if (skcb->mac->suppressed ||
- brcmf_fws_mac_desc_closed(skcb->mac, fifo) ||
+ brcmf_fws_mac_desc_closed(drvr->fws, skcb->mac, fifo) ||
brcmu_pktq_mlen(&skcb->mac->psq, 3 << (fifo * 2)) ||
(!multicast &&
brcmf_fws_consume_credit(drvr->fws, fifo, skb) < 0)) {
--
1.7.10.4
Several firmware signals should be considered as opportunity to
send packets to the firmware. This patch adds conditional scheduling
of the dequeue worker thread while handling those signals.
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c | 36 +++++++++++++-------
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
index 0d2ff60..648f9bd 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
@@ -147,6 +147,9 @@ static const char *brcmf_fws_get_tlv_name(enum brcmf_fws_tlv_type id)
#define BRCMF_FWS_HTOD_FLAG_PKTFROMHOST 0x01
#define BRCMF_FWS_HTOD_FLAG_PKT_REQUESTED 0x02
+#define BRCMF_FWS_RET_OK_NOSCHEDULE 0
+#define BRCMF_FWS_RET_OK_SCHEDULE 1
+
/**
* enum brcmf_fws_skb_state - indicates processing state of skb.
*
@@ -920,12 +923,13 @@ static int brcmf_fws_macdesc_state_indicate(struct brcmf_fws_info *fws,
entry->requested_credit = 0;
if (type == BRCMF_FWS_TYPE_MAC_OPEN) {
entry->state = BRCMF_FWS_STATE_OPEN;
+ return BRCMF_FWS_RET_OK_SCHEDULE;
} else {
entry->state = BRCMF_FWS_STATE_CLOSE;
for (i = BRCMF_FWS_FIFO_AC_BE; i < NL80211_NUM_ACS; i++)
brcmf_fws_tim_update(fws, entry, i);
}
- return 0;
+ return BRCMF_FWS_RET_OK_NOSCHEDULE;
}
static int brcmf_fws_interface_state_indicate(struct brcmf_fws_info *fws,
@@ -952,10 +956,10 @@ static int brcmf_fws_interface_state_indicate(struct brcmf_fws_info *fws,
switch (type) {
case BRCMF_FWS_TYPE_INTERFACE_OPEN:
entry->state = BRCMF_FWS_STATE_OPEN;
- return 0;
+ return BRCMF_FWS_RET_OK_SCHEDULE;
case BRCMF_FWS_TYPE_INTERFACE_CLOSE:
entry->state = BRCMF_FWS_STATE_CLOSE;
- return 0;
+ return BRCMF_FWS_RET_OK_NOSCHEDULE;
default:
ret = -EINVAL;
break;
@@ -985,7 +989,7 @@ static int brcmf_fws_request_indicate(struct brcmf_fws_info *fws, u8 type,
entry->requested_packet = data[0];
entry->ac_bitmap = data[2];
- return 0;
+ return BRCMF_FWS_RET_OK_SCHEDULE;
}
static void brcmf_fws_return_credits(struct brcmf_fws_info *fws,
@@ -1259,7 +1263,7 @@ static int brcmf_fws_fifocreditback_indicate(struct brcmf_fws_info *fws,
if (fws->fcmode != BRCMF_FWS_FCMODE_EXPLICIT_CREDIT) {
brcmf_dbg(INFO, "ignored\n");
- return 0;
+ return BRCMF_FWS_RET_OK_NOSCHEDULE;
}
brcmf_dbg(TRACE, "enter: data %pM\n", data);
@@ -1268,8 +1272,7 @@ static int brcmf_fws_fifocreditback_indicate(struct brcmf_fws_info *fws,
brcmf_dbg(INFO, "map: credit %x delay %x\n", fws->fifo_credit_map,
fws->fifo_delay_map);
- brcmf_fws_schedule_deq(fws);
- return 0;
+ return BRCMF_FWS_RET_OK_SCHEDULE;
}
static int brcmf_fws_txstatus_indicate(struct brcmf_fws_info *fws, u8 *data)
@@ -1353,6 +1356,8 @@ int brcmf_fws_hdrpull(struct brcmf_pub *drvr, int ifidx, s16 signal_len,
u8 type;
u8 len;
u8 *data;
+ s32 status;
+ s32 err;
brcmf_dbg(TRACE, "enter: ifidx %d, skblen %u, sig %d\n",
ifidx, skb->len, signal_len);
@@ -1372,6 +1377,7 @@ int brcmf_fws_hdrpull(struct brcmf_pub *drvr, int ifidx, s16 signal_len,
data_len = signal_len;
signal_data = skb->data;
+ status = BRCMF_FWS_RET_OK_NOSCHEDULE;
while (data_len > 0) {
/* extract tlv info */
type = signal_data[0];
@@ -1397,6 +1403,7 @@ int brcmf_fws_hdrpull(struct brcmf_pub *drvr, int ifidx, s16 signal_len,
if (len != brcmf_fws_get_tlv_len(fws, type))
break;
+ err = BRCMF_FWS_RET_OK_NOSCHEDULE;
switch (type) {
case BRCMF_FWS_TYPE_HOST_REORDER_RXPKTS:
case BRCMF_FWS_TYPE_COMP_TXSTATUS:
@@ -1407,21 +1414,22 @@ int brcmf_fws_hdrpull(struct brcmf_pub *drvr, int ifidx, s16 signal_len,
break;
case BRCMF_FWS_TYPE_MAC_OPEN:
case BRCMF_FWS_TYPE_MAC_CLOSE:
- brcmf_fws_macdesc_state_indicate(fws, type, data);
+ err = brcmf_fws_macdesc_state_indicate(fws, type, data);
break;
case BRCMF_FWS_TYPE_INTERFACE_OPEN:
case BRCMF_FWS_TYPE_INTERFACE_CLOSE:
- brcmf_fws_interface_state_indicate(fws, type, data);
+ err = brcmf_fws_interface_state_indicate(fws, type,
+ data);
break;
case BRCMF_FWS_TYPE_MAC_REQUEST_CREDIT:
case BRCMF_FWS_TYPE_MAC_REQUEST_PACKET:
- brcmf_fws_request_indicate(fws, type, data);
+ err = brcmf_fws_request_indicate(fws, type, data);
break;
case BRCMF_FWS_TYPE_TXSTATUS:
brcmf_fws_txstatus_indicate(fws, data);
break;
case BRCMF_FWS_TYPE_FIFO_CREDITBACK:
- brcmf_fws_fifocreditback_indicate(fws, data);
+ err = brcmf_fws_fifocreditback_indicate(fws, data);
break;
case BRCMF_FWS_TYPE_RSSI:
brcmf_fws_rssi_indicate(fws, *data);
@@ -1435,7 +1443,8 @@ int brcmf_fws_hdrpull(struct brcmf_pub *drvr, int ifidx, s16 signal_len,
fws->stats.tlv_invalid_type++;
break;
}
-
+ if (err == BRCMF_FWS_RET_OK_SCHEDULE)
+ status = BRCMF_FWS_RET_OK_SCHEDULE;
signal_data += len + 2;
data_len -= len + 2;
}
@@ -1443,6 +1452,9 @@ int brcmf_fws_hdrpull(struct brcmf_pub *drvr, int ifidx, s16 signal_len,
if (data_len != 0)
fws->stats.tlv_parse_failed++;
+ if (status == BRCMF_FWS_RET_OK_SCHEDULE)
+ brcmf_fws_schedule_deq(fws);
+
/* signalling processing result does
* not affect the actual ethernet packet.
*/
--
1.7.10.4
The mask was only initialized for the first node, but it should be
done for each node that is handled in the loop.
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
index 1bcd58c..9dae8fc 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
@@ -1070,7 +1070,7 @@ static struct sk_buff *brcmf_fws_deq(struct brcmf_fws_info *fws, int fifo)
int num_nodes;
int node_pos;
int prec_out;
- int pmsk = 3;
+ int pmsk;
int i;
table = (struct brcmf_fws_mac_descriptor *)&fws->desc;
@@ -1084,6 +1084,8 @@ static struct sk_buff *brcmf_fws_deq(struct brcmf_fws_info *fws, int fifo)
if (entry->suppressed)
pmsk = 2;
+ else
+ pmsk = 3;
p = brcmu_pktq_mdeq(&entry->psq, pmsk << (fifo * 2), &prec_out);
if (p == NULL) {
if (entry->suppressed) {
--
1.7.10.4
The function brcmu_pkt_free_skb() use skb->destructor to decide
how the sk_buff should be freed. However, when running AP mode
with iptables configured this results in a kernel warning.
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Piotr Haber <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/brcm80211/brcmutil/utils.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmutil/utils.c b/drivers/net/wireless/brcm80211/brcmutil/utils.c
index bf5e50f..0f7e1c7 100644
--- a/drivers/net/wireless/brcm80211/brcmutil/utils.c
+++ b/drivers/net/wireless/brcm80211/brcmutil/utils.c
@@ -45,17 +45,9 @@ void brcmu_pkt_buf_free_skb(struct sk_buff *skb)
{
if (!skb)
return;
+
WARN_ON(skb->next);
- if (skb->destructor)
- /* cannot kfree_skb() on hard IRQ (net/core/skbuff.c) if
- * destructor exists
- */
- dev_kfree_skb_any(skb);
- else
- /* can free immediately (even in_irq()) if destructor
- * does not exist
- */
- dev_kfree_skb(skb);
+ dev_kfree_skb_any(skb);
}
EXPORT_SYMBOL(brcmu_pkt_buf_free_skb);
--
1.7.10.4
For P2P_DEVICE interface the struct brcmf_if instance is
allocated using kzalloc() which can fail. Add pointer
check and return -ENOMEM if it failed. Fixes the following
smatch error:
"drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c:770
brcmf_add_if()
error: potential null dereference 'ifp'. (kzalloc returns null)"
Reported-by: Dan Carpenter <[email protected]>
Reported-by: Fengguang Wu <[email protected]>
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Piotr Haber <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
index 763a84e..269fde2 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
@@ -754,6 +754,8 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bssidx, s32 ifidx,
/* this is P2P_DEVICE interface */
brcmf_dbg(INFO, "allocate non-netdev interface\n");
ifp = kzalloc(sizeof(*ifp), GFP_KERNEL);
+ if (!ifp)
+ return ERR_PTR(-ENOMEM);
} else {
brcmf_dbg(INFO, "allocate netdev interface\n");
/* Allocate netdev, including space for private structure */
--
1.7.10.4
In function brcmf_add_if() an error message is printed
upon alloc_netdev() failure. The allocation failure itself
spews enough info in the log so remove the error message.
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Piotr Haber <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
index 269fde2..a0afef2 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
@@ -760,10 +760,8 @@ struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bssidx, s32 ifidx,
brcmf_dbg(INFO, "allocate netdev interface\n");
/* Allocate netdev, including space for private structure */
ndev = alloc_netdev(sizeof(*ifp), name, ether_setup);
- if (!ndev) {
- brcmf_err("OOM - alloc_netdev\n");
+ if (!ndev)
return ERR_PTR(-ENOMEM);
- }
ifp = netdev_priv(ndev);
ifp->ndev = ndev;
--
1.7.10.4
In the dequeue worker the function brcmf_commit_skb() is called.
However, instead of increment the credit count upon success it
should break the for loop upon failure. Otherwise, it will result
in an endless loop.
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
index 25eaa13..8ae7da8 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
@@ -1850,10 +1850,9 @@ static void brcmf_fws_dequeue_worker(struct work_struct *worker)
fws->fifo_credit[fifo]);
for (credit = 0; credit < fws->fifo_credit[fifo]; /* nop */) {
skb = brcmf_fws_deq(fws, fifo);
- if (!skb)
+ if (!skb || brcmf_fws_commit_skb(fws, fifo, skb))
break;
- if (!brcmf_fws_commit_skb(fws, fifo, skb) &&
- brcmf_skbcb(skb)->if_flags &
+ if (brcmf_skbcb(skb)->if_flags &
BRCMF_SKB_IF_FLAGS_CREDITCHECK_MASK)
credit++;
}
--
1.7.10.4
All rollback failures should result in freeing of the sk_buff
by calling brcmf_txfinalize().
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
index 1cfec56..13518ec 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
@@ -1616,7 +1616,6 @@ brcmf_fws_rollback_toq(struct brcmf_fws_info *fws, struct sk_buff *skb)
/* free the hanger slot */
brcmf_fws_hanger_poppkt(&fws->hanger, hslot,
&pktout, true);
- brcmf_txfinalize(fws->drvr, skb, false);
rc = -EINVAL;
goto fail;
}
@@ -1650,9 +1649,10 @@ brcmf_fws_rollback_toq(struct brcmf_fws_info *fws, struct sk_buff *skb)
fail:
- if (rc)
+ if (rc) {
+ brcmf_txfinalize(fws->drvr, skb, false);
fws->stats.rollback_failed++;
- else
+ } else
fws->stats.rollback_success++;
return rc;
}
--
1.7.10.4
When deleting an interface in firmware-signalling module it will
clear any destination descriptors. To avoid concurrency issues it
should take the lock using brcmf_fws_lock().
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
index 648f9bd..1cfec56 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
@@ -1821,14 +1821,17 @@ void brcmf_fws_add_interface(struct brcmf_if *ifp)
void brcmf_fws_del_interface(struct brcmf_if *ifp)
{
struct brcmf_fws_mac_descriptor *entry = ifp->fws_desc;
+ ulong flags;
brcmf_dbg(TRACE, "enter: idx=%d\n", ifp->bssidx);
if (!entry)
return;
+ brcmf_fws_lock(ifp->drvr, flags);
ifp->fws_desc = NULL;
brcmf_fws_clear_mac_descriptor(entry);
brcmf_fws_cleanup(ifp->drvr->fws, ifp->ifidx);
+ brcmf_fws_unlock(ifp->drvr, flags);
}
static void brcmf_fws_dequeue_worker(struct work_struct *worker)
--
1.7.10.4
The function brcmf_rollback_toq() is already called in error path
and its result should not override the initial error value. As the
function releases the sk_buff there is no need to return anything
so change return type to void.
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
index 13518ec..25eaa13 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/fwsignal.c
@@ -1574,7 +1574,7 @@ static int brcmf_fws_precommit_skb(struct brcmf_fws_info *fws, int fifo,
return rc;
}
-static int
+static void
brcmf_fws_rollback_toq(struct brcmf_fws_info *fws, struct sk_buff *skb)
{
/*
@@ -1654,7 +1654,6 @@ fail:
fws->stats.rollback_failed++;
} else
fws->stats.rollback_success++;
- return rc;
}
static int brcmf_fws_consume_credit(struct brcmf_fws_info *fws, int fifo,
@@ -1733,7 +1732,7 @@ static int brcmf_fws_commit_skb(struct brcmf_fws_info *fws, int fifo,
return rc;
rollback:
- rc = brcmf_fws_rollback_toq(fws, skb);
+ brcmf_fws_rollback_toq(fws, skb);
return rc;
}
--
1.7.10.4