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

Recovery-31: PTO time MUST NOT be set and interaction with time threshold #4193

Closed
gloinul opened this issue Oct 12, 2020 · 7 comments · Fixed by #4222
Closed

Recovery-31: PTO time MUST NOT be set and interaction with time threshold #4193

gloinul opened this issue Oct 12, 2020 · 7 comments · Fixed by #4222
Assignees
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus. ietf-lc An issue that was raised during IETF Last Call.

Comments

@gloinul
Copy link
Contributor

gloinul commented Oct 12, 2020

Section 6.2.1:

"The probe timer MUST NOT be set if the time threshold (Section 6.1.2) loss detection timer is set. The time threshold loss detection timer is expected to both expire earlier than the PTO and be less likely to spuriously retransmit data."

What does "MUST NOT be set" means. The time threshold loss detection 6.1.2 only occurs if there is acknowledgement received by the sender sufficient late after the transmission. Thus, PTO timer needs to always be set. because even if Time Threshold timer fires it might not declare a loss.

@ianswett
Copy link
Contributor

It means if the time threshold timer is armed, you are not supposed to arm the PTO timer.

If the Time Threshold timer fires and doesn't declare a loss, then I would consider that a bug.

@gloinul
Copy link
Contributor Author

gloinul commented Oct 12, 2020

So the above text about loss timer is only when one are under the condition created by this Section 6.1.2 sentence?

"If packets sent prior to the largest acknowledged packet cannot yet be declared lost, then a timer SHOULD be set for the remaining time."

Does the formulation in 6.1.2 need to be improved, possibly naming the timer?

@ianswett
Copy link
Contributor

I can see the confusion. This can either be a single timer or multiple, and sometimes we refer to it one way and sometimes as separately named timers, ie "The time threshold loss detection timer"

We can either be more consistent about naming the timers or stop naming the timers entirely. Would you prefer the former?

@larseggert larseggert added the ietf-lc An issue that was raised during IETF Last Call. label Oct 13, 2020
@larseggert larseggert added this to Triage in Late Stage Processing via automation Oct 13, 2020
@ianswett ianswett added the editorial An issue that does not affect the design of the protocol; does not require consensus. label Oct 13, 2020
@project-bot project-bot bot moved this from Triage to Editorial Issues in Late Stage Processing Oct 13, 2020
@gloinul
Copy link
Contributor Author

gloinul commented Oct 15, 2020

Frankly I don't know which direction that is best. Could I ask you to take an editorial stab on your most preferred. I would guess when you sit down to do either you realize if it will work or not when you harmonize it.

ianswett added a commit that referenced this issue Oct 15, 2020
That term doesn't appear above, so don't use it in the PTO section.

Fixes #4193
@ianswett
Copy link
Contributor

@gloinul Does PR #4222 resolve this for you?

@larseggert
Copy link
Member

@gloinul we'd need a response, please

@gloinul
Copy link
Contributor Author

gloinul commented Oct 20, 2020

Yes, that simple tweak do make it clearer. So resolved.

Late Stage Processing automation moved this from Editorial Issues to Issue Handled Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-recovery editorial An issue that does not affect the design of the protocol; does not require consensus. ietf-lc An issue that was raised during IETF Last Call.
Projects
Late Stage Processing
  
Issue Handled
Development

Successfully merging a pull request may close this issue.

3 participants