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