netdev - Re: [PATCH net-next v3 2/8] taprio: Add support for frame preemption offload

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 26 Jan 2021 00:09:25 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Vinicius Costa Gomes <vinicius.gomes@...el.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "jhs@...atatu.com" <jhs@...atatu.com>,
        "xiyou.wangcong@...il.com" <xiyou.wangcong@...il.com>,
        "jiri@...nulli.us" <jiri@...nulli.us>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "Jose.Abreu@...opsys.com" <Jose.Abreu@...opsys.com>,
        Po Liu <po.liu@....com>,
        "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
        "anthony.l.nguyen@...el.com" <anthony.l.nguyen@...el.com>,
        "mkubecek@...e.cz" <mkubecek@...e.cz>
Subject: Re: [PATCH net-next v3 2/8] taprio: Add support for frame preemption
 offload

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.