Re: [PATCHv4 net-next 02/14] net: sched: sch_api: handle generic qdisc errors

[Jiri Pirko <jiri@resnulli.us>]

Wed, Dec 20, 2017 at 06:35:12PM CET, aring@mojatatu.com 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.
>
>Cc: David Ahern <dsahern@gmail.com>
>Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
>Signed-off-by: Alexander Aring <aring@mojatatu.com>
>---

[...]


>@@ -1349,21 +1387,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");

This is incorrect. This is hit on successpath as well, confusing user
with the message. I will send fix shortly.


> 					goto create_n_graft;
>-				if (n->nlmsg_flags & NLM_F_EXCL)
>+				}