> +static s64 __scale_delta(s64 diff)
> +{
> +     do_div(diff, (1 << (ALPHA_BETA_GRANULARITY + 1)) - 1);
> +     return diff;
> +}
[...]
> +     delta = __scale_delta(((s64)qdelay - q->pi2.target) * q->pi2.alpha);
> +     delta += __scale_delta(((s64)qdelay - qdelay_old) * q->pi2.beta);

I just noticed that ensuring 64b divide compatibility across platforms 
using do_div() introduced this bug, as do_div() works with unsigned operands.

This will be fixed in a later v5 with the following patch:

---
 net/sched/sch_dualpi2.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/net/sched/sch_dualpi2.c b/net/sched/sch_dualpi2.c
index a6452aa82018..c6c851499d35 100644
--- a/net/sched/sch_dualpi2.c
+++ b/net/sched/sch_dualpi2.c
@@ -385,7 +385,7 @@ static struct sk_buff *dualpi2_qdisc_dequeue(struct Qdisc 
*sch)
        return skb;
 }
 
-static s64 __scale_delta(s64 diff)
+static s64 __scale_delta(u64 diff)
 {
        do_div(diff, (1 << (ALPHA_BETA_GRANULARITY + 1)) - 1);
        return diff;
@@ -406,16 +406,18 @@ static u32 calculate_probability(struct Qdisc *sch)
        /* Alpha and beta take at most 32b, i.e, the delay difference would
         * overflow for queueing delay differences > ~4.2sec.
         */
-       delta = __scale_delta(((s64)qdelay - q->pi2.target) * q->pi2.alpha);
-       delta += __scale_delta(((s64)qdelay - qdelay_old) * q->pi2.beta);
-       new_prob = delta + q->pi2.prob;
+       delta = ((s64)qdelay - q->pi2.target) * q->pi2.alpha;
+       delta += ((s64)qdelay - qdelay_old) * q->pi2.beta;
        /* Prevent overflow */
        if (delta > 0) {
+               new_prob = __scale_delta(delta) + q->pi2.prob;
                if (new_prob < q->pi2.prob)
                        new_prob = MAX_PROB;
+       } else {
+               new_prob = q->pi2.prob - __scale_delta(delta * -1);
                /* Prevent underflow */
-       } else if (new_prob > q->pi2.prob) {
-               new_prob = 0;
+               if (new_prob > q->pi2.prob)
+                       new_prob = 0;
        }
        /* If we do not drop on overload, ensure we cap the L4S probability to
         * 100% to keep window fairness when overflowing.
-- 

Sorry for this.


Best,
Olivier

Reply via email to