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

[Wesley Eddy <[e-mail]>]

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.