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>

diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 38015b4..e19a026 100644 (file)
--- 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 3fcfd4e..f859dd5 100644 (file)
--- 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 3a9569a..9e43ed9 100644 (file)
--- 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 e7a7e87..f6b4fa9 100644 (file)
--- 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 613179c..45c31b1 100644 (file)
--- 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 3a119f5..8a45199 100644 (file)
--- 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);
        }