All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033
@ 2018-10-31 16:19 Leslie Monis
  2018-10-31 16:19 ` [PATCH net-next 1/8] net: sched: pie: change value of QUEUE_THRESHOLD Leslie Monis
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Leslie Monis @ 2018-10-31 16:19 UTC (permalink / raw)
  To: jhs
  Cc: netdev, tahiliani, dhavaljkhandla26, hrishihiraskar,
	bmanish15597, sdp.sachin

The current implementation of PIE queueing discipline is according to an 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
kernel. The following series of patches helps in achieving the same.

This patch series includes:

1. Change the value of QUEUE_THRESHOLD
2. Change the default value of pie_params->target
3. Change the default value of pie_params->tupdate
4. Change the initial value of pie_vars->burst_time
5. Add more conditions to auto-tune alpha and beta
6. Add mechanism to set PIE active/inactive
7. Add a derandomization mechanism
8. Update references

Mohit P. Tahiliani (8):
  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 conditions to auto-tune alpha and beta
  net: sched: pie: add mechanism to set PIE active/inactive
  net: sched: pie: add derandomization mechanism
  net: sched: pie: update references

 net/sched/sch_pie.c | 77 +++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 63 insertions(+), 14 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH net-next 1/8] net: sched: pie: change value of QUEUE_THRESHOLD
  2018-10-31 16:19 [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033 Leslie Monis
@ 2018-10-31 16:19 ` Leslie Monis
  2018-10-31 16:19 ` [PATCH net-next 2/8] net: sched: pie: change default value of pie_params->target Leslie Monis
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Leslie Monis @ 2018-10-31 16:19 UTC (permalink / raw)
  To: jhs
  Cc: netdev, tahiliani, dhavaljkhandla26, hrishihiraskar,
	bmanish15597, sdp.sachin

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

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

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 d142937..56c9e4d 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	/* 16KB i.e. 2^14 bytes */
 #define DQCOUNT_INVALID -1
 #define MAX_PROB  0xffffffff
 #define PIE_SCALE 8
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 2/8] net: sched: pie: change default value of pie_params->target
  2018-10-31 16:19 [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033 Leslie Monis
  2018-10-31 16:19 ` [PATCH net-next 1/8] net: sched: pie: change value of QUEUE_THRESHOLD Leslie Monis
@ 2018-10-31 16:19 ` Leslie Monis
  2018-10-31 16:19 ` [PATCH net-next 3/8] net: sched: pie: change default value of pie_params->tupdate Leslie Monis
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Leslie Monis @ 2018-10-31 16:19 UTC (permalink / raw)
  To: jhs
  Cc: netdev, tahiliani, dhavaljkhandla26, hrishihiraskar,
	bmanish15597, sdp.sachin

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

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

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 56c9e4d..c5d6d6b 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.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 3/8] net: sched: pie: change default value of pie_params->tupdate
  2018-10-31 16:19 [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033 Leslie Monis
  2018-10-31 16:19 ` [PATCH net-next 1/8] net: sched: pie: change value of QUEUE_THRESHOLD Leslie Monis
  2018-10-31 16:19 ` [PATCH net-next 2/8] net: sched: pie: change default value of pie_params->target Leslie Monis
@ 2018-10-31 16:19 ` Leslie Monis
  2018-10-31 16:19 ` [PATCH net-next 4/8] net: sched: pie: change initial value of pie_vars->burst_time Leslie Monis
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Leslie Monis @ 2018-10-31 16:19 UTC (permalink / raw)
  To: jhs
  Cc: netdev, tahiliani, dhavaljkhandla26, hrishihiraskar,
	bmanish15597, sdp.sachin

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

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

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 c5d6d6b..9912d9cc 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.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 4/8] net: sched: pie: change initial value of pie_vars->burst_time
  2018-10-31 16:19 [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033 Leslie Monis
                   ` (2 preceding siblings ...)
  2018-10-31 16:19 ` [PATCH net-next 3/8] net: sched: pie: change default value of pie_params->tupdate Leslie Monis
@ 2018-10-31 16:19 ` Leslie Monis
  2018-10-31 16:19 ` [PATCH net-next 5/8] net: sched: pie: add more conditions to auto-tune alpha and beta Leslie Monis
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Leslie Monis @ 2018-10-31 16:19 UTC (permalink / raw)
  To: jhs
  Cc: netdev, tahiliani, dhavaljkhandla26, hrishihiraskar,
	bmanish15597, sdp.sachin

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 instead of
100 milliseconds.

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 9912d9cc..f4e189a 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.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 5/8] net: sched: pie: add more conditions to auto-tune alpha and beta
  2018-10-31 16:19 [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033 Leslie Monis
                   ` (3 preceding siblings ...)
  2018-10-31 16:19 ` [PATCH net-next 4/8] net: sched: pie: change initial value of pie_vars->burst_time Leslie Monis
@ 2018-10-31 16:19 ` Leslie Monis
  2018-10-31 16:40   ` Stephen Hemminger
  2018-10-31 16:19 ` [PATCH net-next 6/8] net: sched: pie: add mechanism to set PIE active/inactive Leslie Monis
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Leslie Monis @ 2018-10-31 16:19 UTC (permalink / raw)
  To: jhs
  Cc: netdev, tahiliani, dhavaljkhandla26, hrishihiraskar,
	bmanish15597, sdp.sachin

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

The update in drop probability depends on the parameters
alpha and beta, which in turn reflect the current congestion
level. However, the previous if-else cases were recommended
when the supported bandwidth was up to 12 Mbps but, current
data links support a much higher bandwidth, and the
requirement for more bandwidth is in never-ending demand.
Hence, RFC 8033 suggests using more if-else cases for better
fine-tuning of parameters alpha and beta in order to control
the congestion as much as possible.

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 | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index f4e189a..c84e91e 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -343,10 +343,30 @@ static void calculate_probability(struct Qdisc *sch)
 	 * 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.
+	 * We scale alpha and beta differently depending on how heavy the
+	 * congestion is.
 	 */
-	if (q->vars.prob < MAX_PROB / 100) {
+	if (q->vars.prob < MAX_PROB / 1000000) {
+		alpha =
+		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 15;
+		beta =
+		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 15;
+	} else if (q->vars.prob < MAX_PROB / 100000) {
+		alpha =
+		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 13;
+		beta =
+		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 13;
+	} else if (q->vars.prob < MAX_PROB / 10000) {
+		alpha =
+		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 11;
+		beta =
+		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 11;
+	} else if (q->vars.prob < MAX_PROB / 1000) {
+		alpha =
+		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 9;
+		beta =
+		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 9;
+	} else if (q->vars.prob < MAX_PROB / 100) {
 		alpha =
 		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 7;
 		beta =
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 6/8] net: sched: pie: add mechanism to set PIE active/inactive
  2018-10-31 16:19 [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033 Leslie Monis
                   ` (4 preceding siblings ...)
  2018-10-31 16:19 ` [PATCH net-next 5/8] net: sched: pie: add more conditions to auto-tune alpha and beta Leslie Monis
@ 2018-10-31 16:19 ` Leslie Monis
  2018-10-31 16:41   ` Stephen Hemminger
  2018-10-31 16:19 ` [PATCH net-next 7/8] net: sched: pie: add derandomization mechanism Leslie Monis
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Leslie Monis @ 2018-10-31 16:19 UTC (permalink / raw)
  To: jhs
  Cc: netdev, tahiliani, dhavaljkhandla26, hrishihiraskar,
	bmanish15597, sdp.sachin

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

To overcome unnecessary packet drops due to a spurious
uptick in queuing latency caused by fluctuations in a
network, PIE can choose to be active only when the queue
occupancy is over a certain threshold. RFC 8033 suggests
the value of this threshold be 1/3 of the tail drop
threshold. PIE becomes inactive when the congestion ends
i.e., when the drop probability reaches 0, and the current
and previous latency samples are all below half of 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 | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index c84e91e..b68b367 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -57,6 +57,7 @@ struct pie_vars {
 	psched_time_t dq_tstamp;	/* drain rate */
 	u32 avg_dq_rate;	/* bytes per pschedtime tick,scaled */
 	u32 qlen_old;		/* in bytes */
+	bool active;		/* inactive/active */
 };
 
 /* statistics gathering */
@@ -94,6 +95,7 @@ static void pie_vars_init(struct pie_vars *vars)
 	vars->avg_dq_rate = 0;
 	/* default of 150 ms in pschedtime */
 	vars->burst_time = PSCHED_NS2TICKS(150 * NSEC_PER_MSEC);
+	vars->active = true;
 }
 
 static bool drop_early(struct Qdisc *sch, u32 packet_size)
