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] [day] [month] [year] [list]

Message-ID: <xi0MIYyksyLO_1P2VCdlupWbaZk4XFWqX4yq0L-ZjavT4RyEnl7KzGTnk9aAOWdSFAUIhR8nCN3U5_0kwyG6iDDfdTxWeQsaFIJS_WSOEiw=@willsroot.io>
Date: Tue, 15 Jul 2025 18:41:05 +0000
From: William Liu <will@...lsroot.io>
To: Cong Wang <xiyou.wangcong@...il.com>
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 Tuesday, July 15th, 2025 at 6:03 PM, Cong Wang <xiyou.wangcong@...il.com> wrote:

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

I don't really have too strong of an opinion here and personally think it makes more sense to enqueue from the current qdisc under duplication. However, if the code has been performing the enqueue at the root from so many years ago..

I will defer to what others say on this.

> > 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!

If tc_skb_ext is used, then the original behavior of enqueue from root can be preserved and multiple duplicating netems can now exist in the tree in any hierarchy. No potential user setups or expectations will be broken. 

I am not sure how others would feel about propagating this amount of change though just for this one specific use case though (but I am of the opinion that not hardcoding NET_TC_SKB_EXT for one specific use case is a good thing regardless).

Best,
William