Re: [PATCH net-next 1/2] stmmac: xsk: fix underflow of budget in zerocopy mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 22, 2025 at 7:05 AM Jason Xing <kerneljasonxing@xxxxxxxxx> wrote:
>
> On Mon, Jul 21, 2025 at 11:37 PM Stanislav Fomichev
> <stfomichev@xxxxxxxxx> wrote:
> >
> > On 07/21, Jason Xing wrote:
> > > From: Jason Xing <kernelxing@xxxxxxxxxxx>
> > >
> > > The issue can happen when the budget number of descs are consumed. As
> > > 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.
> > >
> > > In this case where all the budget are used up, it means zc function
> > > should return false to let the poll run again because normally we
> > > might have more data to process.
> > >
> > > 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) {
> >
> > There is a continue on line 2621.
>
> Thanks for catching this!
>
> > Should we do 'for (; budget > 0; budget--)'
> > instead? And maybe the same for ixgbe [0]?
>
> Not really. I think I can move the 'budget--' just before the
> 'continue' part. If we convert it to use 'for' loop and then we end up
> with one of 'break' statements, the budget still gets accidently
> increased by one whereas ixgbe driver even doesn't handle the desc
> this time. IIUC, it should not happen, right?

Sorry, I was totally wrong. Your suggestions work and I will revise
them as you said :)

Thanks,
Jason





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux