netdev - Re: [PATCH iproute2-next] tc: pie: change maximum integer value of tc_pie

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: <CAHv+uoE_Q37jCY3=_k_hEoiOrD0Mm67qEd-ALO-E9QjQRkSxBA@mail.gmail.com>
Date:   Tue, 10 Mar 2020 00:12:27 +0530
From:   Leslie Monis <lesliemonis@...il.com>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     David Ahern <dsahern@...il.com>,
        Linux NetDev <netdev@...r.kernel.org>,
        Stephen Hemminger <stephen@...workplumber.org>,
        "Mohit P . Tahiliani" <tahiliani@...k.edu.in>,
        Gautam Ramakrishnan <gautamramk@...il.com>
Subject: Re: [PATCH iproute2-next] tc: pie: change maximum integer value of tc_pie_xstats->prob

On Mon, Mar 9, 2020 at 11:24 PM Eric Dumazet <eric.dumazet@...il.com> wrote:
>
> On 3/9/20 10:48 AM, Eric Dumazet wrote:
> >
> > This means that iproute2 is incompatible with old kernels.
> >
> > commit 105e808c1da2 ("pie: remove pie_vars->accu_prob_overflows") was wrong,
> > it should not have changed user ABI.
> >
> > The rule is : iproute2 v-X should work with linux-<whatever-version>
> >

I'm apologize. I wasn't aware of this rule.

> > Since pie MAX_PROB was implicitly in the user ABI, it can not be changed,
> > at least from user point of view.
> >

You're right. It shouldn't have affected user space.
But I'm afraid the value of MAX_PROB in the kernel did change in v5.1.
commit 3f7ae5f3dc52 ("net: sched: pie: add more cases to auto-tune
alpha and beta")
introduced that change. I'm not sure what to do about this. How can I fix it?

>
> So this kernel patch might be needed :
>
> diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
> index f52442d39bf57a7cf7af2595638a277e9c1ecf60..c65077f0c0f39832ee97f4e89f25639306b19281 100644
> --- a/net/sched/sch_pie.c
> +++ b/net/sched/sch_pie.c
> @@ -493,7 +493,7 @@ static int pie_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
>  {
>         struct pie_sched_data *q = qdisc_priv(sch);
>         struct tc_pie_xstats st = {
> -               .prob           = q->vars.prob,
> +               .prob           = q->vars.prob << BITS_PER_BYTE,
>                 .delay          = ((u32)PSCHED_TICKS2NS(q->vars.qdelay)) /
>                                    NSEC_PER_USEC,
>                 .packets_in     = q->stats.packets_in,

Thanks. This is a much better solution.
Should I go ahead and submit this to net-next?
I guess the applied patch (topic of this thread) has to be reverted.