linux-kernel - Re: [net-next PATCH] taprio: don't reject same mqprio settings

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]

Message-ID: <87mucwhm4h.fsf@linux.intel.com>
Date:   Fri, 15 Nov 2019 13:21:02 -0800
From:   Vinicius Costa Gomes <vinicius.gomes@...el.com>
To:     Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>,
        netdev@...r.kernel.org, davem@...emloft.net
Cc:     jhs@...atatu.com, xiyou.wangcong@...il.com, jiri@...nulli.us,
        linux-kernel@...r.kernel.org,
        Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>
Subject: Re: [net-next PATCH] taprio: don't reject same mqprio settings

Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org> writes:

> The taprio qdisc allows to set mqprio setting but only once. In case
> if mqprio settings are provided next time the error is returned as
> it's not allowed to change traffic class mapping in-flignt and that
> is normal. But if configuration is absolutely the same - no need to
> return error. It allows to provide same command couple times,
> changing only base time for instance, or changing only scheds maps,
> but leaving mqprio setting w/o modification. It more corresponds the
> message: "Changing the traffic mapping of a running schedule is not
> supported", so reject mqprio if it's really changed.
>
> Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>
> ---
>  net/sched/sch_taprio.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index 7cd68628c637..bd844f2cbf7a 100644
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -1347,6 +1347,26 @@ static int taprio_parse_clockid(struct Qdisc *sch, struct nlattr **tb,
>  	return err;
>  }
>  
> +static int taprio_mqprio_cmp(struct net_device *dev,
> +			     struct tc_mqprio_qopt *mqprio)

Nitpick: for these kinds of functions I like to add a 'const' to the parameters
at least as documentation that it doesn't modify its arguments.

> +{
> +	int i;
> +
> +	if (mqprio->num_tc != dev->num_tc)
> +		return -1;

Optional: you could move the check for a NULL mqprio inside this
function. Perhaps, for that to make sense you would need to change the
function name to taprio_mqprio_check() or something.

These are all optional.

Acked-by: Vinicius Costa Gomes <vinicius.gomes@...el.com>