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] [day] [month] [year] [list]

Date:   Mon, 9 Mar 2020 11:53:45 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Leslie Monis <lesliemonis@...il.com>,
        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 3/9/20 11:42 AM, Leslie Monis wrote:
> 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?

Sure, please go ahead !

> I guess the applied patch (topic of this thread) has to be reverted.
>