Skip to content
This repository has been archived by the owner on Aug 28, 2023. It is now read-only.

segments_acked rather than bytes_acked; cwnd in segments #67

Closed
larseggert opened this issue Jun 1, 2021 · 10 comments · Fixed by #77
Closed

segments_acked rather than bytes_acked; cwnd in segments #67

larseggert opened this issue Jun 1, 2021 · 10 comments · Fixed by #77
Assignees
Labels

Comments

@larseggert
Copy link
Contributor

Yoshi said:

8: Section 4.3 I am not very sure why segments_acked is used rather than byted_acked here. What is the benefit of it?
How do we calculate when the acks are split?
Also, I think it should be clarified that cwnd is expressed in segments here in this case.

@larseggert larseggert added WG LC help wanted Looking for someone resolve this issue labels Jun 1, 2021
@goelvidhi
Copy link
Contributor

segments_acked is used throughout the document to match the linux code.

Section 4.1.2 (https://datatracker.ietf.org/doc/html/draft-ietf-tcpm-rfc8312bis-02#section-4.1.2) defines the units for all variables including cwnd which is in segments.

How do we calculate when the acks are split?
I am guessing that this problem has already been solved in linux.

@goelvidhi goelvidhi self-assigned this Jun 3, 2021
@goelvidhi goelvidhi removed the help wanted Looking for someone resolve this issue label Jun 3, 2021
@larseggert
Copy link
Contributor Author

@nsdyoshi does this address your comment?

@nsdyoshi
Copy link

nsdyoshi commented Jun 4, 2021

Ok. I see the definition in cwnd. I'm sorry, I overlooked.
But, my main question is why segments_acked is used and byte_acked is not used. I believe we don't need to follow linux code.
Some implementations may want to use byted_acked. Is it allowed? I think it would be better to provide some guidance here.

@goelvidhi
Copy link
Contributor

Yes, implementations can use bytes_acked. Does the below text suffice:

The unit of all window sizes in this document is segments of the
maximum segment size (MSS), and the unit of all times is seconds.

@nsdyoshi
Copy link

nsdyoshi commented Jun 6, 2021

I think it would be better to mention using bytes_acked is acceptable for implementations explicitly.

BTW, when a segments_acked implementation sent a 1000 byte packet and only 500 bytes were acked, does it use fractions or use some other methods?
Another question is if we count sacked segments into this or not.

@goelvidhi
Copy link
Contributor

goelvidhi commented Jun 8, 2021

BTW, when a segments_acked implementation sent a 1000 byte packet and only 500 bytes were acked, does it use fractions or use some other methods?

If the size of each segment = 1 MSS, then segments_acked = bytes_acked / 1 MSS.

Another question is if we count sacked segments into this or not.

Good question, is that being done for new Reno or other congestion controllers? I looked at a few RFCs and didn't find any mention of using SACK bytes for congestion window. Do you know if any document has guidance on this?

@nsdyoshi
Copy link

If the size of each segment = 1 MSS, then segments_acked = bytes_acked / 1 MSS.
I was wondering the case where the size of segment is not 1 MSS or the case where the size of segment is 1 MSS, but the received ACK covers only part of the segment. It seems to me that this point is a bit ambiguous in the draft.

Good question, is that being done for new Reno or other congestion controllers?

Right. It's described in RFC6582 and RFC6675.
I've thought about this and I think the explanation of the draft on this point is sufficient.

@goelvidhi
Copy link
Contributor

I was wondering the case where the size of segment is not 1 MSS or the case where the size of segment is 1 MSS, but the received ACK covers only part of the segment. It seems to me that this point is a bit ambiguous in the draft.

If I were to use segments_acked as an implementor, I would just do bytes_acked / 1 MSS, whatever may be the size of the segment sent or ACKed. For example if MSS = 1448 and 1000 byte segment was sent and acked, then segments_acked = 1000/1448 and if 500 bytes were acked, then segments_acked = 500/1448.

If OTOH I was using bytes_acked in my implementation, then all one needs to do multiple or divide by 1MSS wherever necessary.

@nsdyoshi
Copy link

OK. In this case, I think it would be better to mention segments_acked is float number and can be less than 1.
Also, I think it would be good to provide a guidance for an implementation uses bytes_acked instead of segments_acked.
Something like multiplying segment-based parameters by 1 MSS.

@goelvidhi
Copy link
Contributor

ok, please review #77.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants