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

[Cong Wang <xiyou.wangcong@gmail.com>]

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@gmail.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.