Hi,

Thanks for getting back to me,

* Chip: chip 43340 rev 2 pmurev 20
* I'm running kernel 4.1 (backports) for the wireless parts
* I have not to setup my environment for the latest kernel yet.
  It's on my todo list.

Looking at the code you pointed out I realize my fix i misplaced.
It's strange that I don't end up with a negative count since
the counter would be decreased twice, with my patch in place.
I took a closer look at the execution flow and made the following
observation:

The eh->h_proto has changed when reaching the txfinalize() function.
This only happens in case of packet drop.

Simplified execution flow.

brcmf_fws_process_skb():
  # At this point ntohs(eh->h_proto) == 0x888E (ETH_P_PAE)
  rc = brcmf_proto_txdata() ->
    brcmf_proto_bcdc_txdata() ->
      brcmf_sdio_bus_txdata()
  # At this point it has changed to: ntohs(eh->h_proto) == 0x8939 (?)
  if (rc < 0)
    # When executing this function the cnt will not be decreased due to
    # eh->h_proto being changed.
    brcmf_txfinalize()

I need to continue my investigation to find out why the h_proto got changed.
I assume this is not an expected behavior?

BR
Per


2016-04-06 10:22 GMT+02:00 Arend Van Spriel <arend.vanspr...@broadcom.com>:
> On 5-4-2016 22:01, per.for...@gmail.com wrote:
>> From: Per Forlin <per.for...@gmail.com>
>>
>> The pend_8021x_cnt gets into a state where it's not being decreased.
>> When the brcmf_netdev_wait_pend8021x is called it will timeout
>> because there are no pending packets to be consumed. There is no
>> easy way of getting out of this state once it has happened.
>> Preventing the counter from being increased in case of dropped
>> packets seem like a reasonable solution.
>>
>> Log showing overflow txq and increased cnt:
>>
>> // Extra debug prints
>> brcmfmac: [brcmf_netdev_wait_pend8021x] pend_8021x_cnt == 0
>> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 1
>> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 2
>>
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>> brcmfmac: brcmf_sdio_bus_txdata: out of bus->txq !!!
>>
>> // Extra debug prints
>> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 3
>> brcmfmac: [brcmf_netdev_start_xmit] pend_8021x_cnt == 4
>> brcmfmac: [brcmf_netdev_wait_pend8021x] pend_8021x_cnt == 4
>>
>> WARNING: at .../brcmfmac/core.c:1289 brcmf_netdev_wait_pend8021x
>>   (warn_slowpath_common)
>>   (warn_slowpath_null)
>>   (brcmf_netdev_wait_pend8021x [brcmfmac])
>>   (send_key_to_dongle [brcmfmac])
>>   (brcmf_cfg80211_del_key [brcmfmac])
>>   (nl80211_del_key [cfg80211])
>>   (genl_rcv_msg)
>>   (netlink_rcv_skb)
>>   (genl_rcv)
>>   (netlink_unicast)
>>   (netlink_sendmsg)
>>   (sock_sendmsg)
>>   (___sys_sendmsg)
>>   (__sys_sendmsg)
>>   (SyS_sendmsg)
>>
>> Signed-off-by: Per Forlin <per.for...@gmail.com>
>> ---
>> I came across this bug in an older kernel but it seems relevant
>> for the latest kernel as well. I'm not familiar with the code
>> but I can reproduce the issue and verify a fix for it.
>> With this patch I no longer get stuck with a pend8021_cnt > 0.
>>
>> Change log:
>>  v2 - Add variable to know whether the counter is increased or not
>
> Sorry for the late response. Can you elaborate where you are seeing this.
>
> What kind of chipset are you using?
> What kernel version are you running?
>
> The function brcmf_fws_process_skb() should call brcmf_txfinalize() in
> case of an error and it does in latest kernel. There the count is
> decreased. We had an issue mapping to the right ifp as reported by
> Rafał, but that has also been fixed recently. So main question here:
>
> Do you see the issue in the latest kernel?
>
> Regards,
> Arend
>
>>  drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c 
>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>> index ed9998b..de80ad8 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>> @@ -196,6 +196,7 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct 
>> sk_buff *skb,
>>       struct brcmf_if *ifp = netdev_priv(ndev);
>>       struct brcmf_pub *drvr = ifp->drvr;
>>       struct ethhdr *eh = (struct ethhdr *)(skb->data);
>> +     bool pend_8021x_cnt_increased = false;
>>
>>       brcmf_dbg(DATA, "Enter, bsscfgidx=%d\n", ifp->bsscfgidx);
>>
>> @@ -233,14 +234,18 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct 
>> sk_buff *skb,
>>               goto done;
>>       }
>>
>> -     if (eh->h_proto == htons(ETH_P_PAE))
>> +     if (eh->h_proto == htons(ETH_P_PAE)) {
>>               atomic_inc(&ifp->pend_8021x_cnt);
>> +             pend_8021x_cnt_increased = true;
>> +     }
>>
>>       ret = brcmf_fws_process_skb(ifp, skb);
>>
>>  done:
>>       if (ret) {
>>               ifp->stats.tx_dropped++;
>> +             if (pend_8021x_cnt_increased)
>> +                     atomic_dec(&ifp->pend_8021x_cnt);
>>       } else {
>>               ifp->stats.tx_packets++;
>>               ifp->stats.tx_bytes += skb->len;
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to