* [PATCH iproute2-next] tc: pie: change maximum integer value of tc_pie_xstats->prob
@ 2020-03-05 16:25 Leslie Monis
2020-03-09 2:49 ` David Ahern
0 siblings, 1 reply; 6+ messages in thread
From: Leslie Monis @ 2020-03-05 16:25 UTC (permalink / raw)
To: Linux NetDev
Cc: David Ahern, Stephen Hemminger, Mohit P . Tahiliani,
Gautam Ramakrishnan
Kernel commit 105e808c1da2 ("pie: remove pie_vars->accu_prob_overflows"),
changes the maximum value of tc_pie_xstats->prob from (2^64 - 1) to
(2^56 - 1).
Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Gautam Ramakrishnan <gautamramk@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
---
tc/q_pie.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tc/q_pie.c b/tc/q_pie.c
index 709a78b4..e6939652 100644
--- a/tc/q_pie.c
+++ b/tc/q_pie.c
@@ -223,9 +223,9 @@ static int pie_print_xstats(struct qdisc_util *qu, FILE *f,
st = RTA_DATA(xstats);
- /* prob is returned as a fracion of maximum integer value */
+ /* prob is returned as a fracion of (2^56 - 1) */
print_float(PRINT_ANY, "prob", " prob %lg",
- (double)st->prob / (double)UINT64_MAX);
+ (double)st->prob / (double)(UINT64_MAX >> 8));
print_uint(PRINT_JSON, "delay", NULL, st->delay);
print_string(PRINT_FP, NULL, " delay %s", sprint_time(st->delay, b1));
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH iproute2-next] tc: pie: change maximum integer value of tc_pie_xstats->prob
2020-03-05 16:25 [PATCH iproute2-next] tc: pie: change maximum integer value of tc_pie_xstats->prob Leslie Monis
@ 2020-03-09 2:49 ` David Ahern
2020-03-09 17:48 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: David Ahern @ 2020-03-09 2:49 UTC (permalink / raw)
To: Leslie Monis, Linux NetDev
Cc: Stephen Hemminger, Mohit P . Tahiliani, Gautam Ramakrishnan
On 3/5/20 9:25 AM, Leslie Monis wrote:
> Kernel commit 105e808c1da2 ("pie: remove pie_vars->accu_prob_overflows"),
> changes the maximum value of tc_pie_xstats->prob from (2^64 - 1) to
> (2^56 - 1).
>
> Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
> Signed-off-by: Gautam Ramakrishnan <gautamramk@gmail.com>
> Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
> ---
> tc/q_pie.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
applied to iproute2-next. Thanks
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH iproute2-next] tc: pie: change maximum integer value of tc_pie_xstats->prob
2020-03-09 2:49 ` David Ahern
@ 2020-03-09 17:48 ` Eric Dumazet
2020-03-09 17:54 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2020-03-09 17:48 UTC (permalink / raw)
To: David Ahern, Leslie Monis, Linux NetDev
Cc: Stephen Hemminger, Mohit P . Tahiliani, Gautam Ramakrishnan
On 3/8/20 7:49 PM, David Ahern wrote:
> On 3/5/20 9:25 AM, Leslie Monis wrote:
>> Kernel commit 105e808c1da2 ("pie: remove pie_vars->accu_prob_overflows"),
>> changes the maximum value of tc_pie_xstats->prob from (2^64 - 1) to
>> (2^56 - 1).
>>
>> Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
>> Signed-off-by: Gautam Ramakrishnan <gautamramk@gmail.com>
>> Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
>> ---
>> tc/q_pie.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>
> applied to iproute2-next. Thanks
>
>
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>
Since pie MAX_PROB was implicitly in the user ABI, it can not be changed,
at least from user point of view.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH iproute2-next] tc: pie: change maximum integer value of tc_pie_xstats->prob
2020-03-09 17:48 ` Eric Dumazet
@ 2020-03-09 17:54 ` Eric Dumazet
2020-03-09 18:42 ` Leslie Monis
0 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2020-03-09 17:54 UTC (permalink / raw)
To: David Ahern, Leslie Monis, Linux NetDev
Cc: Stephen Hemminger, Mohit P . Tahiliani, Gautam Ramakrishnan
On 3/9/20 10:48 AM, Eric Dumazet wrote:
>
>
> On 3/8/20 7:49 PM, David Ahern wrote:
>> On 3/5/20 9:25 AM, Leslie Monis wrote:
>>> Kernel commit 105e808c1da2 ("pie: remove pie_vars->accu_prob_overflows"),
>>> changes the maximum value of tc_pie_xstats->prob from (2^64 - 1) to
>>> (2^56 - 1).
>>>
>>> Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
>>> Signed-off-by: Gautam Ramakrishnan <gautamramk@gmail.com>
>>> Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
>>> ---
>>> tc/q_pie.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>
>> applied to iproute2-next. Thanks
>>
>>
>
> 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>
>
> Since pie MAX_PROB was implicitly in the user ABI, it can not be changed,
> at least from user point of view.
>
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,
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH iproute2-next] tc: pie: change maximum integer value of tc_pie_xstats->prob
2020-03-09 17:54 ` Eric Dumazet
@ 2020-03-09 18:42 ` Leslie Monis
2020-03-09 18:53 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Leslie Monis @ 2020-03-09 18:42 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Ahern, Linux NetDev, Stephen Hemminger, Mohit P . Tahiliani,
Gautam Ramakrishnan
On Mon, Mar 9, 2020 at 11:24 PM Eric Dumazet <eric.dumazet@gmail.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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH iproute2-next] tc: pie: change maximum integer value of tc_pie_xstats->prob
2020-03-09 18:42 ` Leslie Monis
@ 2020-03-09 18:53 ` Eric Dumazet
0 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2020-03-09 18:53 UTC (permalink / raw)
To: Leslie Monis, Eric Dumazet
Cc: David Ahern, Linux NetDev, Stephen Hemminger, Mohit P . Tahiliani,
Gautam Ramakrishnan
On 3/9/20 11:42 AM, Leslie Monis wrote:
> On Mon, Mar 9, 2020 at 11:24 PM Eric Dumazet <eric.dumazet@gmail.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.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-09 18:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-05 16:25 [PATCH iproute2-next] tc: pie: change maximum integer value of tc_pie_xstats->prob Leslie Monis
2020-03-09 2:49 ` David Ahern
2020-03-09 17:48 ` Eric Dumazet
2020-03-09 17:54 ` Eric Dumazet
2020-03-09 18:42 ` Leslie Monis
2020-03-09 18:53 ` Eric Dumazet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).