tc: check for errors in gen_rate_estimator creation

The functions gen_new_estimator and gen_replace_estimator can return
errors, but they were being ignored.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

6 files changed, 97 insertions, 41 deletions

diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 38015b493947..e19a0261144a 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -185,14 +185,21 @@ override:
                 if (parm->peakrate.rate) {
                         P_tab = qdisc_get_rtab(&parm->peakrate,
                                                tb[TCA_POLICE_PEAKRATE]);
-                        if (P_tab == NULL) {
-                                qdisc_put_rtab(R_tab);
+                        if (P_tab == NULL)
                                 goto failure;
-                        }
                 }
         }
-        /* No failure allowed after this point */
+
         spin_lock_bh(&police->tcf_lock);
+        if (est) {
+                err = gen_replace_estimator(&police->tcf_bstats,
+                                            &police->tcf_rate_est,
+                                            &police->tcf_lock, est);
+                if (err)
+                        goto failure_unlock;
+        }
+
+        /* No failure allowed after this point */
         if (R_tab != NULL) {
                 qdisc_put_rtab(police->tcfp_R_tab);
                 police->tcfp_R_tab = R_tab;
@@ -217,10 +224,6 @@ override:
 
         if (tb[TCA_POLICE_AVRATE])
                 police->tcfp_ewma_rate = nla_get_u32(tb[TCA_POLICE_AVRATE]);
-        if (est)
-                gen_replace_estimator(&police->tcf_bstats,
-                                      &police->tcf_rate_est,
-                                      &police->tcf_lock, est);
 
         spin_unlock_bh(&police->tcf_lock);
         if (ret != ACT_P_CREATED)
@@ -238,7 +241,13 @@ override:
         a->priv = police;
         return ret;
 
+failure_unlock:
+        spin_unlock_bh(&police->tcf_lock);
 failure:
+        if (P_tab)
+                qdisc_put_rtab(P_tab);
+        if (R_tab)
+                qdisc_put_rtab(R_tab);
         if (ret == ACT_P_CREATED)
                 kfree(police);
         return err;
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 3fcfd4ef11d1..f859dd5fabf4 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -880,9 +880,12 @@ static int qdisc_change(struct Qdisc *sch, struct nlattr **tca)
         sch->stab = stab;
 
         if (tca[TCA_RATE])
+                /* NB: ignores errors from replace_estimator
+                   because change can't be undone. */
                 gen_replace_estimator(&sch->bstats, &sch->rate_est,
-                                      qdisc_root_sleeping_lock(sch),
-                                      tca[TCA_RATE]);
+                                            qdisc_root_sleeping_lock(sch),
+                                            tca[TCA_RATE]);
+
         return 0;
 }
 
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 3a9569a3396c..9e43ed949167 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1765,11 +1765,23 @@ cbq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, struct nlattr **t
                 }
 
                 if (tb[TCA_CBQ_RATE]) {
-                        rtab = qdisc_get_rtab(nla_data(tb[TCA_CBQ_RATE]), tb[TCA_CBQ_RTAB]);
+                        rtab = qdisc_get_rtab(nla_data(tb[TCA_CBQ_RATE]),
+                                              tb[TCA_CBQ_RTAB]);
                         if (rtab == NULL)
                                 return -EINVAL;
                 }
 
+                if (tca[TCA_RATE]) {
+                        err = gen_replace_estimator(&cl->bstats, &cl->rate_est,
+                                                    qdisc_root_sleeping_lock(sch),
+                                                    tca[TCA_RATE]);
+                        if (err) {
+                                if (rtab)
+                                        qdisc_put_rtab(rtab);
+                                return err;
+                        }
+                }
+
                 /* Change class parameters */
                 sch_tree_lock(sch);
 
@@ -1805,10 +1817,6 @@ cbq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, struct nlattr **t
 
                 sch_tree_unlock(sch);
 
-                if (tca[TCA_RATE])
-                        gen_replace_estimator(&cl->bstats, &cl->rate_est,
-                                              qdisc_root_sleeping_lock(sch),
-                                              tca[TCA_RATE]);
                 return 0;
         }
 
@@ -1855,6 +1863,17 @@ cbq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, struct nlattr **t
         cl = kzalloc(sizeof(*cl), GFP_KERNEL);
         if (cl == NULL)
                 goto failure;
+
+        if (tca[TCA_RATE]) {
+                err = gen_new_estimator(&cl->bstats, &cl->rate_est,
+                                        qdisc_root_sleeping_lock(sch),
+                                        tca[TCA_RATE]);
+                if (err) {
+                        kfree(cl);
+                        goto failure;
+                }
+        }
+
         cl->R_tab = rtab;
         rtab = NULL;
         cl->refcnt = 1;
@@ -1896,10 +1915,6 @@ cbq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, struct nlattr **t
 
         qdisc_class_hash_grow(sch, &q->clhash);
 
