[tcpm] Tsvart last call review of draft-ietf-tcpm-yang-tcp-06

[Gorry Fairhurst via Datatracker <[e-mail]>]

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
[e-mail] 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.