Re: [PATCH] hfsc: ensure class is added to eltree exactly once
From: Florian Westphal <fw@strlen.de>
Date: 2016-05-31 23:00:55
Eric Dumazet [off-list ref] wrote:
quoted
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c index d783d7c..0854be3 100644 --- a/net/sched/sch_hfsc.c +++ b/net/sched/sch_hfsc.c@@ -1583,6 +1583,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch) { struct hfsc_class *cl; int uninitialized_var(err); + unsigned int qlen; cl = hfsc_classify(skb, sch, &err); if (cl == NULL) {@@ -1592,6 +1593,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch) return err; } + qlen = cl->qdisc->q.qlen; err = qdisc_enqueue(skb, cl->qdisc); if (unlikely(err != NET_XMIT_SUCCESS)) { if (net_xmit_drop_count(err)) {@@ -1601,7 +1603,7 @@ hfsc_enqueue(struct sk_buff *skb, struct Qdisc *sch) return err; } - if (cl->qdisc->q.qlen == 1) + if (qlen == 0) set_active(cl, qdisc_pkt_len(skb)); sch->q.qlen++;Well, I am not sure HFSC can deal with non work conserving qdisc anyway ?
As long as child qdisc dequeue() returns an skb if qlen > 0 it should work.
Call to set_active(cl, qdisc_pkt_len(skb)); would tell you that HFSC does not expect another packet than @skb being the next dequeued one.
Good point. I'll look into it in more detail but AFAIU this "only" means we might overshoot a defined realtime/deadline criterion, I don't (yet) see how it could explain Miroslavs bug report.
If you want to make HFSC generic, you would need a lot more changes.
Could you elaborate? I'm not interested in e.g. tbf leaves (makes no sense to me), but I think it should play nice with netem, sfq, codel, fq_codel, fq, ... I don't see any problems with fq_codel + hfsc unless I deliberately misconfigure fq_codel (setting extremely low mem limit, e.g. 32kbyte).