Reviewer: Gorry Fairhurst Review result: Ready with Nits This document has been reviewed as part of the transport area review team's ongoing effort to review key IETF documents. These comments were written primarily for the transport area directors, but are copied to the document's authors and WG to allow them to address any issues raised and also to the IETF discussion list for information. When done at the time of IETF Last Call, the authors should consider this review as part of the last-call comments they receive. Please always CC tsv-art@xxxxxxxx if you reply to or forward this review. >From a transport perspective, the document appears ready with minor NiTs on the use of wording and references. It specifies a YANG model for TCP. It is well-written and does not raise any congestion control or other transport-related protocol issues. There are two normative dependencies, both adopted WG items in/completing WGLC. The following NiTs are listed below: /as it allows enabling of TCP-AO/ - are the words /as it/ best here, or would it be better to say /therefore this allows/ or /and this allows/ or something similar? /This issue is further discussed below./ - I did not really see an /issue/ discussed. Is it possible just to say the support for MD5 is discussed or something similar? Is there a suitable RFC ref for TCP_NODELAY (the TCP base spec)?: /algorithm by TCP_NODELAY/. Suggest: /traditionally different TCP implementation/traditionally different TCP implementations/ - i.e.e add /s/? /As the TCP MD5 signature option is obsoleted by TCP-AO, it is strongly RECOMMENDED to use TCP-AO./ - It seems odd to declare this in the scoping section of a document without a REF! Also, what is a "strong" recommendation. Is this simply restating the intent of RFC5925, in which case this could be rephrased a little, perhaps something like: /The TCP MD5 signature option was obsoleted by TCP-AO in 2010. Current implementations are therefore RECOMMENDED to use TCP-AO [RFCxxxx]./ /This version of the module does not cover Multipath TCP [RFC8684]./ - To me, it seems OK that this YANG model does not "specify a method for Multipath TCP", but the implications of the word /cover/ is a little unclear, does that mean other specs might provide additional functions to support multipath? or that that this spec prevents the use of multipath? or can only be used when multipath is not used? There seems a wide variety of options... Section 4 appears to require RFC-Ed action to replace ID labels within the yang models with the published RFC Refs and a note to the editor here could be helpful also at the start of this section. e.g., I-D.ietf-tcpm-rfc793bis. The meaning of the word "segment" is very clear for TCP, but would perhaps be worth prefixing by TCP, within the model since the word /segment/ is also used in other contexts: /The total number of segments received in error/. This might be better as: /The total number of TCP segments received in error/. /The total number of segments received,/ This might be better as: /The total number of TCP segments received,/ and similarly for segments sent and segments retransmitted. -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
