Re: [PATCH] net/sched: netem: account for backlog updates from child qdisc

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 26 Nov 2024 00:18:25 +0100 Martin Ottens wrote:
> @@ -702,8 +706,8 @@ static struct sk_buff *netem_dequeue(struct Qdisc *sch)
>  tfifo_dequeue:
>  	skb = __qdisc_dequeue_head(&sch->q);
>  	if (skb) {
> -		qdisc_qstats_backlog_dec(sch, skb);
>  deliver:
> +		qdisc_qstats_backlog_dec(sch, skb);
>  		qdisc_bstats_update(sch, skb);
>  		return skb;
>  	}

Isn't this sort of a change all we need?
I don't understand why we need to perform packet accounting 
in a separate new member (t_len). You seem to fix qlen accounting,
anyway, and I think sch->limit should apply to the qdisc and all
its children. Not just qdisc directly (since most classful qdiscs
don't hold packets).

I'm not a qdisc expert, so if you feel confident about this code you
need to explain the thinking in the commit message..
-- 
pw-bot: cr