This patch adds extack support for generic qdisc handling. The extack
will be set deeper to each called function which is not part of netdev
core api. A future patchset will add per-qdisc specific changes.

Cc: David Ahern <dsah...@gmail.com>
Signed-off-by: Alexander Aring <ar...@mojatatu.com>
---
 net/sched/sch_api.c | 193 +++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 139 insertions(+), 54 deletions(-)

diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index a48ca41b7ecf..7b52a16d2fea 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -449,7 +449,8 @@ static const struct nla_policy stab_policy[TCA_STAB_MAX + 
1] = {
        [TCA_STAB_DATA] = { .type = NLA_BINARY },
 };
 
-static struct qdisc_size_table *qdisc_get_stab(struct nlattr *opt)
+static struct qdisc_size_table *qdisc_get_stab(struct nlattr *opt,
+                                              struct netlink_ext_ack *extack)
 {
        struct nlattr *tb[TCA_STAB_MAX + 1];
        struct qdisc_size_table *stab;
@@ -458,23 +459,31 @@ static struct qdisc_size_table *qdisc_get_stab(struct 
nlattr *opt)
        u16 *tab = NULL;
        int err;
 
-       err = nla_parse_nested(tb, TCA_STAB_MAX, opt, stab_policy, NULL);
-       if (err < 0)
+       err = nla_parse_nested(tb, TCA_STAB_MAX, opt, stab_policy, extack);
+       if (err < 0) {
+               NL_SET_ERR_MSG(extack, "Failed to parse stab netlink msg");
                return ERR_PTR(err);
-       if (!tb[TCA_STAB_BASE])
+       }
+       if (!tb[TCA_STAB_BASE]) {
+               NL_SET_ERR_MSG(extack, "Stab base is missing");
                return ERR_PTR(-EINVAL);
+       }
 
        s = nla_data(tb[TCA_STAB_BASE]);
 
        if (s->tsize > 0) {
-               if (!tb[TCA_STAB_DATA])
+               if (!tb[TCA_STAB_DATA]) {
+                       NL_SET_ERR_MSG(extack, "Stab data is missing");
                        return ERR_PTR(-EINVAL);
+               }
                tab = nla_data(tb[TCA_STAB_DATA]);
                tsize = nla_len(tb[TCA_STAB_DATA]) / sizeof(u16);
        }
 
-       if (tsize != s->tsize || (!tab && tsize > 0))
+       if (tsize != s->tsize || (!tab && tsize > 0)) {
+               NL_SET_ERR_MSG(extack, "Invalid data length of stab data");
                return ERR_PTR(-EINVAL);
+       }
 
        list_for_each_entry(stab, &qdisc_stab_list, list) {
                if (memcmp(&stab->szopts, s, sizeof(*s)))
@@ -486,8 +495,10 @@ static struct qdisc_size_table *qdisc_get_stab(struct 
nlattr *opt)
        }
 
        stab = kmalloc(sizeof(*stab) + tsize * sizeof(u16), GFP_KERNEL);
-       if (!stab)
+       if (!stab) {
+               NL_SET_ERR_MSG(extack, "Failed to allocate memory for stab 
data");
                return ERR_PTR(-ENOMEM);
+       }
 
        stab->refcnt = 1;
        stab->szopts = *s;
@@ -896,7 +907,8 @@ static void notify_and_destroy(struct net *net, struct 
sk_buff *skb,
 
 static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
                       struct sk_buff *skb, struct nlmsghdr *n, u32 classid,
-                      struct Qdisc *new, struct Qdisc *old)
+                      struct Qdisc *new, struct Qdisc *old,
+                      struct netlink_ext_ack *extack)
 {
        struct Qdisc *q = old;
        struct net *net = dev_net(dev);
@@ -911,8 +923,10 @@ static int qdisc_graft(struct net_device *dev, struct 
Qdisc *parent,
                    (new && new->flags & TCQ_F_INGRESS)) {
                        num_q = 1;
                        ingress = 1;
-                       if (!dev_ingress_queue(dev))
+                       if (!dev_ingress_queue(dev)) {
+                               NL_SET_ERR_MSG(extack, "Cannot find ingress 
queue for specified netdev");
                                return -ENOENT;
+                       }
                }
 
                if (dev->flags & IFF_UP)
@@ -958,10 +972,12 @@ static int qdisc_graft(struct net_device *dev, struct 
Qdisc *parent,
                if (cops && cops->graft) {
                        unsigned long cl = cops->find(parent, classid);
 
-                       if (cl)
+                       if (cl) {
                                err = cops->graft(parent, cl, new, &old);
-                       else
+                       } else {
+                               NL_SET_ERR_MSG(extack, "Specified class not 
found");
                                err = -ENOENT;
+                       }
                }
                if (!err)
                        notify_and_destroy(net, skb, n, classid, old, new);
@@ -982,7 +998,8 @@ static struct lock_class_key qdisc_rx_lock;
 static struct Qdisc *qdisc_create(struct net_device *dev,
                                  struct netdev_queue *dev_queue,
                                  struct Qdisc *p, u32 parent, u32 handle,
-                                 struct nlattr **tca, int *errp)
+                                 struct nlattr **tca, int *errp,
+                                 struct netlink_ext_ack *extack)
 {
        int err;
        struct nlattr *kind = tca[TCA_KIND];
@@ -1020,11 +1037,14 @@ static struct Qdisc *qdisc_create(struct net_device 
*dev,
 #endif
 
        err = -ENOENT;
-       if (!ops)
+       if (!ops) {
+               NL_SET_ERR_MSG(extack, "Specified qdisc not found");
                goto err_out;
+       }
 
        sch = qdisc_alloc(dev_queue, ops);
        if (IS_ERR(sch)) {
+               NL_SET_ERR_MSG(extack, "Failed to allocate qdisc");
                err = PTR_ERR(sch);
                goto err_out2;
        }
@@ -1039,8 +1059,10 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
                if (handle == 0) {
                        handle = qdisc_alloc_handle(dev);
                        err = -ENOMEM;
-                       if (handle == 0)
+                       if (handle == 0) {
+                               NL_SET_ERR_MSG(extack, "Failed to allocate 
qdisc handle");
                                goto err_out3;
+                       }
                }
                lockdep_set_class(qdisc_lock(sch), &qdisc_tx_lock);
                if (!netif_is_multiqueue(dev))
@@ -1069,16 +1091,20 @@ static struct Qdisc *qdisc_create(struct net_device 
*dev,
        if (qdisc_is_percpu_stats(sch)) {
                sch->cpu_bstats =
                        netdev_alloc_pcpu_stats(struct gnet_stats_basic_cpu);
-               if (!sch->cpu_bstats)
+               if (!sch->cpu_bstats) {
+                       NL_SET_ERR_MSG(extack, "Failed to allocate qdisc per 
cpu bstats");
                        goto err_out4;
+               }
 
                sch->cpu_qstats = alloc_percpu(struct gnet_stats_queue);
-               if (!sch->cpu_qstats)
+               if (!sch->cpu_qstats) {
+                       NL_SET_ERR_MSG(extack, "Failed to allocate qdisc per 
cpu qstats");
                        goto err_out4;
+               }
        }
 
        if (tca[TCA_STAB]) {
-               stab = qdisc_get_stab(tca[TCA_STAB]);
+               stab = qdisc_get_stab(tca[TCA_STAB], extack);
                if (IS_ERR(stab)) {
                        err = PTR_ERR(stab);
                        goto err_out4;
@@ -1089,8 +1115,10 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
                seqcount_t *running;
 
                err = -EOPNOTSUPP;
-               if (sch->flags & TCQ_F_MQROOT)
+               if (sch->flags & TCQ_F_MQROOT) {
+                       NL_SET_ERR_MSG(extack, "Invalid qdisc flag 
TCQ_F_MQROOT");
                        goto err_out4;
+               }
 
                if (sch->parent != TC_H_ROOT &&
                    !(sch->flags & TCQ_F_INGRESS) &&
@@ -1105,8 +1133,10 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
                                        NULL,
                                        running,
                                        tca[TCA_RATE]);
-               if (err)
+               if (err) {
+                       NL_SET_ERR_MSG(extack, "Failed to generate new 
estimator");
                        goto err_out4;
+               }
        }
 
        qdisc_hash_add(sch, false);
@@ -1139,21 +1169,24 @@ static struct Qdisc *qdisc_create(struct net_device 
*dev,
        goto err_out3;
 }
 
-static int qdisc_change(struct Qdisc *sch, struct nlattr **tca)
+static int qdisc_change(struct Qdisc *sch, struct nlattr **tca,
+                       struct netlink_ext_ack *extack)
 {
        struct qdisc_size_table *ostab, *stab = NULL;
        int err = 0;
 
        if (tca[TCA_OPTIONS]) {
-               if (!sch->ops->change)
+               if (!sch->ops->change) {
+                       NL_SET_ERR_MSG(extack, "Change operation not supported 
by qdisc");
                        return -EINVAL;
+               }
                err = sch->ops->change(sch, tca[TCA_OPTIONS]);
                if (err)
                        return err;
        }
 
        if (tca[TCA_STAB]) {
-               stab = qdisc_get_stab(tca[TCA_STAB]);
+               stab = qdisc_get_stab(tca[TCA_STAB], extack);
                if (IS_ERR(stab))
                        return PTR_ERR(stab);
        }
@@ -1235,24 +1268,32 @@ static int tc_get_qdisc(struct sk_buff *skb, struct 
nlmsghdr *n,
        int err;
 
        if ((n->nlmsg_type != RTM_GETQDISC) &&
-           !netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN))
+           !netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
+               NL_SET_ERR_MSG(extack, "Net admin permission required for this 
operation");
                return -EPERM;
+       }
 
        err = nlmsg_parse(n, sizeof(*tcm), tca, TCA_MAX, NULL, extack);
-       if (err < 0)
+       if (err < 0) {
+               NL_SET_ERR_MSG(extack, "Failed to parse tcm netlink message");
                return err;
+       }
 
        dev = __dev_get_by_index(net, tcm->tcm_ifindex);
-       if (!dev)
+       if (!dev) {
+               NL_SET_ERR_MSG(extack, "Failed to find specified netdev");
                return -ENODEV;
+       }
 
        clid = tcm->tcm_parent;
        if (clid) {
                if (clid != TC_H_ROOT) {
                        if (TC_H_MAJ(clid) != TC_H_MAJ(TC_H_INGRESS)) {
                                p = qdisc_lookup(dev, TC_H_MAJ(clid));
-                               if (!p)
+                               if (!p) {
+                                       NL_SET_ERR_MSG(extack, "Failed to find 
qdisc with specified classid");
                                        return -ENOENT;
+                               }
                                q = qdisc_leaf(p, clid);
                        } else if (dev_ingress_queue(dev)) {
                                q = dev_ingress_queue(dev)->qdisc_sleeping;
@@ -1260,26 +1301,38 @@ static int tc_get_qdisc(struct sk_buff *skb, struct 
nlmsghdr *n,
                } else {
                        q = dev->qdisc;
                }
-               if (!q)
+               if (!q) {
+                       NL_SET_ERR_MSG(extack, "Cannot find specified qdisc on 
specified netdev");
                        return -ENOENT;
+               }
 
-               if (tcm->tcm_handle && q->handle != tcm->tcm_handle)
+               if (tcm->tcm_handle && q->handle != tcm->tcm_handle) {
+                       NL_SET_ERR_MSG(extack, "Invalid handle");
                        return -EINVAL;
+               }
        } else {
                q = qdisc_lookup(dev, tcm->tcm_handle);
-               if (!q)
+               if (!q) {
+                       NL_SET_ERR_MSG(extack, "Failed to find qdisc with 
specified handle");
                        return -ENOENT;
+               }
        }
 
-       if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], q->ops->id))
+       if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], q->ops->id)) {
+               NL_SET_ERR_MSG(extack, "Invalid qdisc name");
                return -EINVAL;
+       }
 
        if (n->nlmsg_type == RTM_DELQDISC) {
-               if (!clid)
+               if (!clid) {
+                       NL_SET_ERR_MSG(extack, "Classid cannot be zero");
                        return -EINVAL;
-               if (q->handle == 0)
+               }
+               if (q->handle == 0) {
+                       NL_SET_ERR_MSG(extack, "Cannot delete qdisc with handle 
of zero");
                        return -ENOENT;
-               err = qdisc_graft(dev, p, skb, n, clid, NULL, q);
+               }
+               err = qdisc_graft(dev, p, skb, n, clid, NULL, q, extack);
                if (err != 0)
                        return err;
        } else {
@@ -1303,30 +1356,38 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct 
nlmsghdr *n,
        struct Qdisc *q, *p;
        int err;
 
-       if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN))
+       if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN)) {
+               NL_SET_ERR_MSG(extack, "Net admin permission required for this 
operation");
                return -EPERM;
+       }
 
 replay:
        /* Reinit, just in case something touches this. */
        err = nlmsg_parse(n, sizeof(*tcm), tca, TCA_MAX, NULL, extack);
-       if (err < 0)
+       if (err < 0) {
+               NL_SET_ERR_MSG(extack, "Failed to parse netlink TC attributes");
                return err;
+       }
 
        tcm = nlmsg_data(n);
        clid = tcm->tcm_parent;
        q = p = NULL;
 
        dev = __dev_get_by_index(net, tcm->tcm_ifindex);
-       if (!dev)
+       if (!dev) {
+               NL_SET_ERR_MSG(extack, "Failed to find specified netdev");
                return -ENODEV;
+       }
 
 
        if (clid) {
                if (clid != TC_H_ROOT) {
                        if (clid != TC_H_INGRESS) {
                                p = qdisc_lookup(dev, TC_H_MAJ(clid));
-                               if (!p)
+                               if (!p) {
+                                       NL_SET_ERR_MSG(extack, "Failed to find 
specified qdisc");
                                        return -ENOENT;
+                               }
                                q = qdisc_leaf(p, clid);
                        } else if (dev_ingress_queue_create(dev)) {
                                q = dev_ingress_queue(dev)->qdisc_sleeping;
@@ -1341,21 +1402,33 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct 
nlmsghdr *n,
 
                if (!q || !tcm->tcm_handle || q->handle != tcm->tcm_handle) {
                        if (tcm->tcm_handle) {
-                               if (q && !(n->nlmsg_flags & NLM_F_REPLACE))
+                               if (q && !(n->nlmsg_flags & NLM_F_REPLACE)) {
+                                       NL_SET_ERR_MSG(extack, "NLM_F_REPLACE 
needed to override");
                                        return -EEXIST;
-                               if (TC_H_MIN(tcm->tcm_handle))
+                               }
+                               if (TC_H_MIN(tcm->tcm_handle)) {
+                                       NL_SET_ERR_MSG(extack, "Invalid minor 
handle");
                                        return -EINVAL;
+                               }
                                q = qdisc_lookup(dev, tcm->tcm_handle);
-                               if (!q)
+                               if (!q) {
+                                       NL_SET_ERR_MSG(extack, "No qdisc found 
for specified handle");
                                        goto create_n_graft;
-                               if (n->nlmsg_flags & NLM_F_EXCL)
+                               }
+                               if (n->nlmsg_flags & NLM_F_EXCL) {
+                                       NL_SET_ERR_MSG(extack, "Exclusivity 
flag on, cannot override");
                                        return -EEXIST;
+                               }
                                if (tca[TCA_KIND] &&
-                                   nla_strcmp(tca[TCA_KIND], q->ops->id))
+                                   nla_strcmp(tca[TCA_KIND], q->ops->id)) {
+                                       NL_SET_ERR_MSG(extack, "Invalid qdisc 
name");
                                        return -EINVAL;
+                               }
                                if (q == p ||
-                                   (p && check_loop(q, p, 0)))
+                                   (p && check_loop(q, p, 0))) {
+                                       NL_SET_ERR_MSG(extack, "Too many 
references");
                                        return -ELOOP;
+                               }
                                qdisc_refcount_inc(q);
                                goto graft;
                        } else {
@@ -1390,33 +1463,45 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct 
nlmsghdr *n,
                        }
                }
        } else {
-               if (!tcm->tcm_handle)
+               if (!tcm->tcm_handle) {
+                       NL_SET_ERR_MSG(extack, "Handle cannot be zero");
                        return -EINVAL;
+               }
                q = qdisc_lookup(dev, tcm->tcm_handle);
        }
 
        /* Change qdisc parameters */
-       if (!q)
+       if (!q) {
+               NL_SET_ERR_MSG(extack, "No qdisc available");
                return -ENOENT;
-       if (n->nlmsg_flags & NLM_F_EXCL)
+       }
+       if (n->nlmsg_flags & NLM_F_EXCL) {
+               NL_SET_ERR_MSG(extack, "Exclusivity flag on, cannot modify");
                return -EEXIST;
-       if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], q->ops->id))
+       }
+       if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], q->ops->id)) {
+               NL_SET_ERR_MSG(extack, "Invalid qdisc name");
                return -EINVAL;
-       err = qdisc_change(q, tca);
+       }
+       err = qdisc_change(q, tca, extack);
        if (err == 0)
                qdisc_notify(net, skb, n, clid, NULL, q);
        return err;
 
 create_n_graft:
-       if (!(n->nlmsg_flags & NLM_F_CREATE))
+       if (!(n->nlmsg_flags & NLM_F_CREATE)) {
+               NL_SET_ERR_MSG(extack, "Qdisc not found. To create specify 
NLM_F_CREATE flag");
                return -ENOENT;
+       }
        if (clid == TC_H_INGRESS) {
-               if (dev_ingress_queue(dev))
+               if (dev_ingress_queue(dev)) {
                        q = qdisc_create(dev, dev_ingress_queue(dev), p,
                                         tcm->tcm_parent, tcm->tcm_parent,
-                                        tca, &err);
-               else
+                                        tca, &err, extack);
+               } else {
+                       NL_SET_ERR_MSG(extack, "Cannot find ingress queue for 
specified netdev");
                        err = -ENOENT;
+               }
        } else {
                struct netdev_queue *dev_queue;
 
@@ -1429,7 +1514,7 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct 
nlmsghdr *n,
 
                q = qdisc_create(dev, dev_queue, p,
                                 tcm->tcm_parent, tcm->tcm_handle,
-                                tca, &err);
+                                tca, &err, extack);
        }
        if (q == NULL) {
                if (err == -EAGAIN)
@@ -1438,7 +1523,7 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct 
nlmsghdr *n,
        }
 
 graft:
-       err = qdisc_graft(dev, p, skb, n, clid, q, NULL);
+       err = qdisc_graft(dev, p, skb, n, clid, q, NULL, extack);
        if (err) {
                if (q)
                        qdisc_destroy(q);
-- 
2.11.0

Reply via email to