Skip to content

refactor(cell): change PacketQueue::dequeue to dequeue_at(ts) - #198

Merged
Centaurus99 merged 3 commits into
stack-rs:mainfrom
CepheusC:dequeue
Jul 9, 2026
Merged

refactor(cell): change PacketQueue::dequeue to dequeue_at(ts)#198
Centaurus99 merged 3 commits into
stack-rs:mainfrom
CepheusC:dequeue

Conversation

@CepheusC

@CepheusC CepheusC commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Add an Option<Instant> timestamp parameter to PacketQueue::dequeue() to support time-aware queue operations. The CoDel queue uses this parameter to avoid Instant::now() calls, while simple queue implementations (DropHead, DropTail,
Infinite) ignore it with None.

Usage

This change lays the groundwork for PR #197 (redpie branch) by providing the necessary timestamp plumbing in the PacketQueue trait. The subsequent PR will make use of this parameter to implement its feature.

@BobAnkh

BobAnkh commented Jul 3, 2026

Copy link
Copy Markdown
Member

This shall never be an optional argument. And please rename the interface from dequeue to dequeue_at. @CepheusC

For simple queues, they can just ignore the value (but we shall still pass the actual value from the cell), as they do not require it. But the semantics of this interface should be fixed to: the cell wants to dequeue a packet at the timestamp, as in our design philosophy, the queue is a passive-driven component.

@BobAnkh BobAnkh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@BobAnkh

BobAnkh commented Jul 4, 2026

Copy link
Copy Markdown
Member

Wait for extra two approvals (from @Centaurus99 , @un-lock-able or @Lethe10137 ) to merge this.

@un-lock-able un-lock-able left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread rattan-core/src/cells/bandwidth/queue/codel.rs Outdated
Comment thread rattan-core/src/cells/bandwidth/queue/codel.rs Outdated
@Centaurus99

Copy link
Copy Markdown
Member

In addition, please squash the changes into a single commit and update the PR title accordingly. @CepheusC

@BobAnkh

BobAnkh commented Jul 9, 2026

Copy link
Copy Markdown
Member

In addition, please squash the changes into a single commit and update the PR title accordingly. @CepheusC

We could squash ourselves when merging, so he doesn't need to worry about that and you can have a clear history to review changes. Also, we should try to compact the head into 50 characters instead of spliting into two lines.

@Centaurus99 Centaurus99 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Centaurus99 Centaurus99 changed the title refactor(queue): add optional timestamp parameter to PacketQueue::dequeue refactor(cell): change PacketQueue::dequeue to dequeue_at(timestamp) Jul 9, 2026
@Centaurus99 Centaurus99 changed the title refactor(cell): change PacketQueue::dequeue to dequeue_at(timestamp) refactor(cell): change PacketQueue::dequeue to dequeue_at(ts) Jul 9, 2026
@Centaurus99
Centaurus99 merged commit a11e8a1 into stack-rs:main Jul 9, 2026
6 of 13 checks passed
@BobAnkh BobAnkh mentioned this pull request Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants