From patchwork Wed Dec 6 16:08:40 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexander Aring X-Patchwork-Id: 845240 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=mojatatu-com.20150623.gappssmtp.com header.i=@mojatatu-com.20150623.gappssmtp.com header.b="k1rHXd2i"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3ysNqB2ZHNz9s83 for ; Thu, 7 Dec 2017 03:09:42 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752484AbdLFQJk (ORCPT ); Wed, 6 Dec 2017 11:09:40 -0500 Received: from mail-it0-f68.google.com ([209.85.214.68]:46503 "EHLO mail-it0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752294AbdLFQJW (ORCPT ); Wed, 6 Dec 2017 11:09:22 -0500 Received: by mail-it0-f68.google.com with SMTP id t1so7727745ite.5 for ; Wed, 06 Dec 2017 08:09:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mojatatu-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=SXqO55LEt7zLLh1DItcDWs9lCNswt2mSQ1yj1JxIJSs=; b=k1rHXd2ijjCOmkD3V1gD5mqL6L0us/ZvXBPQq++5CR48H8RhCMI4fZT76PPtTZN2IT qwA0E6Dm4hhQjZkgf54iBG1/fL2h4rYG2vQMJd6XNZ3p9O7SmsOMeYXj9xnmMYdjy6m4 Wts4G9UMsJNh8t72OfE4zZhW6yEvdQxyZI5Qnq6c9kpPEF01IwXhsRTzAN4ouEHnGtgS yJRdf9AxWqmdoaUdjYHPgA+Q5oTCHaKn6bPwemZEKadKwaGfVosfeuz31P6g2aulM0wu /Z4lxVwKRUnVUkVD7LoPQPy1HqepGEAz5oj9G5ZwTH/23JgbVGXX65gtO5G0evZ659Dl QC6w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=SXqO55LEt7zLLh1DItcDWs9lCNswt2mSQ1yj1JxIJSs=; b=fcqj066ZSnPww5ztM2PbPmtJxpQ1KijbZaLWIA3LQtryIvuJ+rB6eVDDKIv7OxociP rW3NhUYUSw+d6nEGDr2gMuV8IBLhxcBy1PxjoBSVZzmip/Gk0DkSntIlU20ekNzYOi0O SX8nSSAr9PWGifc5PK3UBvSmMpZmGOa/ocms18EAPEgKAhRRcrKl0MndJEu3D696XtNn NY13uZWHBKtKVDnlcgf2FGlO6JftSyMczdFrcagsESuVgfxXaDA3wPFrsKUni/Wxpxt4 QS/DNyqWG9DIVihxiiYMgwbjbizBJHFahTegoV3iXfpsM1MDPE13UVYleIjKJfiyIKvz 59/A== X-Gm-Message-State: AKGB3mKD/5XQ0JLXve62bHq426mvA1DhIXM7u3JA6i1C/sjlGYRuIv21 +wpnz7qkoREWluThiQOVGOj87Q== X-Google-Smtp-Source: AGs4zMan+JAlPp4TFCS5a5DpQhoBo5tx8FkR6m/Ubr4279Ew5Jnwqk6D0+dE3QQvM5DX0I0wGBPsLA== X-Received: by 10.36.179.7 with SMTP id e7mr23849262itf.139.1512576562123; Wed, 06 Dec 2017 08:09:22 -0800 (PST) Received: from x220t.lan ([64.26.149.125]) by smtp.gmail.com with ESMTPSA id 143sm1499642ioo.31.2017.12.06.08.09.20 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 06 Dec 2017 08:09:21 -0800 (PST) From: Alexander Aring To: davem@davemloft.net Cc: jhs@mojatatu.com, xiyou.wangcong@gmail.com, jiri@resnulli.us, netdev@vger.kernel.org, kernel@mojatatu.com, Alexander Aring , David Ahern Subject: [PATCH net-next 1/6] net: sched: sch_api: handle generic qdisc errors Date: Wed, 6 Dec 2017 11:08:40 -0500 Message-Id: <20171206160845.6646-2-aring@mojatatu.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20171206160845.6646-1-aring@mojatatu.com> References: <20171206160845.6646-1-aring@mojatatu.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 Signed-off-by: Alexander Aring Acked-by: Jamal Hadi Salim --- 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);