-
Notifications
You must be signed in to change notification settings - Fork 4
feat: logical timestamp & token bucket (Rebase of #145) #155
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
def9000 to
9751584
Compare
|
@Lethe10137 Please rebase the PR first. Ping me when it is ready for review |
f6ded6c to
e1a3bdb
Compare
e1a3bdb to
25bc485
Compare
1379328 to
0bd9673
Compare
By modifying the Delay cell, the recent commit (3fb0ac3) has (slightly) better tput than current main branch. |
493239d to
d487b9c
Compare
|
@Lethe10137 cargo-audit fails, which means your new depencency have security vulnerability, please check which depencency use this. |
BobAnkh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't clearly checked the TBF's implementation, but let's first discuss the bandwidth cell. Let's decide how to define our logic and physical meaning of next_available together with the timestamp on the packet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we may need further discussion on how to set the current time for a packet within bandwidth-like cells. Given that we currently send packets at next_available, but they only arrive after next_available + transfer_time, should we update the packet's timestamp to next_available + transfer_time? In the current implementation (including previous versions), our packets are expected to arrive at the next cell at the time of next_available. I believe there may be design flaws in our previous approach. We may need some further discussion with @Lethe10137 and @Centaurus99
|
Please review the bandwidth cell only, @Centaurus99 |
|
@Centaurus99 Please check it, as |
The tput performance of current PR, 5d9b158, is slightly better than the main branch.
|
|
@Lethe10137 Is everything ready for review again? |


Rebase #145 to #150.