Hi Gorry, Thanks a lot for this useful review. And I want to apologize for the late reply. We have tried to consider all suggestions in the new version -07 of draft-ietf-tcpm-yang-tcp. Version -07 tries to address also other IETF last call reviews and thus includes several changes. Please have a look at the new version (https://datatracker.ietf.org/doc/html/draft-ietf-tcpm-yang-tcp-07) or the diff (https://www.ietf.org/rfcdiff?url2=draft-ietf-tcpm-yang-tcp-07) and let us know if more is needed. Best regards Michael > -----Original Message----- > From: Gorry Fairhurst via Datatracker <noreply@xxxxxxxx> > Sent: Monday, February 28, 2022 11:41 AM > To: tsv-art@xxxxxxxx > Cc: draft-ietf-tcpm-yang-tcp.all@xxxxxxxx; last-call@xxxxxxxx; tcpm@xxxxxxxx > Subject: Tsvart last call review of draft-ietf-tcpm-yang-tcp-06 > > 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
