Skip to content
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

Do not count packets that are still within their timeout window in packet loss calculation. #157

Closed
wants to merge 5 commits into from

Conversation

bkuker
Copy link

@bkuker bkuker commented Aug 21, 2019

Fix for issue #156 .

Adds a circular buffer of ping sent times int *window of length at least ceil(timeout/period). Entries are set to the send time when the ping is sent, and cleared (set to int_min) when a response is received.

During the per-receive stats print out www.csh.rit.edu : [3], 228 bytes, 77.4 ms (77.5 avg, 42% loss) these sent times are compared to the current time + the timeout. Any requests that are still unanswered, but within their timeout window, are excluded from the packet loss calculation.

Please excuse all the extra fprintfs, I will clean out my debugging output as I finalize this PR, if you indicate that it might be accepted.

Thank you!

Bill Kuker added 5 commits August 21, 2019 11:42
…ppy ascii art

Todo, put sent times in window instead?
and timeout. Window size is no longer critical, and results are
correct when timeout is not a multiple of period.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) to 79.618% when pulling 17299a1 on bkuker:develop into 12961d5 on schweikert:develop.

@schweikert
Copy link
Owner

Thanks for the patch! I don't have much time to look at this right now, but I certainly will. I worry a bit about performance aspects, so I will need to verify the cost of this.

@schweikert
Copy link
Owner

This is now fixed with a different implementation in #193

@schweikert schweikert closed this Jul 31, 2020
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.

3 participants