add RED, ARED and PIE by CepheusC · Pull Request #197 · stack-rs/rattan

CepheusC · 2026-06-09T03:44:55Z

No description provided.

un-lock-able

Please also remember to fix warnings from cargo clippy (in github actions) and run cargo fmt after modifications.

Copilot

Pull request overview

Adds three Active Queue Management (AQM) queue implementations (RED, Adaptive RED, and PIE) to rattan-core’s bandwidth cell queue module, enabling more realistic congestion signaling behavior than the existing tail/head drop queues.

Changes:

Introduces new queue implementations: RedQueue, AdaptiveRedQueue, and PieQueue, each with its own config struct and unit tests.
Exposes the new queues from rattan-core/src/cells/bandwidth/queue/mod.rs so they can be used by the rest of the crate.
Adds algorithm-specific state tracking (avg queue length / drop probability / burst allowance / rate estimation) and hard-limit enforcement (packet/byte limits).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
rattan-core/src/cells/bandwidth/queue/mod.rs	Registers and re-exports the new AQM queue modules.
rattan-core/src/cells/bandwidth/queue/red.rs	Adds RED queue implementation + tests.
rattan-core/src/cells/bandwidth/queue/ared.rs	Adds Adaptive RED (ARED) implementation + tests.
rattan-core/src/cells/bandwidth/queue/pie.rs	Adds PIE implementation + tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    fn retain<F>(&mut self, mut f: F)
+    where
+        F: FnMut(&P) -> bool,
+    {
+        self.queue.retain(|packet| f(packet));
+    }


+    fn retain<F>(&mut self, mut f: F)
+    where
+        F: FnMut(&P) -> bool,
+    {
+        self.queue.retain(|packet| f(packet));
+    }


+    fn retain<F>(&mut self, mut f: F)
+    where
+        F: FnMut(&P) -> bool,
+    {
+        self.queue.retain(|packet| f(packet));
+    }


+        std::thread::sleep(Duration::from_millis(15)); // Wait to exceed t_update
+
+        // This enqueue will trigger update_drop_probability()
+        queue.enqueue(create_packet(14));


Lethe10137 · 2026-06-12T08:48:08Z

+    fn update_max_p(&mut self) {
+        let target_min =
+            self.config.min_th as f64 + 0.4 * (self.config.max_th - self.config.min_th) as f64;
+        let target_max =
+            self.config.min_th as f64 + 0.6 * (self.config.max_th - self.config.min_th) as f64;
+        if self.average_queue_length > target_max && self.config.max_p <= 0.5 {
+            self.config.max_p += (self.config.max_p / 4.0).min(0.01);
+        } else if self.average_queue_length < target_min && self.config.max_p >= 0.01 {
+            self.config.max_p *= 0.9;
+        }
+    }


The ARED paper ask us to constrain $max_p$ is constrained to remain within the range $[0.01, 0.5]$.

Your implementation pushes 0.4999 to about 0.6249. and 0.01 to 0.009. Actually let's implement that in a simpler way.

Suggested change

fn update_max_p(&mut self) {

let target_min =

self.config.min_th as f64 + 0.4 * (self.config.max_th - self.config.min_th) as f64;

let target_max =

self.config.min_th as f64 + 0.6 * (self.config.max_th - self.config.min_th) as f64;

if self.average_queue_length > target_max && self.config.max_p <= 0.5 {

self.config.max_p += (self.config.max_p / 4.0).min(0.01);

} else if self.average_queue_length < target_min && self.config.max_p >= 0.01 {

self.config.max_p *= 0.9;

}

}

fn update_max_p(&mut self) {

let target_min =

self.config.min_th as f64 + 0.4 * (self.config.max_th - self.config.min_th) as f64;

let target_max =

self.config.min_th as f64 + 0.6 * (self.config.max_th - self.config.min_th) as f64;

if self.average_queue_length > target_max {

self.config.max_p += (self.config.max_p / 4.0).min(0.01);

} else if self.average_queue_length < target_min {

self.config.max_p *= 0.9;

}

// Clamp to [0.01, 0.5]

self.config.max_p = self.config.max_p.clamp(0.01, 0.5);

}

Lethe10137 · 2026-06-12T09:02:44Z

+                p_b / (1.0 - self.count_packet as f64 * p_b)
+            };
+
+            let rand_val = random_range(0.0..1.0);


This adds non-deterministic behavior. You can refer to how we used a seeded random generator to make the queue's behavior deterministic.

Lethe10137 · 2026-06-12T09:08:44Z

+        assert!(
+            drop_count > 0,
+            "Should have dropped some packets probabilistically"
+        );


It's not good enough to just check "drop some packets, but not all of them".

You could try to calculate actually how many packets (an exact number or a range) is expected to be dropped. Thus, when further modifications on the queue's implementation was made, we can be more confident that, there's no unexpected behavior change by the automated test.

Lethe10137

Please refer to #155 about the "logical timestamp". Conceptually these time-based queue should try to avoid calling Instant::now(), but refer to the timestamp of the packets, which is when the packet "should" be here.

Lethe10137 · 2026-06-12T09:16:29Z

+            return false;
+        }
+
+        let rand_val = random_range(0.0..1.0);


Also, try to make this deterministic with seeded Random generator.

Lethe10137 · 2026-06-12T09:17:47Z

+
+    fn update_avg_drate(&mut self, pkt_size: usize) {
+        let now = Instant::now();
+        let dq_threshold = 16384; // 16 KB


16 KiB

Suggested change

let dq_threshold = 16384; // 16 KB

let dq_threshold = 16384; // 16 KiB

Lethe10137 · 2026-06-12T09:29:53Z

+    P: Packet,
+{
+    fn update_drop_probability(&mut self) {
+        let now = Instant::now();


The now should be a parameter passed in, which is the logical timestamp of the packet that triggered this.

Lethe10137 · 2026-06-12T09:31:41Z

+        let interval_update = Instant::now().saturating_duration_since(self.start_update);
+        let t_update = Duration::from_millis(15);
+        if interval_update >= t_update {
+            self.update_drop_probability();


Pass packet.get_timestamp() as an argument. The queue should use this as the "logical time", rather than Instant::now().

Lethe10137 · 2026-06-12T10:19:08Z

+        std::thread::sleep(Duration::from_millis(15)); // Wait to exceed t_update
+
+        // This enqueue will trigger update_drop_probability()
+        queue.enqueue(create_packet(14));


If the queue were working based on the logical timestamp (the timestamp of the packets), you could just modify the timestamp of the packets.

Lethe10137 · 2026-06-12T10:19:10Z

+        queue.enqueue(create_packet(14));
+
+        assert!(
+            queue.p > 0.0,


What probability should it be updated to? Please make this more strict.

Lethe10137 · 2026-06-12T10:19:59Z

+        }
+
+        if let Some(idle_start) = self.idle_start {
+            let now = Instant::now();


Also, try to use the logical timestamp instead.

Lethe10137 · 2026-06-14T05:24:51Z

+    fn enqueue(&mut self, packet: P) {
+        // Simulate time-driven with event-driven approach
+        let interval_update = Instant::now().saturating_duration_since(self.start_update);
+        let t_update = Duration::from_millis(15);


Please this update interval configurable and default to be 15ms, according to RFC 8033:

The update interval, T_UPDATE, is defaulted to be 15 milliseconds.
It MAY be reduced on high-speed links in order to provide smoother
response. The target latency value, QDELAY_REF, SHOULD be set to 15
milliseconds.

EwecaBcD · 2026-06-14T06:35:58Z

+        }
+
+        let packet_size = packet.l3_length() + self.get_extra_length();
+        let pass_hard_limit = self


This pass_hard_limit actually means whether current queue length does NOT pass the packet_limit or byte_limit. Please consider to use a better variable name.

EwecaBcD · 2026-06-14T07:00:06Z

+        }
+
+        let packet_size = packet.l3_length() + self.get_extra_length();
+        let pass_hard_limit = self


Same problem as pass_hard_limit in ARED implementation.

EwecaBcD · 2026-06-14T07:19:03Z

+where
+    P: Packet,
+{
+    fn update_drop_probability(&mut self) {


According to RFC 8033 and the PIE implementation in Linux kernel (see https://github.com/torvalds/linux/blob/master/net/sched/sch_pie.c#L425), this update happens every 15ms periodically. But in your implementation, it seems to happen only when there is a packet trying to enqueue or dequeue. This may lead to inconsistency between your implementation and the standard implementation.

Consider to add a real timer to trigger this update event, or use some circular update logic in enqueue and dequeue just like this:

let mut interval_update = Instant::now().saturating_duration_since(self.start_update); let t_update = Duration::from_millis(15); while interval_update >= t_update { self.update_drop_probability(t_update.as_secs_f64() * 1000.0); interval_update = Instant::now().saturating_duration_since(self.start_update); }

EwecaBcD · 2026-06-14T07:36:19Z

+                        let epsilon = 0.125;
+                        self.avg_drate = (1.0 - epsilon) * self.avg_drate + epsilon * dq_rate;
+                    }
+                    self.start_measurement = Some(now);


It seems that the measurement should be ended here according to the pseudocode in RFC 8033 appendix B:

if ( PIE->dq_count_ >= DQ_THRESHOLD) { ... PIE->in_measurement_ = FALSE; }

Please check your implementation.

EwecaBcD · 2026-06-14T07:44:02Z

+        } else if self.p < 0.1 {
+            p_increment /= 2.0;
+        }
+        self.p += p_increment;


I notice that there is a capping logic in the pseudocode in RFC 8033 appendix B:

if (PIE->drop_prob_ >= 0.1 && p > 0.02) { p = 0.02; }

But you don't have this in your code. Relative content is in RFC 8033 Section 5.5: Cap Drop Adjustment. Please check if you should add this adjustment to your implementation.

EwecaBcD · 2026-06-14T07:49:29Z

+
+        // RFC 8033 Section 4.1: Bypass random drop logic to be work conserving
+        let bypass_drop =
+            (self.old_del < self.config.ref_del / 2.0 && self.p < 0.2) || self.queue.len() <= 2;


In the pseudocode, the bypass_drop is relative to the byte length of the queue, not the packet length. Please check your implementation.

if ( (PIE->qdelay_old_ < QDELAY_REF/2 && PIE->drop_prob_ < 0.2) || (queue_.byte_length() <= 2 * MEAN_PKTSIZE) ) { return ENQUE; }

EwecaBcD · 2026-06-14T08:20:32Z

+        self.update_avg();
+
+        let packet_size = packet.l3_length() + self.get_extra_length();
+        let pass_hard_limit = self


Same problem as pass_hard_limit in ARED implementation.

EwecaBcD · 2026-06-14T08:23:50Z

+                false
+            }
+        } else if avg >= max_th {
+            self.count_packet = 0;


Same problem as your ARED implementation.

EwecaBcD · 2026-06-14T08:24:30Z

+                false
+            }
+        } else if avg >= max_th {
+            self.count_packet = 0;


According to the code in Linux kernel (see https://github.com/torvalds/linux/blob/master/include/net/red.h#L435), the count_packet should be set to -1 when avg >= max_th.

EwecaBcD · 2026-06-14T08:30:00Z

+        if let Some(idle_start) = self.idle_start {
+            let now = Instant::now();
+            let idle_duration = now.saturating_duration_since(idle_start);
+            let pkt_tx_time = 120.0; // 1500 bytes * 8 / 100Mbps = 120 us


Why it's hard-coded 100Mbps here? The bandwidth can be customized to any value.

EwecaBcD · 2026-06-14T08:31:03Z

+        if let Some(idle_start) = self.idle_start {
+            let now = Instant::now();
+            let idle_duration = now.saturating_duration_since(idle_start);
+            let pkt_tx_time = 120.0; // 1500 bytes * 8 / 100Mbps = 120 us


Same problem as your ARED implementation.

duanxy23 added 7 commits June 6, 2026 14:25

add red (without unit tests)

888cedf

add red unit tests

4810dea

add ared & fix red

713d2f1

add ared unit tests

18360e8

add pie (without unit tests)

52df2e9

update pie

1109872

update pie unit tests

608ff1b

BobAnkh requested review from Centaurus99 and un-lock-able June 9, 2026 07:41

Centaurus99 requested a review from EwecaBcD June 9, 2026 07:43

un-lock-able reviewed Jun 9, 2026

View reviewed changes

duanxy23 added 3 commits June 9, 2026 19:00

fix un-lock-able's review on ared

94d0450

fix warnings from cargo clippy (in github actions) and run cargo fmt

d86554a

fix function update_avg of RED

b482bd9

un-lock-able reviewed Jun 9, 2026

View reviewed changes

Comment thread rattan-core/src/cells/bandwidth/queue/red.rs

duanxy23 added 2 commits June 9, 2026 23:43

fix clippy warning & corresponding logic

a035791

revert enqueue() by using early return

affbf9e

Centaurus99 requested review from Lethe10137 and Copilot and removed request for EwecaBcD June 12, 2026 06:04

Copilot started reviewing on behalf of Centaurus99 June 12, 2026 06:05 View session

Copilot AI reviewed Jun 12, 2026

View reviewed changes

Centaurus99 requested a review from EwecaBcD June 12, 2026 07:00

Lethe10137 reviewed Jun 12, 2026

View reviewed changes

Lethe10137 requested changes Jun 12, 2026

View reviewed changes

Lethe10137 reviewed Jun 14, 2026

View reviewed changes

EwecaBcD reviewed Jun 14, 2026

View reviewed changes

	let dq_threshold = 16384; // 16 KB
	let dq_threshold = 16384; // 16 KiB

Conversation

CepheusC commented Jun 9, 2026

Uh oh!

un-lock-able left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Lethe10137 Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lethe10137 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Lethe10137 Jun 12, 2026 •

edited

Loading