Re: [PATCH 1/3] net/sched: netem: use extack

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 02 Feb 2024 11:53:04 +0000 Donald Hunter wrote:
> > Looks like most sch's require opt. Would it be a bad idea to pull 
> > the check out to the caller? Minor simplification, plus the caller
> > has the outer message so they can use NL_SET_ERR_ATTR_MISS() and
> > friends.  
> 
> There's also these which maybe complicates things:
> 
> $ git grep -A1 'if (opt == NULL)' -- net/sched/
> net/sched/cls_flow.c:   if (opt == NULL)
> net/sched/cls_flow.c-           return -EINVAL;
> --
> net/sched/sch_choke.c:  if (opt == NULL)
> net/sched/sch_choke.c-          return -EINVAL;
> --
> net/sched/sch_fifo.c:   if (opt == NULL) {
> net/sched/sch_fifo.c-           u32 limit = qdisc_dev(sch)->tx_queue_len;
> --
> net/sched/sch_hfsc.c:   if (opt == NULL)
> net/sched/sch_hfsc.c-           return -EINVAL;
> --
> net/sched/sch_plug.c:   if (opt == NULL) {
> net/sched/sch_plug.c-           q->limit = qdisc_dev(sch)->tx_queue_len

That's fine, I was thinking opt-in. Add a bit to ops that says
"init_requires_opts" or whatnot. 

> I'm in favour of qdisc specific extack messages.

Most of them just say "$name requires options" in a more or less concise
and more or less well spelled form :( Even if we don't want to depend
purely on ATTR_MISS - extack messages support printf now, and we have
the qdisc name in the ops (ops->id), so we can printf the same string 
in the core.

Just an idea, if you all prefer to keep things as they are, that's fine.
But we've been sprinkling the damn string messages throughout TC for
years now, and still they keep coming and still if you step one foot
away from the most actively developed actions and classifiers, you're
back in the 90s :(