netdev - Re: [PATCH netnext] net: tc: improve qdisc error messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAM0EoMkkk-dT5kQH6OoVp-zs9bhg8ZLW2w+Dp4SYAZnFfw=CTA@mail.gmail.com>
Date: Thu, 16 Jan 2025 15:00:12 -0500
From: Jamal Hadi Salim <jhs@...atatu.com>
To: John Ousterhout <ouster@...stanford.edu>
Cc: netdev@...r.kernel.org, xiyou.wangcong@...il.com, jiri@...nulli.us
Subject: Re: [PATCH netnext] net: tc: improve qdisc error messages

On Thu, Jan 16, 2025 at 2:57 PM John Ousterhout <ouster@...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@...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@...atatu.com>

cheers,
jamal

> 2.34.1
>
>