netdev - Re: [PATCH net v4 1/2] net/sched: Restrict conditions for adding duplicating netems to qdisc tree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aGMSPCjbWsxmlFuO@pop-os.localdomain>
Date: Mon, 30 Jun 2025 15:39:56 -0700
From: Cong Wang <xiyou.wangcong@...il.com>
To: Jamal Hadi Salim <jhs@...atatu.com>
Cc: William Liu <will@...lsroot.io>, netdev@...r.kernel.org,
	victor@...atatu.com, pctammela@...atatu.com, pabeni@...hat.com,
	kuba@...nel.org, stephen@...workplumber.org, dcaratti@...hat.com,
	savy@...t3mfailure.io, jiri@...nulli.us, davem@...emloft.net,
	edumazet@...gle.com, horms@...nel.org
Subject: Re: [PATCH net v4 1/2] net/sched: Restrict conditions for adding
 duplicating netems to qdisc tree

On Mon, Jun 30, 2025 at 07:32:48AM -0400, Jamal Hadi Salim wrote:
> On Sun, Jun 29, 2025 at 4:16 PM Cong Wang <xiyou.wangcong@...il.com> wrote:
> >
> > On Sat, Jun 28, 2025 at 05:25:25PM -0400, Jamal Hadi Salim wrote:
> > > your approach was to overwrite the netem specific cb which is exposed
> > > via the cb ->data that can be overwritten for example by a trivial
> > > ebpf program attach to any level of the hierarchy. This specific
> > > variant from Cong is not accessible to ebpf but as i expressed my view
> > > in other email i feel it is not a good solution.
> > >
> > > https://lore.kernel.org/netdev/CAM0EoMk4dxOFoN_=3yOy+XrtU=yvjJXAw3fVTmN9=M=R=vtbxA@mail.gmail.com/
> >
> > Hi Jamal,
> >
> > I have two concerns regarding your/Will's proposal:
> >
> > 1) I am not sure whether disallowing such case is safe. From my
> > understanding this case is not obviously or logically wrong. So if we
> > disallow it, we may have a chance to break some application.
> >
> 
> I dont intentionaly creating a loop-inside-a-loop as being correct.
> Stephen, is this a legit use case?
> Agreed that we need to be careful about some corner cases which may
> look crazy but are legit.

Maybe I misunderstand your patch, to me duplicating packets in
parallel sub-hierarchy is not wrong, may be even useful.

> 
> > 2) Singling out this case looks not elegant to me.
> 
> My thinking is to long term disallow all nonsense hierarchy use cases,
> such as this one, with some
> "feature bits". ATM, it's easy to catch the bad configs within a
> single qdisc in ->init() but currently not possible if it affects a
> hierarchy.

The problem with this is it becomes harder to get a clear big picture,
today netem, tomorrow maybe hfsc etc.? We could end up with hiding such
bad-config-prevention code in different Qdisc's.

With the approach I suggested, we have a central place (probably
sch_api.c) to have all the logics, nothing is hidden, easier to
understand and easier to introduce more bad-config-prevention code.

I hope this makes sense to you.

Thanks.