Re: [PATCH v4 net-next 6/9] net/sched: mqprio: allow per-TC user input of FP adminStatus

[Jakub Kicinski <kuba@kernel.org>]

On Sat, 8 Apr 2023 00:52:52 +0300 Vladimir Oltean wrote:
> On Fri, Apr 07, 2023 at 05:40:20PM -0400, Jamal Hadi Salim wrote:
> > Yes, it is minor (and usually minor things generate the most emails;->).
> > I may be misunderstanding what you mean by "doesnt justify exporting
> > something to UAPI"  - those definitions are part of uapi and are
> > already being exported.  
> 
> In my proposed patch set there isn't any TC_FP_MAX. I'm saying it
> doesn't help user space, and so, it just pollutes the name space of C
> programs with no good reason.

+1  we tend to sprinkle MAX and UNSPEC into every enum

> > No, no, it is a matter of taste and opinion. You may have noticed,
> > trivial stuff like this gets the most comments and reviews normally(we
> > just spent like 4-5 emails on this?). Poteto/potato: IOW, if i was to
> > do it i would have used a u16 or u32 because i feel it would be more
> > readable. I would have used NLA_U8 because i felt it is more fitting
> > and i would have used a max value because it would save me one line in
> > a patch in the future. I think weve spent enough electrons on this - I
> > defer to you.  
> 
> Ok, I won't change preemptible_tcs from unsigned long to u32.
> Things like for_each_set_bit() take unsigned long, and so, I got used
> to using that consistently for small bitfield types.
> 
> If there's a second opinion stating that I should prefer the smallest
> netlink attribute type that fits the estimated data, then I'll transition
> from NLA_U32 to NLA_U8. Otherwise, I won't :) since I would need to
> change iproute2 too, and I'd have to re-test more thoroughly to make
> sure I don't introduce stupid bugs.

And here also agreed. We should have a patchwork check for new uses of
NLA_*{8,16} if you ask me :S  NLA_FLAG or NLA_U32, anything in between
needs a strong justification.  Until Alex L posts the variable size
ints, then NLA_FLAG or NLA_UINT ;)