Re: [yang-doctors] Yangdoctors early review of draft-ietf-tcpm-yang-tcp-06

[Mahesh Jethanandani <[e-mail]>]

Hi Ebben,

Thanks first of all for reviewing the draft and the model. We have tried to address most of the comments in the -07 version of the draft. Please see inline for more specific comments.

> On Mar 6, 2022, at 4:52 PM, Ebben Aries via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Ebben Aries
> Review result: On the Right Track
> 
> 1 module in this draft:
> - [e-mail]
> 
> YANG compiler errors or warnings (pyang 2.5.2, yanglint 2.0.112, yangson 1.4.13)
> - No compiler errors or warnings.  Instance data verified with features
> 
> Module [e-mail]:
> - Overall the module is concise and conforms to the guidelines put forth in
>  RFC8409.  Only a few minor nits as well as some comments and questions on
>  the draft itself
> - L#91 - Is there any reason to put a range that covers the entire uint8
>  space?
> - L#104 - Same as L#91

When a type is defined, it is not necessary that the entire range is applicable. For example, a VLAN might be defined as a uint16 type, however, the valid range is only 1..4096. By stating the max range, it is making it clear that the entire range is applicable.

> 
> General comments on the draft/modules:
> - The title is "A YANG Model for TCP Configuration" however the intent is to
>  cover TCP connection tables and statistics.  While configuration parameters
>  are introduced, is it not likely that as the model progresses will encompass
>  additional state over time?  I would suggest readjusting wording to
>  accomodate that this is not only geared towards configuration of TCP
>  parameters (which multiple sections concentrate on) and more to TCP
>  configuration, connection tables and state

Ok. Updated the title and description to reflect that the model covers both configuration and state.

> 
> - Section 3.3 - Tree Diagram.  If you are calling out this section is only for
>  the abridged version you may want to include a reference to the full diagram
>  in Appendix C

Added.

> 
> - Appendix B.2 - Use IPv6 addresses from the documentation prefixes defined
>  inRFC3849 (draft-ietf-tcpm-ao-test-vectors should be updated accordingly as
>  well)

With the new model to configure TCP-AO, this comment is no longer applicable.

> 
> - Understood that it is mentioned in the introduction this is a minimal set of
>  features but statistics wise, there are currently only aggregate/global TCP
>  statistics defined. Has there been any consideration for per-connection
>  statistics and additional attributes much like outputs of `ss -tpi`
>  (processes, counters, mss, pmtu, congestion ctl alg, etc..)? Or is the
>  intent this will be served by a future version or separate
>  module/augmentation at a later date?

This was something that was discussed early on when the scope of the draft was being considered in TCPM. At that time, the consensus was to provide only an aggregate set of statistics. It however, does not preclude a subsequent model to augment this model to expand the collection of statistics.

Thanks.

> 
> 
> 


Mahesh Jethanandani
[e-mail]