Last Call Review of draft-ietf-i2nsf-registration-interface-dm-05
review-ietf-i2nsf-registration-interface-dm-04-yangdoctors-lc-rahman-2019-06-28-01

Request Review of draft-ietf-i2nsf-registration-interface-dm-03
Requested rev. 03 (document currently at 05)
Type Last Call Review
Team YANG Doctors (yangdoctors)
Deadline 2019-06-28
Requested 2019-06-07
Requested by Linda Dunbar
Authors Sangwon Hyun, Jaehoon Jeong, TaeKyun Roh, Sarang Wi, Park Jung-Soo
Draft last updated 2019-11-11
Completed reviews Yangdoctors Last Call review of -05 by Reshad Rahman
Assignment Reviewer Reshad Rahman
State Completed
Review review-ietf-i2nsf-registration-interface-dm-04-yangdoctors-lc-rahman-2019-06-28
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/tK2RO2Z6lfypx6LED4EXkH-Lu_U
Reviewed rev. 05
Review result Ready with Issues
Review completed: 2019-11-11

Review
review-ietf-i2nsf-registration-interface-dm-04-yangdoctors-lc-rahman-2019-06-28

YANG Doctor review of draft-ietf-i2nsf-registration-interface-dm-05 (by Reshad Rahman)

Thank you for addressing comments from my earlier review @ https://datatracker.ietf.org/doc/review-ietf-i2nsf-registration-interface-dm-04-yangdoctors-lc-rahman-2019-06-28/

Major comments/questions:
- There is a YANG warning on the datatracker page:
ietf-i2nsf-reg-interface@2019-07-24.yang:54: warning: RFC 8407: 3.1: The IETF Trust Copyright statement seems to be missing (see pyang --ietf-help for details).
To fix this, in the YANG module remove the <> around 2019: Copyright (c) <2019>

- For contact in YANG module, please remove WG chair info (see RFC8407 appendix B for an example)

- For the revision in YANg module, put "Initial version" (even though it's the 5th revision)

- Why define a union of ipv4-address and ipv6-address in typedef nsf-address, why not reuse existing ip-address type from RFC6021?

- For bandwidth, is there a reason why it's limited to uint16? Even though 65Tbps is a lot, I wouldn't limit it to uint16. And aren't there any use-cases for bandwidth smaller than 1 Gbps? If yes, use e.g Mbps as unit and use uint32 instead of uint16? Please use units statement.
 
- It is not clear to me what’s the distinction between nsf-name and nsf-instance-name. In Examples 4 and 5, they have the same value, but not in Example 3.  Might be worth clarifying or giving the same name.

- Having nsf or i2nsf in many node names is redundant, since NSF or I2NSF is in the higher level container name.  e.g, in NSF Capability Registration all nodes seem to have i2nsf or nsf in their name.

- There seems to be some indentation issues in the YANG  module (e.g. P16)

- Abide by order in RFC8407 Appendix B. e.g. RPC statements should be after groupings.

Nits:

- Appendix B: Managmenet -> Management

- Section 6.2: capailities -> capabilities

- Example 5: space in "http_and_h ttps_flood_mitigation_capability"

Regards,
Reshad.