-        if (tca[TCA_RATE])
-                gen_new_estimator(&cl->bstats, &cl->rate_est,
-                                  qdisc_root_sleeping_lock(sch), tca[TCA_RATE]);
-
         *arg = (unsigned long)cl;
         return 0;
 
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index e7a7e87b141a..f6b4fa97df70 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -82,15 +82,19 @@ static int drr_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
                 quantum = psched_mtu(qdisc_dev(sch));
 
         if (cl != NULL) {
+                if (tca[TCA_RATE]) {
+                        err = gen_replace_estimator(&cl->bstats, &cl->rate_est,
+                                                    qdisc_root_sleeping_lock(sch),
+                                                    tca[TCA_RATE]);
+                        if (err)
+                                return err;
+                }
+
                 sch_tree_lock(sch);
                 if (tb[TCA_DRR_QUANTUM])
                         cl->quantum = quantum;
                 sch_tree_unlock(sch);
 
-                if (tca[TCA_RATE])
-                        gen_replace_estimator(&cl->bstats, &cl->rate_est,
-                                              qdisc_root_sleeping_lock(sch),
-                                              tca[TCA_RATE]);
                 return 0;
         }
 
@@ -106,10 +110,16 @@ static int drr_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
         if (cl->qdisc == NULL)
                 cl->qdisc = &noop_qdisc;
 
-        if (tca[TCA_RATE])
-                gen_replace_estimator(&cl->bstats, &cl->rate_est,
-                                      qdisc_root_sleeping_lock(sch),
-                                      tca[TCA_RATE]);
+        if (tca[TCA_RATE]) {
+                err = gen_replace_estimator(&cl->bstats, &cl->rate_est,
+                                            qdisc_root_sleeping_lock(sch),
+                                            tca[TCA_RATE]);
+                if (err) {
+                        qdisc_destroy(cl->qdisc);
+                        kfree(cl);
+                        return err;
+                }
+        }
 
         sch_tree_lock(sch);
         qdisc_class_hash_insert(&q->clhash, &cl->common);
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 613179c9969c..45c31b1a4e1d 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1018,6 +1018,14 @@ hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
                 }
                 cur_time = psched_get_time();
 
+                if (tca[TCA_RATE]) {
+                        err = gen_replace_estimator(&cl->bstats, &cl->rate_est,
+                                              qdisc_root_sleeping_lock(sch),
+                                              tca[TCA_RATE]);
+                        if (err)
+                                return err;
+                }
+
                 sch_tree_lock(sch);
                 if (rsc != NULL)
                         hfsc_change_rsc(cl, rsc, cur_time);
@@ -1034,10 +1042,6 @@ hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
                 }
                 sch_tree_unlock(sch);
 
-                if (tca[TCA_RATE])
-                        gen_replace_estimator(&cl->bstats, &cl->rate_est,
-                                              qdisc_root_sleeping_lock(sch),
-                                              tca[TCA_RATE]);
                 return 0;
         }
 
@@ -1063,6 +1067,16 @@ hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
         if (cl == NULL)
                 return -ENOBUFS;
 
+        if (tca[TCA_RATE]) {
+                err = gen_new_estimator(&cl->bstats, &cl->rate_est,
+                                        qdisc_root_sleeping_lock(sch),
+                                        tca[TCA_RATE]);
+                if (err) {
+                        kfree(cl);
+                        return err;
+                }
+        }
+
         if (rsc != NULL)
                 hfsc_change_rsc(cl, rsc, 0);
         if (fsc != NULL)
@@ -1093,9 +1107,6 @@ hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 
         qdisc_class_hash_grow(sch, &q->clhash);
 
-        if (tca[TCA_RATE])
-                gen_new_estimator(&cl->bstats, &cl->rate_est,
-                                  qdisc_root_sleeping_lock(sch), tca[TCA_RATE]);
         *arg = (unsigned long)cl;
         return 0;
 }
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 3a119f576bb3..8a4519989732 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1332,9 +1332,14 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
                 if ((cl = kzalloc(sizeof(*cl), GFP_KERNEL)) == NULL)
                         goto failure;
 
-                gen_new_estimator(&cl->bstats, &cl->rate_est,
-                                  qdisc_root_sleeping_lock(sch),
-                                  tca[TCA_RATE] ? : &est.nla);
+                err = gen_new_estimator(&cl->bstats, &cl->rate_est,
+                                        qdisc_root_sleeping_lock(sch),
+                                        tca[TCA_RATE] ? : &est.nla);
+                if (err) {
+                        kfree(cl);
+                        goto failure;
+                }
+
                 cl->refcnt = 1;
                 cl->children = 0;
                 INIT_LIST_HEAD(&cl->un.leaf.drop_list);
@@ -1386,10 +1391,13 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
                 if (parent)
                         parent->children++;
         } else {
-                if (tca[TCA_RATE])
-                        gen_replace_estimator(&cl->bstats, &cl->rate_est,
-                                              qdisc_root_sleeping_lock(sch),
-                                              tca[TCA_RATE]);
+                if (tca[TCA_RATE]) {
+                        err = gen_replace_estimator(&cl->bstats, &cl->rate_est,
+                                                    qdisc_root_sleeping_lock(sch),
+                                                    tca[TCA_RATE]);
+                        if (err)
+                                return err;
+                }
                 sch_tree_lock(sch);
         }