Subject: [tcpm] WGLC review of draft-ietf-tcpm-rfc793bis-18

From: Praveen Balasubramanian <[email-masked]>
Date: Fri, 16 Oct 2020 01:54:43 +0000

Following is a WGLC review of draft-ietf-tcpm-rfc793bis-18 by Matt, Yi, and myself.

Section 3.1

The control bits in the header include the RFC 3168 specified CWR and ECE. It might be worth citing 3168 as well in the Abstract.

It is quite a bit surprising that we don't include discussion of the SACK, Timestamp, and Window Scaling options. Even if we don't describe the details, the document should provide references to these almost universally deployed options and recommend that hosts SHOULD implement them. At least the Timestamp option is referred to multiple times in the text without any specification of the format.

Section 3.6.4

"

   If there is unacknowledged data (i.e., SND.NXT > SND.UNA), then the

   sending TCP endpoint buffers all user data (regardless of the PSH

   bit), until the outstanding data has been acknowledged or until the

   TCP endpoint can send a full-sized segment (Eff.snd.MSS bytes).

"

This is slightly different from Nagle's description in RFC 896:

"

The solution is to inhibit the sending of new TCP  segments  when

new  outgoing  data  arrives  from  the  user  if  any previously

transmitted data on the connection remains unacknowledged.

"

I.e., the original algorithm description in RFC 896 didn't include the "or if a full-sized packet can be sent" part. This difference isn't mentioned in 793bis, it ought to be, along with a justification for the difference. Also there is no mention of the interaction problem between Nagle's algorithm and delayed ACKs, either here or in the later section on delayed ACKs.

Section 3.7.3

"However, the values of R1 and R2 may be different for SYN

            and data segments.  In particular, R2 for a SYN segment MUST

            be set large enough to provide retransmission of the segment

            for at least 3 minutes.  The application can close the

            connection (i.e., give up on the open attempt) sooner, of

            course.

"

The MUST here is onerous for what is an ancient timeout value which modern implementations don't adhere to. Recommend changing this to a SHOULD.

Section 3.7.4
"Keep-alive packets MUST only be sent when no data or acknowledgement
   packets have been received for the connection within an interval
   (MUST-26).

"

It's important to call out an additional condition - that sends are not already outstanding. When a send is outstanding, it is retransmitted and serves as an implicit liveness check.

Section 3.7.2

"A TCP endpoint MUST implement RFC 5681 (MUST-19)."
This requirement is onerous because RFC 5681 also specifies the NewReno congestion control algorithms. Modern implementations use other algorithms. We should add clarifying text permitting experimentation and evolution of congestion control algorithms. It is also not required for interop.

Editorial comments:

"

     The window size MUST be treated as an unsigned number, or else

     large window sizes will appear like negative windows and TCP will

     now work (MUST-1).

"

should be "not work".

"

Note that when the receive window is zero no segments should be

   acceptable except ACK segments.  Thus, it is be possible for a TCP

   implementation to maintain a zero receive window while transmitting

   data and receiving ACKs.

"

Remove the "be".

"

The "Quiet Time" concept discussed below addresses this and the

   discussion of it is included for situations where it might be

   relevant, although it is not felt to be necessary in most current

   implementations.  The problem have been more relevant earlier in the

   history of TCP.

"
The problem was more relevant

"

It is

   tempting for a TCP implementation to want to advertise the largest

   possible MSS
"
Remove "want to".

"The TCP implementation will signal a
   user, even if no RECEIVEs are outstanding, that the remote peer has
   closed, so the user can terminate his side gracefully.
"
"The user may CLOSE the connection at any time on his own
         initiative
"
Suggestion to be gender neutral in the text.

"

For IPv4, RFC 1122 provides an IP-layer recommendation

   on the default effective MTU for sending to be less than or equal to

   576 for destinations not directly connected.
"
Should at least be "recommendation for", but consider changing to something more concise like "For IPv4, RFC 1122 recommends an IP-layer default effective MTU less than or equal to 576 for destinations not directly connected."

---

From: Praveen Balasubramanian <[email-masked]>
Date: Fri, 16 Oct 2020 02:06:54 +0000

Adding Matt and Yi to the To line as the first email removed the Cc line.

From: tcpm <[email-masked]> On Behalf Of Praveen Balasubramanian
Sent: Thursday, October 15, 2020 6:55 PM
To: [email-masked]
Subject: [EXTERNAL] [tcpm] WGLC review of draft-ietf-tcpm-rfc793bis-18

Section 3.1

Section 3.6.4

"

"

"

"

Section 3.7.3

            course.

"

Section 3.7.4

"

Section 3.7.2

Editorial comments:

"

     now work (MUST-1).

"

should be "not work".

"

   data and receiving ACKs.

"

Remove the "be".

"

   history of TCP.

"

"

It is

   possible MSS
"
Remove "want to".

"

---

From: Wesley Eddy <[email-masked]>
Date: Fri, 23 Oct 2020 16:37:11 -0400

Thank you for this helpful review; here are my plans/thoughts on 
addressing your comments:

On 10/15/2020 10:06 PM, Praveen Balasubramanian wrote:
>
> Section 3.1
>
> The control bits in the header include the RFC 3168 specified CWR and 
> ECE. It might be worth citing 3168 as well in the Abstract.
>
Agreed; I'll do this in the next revision.

> It is quite a bit surprising that we don't include discussion of the 
> SACK, Timestamp, and Window Scaling options. Even if we don't describe 
> the details, the document should provide references to these almost 
> universally deployed options and recommend that hosts SHOULD implement 
> them. At least the Timestamp option is referred to multiple times in 
> the text without any specification of the format.
>
This makes basically sense to me, and I'll plan to improve on it in the 
next revision.  My plan is to prominently reference the specs that 
define those options, and say that they're recommended, but won't try to 
recreate their definitions here though.

