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

epoch_start #100

Closed
larseggert opened this issue Aug 31, 2021 · 8 comments · Fixed by #119
Closed

epoch_start #100

larseggert opened this issue Aug 31, 2021 · 8 comments · Fixed by #119
Assignees

Comments

@larseggert
Copy link
Member

@larseggert larseggert commented Aug 31, 2021

Markku Kojo said:

epoch_start: needs more accurate and consistent definition when the exactly the epoch starts. Is it when congestion event occurs or when TSP sender enters congestion avoidance first time after an congestion event. If it is different in different scenarios that would be good to present systematically.

@goelvidhi goelvidhi self-assigned this Sep 1, 2021
@goelvidhi
Copy link
Collaborator

@goelvidhi goelvidhi commented Sep 1, 2021

@markkukojo, from the draft,

epoch_start: The time in seconds at which the current congestion
avoidance stage started.

So, it is when the sender enters the congestion avoidance first.

@larseggert
Copy link
Member Author

@larseggert larseggert commented Sep 1, 2021

Markku, OK to close with no action?

@goelvidhi
Copy link
Collaborator

@goelvidhi goelvidhi commented Sep 8, 2021

@markkukojo, could you please let us know?

@larseggert
Copy link
Member Author

@larseggert larseggert commented Sep 15, 2021

No word from @markkukojo, closing this. @markkukojo, please reopen if you disagree.

@larseggert larseggert closed this Sep 15, 2021
@markkukojo
Copy link
Collaborator

@markkukojo markkukojo commented Sep 24, 2021

I made this note to myself in the early phase of reading the draft and it looks like that I was not very clear what seemed to be the problem. The definition of epoch_start in its brevity seems fine. However, now I see that it was more of its relation to cwnd_start what confused me and in particular what the draft says in Sec 4.2:
"where cwnd_start is the congestion window at the beginning of the
current congestion avoidance stage. For example, right after a
congestion event, cwnd_start is equal to the new cwnd calculated as
described in Section 4.6."

The latter sentence starting "For example, ..." is confusing and unnecessary. It kind of hints that cwnd_start would have the same value immediately after a congestion event and in the beginning of the congestion avoidance stage that follows. This is true when a TCP sender receives ECE, for example. I have noticed that some people think that cwnd would also have the same value in the beginning of loss recovery (i.e., after 3 DupAcks triggered Fast Retransmit) and after Fast Recovery completes (i.e., when a TCP sender enters congestion avoidance). Maybe it is because this true for TCP Reno loss recovery. However, it often is not the true with NewReno (RFC 6582) and SACK-based loss recovery (RFC 6675).

I'd suggest deleting the sentence:

"For example, right after a congestion event, cwnd_start is equal to the new cwnd calculated as described in Section 4.6."

because it is unnecessary and more misleading than clarifying.

@larseggert larseggert reopened this Sep 24, 2021
@goelvidhi
Copy link
Collaborator

@goelvidhi goelvidhi commented Sep 25, 2021

ok, IIUC, this is unrelated to the original issue which talk about proper definition of epoch_start.

I can create a PR to delete this line.

@larseggert
Copy link
Member Author

@larseggert larseggert commented Sep 27, 2021

Closing this one in favor of #119.

@larseggert larseggert closed this Sep 27, 2021
@larseggert
Copy link
Member Author

@larseggert larseggert commented Sep 27, 2021

Sorry, thought #119 was an issue, but it's a PR.

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 a pull request may close this issue.

3 participants