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

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Fri, Apr 07, 2023 at 12:22:26PM -0400, Jamal Hadi Salim wrote:
> > +enum {
> > +       TC_FP_EXPRESS = 1,
> > +       TC_FP_PREEMPTIBLE = 2,
> > +};
> 
> Suggestion: Add a MAX to this enum (as is traditionally done)..

Max what? This doesn't count anything, it just expresses whether the
quality of one traffic class, from the Frame Preemption standard's
perspective, is express or preemptible...

> > @@ -145,13 +149,94 @@ static int mqprio_parse_opt(struct net_device *dev, struct tc_mqprio_qopt *qopt,
> >         return 0;
> >  }
> >
> > +static const struct
> > +nla_policy mqprio_tc_entry_policy[TCA_MQPRIO_TC_ENTRY_MAX + 1] = {
> > +       [TCA_MQPRIO_TC_ENTRY_INDEX]     = NLA_POLICY_MAX(NLA_U32,
> > +                                                        TC_QOPT_MAX_QUEUE),
> 
> And use it here...

Where? Above or below the comment? I think you mean below (for the
policy of TCA_MQPRIO_TC_ENTRY_FP)?

> Out of curiosity, could you have more that 16 queues in this spec? I
> noticed 802.1p mentioned somewhere (typically 3 bits).

"This spec" is IEEE 802.1Q :) It doesn't say how many "queues" (struct
netdev_queue) there are, and this UAPI doesn't work with those, either.
The standard defines 8 priority values, groupable in (potentially fewer)
traffic classes. Linux liked to extend the number of traffic classes to
16 (and the skb->priority values are arbitrarily large IIUC) and this is
where that number 16 came from. The number of 16 traffic classes still
allows for more than 16 TXQs though.

> Lead up question: if the max is 16 then can preemptible_tcs for example be u32?

I don't understand this question, sorry. preemptible_tcs is declared as
"unsigned long", which IIUC is at least 32-bit.

> 
> > +       [TCA_MQPRIO_TC_ENTRY_FP]        = NLA_POLICY_RANGE(NLA_U32,
> > +                                                          TC_FP_EXPRESS,
> > +                                                          TC_FP_PREEMPTIBLE),