[PATCH net-next 0/6] net: sched: sch: introduce extack support

[PATCH net-next 0/6] net: sched: sch: introduce extack support

[Alexander Aring]

Hi,

this patch series basically add support for extack in common qdisc handling.
Additional it adds extack pointer to common qdisc callback handling this
offers per qdisc implementation to setting the extack message for each
failure over netlink.

The extack message will be set deeper in qdisc functions but going not
deeper as net core api. For qdisc module callback handling, the extack
will not be set. This will be part of per qdisc extack handling.

I also want to prepare patches to handle extack per qdisc module...
so there will come a lot of more patches, just cut them down to make
it reviewable.

There are some above 80-chars width warnings, which I ignore because
it looks more ugly otherwise.

This patch-series based on patches by David Ahren which gave me some
hints how to deal with extack support.

Cc: David Ahern <dsahern@gmail.com>

- Alex

Alexander Aring (6):
  net: sched: sch_api: handle generic qdisc errors
  net: sched: sch: add extack for init callback
  net: sched: sch: add extack for change qdisc ops
  net: sched: sch: add extack to change class
  net: sched: sch: add extack for block callback
  net: sched: sch: add extack for graft callback

 include/net/sch_generic.h |  15 ++--
 net/sched/cls_api.c       |   4 +-
 net/sched/sch_api.c       | 204 ++++++++++++++++++++++++++++++++--------------
 net/sched/sch_atm.c       |   3 +-
 net/sched/sch_cbq.c       |  10 ++-
 net/sched/sch_cbs.c       |   8 +-
 net/sched/sch_choke.c     |   8 +-
 net/sched/sch_codel.c     |   8 +-
 net/sched/sch_drr.c       |  12 ++-
 net/sched/sch_dsmark.c    |  12 ++-
 net/sched/sch_fifo.c      |   5 +-
 net/sched/sch_fq.c        |   8 +-
 net/sched/sch_fq_codel.c  |  11 ++-
 net/sched/sch_generic.c   |   8 +-
 net/sched/sch_gred.c      |   6 +-
 net/sched/sch_hfsc.c      |  14 ++--
 net/sched/sch_hhf.c       |   8 +-
 net/sched/sch_htb.c       |  10 ++-
 net/sched/sch_ingress.c   |  12 ++-
 net/sched/sch_mq.c        |   5 +-
 net/sched/sch_mqprio.c    |   5 +-
 net/sched/sch_multiq.c    |  13 +--
 net/sched/sch_netem.c     |  10 ++-
 net/sched/sch_pie.c       |   8 +-
 net/sched/sch_plug.c      |   6 +-
 net/sched/sch_prio.c      |  13 +--
 net/sched/sch_qfq.c       |  12 ++-
 net/sched/sch_red.c       |  10 ++-
 net/sched/sch_sfb.c       |  16 ++--
 net/sched/sch_sfq.c       |   6 +-
 net/sched/sch_tbf.c       |  10 ++-
 net/sched/sch_teql.c      |   3 +-
 32 files changed, 321 insertions(+), 162 deletions(-)

-- 
2.11.0

[PATCH net-next 1/6] net: sched: sch_api: handle generic qdisc errors

[Alexander Aring]

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 <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@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

Re: [PATCH net-next 1/6] net: sched: sch_api: handle generic qdisc errors

[Jamal Hadi Salim]

On 17-12-06 11:08 AM, Alexander Aring wrote:
> 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 <dsahern@gmail.com>
> Signed-off-by: Alexander Aring <aring@mojatatu.com>

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

Re: [PATCH net-next 1/6] net: sched: sch_api: handle generic qdisc errors

[David Ahern]

On 12/6/17 9:08 AM, Alexander Aring wrote:
> 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");

you don't want to set extack here; it should be set in nla_parse_nested
since it is passed as an arg.


