On Fri, 15 Apr 2016, Eric Dumazet wrote:
> Then you need to save the initial qdisc (bfifo for TBF) in a special
> place, to make sure the delete operation is guaranteed to succeed.
>
> Or fail the delete if the bfifo can not be allocated.
>
> I can tell that determinism if far more interesting than usability for
> some users occasionally playing with tc.
>
> Surely the silent fallback to noop_qdisc is wrong.
So before we go further and fix the fact that we actually do have hidden
qdiscs (by refactoring qdisc_match_from_root() and friends), I'd still
like to bring the patch below up for consideration.
Thanks.
From: Jiri Kosina <jkos...@suse.cz>
Subject: [PATCH] sch_tbf: avoid silent fallback to noop_qdisc
TBF started its life as a classless qdisc with a single builtin FIFO queue
which was being shaped.
When it got later turned into classful qdisc, it was written in a way that
the fallback qdisc was noop_qdisc, which produces bad user experience (delete
of last manually added class doesn't reset it to initial default, but renders
networking unusable instead).
Switch the default fallback to bfifo; this also mimics how the other guys
(HTB, HFSC, CBQ, ...) are behaving.
Signed-off-by: Jiri Kosina <jkos...@suse.cz>
---
net/sched/sch_tbf.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 3161e49..b06dffe 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -508,8 +508,12 @@ static int tbf_graft(struct Qdisc *sch, unsigned long arg,
struct Qdisc *new,
{
struct tbf_sched_data *q = qdisc_priv(sch);
- if (new == NULL)
- new = &noop_qdisc;
+ if (new == NULL) {
+ /* reset to default qdisc */
+ new = qdisc_create_dflt(sch->dev_queue, &bfifo_qdisc_ops,
sch->parent);
+ if (!new)
+ return -ENOBUFS;
+ }
*old = qdisc_replace(sch, new, &q->qdisc);
return 0;
--
Jiri Kosina
SUSE Labs
