Commit 3d7c8257 authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller

net_sched: prio: insure proper transactional behavior

Now prio_init() can return -ENOMEM, it also has to make sure
any allocated qdiscs are freed, since the caller (qdisc_create()) wont
call ->destroy() handler for us.

More generally, we want a transactional behavior for "tc qdisc
change ...", so prio_tune() should not make modifications if
any error is returned.

It means that we must validate parameters and allocate missing qdisc(s)
before taking root qdisc lock exactly once, to not leave the prio qdisc
in an intermediate state.

Fixes: cbdf4511 ("net_sched: prio: properly report out of memory errors")
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reported-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
Acked-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 0c5ddb51
...@@ -172,8 +172,9 @@ prio_destroy(struct Qdisc *sch) ...@@ -172,8 +172,9 @@ prio_destroy(struct Qdisc *sch)
static int prio_tune(struct Qdisc *sch, struct nlattr *opt) static int prio_tune(struct Qdisc *sch, struct nlattr *opt)
{ {
struct prio_sched_data *q = qdisc_priv(sch); struct prio_sched_data *q = qdisc_priv(sch);
struct Qdisc *queues[TCQ_PRIO_BANDS];
int oldbands = q->bands, i;
struct tc_prio_qopt *qopt; struct tc_prio_qopt *qopt;
int i;
if (nla_len(opt) < sizeof(*qopt)) if (nla_len(opt) < sizeof(*qopt))
return -EINVAL; return -EINVAL;
...@@ -187,54 +188,42 @@ static int prio_tune(struct Qdisc *sch, struct nlattr *opt) ...@@ -187,54 +188,42 @@ static int prio_tune(struct Qdisc *sch, struct nlattr *opt)
return -EINVAL; return -EINVAL;
} }
/* Before commit, make sure we can allocate all new qdiscs */
for (i = oldbands; i < qopt->bands; i++) {
queues[i] = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops,
TC_H_MAKE(sch->handle, i + 1));
if (!queues[i]) {
while (i > oldbands)
qdisc_destroy(queues[--i]);
return -ENOMEM;
}
}
sch_tree_lock(sch); sch_tree_lock(sch);
q->bands = qopt->bands; q->bands = qopt->bands;
memcpy(q->prio2band, qopt->priomap, TC_PRIO_MAX+1); memcpy(q->prio2band, qopt->priomap, TC_PRIO_MAX+1);
for (i = q->bands; i < TCQ_PRIO_BANDS; i++) { for (i = q->bands; i < oldbands; i++) {
struct Qdisc *child = q->queues[i]; struct Qdisc *child = q->queues[i];
q->queues[i] = &noop_qdisc;
if (child != &noop_qdisc) {
qdisc_tree_reduce_backlog(child, child->q.qlen, child->qstats.backlog);
qdisc_destroy(child);
}
}
sch_tree_unlock(sch);
for (i = 0; i < q->bands; i++) { qdisc_tree_reduce_backlog(child, child->q.qlen,
struct Qdisc *child; child->qstats.backlog);
qdisc_destroy(child);
}
if (q->queues[i] != &noop_qdisc) for (i = oldbands; i < q->bands; i++)
continue; q->queues[i] = queues[i];
child = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops, sch_tree_unlock(sch);
TC_H_MAKE(sch->handle, i + 1));
if (!child)
return -ENOMEM;
sch_tree_lock(sch);
q->queues[i] = child;
sch_tree_unlock(sch);
}
return 0; return 0;
} }
static int prio_init(struct Qdisc *sch, struct nlattr *opt) static int prio_init(struct Qdisc *sch, struct nlattr *opt)
{ {
struct prio_sched_data *q = qdisc_priv(sch); if (!opt)
int i;
for (i = 0; i < TCQ_PRIO_BANDS; i++)
q->queues[i] = &noop_qdisc;
if (opt == NULL) {
return -EINVAL; return -EINVAL;
} else {
int err;
if ((err = prio_tune(sch, opt)) != 0) return prio_tune(sch, opt);
return err;
}
return 0;
} }
static int prio_dump(struct Qdisc *sch, struct sk_buff *skb) static int prio_dump(struct Qdisc *sch, struct sk_buff *skb)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment