On Fri, Jan 22, 2021 at 02:44:47PM -0800, Vinicius Costa Gomes wrote:
> + /* It's valid to enable frame preemption without any kind of
> + * offloading being enabled, so keep it separated.
> + */
> + if (tb[TCA_TAPRIO_ATTR_PREEMPT_TCS]) {
> + u32 preempt = nla_get_u32(tb[TCA_TAPRIO_ATTR_PREEMPT_TCS]);
> + struct tc_preempt_qopt_offload qopt = { };
> +
> + if (preempt == U32_MAX) {
> + NL_SET_ERR_MSG(extack, "At least one queue must be not
> be preemptible");
> + err = -EINVAL;
> + goto free_sched;
> + }
> +
> + qopt.preemptible_queues = tc_map_to_queue_mask(dev, preempt);
> +
> + err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_PREEMPT,
> + &qopt);
> + if (err)
> + goto free_sched;
> +
> + q->preemptible_tcs = preempt;
> + }
> +
First I'm interested in the means: why check for preempt == U32_MAX when
you determine that all traffic classes are preemptible? What if less
than 32 traffic classes are used by the netdev? The check will be
bypassed, won't it?
Secondly, why should at least one queue be preemptible? What's wrong
with frame preemption being triggered by a tc-taprio window smaller than
the packet size? This can happen regardless of traffic class.
