On 6-4-2016 17:56, Per Förlin wrote: > 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 (?) How did you check this? The skb->data pointer is moved here [1] as we need to prepend a protocol header. So probably you did not map 'eh' variable over the correct data portion. Regards, Arend [1] http://lxr.free-electrons.com/source/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c#L2708 > 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.vanspriel@xxxxxxxxxxxx>: >> On 5-4-2016 22:01, per.forlin@xxxxxxxxx wrote: >>> From: Per Forlin <per.forlin@xxxxxxxxx> >>> >>> 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.forlin@xxxxxxxxx> >>> --- >>> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
