I have reviewed draft-ietf-ipsecme-yang-iptfs-01.txt. - Looking at [1], following the most popular naming scheme would suggest the title "A YANG Data Model for IP Traffic Flow Security" instead of the current title. - Never data models usually state in the abstract whether the model conforms to the Network Management Datastore Architecture (NMDA). - The document is generally well written and easy to read. Thanks for that. - Not sure what "actively published YANG modules" are. In the YANG world, we usually consider YANG modules under development not really as published. (See how 'published' is used in RFC 7950.) What you are referring to here are modules actively being worked on I think. - You import RFC 2119 terms but then I am not sure they are actually used anywhere. If you do not need RFC 2119 terms, there is also no need to import them. - It seems that [I-D.ietf-i2nsf-sdn-ipsec-flow-protection] has been published as RFC 9061, so please update your document. - "IP-TFS YANG augments IPsec YANG model from" - why not use the actual YANG module names to avoid any confusion? - I did not understand what the purpose of this is: The data model uses following constructs for configuration and management: o Configuration o Operational State The text following this is useful, but the value of the quoted text was unclear to me. - I did not understand what this text means: IP-TFS YANG augments: * Yang catalog entry for ietf-i2nsf-ike@2021-07-14.yang * Yang catalog entry for ietf-i2nsf-ikeless@20202-07-14.yang What is a Yang catalog entry for xxx and how can you augment it? - The YANG module names you are augmenting include the WG name, which is a rather weird construction and generally not recommended. I wonder how this went through. You seem to follow this and you put ipsecme into the name. Ideally, module names would be tied to a technology but not to RFCs or WGs (all of which come and go). - Copyright needs some updating... and in less than 3 months again. - You use type uint64 for counters instead of the yang:counter64 type. Is there a special reason? How would an overflow be handled? Can there be any counter discontinuities during their lifetime? - I found this: // config true; want this so we can refine? Can you elaborate what the question is here? - The congestion control leaf is an enable/disable knob. Is there exactly one and only one way to do congestion control or is there / will there be a need to be more detailed, i.e., does this have to be extensible instead of just a simple boolean? - What happens if I set use-path-mtu-discovery to true and outer-packet-size to 1234? Which takes precedence? Is it desirable to allow both to be configured? - Consider adding a units clause to l2-fixed-rate and l3-fixed-rate. - I did not extract the YANG to do syntax checking etc, I assume that the authors have a proper build automation in place (or any errors will have to be dealt with when the ID has passed WG last call). - I did not verify the xml and json examples in the appendix. However, I suggest to use documentation addresses for prefixes in the examples instead of 1.1.1.1/32 and 2.2.2.2/32 (and some may even want to see some IPv6 prefixes there as well). - Editorial fll out -> fill out Congestion Control With the -> With the [1] https://en.wikipedia.org/wiki/YANG#Standards-track_data_models