netdev - Re: [Patch v3 net 1/4] net_sched: Implement the right netem duplication behavior

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: <aHaX8n8o/fLBi57L@pop-os.localdomain>
Date: Tue, 15 Jul 2025 11:03:30 -0700
From: Cong Wang <xiyou.wangcong@...il.com>
To: William Liu <will@...lsroot.io>
Cc: netdev@...r.kernel.org, jhs@...atatu.com, stephen@...workplumber.org,
	Savino Dicanosa <savy@...t3mfailure.io>
Subject: Re: [Patch v3 net 1/4] net_sched: Implement the right netem
 duplication behavior

On Mon, Jul 14, 2025 at 02:30:26AM +0000, William Liu wrote:
> FWIW, I suggested changing this behavior to not enqueue from the root a while ago too on the security mailing list for the HFSC rsc bug (as the re-entrancy violated assumptions in other qdiscs), but was told some users might be expecting that behavior and we would break their setups.

Thanks for your valuable input.

Instead of arguing on what users expect, I think it is fair to use the
man page as our argreement with user. Please let me know if you have
more reasonable argreement or more reasonable use case for us to justify
updates to the man page.

I have an open mind.

> 
> If we really want to preserve the ability to have multiple duplicating netems in a tree, I think Jamal had a good suggestion here to rely on tc_skb_ext extensions [1].

Do you mind to be more specific here? I don't think I am following you
on why tc_skb_ext is better here.

The reason why I changed back to netem_skb_cb is exactly because of the
enqueue beahvior change, which now only allows the skb to be queued to
the same qdisc.

If you have a specific reasonable use case you suspect my patch might
break, please share it with me. It would help me to understand you
better and more importantly to test this patch more comprehensively,
I'd love to add as many selftests as I can.

> 
> However, I noted that there are implementation issues that we would have to deal with. Copying what I said there [2]:
> 
> "The tc_skb_ext approach has a problem... the config option that enables it is NET_TC_SKB_EXT. I assumed this is a generic name for skb extensions in the tc subsystem, but unfortunately this is hardcoded for NET_CLS_ACT recirculation support.

IMHO, Kconfig is not a problem here, we just need to deal with the
necessary dependency if we really need to use it.

Like I said above, I don't see the problem of using netem_skb_cb after
enqueuing to the same qdisc, this is the only reason why I don't see the
need to changing it to either tc_skb_cb or skb_ext.

Thanks for your review!