| Message ID | 20250116195642.2794-1-ouster@cs.stanford.edu (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [netnext] net: tc: improve qdisc error messages | expand |
On Thu, Jan 16, 2025 at 2:57 PM John Ousterhout <ouster@cs.stanford.edu> wrote: > > The existing error message ("Invalid qdisc name") is confusing > because it suggests that there is no qdisc with the given name. In > fact, the name does refer to a valid qdisc, but it doesn't match > the kind of an existing qdisc being modified or replaced. The > new error message provides more detail to eliminate confusion. > > Signed-off-by: John Ousterhout <ouster@cs.stanford.edu> > --- > net/sched/sch_api.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c > index 300430b8c4d2..5d017c06a96f 100644 > --- a/net/sched/sch_api.c > +++ b/net/sched/sch_api.c > @@ -1560,7 +1560,7 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n, > } > > if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], q->ops->id)) { > - NL_SET_ERR_MSG(extack, "Invalid qdisc name"); > + NL_SET_ERR_MSG(extack, "Invalid qdisc name: must match existing qdisc"); > return -EINVAL; > } > > @@ -1670,7 +1670,7 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n, > } > if (tca[TCA_KIND] && > nla_strcmp(tca[TCA_KIND], q->ops->id)) { > - NL_SET_ERR_MSG(extack, "Invalid qdisc name"); > + NL_SET_ERR_MSG(extack, "Invalid qdisc name: must match existing qdisc"); > return -EINVAL; > } > if (q->flags & TCQ_F_INGRESS) { > @@ -1746,7 +1746,7 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n, > return -EEXIST; > } > if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], q->ops->id)) { > - NL_SET_ERR_MSG(extack, "Invalid qdisc name"); > + NL_SET_ERR_MSG(extack, "Invalid qdisc name: must match existing qdisc"); > return -EINVAL; > } > err = qdisc_change(q, tca, extack); > -- LGTM. Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> cheers, jamal > 2.34.1 > >
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c index 300430b8c4d2..5d017c06a96f 100644 --- a/net/sched/sch_api.c +++ b/net/sched/sch_api.c @@ -1560,7 +1560,7 @@ static int tc_get_qdisc(struct sk_buff *skb, struct nlmsghdr *n, } if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], q->ops->id)) { - NL_SET_ERR_MSG(extack, "Invalid qdisc name"); + NL_SET_ERR_MSG(extack, "Invalid qdisc name: must match existing qdisc"); return -EINVAL; } @@ -1670,7 +1670,7 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n, } if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], q->ops->id)) { - NL_SET_ERR_MSG(extack, "Invalid qdisc name"); + NL_SET_ERR_MSG(extack, "Invalid qdisc name: must match existing qdisc"); return -EINVAL; } if (q->flags & TCQ_F_INGRESS) { @@ -1746,7 +1746,7 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n, return -EEXIST; } if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], q->ops->id)) { - NL_SET_ERR_MSG(extack, "Invalid qdisc name"); + NL_SET_ERR_MSG(extack, "Invalid qdisc name: must match existing qdisc"); return -EINVAL; } err = qdisc_change(q, tca, extack);
The existing error message ("Invalid qdisc name") is confusing because it suggests that there is no qdisc with the given name. In fact, the name does refer to a valid qdisc, but it doesn't match the kind of an existing qdisc being modified or replaced. The new error message provides more detail to eliminate confusion. Signed-off-by: John Ousterhout <ouster@cs.stanford.edu> --- net/sched/sch_api.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- 2.34.1