From: Jakub Kicinski <kuba@kernel.org>
To: justin.chen@broadcom.com
Cc: netdev@vger.kernel.org, pabeni@redhat.com, edumazet@google.com,
davem@davemloft.net, andrew+netdev@lunn.ch,
bcm-kernel-feedback-list@broadcom.com,
florian.fainelli@broadcom.com, opendmb@gmail.com, nb@tipi-net.de
Subject: Re: [PATCH net 1/2] net: bcmgenet: fix leaking free_bds
Date: Fri, 27 Mar 2026 21:11:27 -0700 [thread overview]
Message-ID: <20260327211127.7e9317b3@kernel.org> (raw)
In-Reply-To: <20260326184529.1393438-2-justin.chen@brodcom.com>
On Thu, 26 Mar 2026 11:45:28 -0700 justin.chen@broadcom.com wrote:
> From: Justin Chen <justin.chen@broadcom.com>
>
> While reclaiming the tx queue we fast forward the write pointer to
> drop any data in flight. These dropped frames are not added back
> to the pool of free bds. We also need to tell the netdev that we
> are dropping said data.
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index 482a31e7b72b..3e1fc3bb8297 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1985,6 +1985,7 @@ static unsigned int bcmgenet_tx_reclaim(struct net_device *dev,
> drop = (ring->prod_index - ring->c_index) & DMA_C_INDEX_MASK;
> released += drop;
> ring->prod_index = ring->c_index & DMA_C_INDEX_MASK;
> + ring->free_bds += drop;
> while (drop--) {
> cb_ptr = bcmgenet_put_txcb(priv, ring);
> skb = cb_ptr->skb;
> @@ -1996,6 +1997,7 @@ static unsigned int bcmgenet_tx_reclaim(struct net_device *dev,
> }
> if (skb)
> dev_consume_skb_any(skb);
> + netdev_tx_reset_queue(netdev_get_tx_queue(dev, ring->index));
> bcmgenet_tdma_ring_writel(priv, ring->index,
> ring->prod_index, TDMA_PROD_INDEX);
> wr_ptr = ring->write_ptr * WORDS_PER_BD(priv);
AI says you may be off by one here?
Does this loop miss the oldest dropped descriptor and leak its SKB and
DMA mapping?
Since bcmgenet_get_txcb() increments write_ptr after a transmission,
write_ptr always points to the next available empty slot.
When this loops backwards drop times, the first iteration retrieves that
empty descriptor (where cb->skb is NULL), and the loop will finish before
reaching the oldest uncompleted descriptor at c_index.
When write_ptr is rolled back to c_index, won't subsequent transmissions
overwrite the descriptor at c_index and permanently leak the unmapped DMA
buffer and SKB memory?
next prev parent reply other threads:[~2026-03-28 4:11 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-26 18:45 [PATCH 0/2 net] net: bcmgenet: fix lock up when queues time out justin.chen
2026-03-26 18:45 ` [PATCH net 1/2] net: bcmgenet: fix leaking free_bds justin.chen
2026-03-28 4:11 ` Jakub Kicinski [this message]
2026-03-31 23:11 ` Justin Chen
2026-03-26 18:45 ` [PATCH net 2/2] net: bcmgenet: fix racing timeout handler justin.chen
2026-03-28 4:13 ` Jakub Kicinski
2026-03-31 23:15 ` Justin Chen
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=20260327211127.7e9317b3@kernel.org \
--to=kuba@kernel.org \
--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=justin.chen@broadcom.com \
--cc=nb@tipi-net.de \
--cc=netdev@vger.kernel.org \
--cc=opendmb@gmail.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.