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

On Sun, Jul 13, 2025 at 03:12:20PM -0700, Stephen Hemminger wrote:
> On Sun, 13 Jul 2025 14:47:45 -0700
> Cong Wang <xiyou.wangcong@...il.com> wrote:
> 
> > +	if (q->duplicate) {
> > +		bool dup = true;
> > +
> > +		if (netem_skb_cb(skb)->duplicate) {
> > +			netem_skb_cb(skb)->duplicate = 0;
> > +			dup = false;
> > +		}
> > +		if (dup && q->duplicate >= get_crandom(&q->dup_cor, &q->prng))
> > +			++count;
> > +	}
> 
> Doesn't look ideal.
> 
> Why do yo need the temporary variable here?

It is all because we need to clear the duplicate bit.

> And you risk having bug where first duplicate sets the flag then second clears it
> and a third layer would do duplicate and reset it.

I am not sure I follow you here. After this patch, we only enqueue the
duplicate skb to the same qdisc and this skb's duplicate bit gets
immediately cleared here. They have no chance to traverse to other qdisc
before clearing this bit. This is actually why it is safe to use
netem_skb_cb() now (instead of skb ext or tc_skb_cb). Or am I missing anything?

If you have a specific setup you suspect this may break, please share it
with me and I am happy to test and integrate into TDC.

Thanks for your review!