Dear Jason,
Thank you for your patch.
Am 21.07.25 um 10:33 schrieb Jason Xing:
From: Jason Xing <kernelxing@xxxxxxxxxxx>
The issue can happen when the budget number of descs are consumed. As
Instead of “The issue”, I’d use “An underflow …”.
long as the budget is decreased to zero, it will again go into
while (budget-- > 0) statement and get decreased by one, so the
underflow issue can happen. It will lead to returning true whereas the
expected value should be false.
What is “it”?
In this case where all the budget are used up, it means zc function
*is* used?
should return false to let the poll run again because normally we
might have more data to process.
Do you have a reproducer, you could add to the commit message?
Fixes: 132c32ee5bc0 ("net: stmmac: Add TX via XDP zero-copy socket")
Signed-off-by: Jason Xing <kernelxing@xxxxxxxxxxx>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f350a6662880..ea5541f9e9a6 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2596,7 +2596,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
budget = min(budget, stmmac_tx_avail(priv, queue));
- while (budget-- > 0) {
+ while (budget > 0) {
So, if the while loop should not be entered with budget being 0, then
the line could be changed to `while (--budget > 0) {`? But then it
wouldn’t be called for budget being 1.
A for loop might be the better choice for a loop with budget as counting
variable?
struct stmmac_metadata_request meta_req;
struct xsk_tx_metadata *meta = NULL;
dma_addr_t dma_addr;
@@ -2681,6 +2681,8 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
tx_q->cur_tx = STMMAC_GET_ENTRY(tx_q->cur_tx, priv->dma_conf.dma_tx_size);
entry = tx_q->cur_tx;
+
+ budget--;
}
u64_stats_update_begin(&txq_stats->napi_syncp);
u64_stats_add(&txq_stats->napi.tx_set_ic_bit, tx_set_ic_bit);
Excuse my ignorance, but I do not yet see the problem that the while
loop is entered and buffer is set to 0. Is it later the return condition?
return !!budget && work_done;
Kind regards,
Paul