> I.e., the original algorithm description in RFC 896 didn't include the 
> "or if a full-sized packet can be sent" part. This difference isn't 
> mentioned in 793bis, it ought to be, along with a justification for 
> the difference.
>
This part is directly from RFC 1122 (Section 4.2.3.4), so I think it 
says the proper thing.

> Also there is no mention of the interaction problem between Nagle's 
> algorithm and delayed ACKs, either here or in the later section on 
> delayed ACKs.
>
Very good point!  Currently, the delayed ACK interaction w/ Nagle is 
only mentioned in Appendix A.3.  I think, based on your comment, that we 
should add a mention of this in the main body when Nagle's algorithm is 
discussed, referring to A.3.

> Section 3.7.3
>
> “However, the values of R1 and R2 may be different for SYN
>
>             and data segments.  In particular, R2 for a SYN segment *MUST*
>
> “
> The MUST here is onerous for what is an ancient timeout value which 
> modern implementations don’t adhere to. Recommend changing this to a 
> SHOULD.
>
While I agree, I think this will require some greater working group 
discussion.  If the group and chairs agree, I'm happy to do this.

> Section 3.7.4
>
> “Keep-alive packets MUST only be sent when no data or acknowledgement
>
> “
> It’s important to call out an additional condition – that sends are 
> not already outstanding. When a send is outstanding, it is 
> retransmitted and serves as an implicit liveness check.
>
This is an excellent point, and I think it can be added in the next 
revision, unless there is any objections from the rest of the working group.

> Section 3.7.2
>
> “/A TCP endpoint MUST implement RFC 5681 (MUST-19)./”
>
> This requirement is onerous because RFC 5681 also specifies the 
> NewReno congestion control algorithms. Modern implementations use 
> other algorithms. We should add clarifying text permitting 
> experimentation and evolution of congestion control algorithms. It is 
> also not required for interop.
>
This is a part where maybe we could use some more help/input on the 
exact wording.

I think MUST implement some IETF standard congestion control is the 
right idea, but alternative algorithms that are administratively (or 
otherwise) managed on a host are also fine to implement and enable.  If 
others agree, disagree, or have thoughts on how exactly to phrase this, 
I think it would help to hear from the working group.

Finally, all of your editorial comments are now accounted for in my 
working copy, so thank you for those.

---

From: "Scharf, Michael" <[email-masked]>
Date: Sat, 24 Oct 2020 08:48:58 +0000

Regarding...

> While I agree, I think this will require some greater working group discussion.  If the group and chairs agree, I'm happy to do this.

This MUST is in RFC 1122, i.e., using SHOULD would be a modification to RFC 1122. If we change the wording, I think we need to clearly document that change, e.g. in Section 8, possibly with an explanation.

Personally (i.e., with no hat), I agree that this MUST could be changed to a SHOULD, if the change is properly documented.

I have also thought about other wordings to work around this capital letter MUST, but I haven't found a good alternative so far.

Michael

---

From: Praveen Balasubramanian <[email-masked]>
Date: Thu, 29 Oct 2020 02:44:17 +0000

In draft 19:

Section 2. There is a typo retransmion -> retransmission. I would suggest running this document through a spell checker.

Section 3.1. "Additional RFCs define some other commonly used options that are recommended to implement." --> I think we need stronger language here. The following commonly used options SHOULD be supported.

Please bring up the SYN timeout MUST requirement of 3 minutes and the flexibility of using other congestion controllers (other than Reno as in RFC 5681) up for working group discussion.

Thanks

From: Wesley Eddy <[email-masked]>
Sent: Friday, October 23, 2020 1:37 PM
To: Praveen Balasubramanian <[email-masked]>; [email-masked]; Matt Olson <[email-masked]>; Yi Huang <[email-masked]>
Subject: [EXTERNAL] Re: [tcpm] WGLC review of draft-ietf-tcpm-rfc793bis-18

Thank you for this helpful review; here are my plans/thoughts on addressing your comments:

Section 3.1

This makes basically sense to me, and I'll plan to improve on it in the next revision.  My plan is to prominently reference the specs that define those options, and say that they're recommended, but won't try to recreate their definitions here though.

Section 3.6.4

"

"

"

"

I.e., the original algorithm description in RFC 896 didn't include the "or if a full-sized packet can be sent" part. This difference isn't mentioned in 793bis, it ought to be, along with a justification for the difference.

This part is directly from RFC 1122 (Section 4.2.3.4), so I think it says the proper thing.

Also there is no mention of the interaction problem between Nagle's algorithm and delayed ACKs, either here or in the later section on delayed ACKs.

Very good point!  Currently, the delayed ACK interaction w/ Nagle is only mentioned in Appendix A.3.  I think, based on your comment, that we should add a mention of this in the main body when Nagle's algorithm is discussed, referring to A.3.

Section 3.7.3

            course.

"

Section 3.7.4

"

This is an excellent point, and I think it can be added in the next revision, unless there is any objections from the rest of the working group.

Section 3.7.2

This is a part where maybe we could use some more help/input on the exact wording.

I think MUST implement some IETF standard congestion control is the right idea, but alternative algorithms that are administratively (or otherwise) managed on a host are also fine to implement and enable.  If others agree, disagree, or have thoughts on how exactly to phrase this, I think it would help to hear from the working group.

Finally, all of your editorial comments are now accounted for in my working copy, so thank you for those.