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

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

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