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

[Praveen Balasubramanian <[e-mail]>]

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 <[e-mail]>
Sent: Friday, October 23, 2020 1:37 PM
To: Praveen Balasubramanian <[e-mail]>; [e-mail]; Matt Olson <[e-mail]>; Yi Huang <[e-mail]>
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:


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.



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.

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

            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.

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
   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.

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.