[PATCH net-next v2 0/7] net: sched: pie: align PIE implementation with RFC 8033

[PATCH net-next v2 0/7] net: sched: pie: align PIE implementation with RFC 8033

[Leslie Monis]

The current implementation of the PIE queuing discipline is according to the
IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and the paper
[PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
However, a lot of necessary modifications and enhancements have been proposed
in RFC 8033, which have not yet been incorporated in the source code of Linux.
This patch series helps in achieving the same.

Performance tests carried out using Flent [https://flent.org/]

Changes from v1 to v2:
  - Excluded the patch setting PIE dynamically active/inactive as the test
    results were unsatisfactory
  - Fixed a scaling issue when adding more auto-tuning cases which caused
    local variables to underflow
  - Changed the long if/else chain to a loop as suggested by Stephen
  - Changed the position of the accu_prob variable in the pie_vars
    structure as recommended by Stephen

Mohit P. Tahiliani (7):
  net: sched: pie: change value of QUEUE_THRESHOLD
  net: sched: pie: change default value of pie_params->target
  net: sched: pie: change default value of pie_params->tupdate
  net: sched: pie: change initial value of pie_vars->burst_time
  net: sched: pie: add more cases to auto-tune alpha and beta
  net: sched: pie: add derandomization mechanism
  net: sched: pie: update references

 include/uapi/linux/pkt_sched.h |   2 +-
 net/sched/sch_pie.c            | 107 ++++++++++++++++++++-------------
 2 files changed, 66 insertions(+), 43 deletions(-)

-- 
2.17.1

[PATCH net-next v2 1/7] net: sched: pie: change value of QUEUE_THRESHOLD

[Leslie Monis]

From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>

RFC 8033 recommends a value of 16384 bytes for the queue
threshold.

Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
---
 net/sched/sch_pie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index d1429371592f..7778eff6cdb7 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -31,7 +31,7 @@
 #include <net/pkt_sched.h>
 #include <net/inet_ecn.h>
 
-#define QUEUE_THRESHOLD 10000
+#define QUEUE_THRESHOLD 16384
 #define DQCOUNT_INVALID -1
 #define MAX_PROB  0xffffffff
 #define PIE_SCALE 8
-- 
2.17.1

[PATCH net-next v2 2/7] net: sched: pie: change default value of pie_params->target

[Leslie Monis]

From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>

RFC 8033 suggests a default value of 15 milliseconds for the
target queue delay.

Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
---
 net/sched/sch_pie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 7778eff6cdb7..91af9bf19852 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -83,7 +83,7 @@ static void pie_params_init(struct pie_params *params)
 	params->beta = 20;
 	params->tupdate = usecs_to_jiffies(30 * USEC_PER_MSEC);	/* 30 ms */
 	params->limit = 1000;	/* default of 1000 packets */
-	params->target = PSCHED_NS2TICKS(20 * NSEC_PER_MSEC);	/* 20 ms */
+	params->target = PSCHED_NS2TICKS(15 * NSEC_PER_MSEC);	/* 15 ms */
 	params->ecn = false;
 	params->bytemode = false;
 }
-- 
2.17.1

[PATCH net-next v2 3/7] net: sched: pie: change default value of pie_params->tupdate

[Leslie Monis]

From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>

RFC 8033 suggests a default value of 15 milliseconds for the
update interval.

Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
---
 net/sched/sch_pie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 91af9bf19852..702f75afc312 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -81,7 +81,7 @@ static void pie_params_init(struct pie_params *params)
 {
 	params->alpha = 2;
 	params->beta = 20;
-	params->tupdate = usecs_to_jiffies(30 * USEC_PER_MSEC);	/* 30 ms */
+	params->tupdate = usecs_to_jiffies(15 * USEC_PER_MSEC);	/* 15 ms */
 	params->limit = 1000;	/* default of 1000 packets */
 	params->target = PSCHED_NS2TICKS(15 * NSEC_PER_MSEC);	/* 15 ms */
 	params->ecn = false;
-- 
2.17.1

[PATCH net-next v2 4/7] net: sched: pie: change initial value of pie_vars->burst_time

[Leslie Monis]

From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>

RFC 8033 suggests an initial value of 150 milliseconds for
the maximum time allowed for a burst of packets.

Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
---
 net/sched/sch_pie.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 702f75afc312..d88ab53593b3 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -92,8 +92,8 @@ static void pie_vars_init(struct pie_vars *vars)
 {
 	vars->dq_count = DQCOUNT_INVALID;
 	vars->avg_dq_rate = 0;
-	/* default of 100 ms in pschedtime */
-	vars->burst_time = PSCHED_NS2TICKS(100 * NSEC_PER_MSEC);
+	/* default of 150 ms in pschedtime */
+	vars->burst_time = PSCHED_NS2TICKS(150 * NSEC_PER_MSEC);
 }
 
 static bool drop_early(struct Qdisc *sch, u32 packet_size)
-- 
2.17.1

[PATCH net-next v2 5/7] net: sched: pie: add more cases to auto-tune alpha and beta

[Leslie Monis]

From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>

The current implementation scales the local alpha and beta
variables in the calculate_probability function by the same
amount for all values of drop probability below 1%.

RFC 8033 suggests using additional cases for auto-tuning
alpha and beta when the drop probability is less than 1%.

In order to add more auto-tuning cases, MAX_PROB must be
scaled by u64 instead of u32 to prevent underflow when
scaling the local alpha and beta variables in the
calculate_probability function.

Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
---
 include/uapi/linux/pkt_sched.h |  2 +-
 net/sched/sch_pie.c            | 65 +++++++++++++++++-----------------
 2 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 0d18b1d1fbbc..1eb572ef3f27 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -954,7 +954,7 @@ enum {
 #define TCA_PIE_MAX   (__TCA_PIE_MAX - 1)
 
 struct tc_pie_xstats {
-	__u32 prob;             /* current probability */
+	__u64 prob;             /* current probability */
 	__u32 delay;            /* current delay in ms */
 	__u32 avg_dq_rate;      /* current average dq_rate in bits/pie_time */
 	__u32 packets_in;       /* total number of packets enqueued */
diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index d88ab53593b3..b9f586cfb51c 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -33,7 +33,7 @@
 
 #define QUEUE_THRESHOLD 16384
 #define DQCOUNT_INVALID -1
-#define MAX_PROB  0xffffffff
+#define MAX_PROB 0xffffffffffffffff
 #define PIE_SCALE 8
 
 /* parameters used */
@@ -49,7 +49,7 @@ struct pie_params {
 
 /* variables used */
 struct pie_vars {
-	u32 prob;		/* probability but scaled by u32 limit. */
+	u64 prob;		/* probability but scaled by u64 limit. */
 	psched_time_t burst_time;
 	psched_time_t qdelay;
 	psched_time_t qdelay_old;
@@ -99,8 +99,8 @@ static void pie_vars_init(struct pie_vars *vars)
 static bool drop_early(struct Qdisc *sch, u32 packet_size)
 {
 	struct pie_sched_data *q = qdisc_priv(sch);
-	u32 rnd;
-	u32 local_prob = q->vars.prob;
+	u64 rnd;
+	u64 local_prob = q->vars.prob;
 	u32 mtu = psched_mtu(qdisc_dev(sch));
 
 	/* If there is still burst allowance left skip random early drop */
@@ -124,11 +124,11 @@ static bool drop_early(struct Qdisc *sch, u32 packet_size)
 	 * probablity. Smaller packets will have lower drop prob in this case
 	 */
 	if (q->params.bytemode && packet_size <= mtu)
-		local_prob = (local_prob / mtu) * packet_size;
+		local_prob = (local_prob / (u64)mtu) * (u64)packet_size;
 	else
 		local_prob = q->vars.prob;
 
-	rnd = prandom_u32();
+	prandom_bytes(&rnd, 8);
 	if (rnd < local_prob)
 		return true;
 
@@ -317,9 +317,10 @@ static void calculate_probability(struct Qdisc *sch)
 	u32 qlen = sch->qstats.backlog;	/* queue size in bytes */
 	psched_time_t qdelay = 0;	/* in pschedtime */
 	psched_time_t qdelay_old = q->vars.qdelay;	/* in pschedtime */
-	s32 delta = 0;		/* determines the change in probability */
-	u32 oldprob;
-	u32 alpha, beta;
+	s64 delta = 0;		/* determines the change in probability */
+	u64 oldprob;
+	u64 alpha, beta;
+	u32 power;
 	bool update_prob = true;
 
 	q->vars.qdelay_old = q->vars.qdelay;
@@ -339,38 +340,36 @@ static void calculate_probability(struct Qdisc *sch)
 	 * value for alpha as 0.125. In this implementation, we use values 0-32
 	 * passed from user space to represent this. Also, alpha and beta have
 	 * unit of HZ and need to be scaled before they can used to update
-	 * probability. alpha/beta are updated locally below by 1) scaling them
-	 * appropriately 2) scaling down by 16 to come to 0-2 range.
-	 * Please see paper for details.
-	 *
-	 * We scale alpha and beta differently depending on whether we are in
-	 * light, medium or high dropping mode.
+	 * probability. alpha/beta are updated locally below by scaling down
+	 * by 16 to come to 0-2 range.
 	 */
-	if (q->vars.prob < MAX_PROB / 100) {
-		alpha =
-		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 7;
-		beta =
-		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 7;
-	} else if (q->vars.prob < MAX_PROB / 10) {
-		alpha =
-		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 5;
-		beta =
-		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 5;
-	} else {
-		alpha =
-		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 4;
-		beta =
-		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 4;
+	alpha = ((u64)q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 4;
+	beta = ((u64)q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 4;
+
+	/* We scale alpha and beta differently depending on how heavy the
+	 * congestion is. Please see RFC 8033 for details.
+	 */
+	if (q->vars.prob < MAX_PROB / 10) {
+		alpha >>= 1;
+		beta >>= 1;
+
+		power = 100;
+		while (q->vars.prob < MAX_PROB / (u64)power &&
+		       power <= 1000000) {
+			alpha >>= 2;
+			beta >>= 2;
+			power *= 10;
+		}
 	}
 
 	/* alpha and beta should be between 0 and 32, in multiples of 1/16 */
-	delta += alpha * ((qdelay - q->params.target));
-	delta += beta * ((qdelay - qdelay_old));
+	delta += alpha * (u64)(qdelay - q->params.target);
+	delta += beta * (u64)(qdelay - qdelay_old);
 
 	oldprob = q->vars.prob;
 
 	/* to ensure we increase probability in steps of no more than 2% */
-	if (delta > (s32)(MAX_PROB / (100 / 2)) &&
+	if (delta > (s64)(MAX_PROB / (100 / 2)) &&
 	    q->vars.prob >= MAX_PROB / 10)
 		delta = (MAX_PROB / 100) * 2;
 
-- 
2.17.1

[PATCH net-next v2 6/7] net: sched: pie: add derandomization mechanism

[Leslie Monis]

From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>

Random dropping of packets to achieve latency control may
introduce outlier situations where packets are dropped too
close to each other or too far from each other. This can
cause the real drop percentage to temporarily deviate from
the intended drop probability. In certain scenarios, such
as a small number of simultaneous TCP flows, these
deviations can cause significant deviations in link
utilization and queuing latency.

RFC 8033 suggests using a derandomization mechanism to avoid
these deviations.

Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
---
 net/sched/sch_pie.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index b9f586cfb51c..8bfd8ca500f9 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -55,8 +55,10 @@ struct pie_vars {
 	psched_time_t qdelay_old;
 	u64 dq_count;		/* measured in bytes */
 	psched_time_t dq_tstamp;	/* drain rate */
+	u64 accu_prob;		/* accumulated drop probability */
 	u32 avg_dq_rate;	/* bytes per pschedtime tick,scaled */
 	u32 qlen_old;		/* in bytes */
+	u8 accu_prob_overflows;	/* overflows of accu_prob */
 };
 
 /* statistics gathering */
@@ -91,9 +93,11 @@ static void pie_params_init(struct pie_params *params)
 static void pie_vars_init(struct pie_vars *vars)
 {
 	vars->dq_count = DQCOUNT_INVALID;
+	vars->accu_prob = 0;
 	vars->avg_dq_rate = 0;
 	/* default of 150 ms in pschedtime */
 	vars->burst_time = PSCHED_NS2TICKS(150 * NSEC_PER_MSEC);
+	vars->accu_prob_overflows = 0;
 }
 
 static bool drop_early(struct Qdisc *sch, u32 packet_size)
@@ -128,9 +132,29 @@ static bool drop_early(struct Qdisc *sch, u32 packet_size)
 	else
 		local_prob = q->vars.prob;
 
+	if (local_prob == 0) {
+		q->vars.accu_prob = 0;
+		q->vars.accu_prob_overflows = 0;
+	}
+
+	if (local_prob > MAX_PROB - q->vars.accu_prob)
+		q->vars.accu_prob_overflows++;
+
+	q->vars.accu_prob += local_prob;
+
+	if (q->vars.accu_prob_overflows == 0 &&
+	    q->vars.accu_prob < (MAX_PROB / 100) * 85)
+		return false;
+	if (q->vars.accu_prob_overflows == 8 &&
+	    q->vars.accu_prob >= MAX_PROB / 2)
+		return true;
+
 	prandom_bytes(&rnd, 8);
-	if (rnd < local_prob)
+	if (rnd < local_prob) {
+		q->vars.accu_prob = 0;
+		q->vars.accu_prob_overflows = 0;
 		return true;
+	}
 
 	return false;
 }
@@ -168,6 +192,8 @@ static int pie_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 
 out:
 	q->stats.dropped++;
+	q->vars.accu_prob = 0;
+	q->vars.accu_prob_overflows = 0;
 	return qdisc_drop(skb, sch, to_free);
 }
 
-- 
2.17.1

[PATCH net-next v2 7/7] net: sched: pie: update references

[Leslie Monis]

From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>

RFC 8033 replaces the IETF draft for PIE

Signed-off-by: Mohit P. Tahiliani <tahiliani@nitk.edu.in>
Signed-off-by: Dhaval Khandla <dhavaljkhandla26@gmail.com>
Signed-off-by: Hrishikesh Hiraskar <hrishihiraskar@gmail.com>
Signed-off-by: Manish Kumar B <bmanish15597@gmail.com>
Signed-off-by: Sachin D. Patil <sdp.sachin@gmail.com>
Signed-off-by: Leslie Monis <lesliemonis@gmail.com>
---
 net/sched/sch_pie.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 8bfd8ca500f9..78658080fcc4 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -17,9 +17,7 @@
  * University of Oslo, Norway.
  *
  * References:
- * IETF draft submission: http://tools.ietf.org/html/draft-pan-aqm-pie-00
- * IEEE  Conference on High Performance Switching and Routing 2013 :
- * "PIE: A * Lightweight Control Scheme to Address the Bufferbloat Problem"
+ * RFC 8033: https://tools.ietf.org/html/rfc8034
  */
 
 #include <linux/module.h>
-- 
2.17.1

Re: [PATCH net-next v2 0/7] net: sched: pie: align PIE implementation with RFC 8033

[Jamal Hadi Salim]

On 2019-02-25 5:20 a.m., Leslie Monis wrote:
> The current implementation of the PIE queuing discipline is according to the
> IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and the paper
> [PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
> However, a lot of necessary modifications and enhancements have been proposed
> in RFC 8033, which have not yet been incorporated in the source code of Linux.
> This patch series helps in achieving the same.
> 
> Performance tests carried out using Flent [https://flent.org/]
> 

+Cc the authors of PIE to double check these values.
Please respond (it is how open source works!) and N/ACK

cheers,
jamal

Re: [PATCH net-next v2 0/7] net: sched: pie: align PIE implementation with RFC 8033

[Jamal Hadi Salim]

On 2019-02-25 8:43 a.m., Jamal Hadi Salim wrote:
> On 2019-02-25 5:20 a.m., Leslie Monis wrote:
>> The current implementation of the PIE queuing discipline is according 
>> to the
>> IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and the 
>> paper
>> [PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
>> However, a lot of necessary modifications and enhancements have been 
>> proposed
>> in RFC 8033, which have not yet been incorporated in the source code 
>> of Linux.
>> This patch series helps in achieving the same.
>>
>> Performance tests carried out using Flent [https://flent.org/]
>>
> 
> +Cc the authors of PIE to double check these values.
> Please respond (it is how open source works!) and N/ACK

Great. Bouncing addresses.
D. Taht - can you look at this?

cheers,
jamal

Re: [PATCH net-next v2 0/7] net: sched: pie: align PIE implementation with RFC 8033

[Dave Taht]

Jamal Hadi Salim <jhs@mojatatu.com> writes:

> On 2019-02-25 8:43 a.m., Jamal Hadi Salim wrote:
>> On 2019-02-25 5:20 a.m., Leslie Monis wrote:
>>> The current implementation of the PIE queuing discipline is
>>> according to the
>>> IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and
>>> the paper
>>> [PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
>>> However, a lot of necessary modifications and enhancements have
>>> been proposed
>>> in RFC 8033, which have not yet been incorporated in the source
>>> code of Linux.
>>> This patch series helps in achieving the same.
>>>
>>> Performance tests carried out using Flent [https://flent.org/]
>>>
>>
>> +Cc the authors of PIE to double check these values.
>> Please respond (it is how open source works!) and N/ACK
>
> Great. Bouncing addresses.
> D. Taht - can you look at this?

Whose addresses are bouncing? The folk that did pie originally were
mostly contractors and long ago moved on to other things.

My taht.net address tends to bounce to vger as I long ago mandated
starttls on all email transactions which is why I use my gmail account
for postings there. I keep hoping that one day vger will support
starttls... posting it here as I'm an optimist. I wasn't expecting to be
cc'd on this submittal....

Pie seems to be mostly an abandoned CISCO effort outside of docsis-pie,
at least at the moment. I've heard not a peep from them in years.
(fq_codel seems to have mostly "won" in our world)

I did review all these changes when they went by in v1, and aside from
the ecn mistake ending up in the final RFC when I wasn't looking[1],
approve of these changes to sch_pie to make it compliant with the rfc,
finally.

[1] We proposed refining pie's ecn handling here:

https://github.com/gautamramk/FQ-PIE-for-Linux-Kernel/issues/2

Anyway:

Acked-by: Dave Taht <dave.taht@gmail.com>

Re: [PATCH net-next v2 0/7] net: sched: pie: align PIE implementation with RFC 8033

[Jamal Hadi Salim]

On 2019-02-25 9:11 a.m., Dave Taht wrote:
> Jamal Hadi Salim <jhs@mojatatu.com> writes:
> 
>> On 2019-02-25 8:43 a.m., Jamal Hadi Salim wrote:
>>> On 2019-02-25 5:20 a.m., Leslie Monis wrote:
>>>> The current implementation of the PIE queuing discipline is
>>>> according to the
>>>> IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and
>>>> the paper
>>>> [PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
>>>> However, a lot of necessary modifications and enhancements have
>>>> been proposed
>>>> in RFC 8033, which have not yet been incorporated in the source
>>>> code of Linux.
>>>> This patch series helps in achieving the same.
>>>>
>>>> Performance tests carried out using Flent [https://flent.org/]
>>>>
>>>
>>> +Cc the authors of PIE to double check these values.
>>> Please respond (it is how open source works!) and N/ACK
>>
>> Great. Bouncing addresses.
>> D. Taht - can you look at this?
> 
> Whose addresses are bouncing? The folk that did pie originally were
> mostly contractors and long ago moved on to other things.
> 

The Cisco addresses - listed as authors of PIE were bouncing.
Note: they are still listed as authors of the referenced
IETF draft with those email addresses.

> My taht.net address tends to bounce to vger as I long ago mandated
> starttls on all email transactions which is why I use my gmail account
> for postings there. I keep hoping that one day vger will support
> starttls... posting it here as I'm an optimist. I wasn't expecting to be
> cc'd on this submittal....
> 
> Pie seems to be mostly an abandoned CISCO effort outside of docsis-pie,
> at least at the moment. I've heard not a peep from them in years.
> (fq_codel seems to have mostly "won" in our world)
> 
> I did review all these changes when they went by in v1, and aside from
> the ecn mistake ending up in the final RFC when I wasn't looking[1],
> approve of these changes to sch_pie to make it compliant with the rfc,
> finally.
> 
> [1] We proposed refining pie's ecn handling here:
> 
> https://github.com/gautamramk/FQ-PIE-for-Linux-Kernel/issues/2
> 
> Anyway:
> 
> Acked-by: Dave Taht <dave.taht@gmail.com>
> 

Thats what i was hoping for. FWIW,

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>


cheers,
jamal

Re: [PATCH net-next v2 0/7] net: sched: pie: align PIE implementation with RFC 8033

[Leslie Monis]

On Mon, Feb 25, 2019 at 06:11:25AM -0800, Dave Taht wrote:
> Jamal Hadi Salim <jhs@mojatatu.com> writes:
> 
> > On 2019-02-25 8:43 a.m., Jamal Hadi Salim wrote:
> >> On 2019-02-25 5:20 a.m., Leslie Monis wrote:
> >>> The current implementation of the PIE queuing discipline is
> >>> according to the
> >>> IETF draft [http://tools.ietf.org/html/draft-pan-aqm-pie-00] and
> >>> the paper
> >>> [PIE: A Lightweight Control Scheme to Address the Bufferbloat Problem].
> >>> However, a lot of necessary modifications and enhancements have
> >>> been proposed
> >>> in RFC 8033, which have not yet been incorporated in the source
> >>> code of Linux.
> >>> This patch series helps in achieving the same.
> >>>
> >>> Performance tests carried out using Flent [https://flent.org/]
> >>>
> >>
> >> +Cc the authors of PIE to double check these values.
> >> Please respond (it is how open source works!) and N/ACK
> >
> > Great. Bouncing addresses.
> > D. Taht - can you look at this?
> 
> Whose addresses are bouncing? The folk that did pie originally were
> mostly contractors and long ago moved on to other things.
> 
> My taht.net address tends to bounce to vger as I long ago mandated
> starttls on all email transactions which is why I use my gmail account
> for postings there. I keep hoping that one day vger will support
> starttls... posting it here as I'm an optimist. I wasn't expecting to be
> cc'd on this submittal....
> 
> Pie seems to be mostly an abandoned CISCO effort outside of docsis-pie,
> at least at the moment. I've heard not a peep from them in years.
> (fq_codel seems to have mostly "won" in our world)
> 
> I did review all these changes when they went by in v1, and aside from
> the ecn mistake ending up in the final RFC when I wasn't looking[1],
> approve of these changes to sch_pie to make it compliant with the rfc,
> finally.
> 
> [1] We proposed refining pie's ecn handling here:
> 
> https://github.com/gautamramk/FQ-PIE-for-Linux-Kernel/issues/2
> 
> Anyway:
> 
> Acked-by: Dave Taht <dave.taht@gmail.com>
> 

Hi Dave,

For now we would just like to make the PIE implementation meet the 
standards as expressed by the RFC. As ECN handling is outside the 
scope of the RFC we can consider refinements to it in the code later.

Thanks,
Leslie

Re: [PATCH net-next v2 5/7] net: sched: pie: add more cases to auto-tune alpha and beta

[David Miller]

From: Leslie Monis <lesliemonis@gmail.com>
Date: Mon, 25 Feb 2019 15:50:49 +0530

> +		power = 100;
> +		while (q->vars.prob < MAX_PROB / (u64)power &&

You have to use div_u64() or similar when doing 64-bit divisons otherwise
you'll get build failures on 32-bit platforms.

Re: [PATCH net-next v2 5/7] net: sched: pie: add more cases to auto-tune alpha and beta

[Leslie Monis]

On Mon, Feb 25, 2019 at 09:40:38AM -0800, David Miller wrote:
> From: Leslie Monis <lesliemonis@gmail.com>
> Date: Mon, 25 Feb 2019 15:50:49 +0530
> 
> > +		power = 100;
> > +		while (q->vars.prob < MAX_PROB / (u64)power &&
> 
> You have to use div_u64() or similar when doing 64-bit divisons otherwise
> you'll get build failures on 32-bit platforms.

Alright. Thanks for the feedback.

Re: [PATCH net-next v2 5/7] net: sched: pie: add more cases to auto-tune alpha and beta

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1179 bytes --]

Hi Mohit,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Leslie-Monis/net-sched-pie-align-PIE-implementation-with-RFC-8033/20190226-041701
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.2.0 make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   arm-linux-gnueabi-ld: net/sched/sch_pie.o: in function `pie_timer':
>> sch_pie.c:(.text+0xc3c): undefined reference to `__aeabi_uldivmod'
>> arm-linux-gnueabi-ld: sch_pie.c:(.text+0xed8): undefined reference to `__aeabi_uldivmod'
   arm-linux-gnueabi-ld: net/sched/sch_pie.o: in function `pie_qdisc_enqueue':
   sch_pie.c:(.text+0x168c): undefined reference to `__aeabi_uldivmod'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67762 bytes --]

Re: [PATCH net-next v2 5/7] net: sched: pie: add more cases to auto-tune alpha and beta

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 851 bytes --]

Hi Mohit,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Leslie-Monis/net-sched-pie-align-PIE-implementation-with-RFC-8033/20190226-041701
config: mips-allmodconfig (attached as .config)
compiler: mips-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.2.0 make.cross ARCH=mips 

All errors (new ones prefixed by >>):

>> ERROR: "__udivdi3" [net/sched/sch_pie.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 58625 bytes --]