>  		return ERR_PTR(err);
> -	if (!tb[TCA_STAB_BASE])
> +	}
> +	if (!tb[TCA_STAB_BASE]) {
> +		NL_SET_ERR_MSG(extack, "Stab base is missing");

"stab base" is the name of the netlink attribute, but it does not
describe *what* is missing. The key here is returning a message that
tells an average user what they did wrong.


>  		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");

ditto here.

Looking at the code for the tc command, stab appears to be table size;
perhaps a tc author can elaborate on what STAB really means.


>  			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");

ENOMEM does not need a text message. Which memory allocation failed is
not really relevant.


>  		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");

netdev is a kernel term. 'device' is in the tc man page so a relevant
term for the message.


I think the above gives you an idea. Apply those comments to the rest of
this patch and the next ones.

Re: [PATCH net-next 1/6] net: sched: sch_api: handle generic qdisc errors

[Jamal Hadi Salim]

On 17-12-07 12:28 AM, David Ahern wrote:

>>   
>> -	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");
> 
> you don't want to set extack here; it should be set in nla_parse_nested
> since it is passed as an arg.
> 

Can you really have a generic message in nla_parse(_nested)? What
would it say?
Note: the bad attribute is saved in the bowels of nla_parsing.

-
>>   	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");
> 
> ENOMEM does not need a text message. Which memory allocation failed is
> not really relevant.
> 

YMMV.
On the one hand it is useful to distinguish which allocation
in the code path failed(if there was a bug for example).
On the other hand you could argue an alloc failure is as dramatic
as the "sky is falling" - doesnt matter what part of the globe it is 
falling at. If the cost of sending or not is the same why not include
the message?

cheers,
jamal

Re: [PATCH net-next 1/6] net: sched: sch_api: handle generic qdisc errors

[David Ahern]

On 12/7/17 5:04 AM, Jamal Hadi Salim wrote:
> On 17-12-07 12:28 AM, David Ahern wrote:
> 
>>>   -    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");
>>
>> you don't want to set extack here; it should be set in nla_parse_nested
>> since it is passed as an arg.
>>
> 
> Can you really have a generic message in nla_parse(_nested)? What
> would it say?
> Note: the bad attribute is saved in the bowels of nla_parsing.

sure, nla_parse only sets the bad attr arg. If you keep the above, then
it needs to be corrected - it is failing to parse the TCA_STAB nested
attribute.
> 
> -
>>>       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");
>>
>> ENOMEM does not need a text message. Which memory allocation failed is
>> not really relevant.
>>
> 
> YMMV.
> On the one hand it is useful to distinguish which allocation
> in the code path failed(if there was a bug for example).
> On the other hand you could argue an alloc failure is as dramatic
> as the "sky is falling" - doesnt matter what part of the globe it is
> falling at. If the cost of sending or not is the same why not include
> the message?

What value are the messages providing above and beyond the standard libc
strerror(errno)? In the case of ENOMEM, there is nothing in the user can
do to fix that particular command, so why bloat the code with extraneous
messages? Similarly with other errors like ENODEV -- if there is only 1
device in question AND the user specified the device then you do not
need to augment with an additional error message.

The value of extack is in explaining EINVAL, EOPNOTSUPP, or the
confusing inter-dependencies in command arguments. It is not a matter of
adding an extack message for every single error path; add messages that
provide value in helping the user.

Re: [PATCH net-next 1/6] net: sched: sch_api: handle generic qdisc errors

[David Miller]

From: David Ahern <dsahern@gmail.com>
Date: Thu, 7 Dec 2017 10:52:01 -0700

> What value are the messages providing above and beyond the standard libc
> strerror(errno)? In the case of ENOMEM, there is nothing in the user can
> do to fix that particular command, so why bloat the code with extraneous
> messages? Similarly with other errors like ENODEV -- if there is only 1
> device in question AND the user specified the device then you do not
> need to augment with an additional error message.

Agreed, all of this stuff is bloat and not useful in any way to the user.

[PATCH net-next 2/6] net: sched: sch: add extack for init callback

[Alexander Aring]

This patch adds extack support for init callback to prepare per-qdisc
specific changes for extack.

Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 include/net/sch_generic.h |  3 ++-
 net/sched/sch_api.c       |  2 +-
 net/sched/sch_cbq.c       |  3 ++-
 net/sched/sch_cbs.c       |  3 ++-
 net/sched/sch_choke.c     |  3 ++-
 net/sched/sch_codel.c     |  3 ++-
 net/sched/sch_drr.c       |  3 ++-
 net/sched/sch_dsmark.c    |  3 ++-
 net/sched/sch_fifo.c      | 14 ++++++++++----
 net/sched/sch_fq.c        |  3 ++-
 net/sched/sch_fq_codel.c  |  3 ++-
 net/sched/sch_generic.c   |  8 +++++---
 net/sched/sch_gred.c      |  3 ++-
 net/sched/sch_hfsc.c      |  3 ++-
 net/sched/sch_hhf.c       |  3 ++-
 net/sched/sch_htb.c       |  3 ++-
 net/sched/sch_ingress.c   |  6 ++++--
 net/sched/sch_mq.c        |  3 ++-
 net/sched/sch_mqprio.c    |  3 ++-
 net/sched/sch_multiq.c    |  3 ++-
 net/sched/sch_netem.c     |  3 ++-
 net/sched/sch_pie.c       |  3 ++-
 net/sched/sch_plug.c      |  3 ++-
 net/sched/sch_prio.c      |  3 ++-
 net/sched/sch_qfq.c       |  3 ++-
 net/sched/sch_red.c       |  3 ++-
 net/sched/sch_sfb.c       |  3 ++-
 net/sched/sch_sfq.c       |  3 ++-
 net/sched/sch_tbf.c       |  3 ++-
 net/sched/sch_teql.c      |  3 ++-
 30 files changed, 72 insertions(+), 36 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 7dd8b0b0d244..a6974a46c9da 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -186,7 +186,8 @@ struct Qdisc_ops {
 	struct sk_buff *	(*dequeue)(struct Qdisc *);
 	struct sk_buff *	(*peek)(struct Qdisc *);
 
-	int			(*init)(struct Qdisc *sch, struct nlattr *arg);
+	int			(*init)(struct Qdisc *sch, struct nlattr *arg,
+					struct netlink_ext_ack *extack);
 	void			(*reset)(struct Qdisc *);
 	void			(*destroy)(struct Qdisc *);
 	int			(*change)(struct Qdisc *sch,
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 7b52a16d2fea..c9b9136a3e30 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1083,7 +1083,7 @@ static struct Qdisc *qdisc_create(struct net_device *dev,
 	}
 
 	if (ops->init) {
-		err = ops->init(sch, tca[TCA_OPTIONS]);
+		err = ops->init(sch, tca[TCA_OPTIONS], extack);
 		if (err != 0)
 			goto err_out5;
 	}
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 525eb3a6d625..845f1ebbb217 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1132,7 +1132,8 @@ static const struct nla_policy cbq_policy[TCA_CBQ_MAX + 1] = {
 	[TCA_CBQ_POLICE]	= { .len = sizeof(struct tc_cbq_police) },
 };
 
-static int cbq_init(struct Qdisc *sch, struct nlattr *opt)
+static int cbq_init(struct Qdisc *sch, struct nlattr *opt,
+		    struct netlink_ext_ack *extack)
 {
 	struct cbq_sched_data *q = qdisc_priv(sch);
 	struct nlattr *tb[TCA_CBQ_MAX + 1];
diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c
index 7a72980c1509..d77c632a276c 100644
--- a/net/sched/sch_cbs.c
+++ b/net/sched/sch_cbs.c
@@ -291,7 +291,8 @@ static int cbs_change(struct Qdisc *sch, struct nlattr *opt)
 	return 0;
 }
 
-static int cbs_init(struct Qdisc *sch, struct nlattr *opt)
+static int cbs_init(struct Qdisc *sch, struct nlattr *opt,
+		    struct netlink_ext_ack *extack)
 {
 	struct cbs_sched_data *q = qdisc_priv(sch);
 	struct net_device *dev = qdisc_dev(sch);
diff --git a/net/sched/sch_choke.c b/net/sched/sch_choke.c
index b30a2c70bd48..f3f773ff9ac4 100644
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -428,7 +428,8 @@ static int choke_change(struct Qdisc *sch, struct nlattr *opt)
 	return 0;
 }
 
-static int choke_init(struct Qdisc *sch, struct nlattr *opt)
+static int choke_init(struct Qdisc *sch, struct nlattr *opt,
+		      struct netlink_ext_ack *extack)
 {
 	return choke_change(sch, opt);
 }
diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c
index c518a1efcb9d..7221244e7f3b 100644
--- a/net/sched/sch_codel.c
+++ b/net/sched/sch_codel.c
@@ -184,7 +184,8 @@ static int codel_change(struct Qdisc *sch, struct nlattr *opt)
 	return 0;
 }
 
-static int codel_init(struct Qdisc *sch, struct nlattr *opt)
+static int codel_init(struct Qdisc *sch, struct nlattr *opt,
+		      struct netlink_ext_ack *extack)
 {
 	struct codel_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index 5bbcef3dcd8c..1a88473cd768 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -408,7 +408,8 @@ static struct sk_buff *drr_dequeue(struct Qdisc *sch)
 	return NULL;
 }
 
-static int drr_init_qdisc(struct Qdisc *sch, struct nlattr *opt)
+static int drr_init_qdisc(struct Qdisc *sch, struct nlattr *opt,
+			  struct netlink_ext_ack *extack)
 {
 	struct drr_sched *q = qdisc_priv(sch);
 	int err;
diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
index fb4fb71c68cf..16dd480b5583 100644
--- a/net/sched/sch_dsmark.c
+++ b/net/sched/sch_dsmark.c
@@ -330,7 +330,8 @@ static struct sk_buff *dsmark_peek(struct Qdisc *sch)
 	return p->q->ops->peek(p->q);
 }
 
-static int dsmark_init(struct Qdisc *sch, struct nlattr *opt)
+static int dsmark_init(struct Qdisc *sch, struct nlattr *opt,
+		       struct netlink_ext_ack *extack)
 {
 	struct dsmark_qdisc_data *p = qdisc_priv(sch);
 	struct nlattr *tb[TCA_DSMARK_MAX + 1];
diff --git a/net/sched/sch_fifo.c b/net/sched/sch_fifo.c
index 1e37247656f8..a2d1c9f9b798 100644
--- a/net/sched/sch_fifo.c
+++ b/net/sched/sch_fifo.c
@@ -55,7 +55,8 @@ static int pfifo_tail_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	return NET_XMIT_CN;
 }
 
-static int fifo_init(struct Qdisc *sch, struct nlattr *opt)
+static int fifo_init(struct Qdisc *sch, struct nlattr *opt,
+		     struct netlink_ext_ack *extack)
 {
 	bool bypass;
 	bool is_bfifo = sch->ops == &bfifo_qdisc_ops;
@@ -88,6 +89,11 @@ static int fifo_init(struct Qdisc *sch, struct nlattr *opt)
 	return 0;
 }
 
+static int fifo_change(struct Qdisc *sch, struct nlattr *opt)
+{
+	return fifo_init(sch, opt, NULL);
+}
+
 static int fifo_dump(struct Qdisc *sch, struct sk_buff *skb)
 {
 	struct tc_fifo_qopt opt = { .limit = sch->limit };
@@ -108,7 +114,7 @@ struct Qdisc_ops pfifo_qdisc_ops __read_mostly = {
 	.peek		=	qdisc_peek_head,
 	.init		=	fifo_init,
 	.reset		=	qdisc_reset_queue,
-	.change		=	fifo_init,
+	.change		=	fifo_change,
 	.dump		=	fifo_dump,
 	.owner		=	THIS_MODULE,
 };
@@ -122,7 +128,7 @@ struct Qdisc_ops bfifo_qdisc_ops __read_mostly = {
 	.peek		=	qdisc_peek_head,
 	.init		=	fifo_init,
 	.reset		=	qdisc_reset_queue,
-	.change		=	fifo_init,
+	.change		=	fifo_change,
 	.dump		=	fifo_dump,
 	.owner		=	THIS_MODULE,
 };
@@ -136,7 +142,7 @@ struct Qdisc_ops pfifo_head_drop_qdisc_ops __read_mostly = {
 	.peek		=	qdisc_peek_head,
 	.init		=	fifo_init,
 	.reset		=	qdisc_reset_queue,
-	.change		=	fifo_init,
+	.change		=	fifo_change,
 	.dump		=	fifo_dump,
 	.owner		=	THIS_MODULE,
 };
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 263d16e3219e..c9f61ffe220e 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -788,7 +788,8 @@ static void fq_destroy(struct Qdisc *sch)
 	qdisc_watchdog_cancel(&q->watchdog);
 }
 
-static int fq_init(struct Qdisc *sch, struct nlattr *opt)
+static int fq_init(struct Qdisc *sch, struct nlattr *opt,
+		   struct netlink_ext_ack *extack)
 {
 	struct fq_sched_data *q = qdisc_priv(sch);
 	int err;
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index 0305d791ea94..5d0b20898ffa 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -458,7 +458,8 @@ static void fq_codel_destroy(struct Qdisc *sch)
 	kvfree(q->flows);
 }
 
-static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt)
+static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt,
+			 struct netlink_ext_ack *extack)
 {
 	struct fq_codel_sched_data *q = qdisc_priv(sch);
 	int i;
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 3839cbbdc32b..0f26a6026988 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -434,7 +434,8 @@ struct Qdisc noop_qdisc = {
 };
 EXPORT_SYMBOL(noop_qdisc);
 
-static int noqueue_init(struct Qdisc *qdisc, struct nlattr *opt)
+static int noqueue_init(struct Qdisc *qdisc, struct nlattr *opt,
+			struct netlink_ext_ack *extack)
 {
 	/* register_qdisc() assigns a default of noop_enqueue if unset,
 	 * but __dev_queue_xmit() treats noqueue only as such
@@ -567,7 +568,8 @@ static int pfifo_fast_dump(struct Qdisc *qdisc, struct sk_buff *skb)
 	return -1;
 }
 
-static int pfifo_fast_init(struct Qdisc *qdisc, struct nlattr *opt)
+static int pfifo_fast_init(struct Qdisc *qdisc, struct nlattr *opt,
+			   struct netlink_ext_ack *extack)
 {
 	int prio;
 	struct pfifo_fast_priv *priv = qdisc_priv(qdisc);
@@ -666,7 +668,7 @@ struct Qdisc *qdisc_create_dflt(struct netdev_queue *dev_queue,
 	}
 	sch->parent = parentid;
 
-	if (!ops->init || ops->init(sch, NULL) == 0)
+	if (!ops->init || ops->init(sch, NULL, NULL) == 0)
 		return sch;
 
 	qdisc_destroy(sch);
diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c
index 17c7130454bd..be22b1f4ee12 100644
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -462,7 +462,8 @@ static int gred_change(struct Qdisc *sch, struct nlattr *opt)
 	return err;
 }
 
-static int gred_init(struct Qdisc *sch, struct nlattr *opt)
+static int gred_init(struct Qdisc *sch, struct nlattr *opt,
+		     struct netlink_ext_ack *extack)
 {
 	struct nlattr *tb[TCA_GRED_MAX + 1];
 	int err;
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index d04068a97d81..eca5e34ceb6f 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1388,7 +1388,8 @@ hfsc_schedule_watchdog(struct Qdisc *sch)
 }
 
 static int
-hfsc_init_qdisc(struct Qdisc *sch, struct nlattr *opt)
+hfsc_init_qdisc(struct Qdisc *sch, struct nlattr *opt,
+		struct netlink_ext_ack *extack)
 {
 	struct hfsc_sched *q = qdisc_priv(sch);
 	struct tc_hfsc_qopt *qopt;
diff --git a/net/sched/sch_hhf.c b/net/sched/sch_hhf.c
index 73a53c08091b..b3a80f0ed4b0 100644
--- a/net/sched/sch_hhf.c
+++ b/net/sched/sch_hhf.c
@@ -571,7 +571,8 @@ static int hhf_change(struct Qdisc *sch, struct nlattr *opt)
 	return 0;
 }
 
-static int hhf_init(struct Qdisc *sch, struct nlattr *opt)
+static int hhf_init(struct Qdisc *sch, struct nlattr *opt,
+		    struct netlink_ext_ack *extack)
 {
 	struct hhf_sched_data *q = qdisc_priv(sch);
 	int i;
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index fa0380730ff0..41d9b7da9273 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1017,7 +1017,8 @@ static void htb_work_func(struct work_struct *work)
 	rcu_read_unlock();
 }
 
-static int htb_init(struct Qdisc *sch, struct nlattr *opt)
+static int htb_init(struct Qdisc *sch, struct nlattr *opt,
+		    struct netlink_ext_ack *extack)
 {
 	struct htb_sched *q = qdisc_priv(sch);
 	struct nlattr *tb[TCA_HTB_MAX + 1];
diff --git a/net/sched/sch_ingress.c b/net/sched/sch_ingress.c
index 5ecc38f35d47..c703cf3a0bed 100644
--- a/net/sched/sch_ingress.c
+++ b/net/sched/sch_ingress.c
@@ -62,7 +62,8 @@ static void clsact_chain_head_change(struct tcf_proto *tp_head, void *priv)
 	mini_qdisc_pair_swap(miniqp, tp_head);
 }
 
-static int ingress_init(struct Qdisc *sch, struct nlattr *opt)
+static int ingress_init(struct Qdisc *sch, struct nlattr *opt,
+			struct netlink_ext_ack *extack)
 {
 	struct ingress_sched_data *q = qdisc_priv(sch);
 	struct net_device *dev = qdisc_dev(sch);
@@ -166,7 +167,8 @@ static struct tcf_block *clsact_tcf_block(struct Qdisc *sch, unsigned long cl)
 	}
 }
 
-static int clsact_init(struct Qdisc *sch, struct nlattr *opt)
+static int clsact_init(struct Qdisc *sch, struct nlattr *opt,
+		       struct netlink_ext_ack *extack)
 {
 	struct clsact_sched_data *q = qdisc_priv(sch);
 	struct net_device *dev = qdisc_dev(sch);
diff --git a/net/sched/sch_mq.c b/net/sched/sch_mq.c
index 213b586a06a0..c4f160495366 100644
--- a/net/sched/sch_mq.c
+++ b/net/sched/sch_mq.c
@@ -35,7 +35,8 @@ static void mq_destroy(struct Qdisc *sch)
 	kfree(priv->qdiscs);
 }
 
-static int mq_init(struct Qdisc *sch, struct nlattr *opt)
+static int mq_init(struct Qdisc *sch, struct nlattr *opt,
+		   struct netlink_ext_ack *extack)
 {
 	struct net_device *dev = qdisc_dev(sch);
 	struct mq_sched *priv = qdisc_priv(sch);
diff --git a/net/sched/sch_mqprio.c b/net/sched/sch_mqprio.c
index b85885a9d8a1..9355c4a4686c 100644
--- a/net/sched/sch_mqprio.c
+++ b/net/sched/sch_mqprio.c
@@ -132,7 +132,8 @@ static int parse_attr(struct nlattr *tb[], int maxtype, struct nlattr *nla,
 	return 0;
 }
 
-static int mqprio_init(struct Qdisc *sch, struct nlattr *opt)
+static int mqprio_init(struct Qdisc *sch, struct nlattr *opt,
+		       struct netlink_ext_ack *extack)
 {
 	struct net_device *dev = qdisc_dev(sch);
 	struct mqprio_sched *priv = qdisc_priv(sch);
diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c
index 012216386c0b..7ed50e653d1a 100644
--- a/net/sched/sch_multiq.c
+++ b/net/sched/sch_multiq.c
@@ -236,7 +236,8 @@ static int multiq_tune(struct Qdisc *sch, struct nlattr *opt)
 	return 0;
 }
 
-static int multiq_init(struct Qdisc *sch, struct nlattr *opt)
+static int multiq_init(struct Qdisc *sch, struct nlattr *opt,
+		       struct netlink_ext_ack *extack)
 {
 	struct multiq_sched_data *q = qdisc_priv(sch);
 	int i, err;
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index dd70924cbcdf..6490ce08d29e 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -984,7 +984,8 @@ static int netem_change(struct Qdisc *sch, struct nlattr *opt)
 	return ret;
 }
 
-static int netem_init(struct Qdisc *sch, struct nlattr *opt)
+static int netem_init(struct Qdisc *sch, struct nlattr *opt,
+		      struct netlink_ext_ack *extack)
 {
 	struct netem_sched_data *q = qdisc_priv(sch);
 	int ret;
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 776c694c77c7..c4c87ed3971f 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -439,7 +439,8 @@ static void pie_timer(struct timer_list *t)
 
 }
 
-static int pie_init(struct Qdisc *sch, struct nlattr *opt)
+static int pie_init(struct Qdisc *sch, struct nlattr *opt,
+		    struct netlink_ext_ack *extack)
 {
 	struct pie_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_plug.c b/net/sched/sch_plug.c
index 1c6cbab3e7b9..d9c6fbe55ae5 100644
--- a/net/sched/sch_plug.c
+++ b/net/sched/sch_plug.c
@@ -123,7 +123,8 @@ static struct sk_buff *plug_dequeue(struct Qdisc *sch)
 	return qdisc_dequeue_head(sch);
 }
 
-static int plug_init(struct Qdisc *sch, struct nlattr *opt)
+static int plug_init(struct Qdisc *sch, struct nlattr *opt,
+		     struct netlink_ext_ack *extack)
 {
 	struct plug_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
index 2c79559a0d31..8632d795e6ee 100644
--- a/net/sched/sch_prio.c
+++ b/net/sched/sch_prio.c
@@ -205,7 +205,8 @@ static int prio_tune(struct Qdisc *sch, struct nlattr *opt)
 	return 0;
 }
 
-static int prio_init(struct Qdisc *sch, struct nlattr *opt)
+static int prio_init(struct Qdisc *sch, struct nlattr *opt,
+		     struct netlink_ext_ack *extack)
 {
 	struct prio_sched_data *q = qdisc_priv(sch);
 	int err;
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 6962b37a3ad3..7c1b976314bd 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -1413,7 +1413,8 @@ static void qfq_qlen_notify(struct Qdisc *sch, unsigned long arg)
 	qfq_deactivate_class(q, cl);
 }
 
-static int qfq_init_qdisc(struct Qdisc *sch, struct nlattr *opt)
+static int qfq_init_qdisc(struct Qdisc *sch, struct nlattr *opt,
+			  struct netlink_ext_ack *extack)
 {
 	struct qfq_sched *q = qdisc_priv(sch);
 	struct qfq_group *grp;
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index 7f8ea9e297c3..bf1b72b8f0de 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -262,7 +262,8 @@ static inline void red_adaptative_timer(struct timer_list *t)
 	spin_unlock(root_lock);
 }
 
-static int red_init(struct Qdisc *sch, struct nlattr *opt)
+static int red_init(struct Qdisc *sch, struct nlattr *opt,
+		    struct netlink_ext_ack *extack)
 {
 	struct red_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c
index 0678debdd856..b2205eaa0f51 100644
--- a/net/sched/sch_sfb.c
+++ b/net/sched/sch_sfb.c
@@ -549,7 +549,8 @@ static int sfb_change(struct Qdisc *sch, struct nlattr *opt)
 	return 0;
 }
 
-static int sfb_init(struct Qdisc *sch, struct nlattr *opt)
+static int sfb_init(struct Qdisc *sch, struct nlattr *opt,
+		    struct netlink_ext_ack *extack)
 {
 	struct sfb_sched_data *q = qdisc_priv(sch);
 	int err;
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 09c1203c1711..0af813ed67c2 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -718,7 +718,8 @@ static void sfq_destroy(struct Qdisc *sch)
 	kfree(q->red_parms);
 }
 
-static int sfq_init(struct Qdisc *sch, struct nlattr *opt)
+static int sfq_init(struct Qdisc *sch, struct nlattr *opt,
+		    struct netlink_ext_ack *extack)
 {
 	struct sfq_sched_data *q = qdisc_priv(sch);
 	int i;
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 120f4f365967..6fbfce047d4c 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -421,7 +421,8 @@ static int tbf_change(struct Qdisc *sch, struct nlattr *opt)
 	return err;
 }
 
-static int tbf_init(struct Qdisc *sch, struct nlattr *opt)
+static int tbf_init(struct Qdisc *sch, struct nlattr *opt,
+		    struct netlink_ext_ack *extack)
 {
 	struct tbf_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index 9fe6b427afed..93f04cf5cac1 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -167,7 +167,8 @@ teql_destroy(struct Qdisc *sch)
 	}
 }
 
-static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt)
+static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt,
+			   struct netlink_ext_ack *extack)
 {
 	struct net_device *dev = qdisc_dev(sch);
 	struct teql_master *m = (struct teql_master *)sch->ops;
-- 
2.11.0

Re: [PATCH net-next 2/6] net: sched: sch: add extack for init callback

[Jamal Hadi Salim]

On 17-12-06 11:08 AM, Alexander Aring wrote:
> This patch adds extack support for init callback to prepare per-qdisc
> specific changes for extack.
> 
> Cc: David Ahern<dsahern@gmail.com>
> Signed-off-by: Alexander Aring<aring@mojatatu.com>

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

[PATCH net-next 3/6] net: sched: sch: add extack for change qdisc ops

[Alexander Aring]

This patch adds extack support for change callback for qdisc ops
structtur to prepare per-qdisc specific changes for extack.

Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 include/net/sch_generic.h |  3 ++-
 net/sched/sch_api.c       |  2 +-
 net/sched/sch_cbs.c       |  5 +++--
 net/sched/sch_choke.c     |  5 +++--
 net/sched/sch_codel.c     |  5 +++--
 net/sched/sch_fifo.c      | 13 ++++---------
 net/sched/sch_fq.c        |  5 +++--
 net/sched/sch_fq_codel.c  |  5 +++--
 net/sched/sch_gred.c      |  3 ++-
 net/sched/sch_hfsc.c      |  3 ++-
 net/sched/sch_hhf.c       |  5 +++--
 net/sched/sch_multiq.c    |  5 +++--
 net/sched/sch_netem.c     |  5 +++--
 net/sched/sch_pie.c       |  5 +++--
 net/sched/sch_plug.c      |  3 ++-
 net/sched/sch_prio.c      |  5 +++--
 net/sched/sch_red.c       |  5 +++--
 net/sched/sch_sfb.c       |  5 +++--
 net/sched/sch_tbf.c       |  5 +++--
 19 files changed, 52 insertions(+), 40 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index a6974a46c9da..f7b831ff1e94 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -191,7 +191,8 @@ struct Qdisc_ops {
 	void			(*reset)(struct Qdisc *);
 	void			(*destroy)(struct Qdisc *);
 	int			(*change)(struct Qdisc *sch,
-					  struct nlattr *arg);
+					  struct nlattr *arg,
+					  struct netlink_ext_ack *extack);
 	void			(*attach)(struct Qdisc *sch);
 
 	int			(*dump)(struct Qdisc *, struct sk_buff *);
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index c9b9136a3e30..3e13e7a24221 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1180,7 +1180,7 @@ static int qdisc_change(struct Qdisc *sch, struct nlattr **tca,
 			NL_SET_ERR_MSG(extack, "Change operation not supported by qdisc");
 			return -EINVAL;
 		}
-		err = sch->ops->change(sch, tca[TCA_OPTIONS]);
+		err = sch->ops->change(sch, tca[TCA_OPTIONS], extack);
 		if (err)
 			return err;
 	}
diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c
index d77c632a276c..8bf6e163d29c 100644
--- a/net/sched/sch_cbs.c
+++ b/net/sched/sch_cbs.c
@@ -246,7 +246,8 @@ static int cbs_enable_offload(struct net_device *dev, struct cbs_sched_data *q,
 	return 0;
 }
 
-static int cbs_change(struct Qdisc *sch, struct nlattr *opt)
+static int cbs_change(struct Qdisc *sch, struct nlattr *opt,
+		      struct netlink_ext_ack *extack)
 {
 	struct cbs_sched_data *q = qdisc_priv(sch);
 	struct net_device *dev = qdisc_dev(sch);
@@ -307,7 +308,7 @@ static int cbs_init(struct Qdisc *sch, struct nlattr *opt,
 
 	qdisc_watchdog_init(&q->watchdog, sch);
 
-	return cbs_change(sch, opt);
+	return cbs_change(sch, opt, extack);
 }
 
 static void cbs_destroy(struct Qdisc *sch)
diff --git a/net/sched/sch_choke.c b/net/sched/sch_choke.c
index f3f773ff9ac4..9a93e82c6126 100644
--- a/net/sched/sch_choke.c
+++ b/net/sched/sch_choke.c
@@ -344,7 +344,8 @@ static void choke_free(void *addr)
 	kvfree(addr);
 }
 
-static int choke_change(struct Qdisc *sch, struct nlattr *opt)
+static int choke_change(struct Qdisc *sch, struct nlattr *opt,
+			struct netlink_ext_ack *extack)
 {
 	struct choke_sched_data *q = qdisc_priv(sch);
 	struct nlattr *tb[TCA_CHOKE_MAX + 1];
@@ -431,7 +432,7 @@ static int choke_change(struct Qdisc *sch, struct nlattr *opt)
 static int choke_init(struct Qdisc *sch, struct nlattr *opt,
 		      struct netlink_ext_ack *extack)
 {
-	return choke_change(sch, opt);
+	return choke_change(sch, opt, extack);
 }
 
 static int choke_dump(struct Qdisc *sch, struct sk_buff *skb)
diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c
index 7221244e7f3b..17cd81f84b5d 100644
--- a/net/sched/sch_codel.c
+++ b/net/sched/sch_codel.c
@@ -130,7 +130,8 @@ static const struct nla_policy codel_policy[TCA_CODEL_MAX + 1] = {
 	[TCA_CODEL_CE_THRESHOLD]= { .type = NLA_U32 },
 };
 
-static int codel_change(struct Qdisc *sch, struct nlattr *opt)
+static int codel_change(struct Qdisc *sch, struct nlattr *opt,
+			struct netlink_ext_ack *extack)
 {
 	struct codel_sched_data *q = qdisc_priv(sch);
 	struct nlattr *tb[TCA_CODEL_MAX + 1];
@@ -197,7 +198,7 @@ static int codel_init(struct Qdisc *sch, struct nlattr *opt,
 	q->params.mtu = psched_mtu(qdisc_dev(sch));
 
 	if (opt) {
-		int err = codel_change(sch, opt);
+		int err = codel_change(sch, opt, extack);
 
 		if (err)
 			return err;
diff --git a/net/sched/sch_fifo.c b/net/sched/sch_fifo.c
index a2d1c9f9b798..c65f23c70f40 100644
--- a/net/sched/sch_fifo.c
+++ b/net/sched/sch_fifo.c
@@ -89,11 +89,6 @@ static int fifo_init(struct Qdisc *sch, struct nlattr *opt,
 	return 0;
 }
 
-static int fifo_change(struct Qdisc *sch, struct nlattr *opt)
-{
-	return fifo_init(sch, opt, NULL);
-}
-
 static int fifo_dump(struct Qdisc *sch, struct sk_buff *skb)
 {
 	struct tc_fifo_qopt opt = { .limit = sch->limit };
@@ -114,7 +109,7 @@ struct Qdisc_ops pfifo_qdisc_ops __read_mostly = {
 	.peek		=	qdisc_peek_head,
 	.init		=	fifo_init,
 	.reset		=	qdisc_reset_queue,
-	.change		=	fifo_change,
+	.change		=	fifo_init,
 	.dump		=	fifo_dump,
 	.owner		=	THIS_MODULE,
 };
@@ -128,7 +123,7 @@ struct Qdisc_ops bfifo_qdisc_ops __read_mostly = {
 	.peek		=	qdisc_peek_head,
 	.init		=	fifo_init,
 	.reset		=	qdisc_reset_queue,
-	.change		=	fifo_change,
+	.change		=	fifo_init,
 	.dump		=	fifo_dump,
 	.owner		=	THIS_MODULE,
 };
@@ -142,7 +137,7 @@ struct Qdisc_ops pfifo_head_drop_qdisc_ops __read_mostly = {
 	.peek		=	qdisc_peek_head,
 	.init		=	fifo_init,
 	.reset		=	qdisc_reset_queue,
-	.change		=	fifo_change,
+	.change		=	fifo_init,
 	.dump		=	fifo_dump,
 	.owner		=	THIS_MODULE,
 };
@@ -163,7 +158,7 @@ int fifo_set_limit(struct Qdisc *q, unsigned int limit)
 		nla->nla_len = nla_attr_size(sizeof(struct tc_fifo_qopt));
 		((struct tc_fifo_qopt *)nla_data(nla))->limit = limit;
 
-		ret = q->ops->change(q, nla);
+		ret = q->ops->change(q, nla, NULL);
 		kfree(nla);
 	}
 	return ret;
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index c9f61ffe220e..a366e4c9413a 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -685,7 +685,8 @@ static const struct nla_policy fq_policy[TCA_FQ_MAX + 1] = {
 	[TCA_FQ_LOW_RATE_THRESHOLD]	= { .type = NLA_U32 },
 };
 
-static int fq_change(struct Qdisc *sch, struct nlattr *opt)
+static int fq_change(struct Qdisc *sch, struct nlattr *opt,
+		     struct netlink_ext_ack *extack)
 {
 	struct fq_sched_data *q = qdisc_priv(sch);
 	struct nlattr *tb[TCA_FQ_MAX + 1];
@@ -812,7 +813,7 @@ static int fq_init(struct Qdisc *sch, struct nlattr *opt,
 	qdisc_watchdog_init(&q->watchdog, sch);
 
 	if (opt)
-		err = fq_change(sch, opt);
+		err = fq_change(sch, opt, extack);
 	else
 		err = fq_resize(sch, q->fq_trees_log);
 
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index 5d0b20898ffa..d798c93f7c96 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -377,7 +377,8 @@ static const struct nla_policy fq_codel_policy[TCA_FQ_CODEL_MAX + 1] = {
 	[TCA_FQ_CODEL_MEMORY_LIMIT] = { .type = NLA_U32 },
 };
 
-static int fq_codel_change(struct Qdisc *sch, struct nlattr *opt)
+static int fq_codel_change(struct Qdisc *sch, struct nlattr *opt,
+			   struct netlink_ext_ack *extack)
 {
 	struct fq_codel_sched_data *q = qdisc_priv(sch);
 	struct nlattr *tb[TCA_FQ_CODEL_MAX + 1];
@@ -478,7 +479,7 @@ static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt,
 	q->cparams.mtu = psched_mtu(qdisc_dev(sch));
 
 	if (opt) {
-		int err = fq_codel_change(sch, opt);
+		int err = fq_codel_change(sch, opt, NULL);
 		if (err)
 			return err;
 	}
diff --git a/net/sched/sch_gred.c b/net/sched/sch_gred.c
index be22b1f4ee12..c3bc82197b63 100644
--- a/net/sched/sch_gred.c
+++ b/net/sched/sch_gred.c
@@ -388,7 +388,8 @@ static const struct nla_policy gred_policy[TCA_GRED_MAX + 1] = {
 	[TCA_GRED_LIMIT]	= { .type = NLA_U32 },
 };
 
-static int gred_change(struct Qdisc *sch, struct nlattr *opt)
+static int gred_change(struct Qdisc *sch, struct nlattr *opt,
+		       struct netlink_ext_ack *extack)
 {
 	struct gred_sched *table = qdisc_priv(sch);
 	struct tc_gred_qopt *ctl;
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index eca5e34ceb6f..ed73ad6c4aaf 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1430,7 +1430,8 @@ hfsc_init_qdisc(struct Qdisc *sch, struct nlattr *opt,
 }
 
 static int
-hfsc_change_qdisc(struct Qdisc *sch, struct nlattr *opt)
+hfsc_change_qdisc(struct Qdisc *sch, struct nlattr *opt,
+		  struct netlink_ext_ack *extack)
 {
 	struct hfsc_sched *q = qdisc_priv(sch);
 	struct tc_hfsc_qopt *qopt;
diff --git a/net/sched/sch_hhf.c b/net/sched/sch_hhf.c
index b3a80f0ed4b0..bce2632212d3 100644
--- a/net/sched/sch_hhf.c
+++ b/net/sched/sch_hhf.c
@@ -504,7 +504,8 @@ static const struct nla_policy hhf_policy[TCA_HHF_MAX + 1] = {
 	[TCA_HHF_NON_HH_WEIGHT]	 = { .type = NLA_U32 },
 };
 
-static int hhf_change(struct Qdisc *sch, struct nlattr *opt)
+static int hhf_change(struct Qdisc *sch, struct nlattr *opt,
+		      struct netlink_ext_ack *extack)
 {
 	struct hhf_sched_data *q = qdisc_priv(sch);
 	struct nlattr *tb[TCA_HHF_MAX + 1];
@@ -590,7 +591,7 @@ static int hhf_init(struct Qdisc *sch, struct nlattr *opt,
 	q->hhf_non_hh_weight = 2;
 
 	if (opt) {
-		int err = hhf_change(sch, opt);
+		int err = hhf_change(sch, opt, extack);
 
 		if (err)
 			return err;
diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c
index 7ed50e653d1a..fbd15b5524bb 100644
--- a/net/sched/sch_multiq.c
+++ b/net/sched/sch_multiq.c
@@ -180,7 +180,8 @@ multiq_destroy(struct Qdisc *sch)
 	kfree(q->queues);
 }
 
-static int multiq_tune(struct Qdisc *sch, struct nlattr *opt)
+static int multiq_tune(struct Qdisc *sch, struct nlattr *opt,
+		       struct netlink_ext_ack *extack)
 {
 	struct multiq_sched_data *q = qdisc_priv(sch);
 	struct tc_multiq_qopt *qopt;
@@ -259,7 +260,7 @@ static int multiq_init(struct Qdisc *sch, struct nlattr *opt,
 	for (i = 0; i < q->max_bands; i++)
 		q->queues[i] = &noop_qdisc;
 
-	return multiq_tune(sch, opt);
+	return multiq_tune(sch, opt, extack);
 }
 
 static int multiq_dump(struct Qdisc *sch, struct sk_buff *skb)
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 6490ce08d29e..f45040b55531 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -893,7 +893,8 @@ static int parse_attr(struct nlattr *tb[], int maxtype, struct nlattr *nla,
 }
 
 /* Parse netlink message to set options */
-static int netem_change(struct Qdisc *sch, struct nlattr *opt)
+static int netem_change(struct Qdisc *sch, struct nlattr *opt,
+			struct netlink_ext_ack *extack)
 {
 	struct netem_sched_data *q = qdisc_priv(sch);
 	struct nlattr *tb[TCA_NETEM_MAX + 1];
@@ -996,7 +997,7 @@ static int netem_init(struct Qdisc *sch, struct nlattr *opt,
 		return -EINVAL;
 
 	q->loss_model = CLG_RANDOM;
-	ret = netem_change(sch, opt);
+	ret = netem_change(sch, opt, extack);
 	if (ret)
 		pr_info("netem: change failed\n");
 	return ret;
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index c4c87ed3971f..18d30bb86881 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -181,7 +181,8 @@ static const struct nla_policy pie_policy[TCA_PIE_MAX + 1] = {
 	[TCA_PIE_BYTEMODE] = {.type = NLA_U32},
 };
 
-static int pie_change(struct Qdisc *sch, struct nlattr *opt)
+static int pie_change(struct Qdisc *sch, struct nlattr *opt,
+		      struct netlink_ext_ack *extack)
 {
 	struct pie_sched_data *q = qdisc_priv(sch);
 	struct nlattr *tb[TCA_PIE_MAX + 1];
@@ -452,7 +453,7 @@ static int pie_init(struct Qdisc *sch, struct nlattr *opt,
 	timer_setup(&q->adapt_timer, pie_timer, 0);
 
 	if (opt) {
-		int err = pie_change(sch, opt);
+		int err = pie_change(sch, opt, extack);
 
 		if (err)
 			return err;
diff --git a/net/sched/sch_plug.c b/net/sched/sch_plug.c
index d9c6fbe55ae5..5619d2eb17b6 100644
--- a/net/sched/sch_plug.c
+++ b/net/sched/sch_plug.c
@@ -159,7 +159,8 @@ static int plug_init(struct Qdisc *sch, struct nlattr *opt,
  *   command is received (just act as a pass-thru queue).
  * TCQ_PLUG_LIMIT: Increase/decrease queue size
  */
-static int plug_change(struct Qdisc *sch, struct nlattr *opt)
+static int plug_change(struct Qdisc *sch, struct nlattr *opt,
+		       struct netlink_ext_ack *extack)
 {
 	struct plug_sched_data *q = qdisc_priv(sch);
 	struct tc_plug_qopt *msg;
diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
index 8632d795e6ee..5f8ecbaa2610 100644
--- a/net/sched/sch_prio.c
+++ b/net/sched/sch_prio.c
@@ -153,7 +153,8 @@ prio_destroy(struct Qdisc *sch)
 		qdisc_destroy(q->queues[prio]);
 }
 
-static int prio_tune(struct Qdisc *sch, struct nlattr *opt)
+static int prio_tune(struct Qdisc *sch, struct nlattr *opt,
+		     struct netlink_ext_ack *extack)
 {
 	struct prio_sched_data *q = qdisc_priv(sch);
 	struct Qdisc *queues[TCQ_PRIO_BANDS];
@@ -218,7 +219,7 @@ static int prio_init(struct Qdisc *sch, struct nlattr *opt,
 	if (err)
 		return err;
 
-	return prio_tune(sch, opt);
+	return prio_tune(sch, opt, extack);
 }
 
 static int prio_dump(struct Qdisc *sch, struct sk_buff *skb)
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index bf1b72b8f0de..180bdd206b99 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -189,7 +189,8 @@ static const struct nla_policy red_policy[TCA_RED_MAX + 1] = {
 	[TCA_RED_MAX_P] = { .type = NLA_U32 },
 };
 
-static int red_change(struct Qdisc *sch, struct nlattr *opt)
+static int red_change(struct Qdisc *sch, struct nlattr *opt,
+		      struct netlink_ext_ack *extack)
 {
 	struct red_sched_data *q = qdisc_priv(sch);
 	struct nlattr *tb[TCA_RED_MAX + 1];
@@ -270,7 +271,7 @@ static int red_init(struct Qdisc *sch, struct nlattr *opt,
 	q->qdisc = &noop_qdisc;
 	q->sch = sch;
 	timer_setup(&q->adapt_timer, red_adaptative_timer, 0);
-	return red_change(sch, opt);
+	return red_change(sch, opt, extack);
 }
 
 static int red_dump_offload(struct Qdisc *sch, struct tc_red_qopt *opt)
diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c
index b2205eaa0f51..1b9d69bd6ed6 100644
--- a/net/sched/sch_sfb.c
+++ b/net/sched/sch_sfb.c
@@ -488,7 +488,8 @@ static const struct tc_sfb_qopt sfb_default_ops = {
 	.penalty_burst = 20,
 };
 
-static int sfb_change(struct Qdisc *sch, struct nlattr *opt)
+static int sfb_change(struct Qdisc *sch, struct nlattr *opt,
+		      struct netlink_ext_ack *extack)
 {
 	struct sfb_sched_data *q = qdisc_priv(sch);
 	struct Qdisc *child;
@@ -560,7 +561,7 @@ static int sfb_init(struct Qdisc *sch, struct nlattr *opt,
 		return err;
 
 	q->qdisc = &noop_qdisc;
-	return sfb_change(sch, opt);
+	return sfb_change(sch, opt, extack);
 }
 
 static int sfb_dump(struct Qdisc *sch, struct sk_buff *skb)
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 6fbfce047d4c..49002653e9dd 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -302,7 +302,8 @@ static const struct nla_policy tbf_policy[TCA_TBF_MAX + 1] = {
 	[TCA_TBF_PBURST] = { .type = NLA_U32 },
 };
 
-static int tbf_change(struct Qdisc *sch, struct nlattr *opt)
+static int tbf_change(struct Qdisc *sch, struct nlattr *opt,
+		      struct netlink_ext_ack *extack)
 {
 	int err;
 	struct tbf_sched_data *q = qdisc_priv(sch);
@@ -434,7 +435,7 @@ static int tbf_init(struct Qdisc *sch, struct nlattr *opt,
 
 	q->t_c = ktime_get_ns();
 
-	return tbf_change(sch, opt);
+	return tbf_change(sch, opt, extack);
 }
 
 static void tbf_destroy(struct Qdisc *sch)
-- 
2.11.0

Re: [PATCH net-next 3/6] net: sched: sch: add extack for change qdisc ops

[Jamal Hadi Salim]

On 17-12-06 11:08 AM, Alexander Aring wrote:
> This patch adds extack support for change callback for qdisc ops
> structtur to prepare per-qdisc specific changes for extack.
> 
> Cc: David Ahern<dsahern@gmail.com>
> Signed-off-by: Alexander Aring<aring@mojatatu.com>


Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>


cheers,
jamal

[PATCH net-next 4/6] net: sched: sch: add extack to change class

[Alexander Aring]

This patch adds extack support for class change callback api. This prepares
to handle extack support inside each specific class implementation.

Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 include/net/sch_generic.h | 3 ++-
 net/sched/sch_api.c       | 2 +-
 net/sched/sch_atm.c       | 3 ++-
 net/sched/sch_cbq.c       | 2 +-
 net/sched/sch_drr.c       | 3 ++-
 net/sched/sch_dsmark.c    | 3 ++-
 net/sched/sch_hfsc.c      | 3 ++-
 net/sched/sch_htb.c       | 2 +-
 net/sched/sch_qfq.c       | 3 ++-
 net/sched/sch_sfb.c       | 3 ++-
 10 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index f7b831ff1e94..d5a545a9d13e 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -156,7 +156,8 @@ struct Qdisc_class_ops {
 	/* Class manipulation routines */
 	unsigned long		(*find)(struct Qdisc *, u32 classid);
 	int			(*change)(struct Qdisc *, u32, u32,
-					struct nlattr **, unsigned long *);
+					struct nlattr **, unsigned long *,
+					struct netlink_ext_ack *);
 	int			(*delete)(struct Qdisc *, unsigned long);
 	void			(*walk)(struct Qdisc *, struct qdisc_walker * arg);
 
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 3e13e7a24221..87c974d2d576 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1922,7 +1922,7 @@ static int tc_ctl_tclass(struct sk_buff *skb, struct nlmsghdr *n,
 	new_cl = cl;
 	err = -EOPNOTSUPP;
 	if (cops->change)
-		err = cops->change(q, clid, portid, tca, &new_cl);
+		err = cops->change(q, clid, portid, tca, &new_cl, extack);
 	if (err == 0) {
 		tclass_notify(net, skb, n, q, new_cl, RTM_NEWTCLASS);
 		/* We just create a new class, need to do reverse binding. */
diff --git a/net/sched/sch_atm.c b/net/sched/sch_atm.c
index 2dbd249c0b2f..07bffd1b537c 100644
--- a/net/sched/sch_atm.c
+++ b/net/sched/sch_atm.c
@@ -191,7 +191,8 @@ static const struct nla_policy atm_policy[TCA_ATM_MAX + 1] = {
 };
 
 static int atm_tc_change(struct Qdisc *sch, u32 classid, u32 parent,
-			 struct nlattr **tca, unsigned long *arg)
+			 struct nlattr **tca, unsigned long *arg,
+			 struct netlink_ext_ack *extack)
 {
 	struct atm_qdisc_data *p = qdisc_priv(sch);
 	struct atm_flow_data *flow = (struct atm_flow_data *)*arg;
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 845f1ebbb217..55cb7c035b19 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1451,7 +1451,7 @@ static void cbq_destroy(struct Qdisc *sch)
 
 static int
 cbq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, struct nlattr **tca,
-		 unsigned long *arg)
+		 unsigned long *arg, struct netlink_ext_ack *extack)
 {
 	int err;
 	struct cbq_sched_data *q = qdisc_priv(sch);
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index 1a88473cd768..73b914bc47a4 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -64,7 +64,8 @@ static const struct nla_policy drr_policy[TCA_DRR_MAX + 1] = {
 };
 
 static int drr_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
-			    struct nlattr **tca, unsigned long *arg)
+			    struct nlattr **tca, unsigned long *arg,
+			    struct netlink_ext_ack *extack)
 {
 	struct drr_sched *q = qdisc_priv(sch);
 	struct drr_class *cl = (struct drr_class *)*arg;
diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
index 16dd480b5583..89e433bbd590 100644
--- a/net/sched/sch_dsmark.c
+++ b/net/sched/sch_dsmark.c
@@ -112,7 +112,8 @@ static const struct nla_policy dsmark_policy[TCA_DSMARK_MAX + 1] = {
 };
 
 static int dsmark_change(struct Qdisc *sch, u32 classid, u32 parent,
-			 struct nlattr **tca, unsigned long *arg)
+			 struct nlattr **tca, unsigned long *arg,
+			 struct netlink_ext_ack *extack)
 {
 	struct dsmark_qdisc_data *p = qdisc_priv(sch);
 	struct nlattr *opt = tca[TCA_OPTIONS];
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index ed73ad6c4aaf..008524d6bb74 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -921,7 +921,8 @@ static const struct nla_policy hfsc_policy[TCA_HFSC_MAX + 1] = {
 
 static int
 hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
-		  struct nlattr **tca, unsigned long *arg)
+		  struct nlattr **tca, unsigned long *arg,
+		  struct netlink_ext_ack *extack)
 {
 	struct hfsc_sched *q = qdisc_priv(sch);
 	struct hfsc_class *cl = (struct hfsc_class *)*arg;
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 41d9b7da9273..eb535a23a69b 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1327,7 +1327,7 @@ static int htb_delete(struct Qdisc *sch, unsigned long arg)
 
 static int htb_change_class(struct Qdisc *sch, u32 classid,
 			    u32 parentid, struct nlattr **tca,
-			    unsigned long *arg)
+			    unsigned long *arg, struct netlink_ext_ack *extack)
 {
 	int err = -EINVAL;
 	struct htb_sched *q = qdisc_priv(sch);
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 7c1b976314bd..1f4a84b687d2 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -402,7 +402,8 @@ static int qfq_change_agg(struct Qdisc *sch, struct qfq_class *cl, u32 weight,
 }
 
 static int qfq_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
-			    struct nlattr **tca, unsigned long *arg)
+			    struct nlattr **tca, unsigned long *arg,
+			    struct netlink_ext_ack *extack)
 {
 	struct qfq_sched *q = qdisc_priv(sch);
 	struct qfq_class *cl = (struct qfq_class *)*arg;
diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c
index 1b9d69bd6ed6..d70d470361be 100644
--- a/net/sched/sch_sfb.c
+++ b/net/sched/sch_sfb.c
@@ -645,7 +645,8 @@ static void sfb_unbind(struct Qdisc *sch, unsigned long arg)
 }
 
 static int sfb_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
-			    struct nlattr **tca, unsigned long *arg)
+			    struct nlattr **tca, unsigned long *arg,
+			    struct netlink_ext_ack *extack)
 {
 	return -ENOSYS;
 }
-- 
2.11.0

Re: [PATCH net-next 4/6] net: sched: sch: add extack to change class

[Jamal Hadi Salim]

On 17-12-06 11:08 AM, Alexander Aring wrote:
> This patch adds extack support for class change callback api. This prepares
> to handle extack support inside each specific class implementation.
> 
> Cc: David Ahern<dsahern@gmail.com>
> Signed-off-by: Alexander Aring<aring@mojatatu.com>

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

[PATCH net-next 5/6] net: sched: sch: add extack for block callback

[Alexander Aring]

This patch adds extack support for block callback to prepare per-qdisc
specific changes for extack.

Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 include/net/sch_generic.h | 3 ++-
 net/sched/cls_api.c       | 4 ++--
 net/sched/sch_api.c       | 2 +-
 net/sched/sch_cbq.c       | 3 ++-
 net/sched/sch_drr.c       | 3 ++-
 net/sched/sch_dsmark.c    | 3 ++-
 net/sched/sch_fq_codel.c  | 3 ++-
 net/sched/sch_hfsc.c      | 3 ++-
 net/sched/sch_htb.c       | 3 ++-
 net/sched/sch_ingress.c   | 6 ++++--
 net/sched/sch_multiq.c    | 3 ++-
 net/sched/sch_prio.c      | 3 ++-
 net/sched/sch_qfq.c       | 3 ++-
 net/sched/sch_sfb.c       | 3 ++-
 net/sched/sch_sfq.c       | 3 ++-
 15 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index d5a545a9d13e..b9940ea8b25e 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -163,7 +163,8 @@ struct Qdisc_class_ops {
 
 	/* Filter manipulation */
 	struct tcf_block *	(*tcf_block)(struct Qdisc *sch,
-					     unsigned long arg);
+					     unsigned long arg,
+					     struct netlink_ext_ack *extack);
 	unsigned long		(*bind_tcf)(struct Qdisc *, unsigned long,
 					u32 classid);
 	void			(*unbind_tcf)(struct Qdisc *, unsigned long);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index d51051dd8f7d..3f9e9e7cf3f9 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -793,7 +793,7 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 	}
 
 	/* And the last stroke */
-	block = cops->tcf_block(q, cl);
+	block = cops->tcf_block(q, cl, extack);
 	if (!block) {
 		err = -EINVAL;
 		goto errout;
@@ -1040,7 +1040,7 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 		if (cl == 0)
 			goto out;
 	}
-	block = cops->tcf_block(q, cl);
+	block = cops->tcf_block(q, cl, NULL);
 	if (!block)
 		goto out;
 
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 87c974d2d576..d1ef0df4d628 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1775,7 +1775,7 @@ static void tc_bind_tclass(struct Qdisc *q, u32 portid, u32 clid,
 	cl = cops->find(q, portid);
 	if (!cl)
 		return;
-	block = cops->tcf_block(q, cl);
+	block = cops->tcf_block(q, cl, NULL);
 	if (!block)
 		return;
 	list_for_each_entry(chain, &block->chain_list, list) {
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 55cb7c035b19..aba17cc34050 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1679,7 +1679,8 @@ static int cbq_delete(struct Qdisc *sch, unsigned long arg)
 	return 0;
 }
 
-static struct tcf_block *cbq_tcf_block(struct Qdisc *sch, unsigned long arg)
+static struct tcf_block *cbq_tcf_block(struct Qdisc *sch, unsigned long arg,
+				       struct netlink_ext_ack *extack)
 {
 	struct cbq_sched_data *q = qdisc_priv(sch);
 	struct cbq_class *cl = (struct cbq_class *)arg;
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index 73b914bc47a4..44a2870f6f10 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -173,7 +173,8 @@ static unsigned long drr_search_class(struct Qdisc *sch, u32 classid)
 	return (unsigned long)drr_find_class(sch, classid);
 }
 
-static struct tcf_block *drr_tcf_block(struct Qdisc *sch, unsigned long cl)
+static struct tcf_block *drr_tcf_block(struct Qdisc *sch, unsigned long cl,
+				       struct netlink_ext_ack *extack)
 {
 	struct drr_sched *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
index 89e433bbd590..5dc5d5216fbb 100644
--- a/net/sched/sch_dsmark.c
+++ b/net/sched/sch_dsmark.c
@@ -185,7 +185,8 @@ static void dsmark_walk(struct Qdisc *sch, struct qdisc_walker *walker)
 	}
 }
 
-static struct tcf_block *dsmark_tcf_block(struct Qdisc *sch, unsigned long cl)
+static struct tcf_block *dsmark_tcf_block(struct Qdisc *sch, unsigned long cl,
+					  struct netlink_ext_ack *extack)
 {
 	struct dsmark_qdisc_data *p = qdisc_priv(sch);
 
diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c
index d798c93f7c96..8c5cea978d16 100644
--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -597,7 +597,8 @@ static void fq_codel_unbind(struct Qdisc *q, unsigned long cl)
 {
 }
 
-static struct tcf_block *fq_codel_tcf_block(struct Qdisc *sch, unsigned long cl)
+static struct tcf_block *fq_codel_tcf_block(struct Qdisc *sch, unsigned long cl,
+					    struct netlink_ext_ack *extack)
 {
 	struct fq_codel_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 008524d6bb74..bf86a8aa67eb 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1247,7 +1247,8 @@ hfsc_unbind_tcf(struct Qdisc *sch, unsigned long arg)
 	cl->filter_cnt--;
 }
 
-static struct tcf_block *hfsc_tcf_block(struct Qdisc *sch, unsigned long arg)
+static struct tcf_block *hfsc_tcf_block(struct Qdisc *sch, unsigned long arg,
+					struct netlink_ext_ack *extack)
 {
 	struct hfsc_sched *q = qdisc_priv(sch);
 	struct hfsc_class *cl = (struct hfsc_class *)arg;
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index eb535a23a69b..79cf24468a38 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1525,7 +1525,8 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
 	return err;
 }
 
-static struct tcf_block *htb_tcf_block(struct Qdisc *sch, unsigned long arg)
+static struct tcf_block *htb_tcf_block(struct Qdisc *sch, unsigned long arg,
+				       struct netlink_ext_ack *extack)
 {
 	struct htb_sched *q = qdisc_priv(sch);
 	struct htb_class *cl = (struct htb_class *)arg;
diff --git a/net/sched/sch_ingress.c b/net/sched/sch_ingress.c
index c703cf3a0bed..17fa8969f7e7 100644
--- a/net/sched/sch_ingress.c
+++ b/net/sched/sch_ingress.c
@@ -48,7 +48,8 @@ static void ingress_walk(struct Qdisc *sch, struct qdisc_walker *walker)
 {
 }
 
-static struct tcf_block *ingress_tcf_block(struct Qdisc *sch, unsigned long cl)
+static struct tcf_block *ingress_tcf_block(struct Qdisc *sch, unsigned long cl,
+					   struct netlink_ext_ack *extack)
 {
 	struct ingress_sched_data *q = qdisc_priv(sch);
 
@@ -153,7 +154,8 @@ static unsigned long clsact_bind_filter(struct Qdisc *sch,
 	return clsact_find(sch, classid);
 }
 
-static struct tcf_block *clsact_tcf_block(struct Qdisc *sch, unsigned long cl)
+static struct tcf_block *clsact_tcf_block(struct Qdisc *sch, unsigned long cl,
+					  struct netlink_ext_ack *extack)
 {
 	struct clsact_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c
index fbd15b5524bb..a993ac4e1979 100644
--- a/net/sched/sch_multiq.c
+++ b/net/sched/sch_multiq.c
@@ -371,7 +371,8 @@ static void multiq_walk(struct Qdisc *sch, struct qdisc_walker *arg)
 	}
 }
 
-static struct tcf_block *multiq_tcf_block(struct Qdisc *sch, unsigned long cl)
+static struct tcf_block *multiq_tcf_block(struct Qdisc *sch, unsigned long cl,
+					  struct netlink_ext_ack *extack)
 {
 	struct multiq_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
index 5f8ecbaa2610..077af4730749 100644
--- a/net/sched/sch_prio.c
+++ b/net/sched/sch_prio.c
@@ -329,7 +329,8 @@ static void prio_walk(struct Qdisc *sch, struct qdisc_walker *arg)
 	}
 }
 
-static struct tcf_block *prio_tcf_block(struct Qdisc *sch, unsigned long cl)
+static struct tcf_block *prio_tcf_block(struct Qdisc *sch, unsigned long cl,
+					struct netlink_ext_ack *extack)
 {
 	struct prio_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index 1f4a84b687d2..e77e7131e620 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -565,7 +565,8 @@ static unsigned long qfq_search_class(struct Qdisc *sch, u32 classid)
 	return (unsigned long)qfq_find_class(sch, classid);
 }
 
-static struct tcf_block *qfq_tcf_block(struct Qdisc *sch, unsigned long cl)
+static struct tcf_block *qfq_tcf_block(struct Qdisc *sch, unsigned long cl,
+				       struct netlink_ext_ack *extack)
 {
 	struct qfq_sched *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c
index d70d470361be..9e01b80edfe7 100644
--- a/net/sched/sch_sfb.c
+++ b/net/sched/sch_sfb.c
@@ -668,7 +668,8 @@ static void sfb_walk(struct Qdisc *sch, struct qdisc_walker *walker)
 	}
 }
 
-static struct tcf_block *sfb_tcf_block(struct Qdisc *sch, unsigned long cl)
+static struct tcf_block *sfb_tcf_block(struct Qdisc *sch, unsigned long cl,
+				       struct netlink_ext_ack *extack)
 {
 	struct sfb_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 0af813ed67c2..0efd47098c5c 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -834,7 +834,8 @@ static void sfq_unbind(struct Qdisc *q, unsigned long cl)
 {
 }
 
-static struct tcf_block *sfq_tcf_block(struct Qdisc *sch, unsigned long cl)
+static struct tcf_block *sfq_tcf_block(struct Qdisc *sch, unsigned long cl,
+				       struct netlink_ext_ack *extack)
 {
 	struct sfq_sched_data *q = qdisc_priv(sch);
 
-- 
2.11.0

Re: [PATCH net-next 5/6] net: sched: sch: add extack for block callback

[Jamal Hadi Salim]

On 17-12-06 11:08 AM, Alexander Aring wrote:
> This patch adds extack support for block callback to prepare per-qdisc
> specific changes for extack.
> 
> Cc: David Ahern<dsahern@gmail.com>
> Signed-off-by: Alexander Aring<aring@mojatatu.com>

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

[PATCH net-next 6/6] net: sched: sch: add extack for graft callback

[Alexander Aring]

This patch adds extack support for graft callback to prepare per-qdisc
specific changes for extack.

Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Alexander Aring <aring@mojatatu.com>
---
 include/net/sch_generic.h | 3 ++-
 net/sched/sch_api.c       | 3 ++-
 net/sched/sch_cbq.c       | 2 +-
 net/sched/sch_drr.c       | 3 ++-
 net/sched/sch_dsmark.c    | 3 ++-
 net/sched/sch_hfsc.c      | 2 +-
 net/sched/sch_htb.c       | 2 +-
 net/sched/sch_mq.c        | 2 +-
 net/sched/sch_mqprio.c    | 2 +-
 net/sched/sch_multiq.c    | 2 +-
 net/sched/sch_netem.c     | 2 +-
 net/sched/sch_prio.c      | 2 +-
 net/sched/sch_qfq.c       | 3 ++-
 net/sched/sch_red.c       | 2 +-
 net/sched/sch_sfb.c       | 2 +-
 net/sched/sch_tbf.c       | 2 +-
 16 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index b9940ea8b25e..c57a8955cdc9 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -149,7 +149,8 @@ struct Qdisc_class_ops {
 	/* Child qdisc manipulation */
 	struct netdev_queue *	(*select_queue)(struct Qdisc *, struct tcmsg *);
 	int			(*graft)(struct Qdisc *, unsigned long cl,
-					struct Qdisc *, struct Qdisc **);
+					struct Qdisc *, struct Qdisc **,
+					struct netlink_ext_ack *extack);
 	struct Qdisc *		(*leaf)(struct Qdisc *, unsigned long cl);
 	void			(*qlen_notify)(struct Qdisc *, unsigned long);
 
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index d1ef0df4d628..9187b16ccec1 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -973,7 +973,8 @@ static int qdisc_graft(struct net_device *dev, struct Qdisc *parent,
 			unsigned long cl = cops->find(parent, classid);
 
 			if (cl) {
-				err = cops->graft(parent, cl, new, &old);
+				err = cops->graft(parent, cl, new, &old,
+						  extack);
 			} else {
 				NL_SET_ERR_MSG(extack, "Specified class not found");
 				err = -ENOENT;
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index aba17cc34050..2ac3290af472 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1370,7 +1370,7 @@ cbq_dump_class_stats(struct Qdisc *sch, unsigned long arg,
 }
 
 static int cbq_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
-		     struct Qdisc **old)
+		     struct Qdisc **old, struct netlink_ext_ack *extack)
 {
 	struct cbq_class *cl = (struct cbq_class *)arg;
 
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index 44a2870f6f10..30e9cba54ddb 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -203,7 +203,8 @@ static void drr_unbind_tcf(struct Qdisc *sch, unsigned long arg)
 }
 
 static int drr_graft_class(struct Qdisc *sch, unsigned long arg,
-			   struct Qdisc *new, struct Qdisc **old)
+			   struct Qdisc *new, struct Qdisc **old,
+			   struct netlink_ext_ack *extack)
 {
 	struct drr_class *cl = (struct drr_class *)arg;
 
diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
index 5dc5d5216fbb..92a36aa4c713 100644
--- a/net/sched/sch_dsmark.c
+++ b/net/sched/sch_dsmark.c
@@ -61,7 +61,8 @@ static inline int dsmark_valid_index(struct dsmark_qdisc_data *p, u16 index)
 /* ------------------------- Class/flow operations ------------------------- */
 
 static int dsmark_graft(struct Qdisc *sch, unsigned long arg,
-			struct Qdisc *new, struct Qdisc **old)
+			struct Qdisc *new, struct Qdisc **old,
+			struct netlink_ext_ack *extack)
 {
 	struct dsmark_qdisc_data *p = qdisc_priv(sch);
 
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index bf86a8aa67eb..bd0546044326 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1177,7 +1177,7 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr)
 
 static int
 hfsc_graft_class(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
-		 struct Qdisc **old)
+		 struct Qdisc **old, struct netlink_ext_ack *extack)
 {
 	struct hfsc_class *cl = (struct hfsc_class *)arg;
 
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 79cf24468a38..65762d57a70d 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1172,7 +1172,7 @@ htb_dump_class_stats(struct Qdisc *sch, unsigned long arg, struct gnet_dump *d)
 }
 
 static int htb_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
-		     struct Qdisc **old)
+		     struct Qdisc **old, struct netlink_ext_ack *extack)
 {
 	struct htb_class *cl = (struct htb_class *)arg;
 
diff --git a/net/sched/sch_mq.c b/net/sched/sch_mq.c
index c4f160495366..c252867f541e 100644
--- a/net/sched/sch_mq.c
+++ b/net/sched/sch_mq.c
@@ -135,7 +135,7 @@ static struct netdev_queue *mq_select_queue(struct Qdisc *sch,
 }
 
 static int mq_graft(struct Qdisc *sch, unsigned long cl, struct Qdisc *new,
-		    struct Qdisc **old)
+		    struct Qdisc **old, struct netlink_ext_ack *extack)
 {
 	struct netdev_queue *dev_queue = mq_queue_get(sch, cl);
 	struct net_device *dev = qdisc_dev(sch);
diff --git a/net/sched/sch_mqprio.c b/net/sched/sch_mqprio.c
index 9355c4a4686c..75ab78a13e28 100644
--- a/net/sched/sch_mqprio.c
+++ b/net/sched/sch_mqprio.c
@@ -320,7 +320,7 @@ static struct netdev_queue *mqprio_queue_get(struct Qdisc *sch,
 }
 
 static int mqprio_graft(struct Qdisc *sch, unsigned long cl, struct Qdisc *new,
-		    struct Qdisc **old)
+			struct Qdisc **old, struct netlink_ext_ack *extack)
 {
 	struct net_device *dev = qdisc_dev(sch);
 	struct netdev_queue *dev_queue = mqprio_queue_get(sch, cl);
diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c
index a993ac4e1979..28c7d98d04bf 100644
--- a/net/sched/sch_multiq.c
+++ b/net/sched/sch_multiq.c
@@ -283,7 +283,7 @@ static int multiq_dump(struct Qdisc *sch, struct sk_buff *skb)
 }
 
 static int multiq_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
-		      struct Qdisc **old)
+			struct Qdisc **old, struct netlink_ext_ack *extack)
 {
 	struct multiq_sched_data *q = qdisc_priv(sch);
 	unsigned long band = arg - 1;
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index f45040b55531..7bbc13b8ca47 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -1159,7 +1159,7 @@ static int netem_dump_class(struct Qdisc *sch, unsigned long cl,
 }
 
 static int netem_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
-		     struct Qdisc **old)
+		     struct Qdisc **old, struct netlink_ext_ack *extack)
 {
 	struct netem_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
index 077af4730749..8fbd65661d77 100644
--- a/net/sched/sch_prio.c
+++ b/net/sched/sch_prio.c
@@ -242,7 +242,7 @@ static int prio_dump(struct Qdisc *sch, struct sk_buff *skb)
 }
 
 static int prio_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
-		      struct Qdisc **old)
+		      struct Qdisc **old, struct netlink_ext_ack *extack)
 {
 	struct prio_sched_data *q = qdisc_priv(sch);
 	unsigned long band = arg - 1;
diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index e77e7131e620..7ec893f770d2 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -595,7 +595,8 @@ static void qfq_unbind_tcf(struct Qdisc *sch, unsigned long arg)
 }
 
 static int qfq_graft_class(struct Qdisc *sch, unsigned long arg,
-			   struct Qdisc *new, struct Qdisc **old)
+			   struct Qdisc *new, struct Qdisc **old,
+			   struct netlink_ext_ack *extack)
 {
 	struct qfq_class *cl = (struct qfq_class *)arg;
 
diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index 180bdd206b99..49efad96f806 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -381,7 +381,7 @@ static int red_dump_class(struct Qdisc *sch, unsigned long cl,
 }
 
 static int red_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
-		     struct Qdisc **old)
+		     struct Qdisc **old, struct netlink_ext_ack *extack)
 {
 	struct red_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c
index 9e01b80edfe7..1a33d6c3ac42 100644
--- a/net/sched/sch_sfb.c
+++ b/net/sched/sch_sfb.c
@@ -617,7 +617,7 @@ static int sfb_dump_class(struct Qdisc *sch, unsigned long cl,
 }
 
 static int sfb_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
-		     struct Qdisc **old)
+		     struct Qdisc **old, struct netlink_ext_ack *extack)
 {
 	struct sfb_sched_data *q = qdisc_priv(sch);
 
diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
index 49002653e9dd..598cef1d2a9d 100644
--- a/net/sched/sch_tbf.c
+++ b/net/sched/sch_tbf.c
@@ -496,7 +496,7 @@ static int tbf_dump_class(struct Qdisc *sch, unsigned long cl,
 }
 
 static int tbf_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
-		     struct Qdisc **old)
+		     struct Qdisc **old, struct netlink_ext_ack *extack)
 {
 	struct tbf_sched_data *q = qdisc_priv(sch);
 
-- 
2.11.0

Re: [PATCH net-next 6/6] net: sched: sch: add extack for graft callback

[Jamal Hadi Salim]

On 17-12-06 11:08 AM, Alexander Aring wrote:
> This patch adds extack support for graft callback to prepare per-qdisc
> specific changes for extack.
> 
> Cc: David Ahern<dsahern@gmail.com>
> Signed-off-by: Alexander Aring<aring@mojatatu.com>

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

Re: [PATCH net-next 0/6] net: sched: sch: introduce extack support

[Cong Wang]

On Wed, Dec 6, 2017 at 8:08 AM, Alexander Aring <aring@mojatatu.com> wrote:
> Hi,
>
> this patch series basically add support for extack in common qdisc handling.
> Additional it adds extack pointer to common qdisc callback handling this
> offers per qdisc implementation to setting the extack message for each
> failure over netlink.
>
> The extack message will be set deeper in qdisc functions but going not
> deeper as net core api. For qdisc module callback handling, the extack
> will not be set. This will be part of per qdisc extack handling.
>
> I also want to prepare patches to handle extack per qdisc module...
> so there will come a lot of more patches, just cut them down to make
> it reviewable.
>
> There are some above 80-chars width warnings, which I ignore because
> it looks more ugly otherwise.
>
> This patch-series based on patches by David Ahren which gave me some
> hints how to deal with extack support.
>
> Cc: David Ahern <dsahern@gmail.com>

Acked-by: Cong Wang <xiyou.wangcong@gmail.com>

Re: [PATCH net-next 0/6] net: sched: sch: introduce extack support

[David Miller]

From: Alexander Aring <aring@mojatatu.com>
Date: Wed,  6 Dec 2017 11:08:39 -0500

> this patch series basically add support for extack in common qdisc handling.
> Additional it adds extack pointer to common qdisc callback handling this
> offers per qdisc implementation to setting the extack message for each
> failure over netlink.
> 
> The extack message will be set deeper in qdisc functions but going not
> deeper as net core api. For qdisc module callback handling, the extack
> will not be set. This will be part of per qdisc extack handling.
> 
> I also want to prepare patches to handle extack per qdisc module...
> so there will come a lot of more patches, just cut them down to make
> it reviewable.
> 
> There are some above 80-chars width warnings, which I ignore because
> it looks more ugly otherwise.
> 
> This patch-series based on patches by David Ahren which gave me some
> hints how to deal with extack support.
> 
> Cc: David Ahern <dsahern@gmail.com>

Only add the plumbing when you have actual extack messages you are
adding as an example use case.

Thank you.

Re: [PATCH net-next 0/6] net: sched: sch: introduce extack support

[Alexander Aring]

Hi,

On Wed, Dec 6, 2017 at 3:40 PM, David Miller <davem@davemloft.net> wrote:
> From: Alexander Aring <aring@mojatatu.com>
> Date: Wed,  6 Dec 2017 11:08:39 -0500
>
>> this patch series basically add support for extack in common qdisc handling.
>> Additional it adds extack pointer to common qdisc callback handling this
>> offers per qdisc implementation to setting the extack message for each
>> failure over netlink.
>>
>> The extack message will be set deeper in qdisc functions but going not
>> deeper as net core api. For qdisc module callback handling, the extack
>> will not be set. This will be part of per qdisc extack handling.
>>
>> I also want to prepare patches to handle extack per qdisc module...
>> so there will come a lot of more patches, just cut them down to make
>> it reviewable.
>>
>> There are some above 80-chars width warnings, which I ignore because
>> it looks more ugly otherwise.
>>
>> This patch-series based on patches by David Ahren which gave me some
>> hints how to deal with extack support.
>>
>> Cc: David Ahern <dsahern@gmail.com>
>
> Only add the plumbing when you have actual extack messages you are
> adding as an example use case.
>

I did not understand. I have a lot of patches which make use of these
changes. Do you want me to submit me these in one shot (patch-series)?
I was hoping to making it in smaller patch-series for easier review.

- Alex

Re: [PATCH net-next 0/6] net: sched: sch: introduce extack support

[David Miller]

From: Alexander Aring <aring@mojatatu.com>
Date: Wed, 6 Dec 2017 17:34:08 -0500

> Hi,
> 
> On Wed, Dec 6, 2017 at 3:40 PM, David Miller <davem@davemloft.net> wrote:
>> From: Alexander Aring <aring@mojatatu.com>
>> Date: Wed,  6 Dec 2017 11:08:39 -0500
>>
>>> this patch series basically add support for extack in common qdisc handling.
>>> Additional it adds extack pointer to common qdisc callback handling this
>>> offers per qdisc implementation to setting the extack message for each
>>> failure over netlink.
>>>
>>> The extack message will be set deeper in qdisc functions but going not
>>> deeper as net core api. For qdisc module callback handling, the extack
>>> will not be set. This will be part of per qdisc extack handling.
>>>
>>> I also want to prepare patches to handle extack per qdisc module...
>>> so there will come a lot of more patches, just cut them down to make
>>> it reviewable.
>>>
>>> There are some above 80-chars width warnings, which I ignore because
>>> it looks more ugly otherwise.
>>>
>>> This patch-series based on patches by David Ahren which gave me some
>>> hints how to deal with extack support.
>>>
>>> Cc: David Ahern <dsahern@gmail.com>
>>
>> Only add the plumbing when you have actual extack messages you are
>> adding as an example use case.
>>
> 
> I did not understand. I have a lot of patches which make use of these
> changes. Do you want me to submit me these in one shot (patch-series)?
> I was hoping to making it in smaller patch-series for easier review.

Submit one plumbing patch alongside the changes that actually add
messages in those code paths.

This patch series did plumbing in many spots, one patch at a time,
but added no users except in the initial path.

That's what I don't like.