[PATCH netnext] net: tc: improve qdisc error messages

[PATCH netnext] net: tc: improve qdisc error messages

[John Ousterhout]

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

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

[Jamal Hadi Salim]

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
>
>

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

[John Ousterhout]

On Thu, Jan 16, 2025 at 12:00 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> LGTM.
> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
>
> cheers,
> jamal
>
> > 2.34.1

Sorry for my newbie ignorance (this will be my first accepted patch,
assuming it's accepted) but what happens next on this? Is there
anything else I need to do? I mistyped "net-next" as "netnext" in the
subject and patchwork complained about that; do I need to resubmit to
fix this?

-John-

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

[ericnetdev dumazet]

Le ven. 31 janv. 2025 à 07:39, John Ousterhout
<ouster@cs.stanford.edu> a écrit :
>
> On Thu, Jan 16, 2025 at 12:00 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> >
> > LGTM.
> > Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
> >
> > cheers,
> > jamal
> >
> > > 2.34.1
>
> Sorry for my newbie ignorance (this will be my first accepted patch,
> assuming it's accepted) but what happens next on this? Is there
> anything else I need to do? I mistyped "net-next" as "netnext" in the
> subject and patchwork complained about that; do I need to resubmit to
> fix this?

net-next was closed during the merge window, and still is.

Details in Documentation/process/maintainer-netdev.rst

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

[Jamal Hadi Salim]

Hi John,

On Fri, Jan 31, 2025 at 1:39 AM John Ousterhout <ouster@cs.stanford.edu> wrote:
>
> On Thu, Jan 16, 2025 at 12:00 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> >
> > LGTM.
> > Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
> >
> > cheers,
> > jamal
> >
> > > 2.34.1
>
> Sorry for my newbie ignorance (this will be my first accepted patch,
> assuming it's accepted) but what happens next on this? Is there
> anything else I need to do? I mistyped "net-next" as "netnext" in the
> subject and patchwork complained about that; do I need to resubmit to
> fix this?
>

My bad, I should have caught that subject and patchwork wont notify
you if it didnt like something.

Re-submit when net-next opens with --subject-prefix="PATCH net-next"
--subject="net: sched: improve qdisc error messages"
You can add to the submission:
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

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

[Jakub Kicinski]

On Fri, 31 Jan 2025 08:49:31 -0500 Jamal Hadi Salim wrote:
> Re-submit when net-next opens with --subject-prefix="PATCH net-next"
> --subject="net: sched: improve qdisc error messages"

Sorry for the confusion, the pw-bot didn't reply and someone
manually marked this as changes requested in patchwork :(

The patch has in fact already been applied. It is commmit 
f16312b0b9c0 ("net: tc: improve qdisc error messages"),
and already in Linus's tree.