[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAM0EoMkO0vZ4ZtODLJEBP5FiA0+ofVNOSf-BxCOGOyWAZDHdTg@mail.gmail.com>
Date: Sat, 24 May 2025 09:07:41 -0400
From: Jamal Hadi Salim <jhs@...atatu.com>
To: William Liu <will@...lsroot.io>
Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, Savy <savy@...t3mfailure.io>,
Cong Wang <xiyou.wangcong@...il.com>, Victor Nogueira <victor@...atatu.com>,
Pedro Tammela <pctammela@...atatu.com>, Paolo Abeni <pabeni@...hat.com>,
Jakub Kicinski <kuba@...nel.org>, Stephen Hemminger <stephen@...workplumber.org>,
Davide Caratti <dcaratti@...hat.com>
Subject: Re: [BUG] net/sched: Soft Lockup/Task Hang and OOM Loop in netem_dequeue
On Fri, May 23, 2025 at 11:23 AM William Liu <will@...lsroot.io> wrote:
>
>
> On Friday, May 23rd, 2025 at 11:01 AM, Jamal Hadi Salim <jhs@...atatu.com> wrote:
>
> >
> >
> > looks ok from 30k feet. Comments:
> > You dont need a count variable anymore because the per-cpu variable
> > serves the same goal - so please get rid of it.
> > Submit a formal patchset - including at least one tdc test(your
> > validation tests are sufficient) then we can do a better review.
> > And no netdev comments are complete if we dont talk about the xmas
> > tree variable layout. Not your fault on the state of the lights on
> > that tree but dont make it worse;-> move the nest_level assignment as
> >
> > the first line in the function.
> >
> > cheers,
> > jamal
>
> Ok will do. Can you clarify the count variable issue? My understanding of the count variable is that it accounts for both the possibility of simulated duplication or loss. The nest_level I added is to prevent against duplication of a duplicate due to re-entrancy.
"count == 0" seems to be only needed for the loss when a drop decision is made.
Slight tangent: Looking at init() the setup does allow for both to be
on (i.e could be "and" not just "or" as you state above). It feels
sensible to me that if the loss function decided the packet is to be
dropped then that would override the duplication. Stephen? IOW, I
highly doubt anyone is using a setup with both on - but doesnt mean
someone wouldnt set both if the system allows it.
Would be useful to test with loss probability of 100% and set
duplication on to see what kind of smoke comes out.
In any case - here's what i had in mind, of course not even compile tested.
cheers,
jamal
Download attachment "netem-patchlet" of type "application/octet-stream" (3104 bytes)
Powered by blists - more mailing lists