@@ -141,12 +143,23 @@ static int pie_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 	struct pie_sched_data *q = qdisc_priv(sch);
 	bool enqueue = false;
 
+	if (!q->vars.active && qdisc_qlen(sch) >= sch->limit / 3) {
+		/* If the queue occupancy is over 1/3 of the tail drop
+		 * threshold, turn on PIE.
+		 */
+		pie_vars_init(&q->vars);
+		q->vars.prob = 0;
+		q->vars.qdelay_old = 0;
+		q->vars.dq_count = 0;
+		q->vars.dq_tstamp = psched_get_time();
+	}
+
 	if (unlikely(qdisc_qlen(sch) >= sch->limit)) {
 		q->stats.overlimit++;
 		goto out;
 	}
 
-	if (!drop_early(sch, skb->len)) {
+	if (!q->vars.active || !drop_early(sch, skb->len)) {
 		enqueue = true;
 	} else if (q->params.ecn && (q->vars.prob <= MAX_PROB / 10) &&
 		   INET_ECN_set_ce(skb)) {
@@ -431,7 +444,7 @@ static void calculate_probability(struct Qdisc *sch)
 	q->vars.qdelay = qdelay;
 	q->vars.qlen_old = qlen;
 
-	/* We restart the measurement cycle if the following conditions are met
+	/* We turn off PIE if the following conditions are met
 	 * 1. If the delay has been low for 2 consecutive Tupdate periods
 	 * 2. Calculated drop probability is zero
 	 * 3. We have atleast one estimate for the avg_dq_rate ie.,
@@ -441,7 +454,7 @@ static void calculate_probability(struct Qdisc *sch)
 	    (q->vars.qdelay_old < q->params.target / 2) &&
 	    q->vars.prob == 0 &&
 	    q->vars.avg_dq_rate > 0)
-		pie_vars_init(&q->vars);
+		q->vars.active = false;
 }
 
 static void pie_timer(struct timer_list *t)
@@ -451,7 +464,8 @@ static void pie_timer(struct timer_list *t)
 	spinlock_t *root_lock = qdisc_lock(qdisc_root_sleeping(sch));
 
 	spin_lock(root_lock);
-	calculate_probability(sch);
+	if (q->vars.active)
+		calculate_probability(sch);
 
 	/* reset the timer to fire after 'tupdate'. tupdate is in jiffies. */
 	if (q->params.tupdate)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 7/8] net: sched: pie: add derandomization mechanism
  2018-10-31 16:19 [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033 Leslie Monis
                   ` (5 preceding siblings ...)
  2018-10-31 16:19 ` [PATCH net-next 6/8] net: sched: pie: add mechanism to set PIE active/inactive Leslie Monis
@ 2018-10-31 16:19 ` Leslie Monis
  2018-10-31 16:38   ` Stephen Hemminger
  2018-10-31 16:19 ` [PATCH net-next 8/8] net: sched: pie: update references Leslie Monis
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Leslie Monis @ 2018-10-31 16:19 UTC (permalink / raw)
  To: jhs
  Cc: netdev, tahiliani, dhavaljkhandla26, hrishihiraskar,
	bmanish15597, sdp.sachin

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 | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index b68b367..88e605c 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -58,6 +58,7 @@ struct pie_vars {
 	u32 avg_dq_rate;	/* bytes per pschedtime tick,scaled */
 	u32 qlen_old;		/* in bytes */
 	bool active;		/* inactive/active */
+	u64 accu_prob;		/* accumulated drop probability */
 };
 
 /* statistics gathering */
@@ -96,6 +97,7 @@ static void pie_vars_init(struct pie_vars *vars)
 	/* default of 150 ms in pschedtime */
 	vars->burst_time = PSCHED_NS2TICKS(150 * NSEC_PER_MSEC);
 	vars->active = true;
+	vars->accu_prob = 0;
 }
 
 static bool drop_early(struct Qdisc *sch, u32 packet_size)
@@ -130,9 +132,21 @@ 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 += local_prob;
+
+	if (q->vars.accu_prob < (MAX_PROB / 100) * 85)
+		return false;
+	if (q->vars.accu_prob >= ((u64)MAX_PROB * 17) / 2)
+		return true;
+
 	rnd = prandom_u32();
-	if (rnd < local_prob)
+	if (rnd < local_prob) {
+		q->vars.accu_prob = 0;
 		return true;
+	}
 
 	return false;
 }
@@ -181,6 +195,7 @@ static int pie_qdisc_enqueue(struct sk_buff *skb, struct Qdisc *sch,
 
 out:
 	q->stats.dropped++;
+	q->vars.accu_prob = 0;
 	return qdisc_drop(skb, sch, to_free);
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next 8/8] net: sched: pie: update references
  2018-10-31 16:19 [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033 Leslie Monis
                   ` (6 preceding siblings ...)
  2018-10-31 16:19 ` [PATCH net-next 7/8] net: sched: pie: add derandomization mechanism Leslie Monis
@ 2018-10-31 16:19 ` Leslie Monis
  2018-10-31 16:36 ` [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033 Stephen Hemminger
  2018-10-31 17:43 ` David Miller
  9 siblings, 0 replies; 14+ messages in thread
From: Leslie Monis @ 2018-10-31 16:19 UTC (permalink / raw)
  To: jhs
  Cc: netdev, tahiliani, dhavaljkhandla26, hrishihiraskar,
	bmanish15597, sdp.sachin

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 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
index 88e605c..708e8ad 100644
--- a/net/sched/sch_pie.c
+++ b/net/sched/sch_pie.c
@@ -17,7 +17,7 @@
  * University of Oslo, Norway.
  *
  * References:
- * IETF draft submission: http://tools.ietf.org/html/draft-pan-aqm-pie-00
+ * RFC 8033: https://tools.ietf.org/html/rfc8033
  * IEEE  Conference on High Performance Switching and Routing 2013 :
  * "PIE: A * Lightweight Control Scheme to Address the Bufferbloat Problem"
  */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033
  2018-10-31 16:19 [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033 Leslie Monis
                   ` (7 preceding siblings ...)
  2018-10-31 16:19 ` [PATCH net-next 8/8] net: sched: pie: update references Leslie Monis
@ 2018-10-31 16:36 ` Stephen Hemminger
  2018-10-31 17:43 ` David Miller
  9 siblings, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2018-10-31 16:36 UTC (permalink / raw)
  To: Leslie Monis
  Cc: jhs, netdev, tahiliani, dhavaljkhandla26, hrishihiraskar,
	bmanish15597, sdp.sachin

On Wed, 31 Oct 2018 21:49:24 +0530
Leslie Monis <lesliemonis@gmail.com> wrote:

> The current implementation of PIE queueing discipline is according to an 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
> kernel. The following series of patches helps in achieving the same.
> 
> This patch series includes:
> 
> 1. Change the value of QUEUE_THRESHOLD
> 2. Change the default value of pie_params->target
> 3. Change the default value of pie_params->tupdate
> 4. Change the initial value of pie_vars->burst_time
> 5. Add more conditions to auto-tune alpha and beta
> 6. Add mechanism to set PIE active/inactive
> 7. Add a derandomization mechanism
> 8. Update references
> 
> Mohit P. Tahiliani (8):
>   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 conditions to auto-tune alpha and beta
>   net: sched: pie: add mechanism to set PIE active/inactive
>   net: sched: pie: add derandomization mechanism
>   net: sched: pie: update references
> 
>  net/sched/sch_pie.c | 77 +++++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 63 insertions(+), 14 deletions(-)
> 

Did you do performance tests? Often the RFC is out of date and
the actual values are better than those in the standard.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next 7/8] net: sched: pie: add derandomization mechanism
  2018-10-31 16:19 ` [PATCH net-next 7/8] net: sched: pie: add derandomization mechanism Leslie Monis
@ 2018-10-31 16:38   ` Stephen Hemminger
  0 siblings, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2018-10-31 16:38 UTC (permalink / raw)
  To: Leslie Monis
  Cc: jhs, netdev, tahiliani, dhavaljkhandla26, hrishihiraskar,
	bmanish15597, sdp.sachin

On Wed, 31 Oct 2018 21:49:31 +0530
Leslie Monis <lesliemonis@gmail.com> wrote:

> diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
> index b68b367..88e605c 100644
> --- a/net/sched/sch_pie.c
> +++ b/net/sched/sch_pie.c
> @@ -58,6 +58,7 @@ struct pie_vars {
>  	u32 avg_dq_rate;	/* bytes per pschedtime tick,scaled */
>  	u32 qlen_old;		/* in bytes */
>  	bool active;		/* inactive/active */
> +	u64 accu_prob;		/* accumulated drop probability */
>  };
>  

Although putting it at the end seems like a natural place, it creates
holes in the c structure. My recommendation would be to put the new
field after dq_tsstamp.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next 5/8] net: sched: pie: add more conditions to auto-tune alpha and beta
  2018-10-31 16:19 ` [PATCH net-next 5/8] net: sched: pie: add more conditions to auto-tune alpha and beta Leslie Monis
@ 2018-10-31 16:40   ` Stephen Hemminger
  0 siblings, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2018-10-31 16:40 UTC (permalink / raw)
  To: Leslie Monis
  Cc: jhs, netdev, tahiliani, dhavaljkhandla26, hrishihiraskar,
	bmanish15597, sdp.sachin

On Wed, 31 Oct 2018 21:49:29 +0530
Leslie Monis <lesliemonis@gmail.com> wrote:

> From: "Mohit P. Tahiliani" <tahiliani@nitk.edu.in>
> 
> The update in drop probability depends on the parameters
> alpha and beta, which in turn reflect the current congestion
> level. However, the previous if-else cases were recommended
> when the supported bandwidth was up to 12 Mbps but, current
> data links support a much higher bandwidth, and the
> requirement for more bandwidth is in never-ending demand.
> Hence, RFC 8033 suggests using more if-else cases for better
> fine-tuning of parameters alpha and beta in order to control
> the congestion as much as possible.
> 
> 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 | 26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
> index f4e189a..c84e91e 100644
> --- a/net/sched/sch_pie.c
> +++ b/net/sched/sch_pie.c
> @@ -343,10 +343,30 @@ static void calculate_probability(struct Qdisc *sch)
>  	 * 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.
> +	 * We scale alpha and beta differently depending on how heavy the
> +	 * congestion is.
>  	 */
> -	if (q->vars.prob < MAX_PROB / 100) {
> +	if (q->vars.prob < MAX_PROB / 1000000) {
> +		alpha =
> +		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 15;
> +		beta =
> +		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 15;
> +	} else if (q->vars.prob < MAX_PROB / 100000) {
> +		alpha =
> +		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 13;
> +		beta =
> +		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 13;
> +	} else if (q->vars.prob < MAX_PROB / 10000) {
> +		alpha =
> +		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 11;
> +		beta =
> +		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 11;
> +	} else if (q->vars.prob < MAX_PROB / 1000) {
> +		alpha =
> +		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 9;
> +		beta =
> +		    (q->params.beta * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 9;
> +	} else if (q->vars.prob < MAX_PROB / 100) {
>  		alpha =
>  		    (q->params.alpha * (MAX_PROB / PSCHED_TICKS_PER_SEC)) >> 7;
>  		beta =


Seems like the if/else chain is getting long in the tail. Maybe a loop
or table driven approach would be clearer.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next 6/8] net: sched: pie: add mechanism to set PIE active/inactive
  2018-10-31 16:19 ` [PATCH net-next 6/8] net: sched: pie: add mechanism to set PIE active/inactive Leslie Monis
@ 2018-10-31 16:41   ` Stephen Hemminger
  0 siblings, 0 replies; 14+ messages in thread
From: Stephen Hemminger @ 2018-10-31 16:41 UTC (permalink / raw)
  To: Leslie Monis
  Cc: jhs, netdev, tahiliani, dhavaljkhandla26, hrishihiraskar,
	bmanish15597, sdp.sachin

On Wed, 31 Oct 2018 21:49:30 +0530
Leslie Monis <lesliemonis@gmail.com> wrote:

> diff --git a/net/sched/sch_pie.c b/net/sched/sch_pie.c
> index c84e91e..b68b367 100644
> --- a/net/sched/sch_pie.c
> +++ b/net/sched/sch_pie.c
> @@ -57,6 +57,7 @@ struct pie_vars {
>  	psched_time_t dq_tstamp;	/* drain rate */
>  	u32 avg_dq_rate;	/* bytes per pschedtime tick,scaled */
>  	u32 qlen_old;		/* in bytes */
> +	bool active;		/* inactive/active */
>  };

Current Linux best practice is to not use bool for true/false values
in a structure. This is because the size of bool is not obvious and
can cause padding.

Recommend using u8 instead.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033
  2018-10-31 16:19 [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033 Leslie Monis
                   ` (8 preceding siblings ...)
  2018-10-31 16:36 ` [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033 Stephen Hemminger
@ 2018-10-31 17:43 ` David Miller
  9 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2018-10-31 17:43 UTC (permalink / raw)
  To: lesliemonis
  Cc: jhs, netdev, tahiliani, dhavaljkhandla26, hrishihiraskar,
	bmanish15597, sdp.sachin


net-next is closed, please resubmit this when net-next opens back up.

Thank you.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2018-11-01  2:42 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-31 16:19 [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033 Leslie Monis
2018-10-31 16:19 ` [PATCH net-next 1/8] net: sched: pie: change value of QUEUE_THRESHOLD Leslie Monis
2018-10-31 16:19 ` [PATCH net-next 2/8] net: sched: pie: change default value of pie_params->target Leslie Monis
2018-10-31 16:19 ` [PATCH net-next 3/8] net: sched: pie: change default value of pie_params->tupdate Leslie Monis
2018-10-31 16:19 ` [PATCH net-next 4/8] net: sched: pie: change initial value of pie_vars->burst_time Leslie Monis
2018-10-31 16:19 ` [PATCH net-next 5/8] net: sched: pie: add more conditions to auto-tune alpha and beta Leslie Monis
2018-10-31 16:40   ` Stephen Hemminger
2018-10-31 16:19 ` [PATCH net-next 6/8] net: sched: pie: add mechanism to set PIE active/inactive Leslie Monis
2018-10-31 16:41   ` Stephen Hemminger
2018-10-31 16:19 ` [PATCH net-next 7/8] net: sched: pie: add derandomization mechanism Leslie Monis
2018-10-31 16:38   ` Stephen Hemminger
2018-10-31 16:19 ` [PATCH net-next 8/8] net: sched: pie: update references Leslie Monis
2018-10-31 16:36 ` [PATCH net-next 0/8] net: sched: pie: align PIE implementation with RFC 8033 Stephen Hemminger
2018-10-31 17:43 ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.