From: Justin Chen <justin.chen@broadcom.com>
To: Nicolai Buchwitz <nb@tipi-net.de>
Cc: netdev@vger.kernel.org, bcm-kernel-feedback-list@broadcom.com,
horms@kernel.org, pabeni@redhat.com, kuba@kernel.org,
edumazet@google.com, davem@davemloft.net, andrew+netdev@lunn.ch,
florian.fainelli@broadcom.com
Subject: Re: [PATCH net] net: bcmasp: fix lost TX wakeup race with lockless queue API
Date: Mon, 28 Sep 2026 14:56:38 -0700 [thread overview]
Message-ID: <bbbb6f23-44b6-4794-8bc4-6de58c7fb191@broadcom.com> (raw)
In-Reply-To: <a65f6e3445b1ea32becb8d974bc147ad@tipi-net.de>
On 9/28/26 2:35 PM, Nicolai Buchwitz wrote:
> Hi Justin
>
> On 28.9.2026 22:18, justin.chen@broadcom.com wrote:
>> From: Justin Chen <justin.chen@broadcom.com>
>>
>> netif_stop_queue() in bcmasp_xmit() raced with netif_wake_queue() in
>> bcmasp_tx_poll(): a reclaim landing between the ring-full check and
>> the stop call left the queue stopped despite free descriptors,
>> hanging TX until timeout.
>>
>> Switch to netif_txq_maybe_stop()/netif_txq_completed_wake() (the
>> lockless TX queue API, which also brings in BQL), replace
>> tx_spb_ring_full() with a live bcmasp_tx_avail() count.
>>
>> Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet
>> controller")
>> Signed-off-by: Justin Chen <justin.chen@broadcom.com>
>> Assisted-by: Claude:claude-sonnet-5
>> ---
>> .../net/ethernet/broadcom/asp2/bcmasp_intf.c | 68 +++++++++++--------
>
>> [...]
>
>> static struct sk_buff *bcmasp_csum_offload(struct net_device *dev,
>> @@ -241,16 +239,18 @@ static netdev_tx_t bcmasp_xmit(struct sk_buff
>> *skb, struct net_device *dev)
>> struct bcmasp_tx_cb *txcb;
>> dma_addr_t mapping, valid;
>> struct bcmasp_desc *desc;
>> + struct netdev_queue *txq;
>> bool csum_hw = false;
>> struct device *kdev;
>> skb_frag_t *frag;
>>
>> kdev = &intf->parent->pdev->dev;
>> + txq = netdev_get_tx_queue(dev, 0);
>>
>> nr_frags = skb_shinfo(skb)->nr_frags;
>>
>> - if (tx_spb_ring_full(intf, nr_frags + 1)) {
>> - netif_stop_queue(dev);
>> + if (unlikely(bcmasp_tx_avail(intf) < nr_frags + 1)) {
>> + netif_tx_stop_queue(txq);
>
> nit: AFAIU this is unreachable for now, but to prevent future races maybe
> better use netif_txq_try_stop()?
Will use netif_txq_try_stop() in v2. My understanding is this is still
reachable since the maybe_stop call at the end of the xmit may have
false wake ups when the ring is full. That is what is documented in the
function header description for netif_txq_try_stop().
Thanks,
Justin
>
>> [...]
>
> Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
>
> Thanks,
> Nicolai
next prev parent reply other threads:[~2026-09-28 21:56 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-28 20:18 [PATCH net] net: bcmasp: fix lost TX wakeup race with lockless queue API justin.chen
2026-09-28 20:25 ` netdev-bot+sinfo
2026-09-28 20:39 ` Florian Fainelli
2026-09-28 21:35 ` Nicolai Buchwitz
2026-09-28 21:56 ` Justin Chen [this message]
2026-10-01 23:18 ` netdev-bot+sashiko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bbbb6f23-44b6-4794-8bc4-6de58c7fb191@broadcom.com \
--to=justin.chen@broadcom.com \
--cc=andrew+netdev@lunn.ch \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=florian.fainelli@broadcom.com \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=nb@tipi-net.de \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox