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

My comments have been addressed in rev-08.

Regards,
Reshad.

==== review of 07 ====

Hi Paul,

Apologies for the delay. I took a look at rev-07 and here are some things which
I missed in previous reviews:

1.      draft-ietf-i2nsf-capability-data-model is an informative reference, it
should be a normative reference as per
https://tools.ietf.org/html/rfc8407#section-3.9 2.      There are references to
draft-ietf-i2nsf-capability-data-model in the YANG model which should be
qualified with a note to the editor (that draft will become an RFC). As an
example search for YYYY in https://www.ietf.org/id/draft-ietf-bfd-yang-17.txt
3.       The editor’s note should also request to change XXXX to actual RFC
number when the document is published. 4.      Leaf nodes under container
processing should have unit GHz. On this note, I’m not sure how clock speed
uniquely identifies performance, but I won’t pretend to be an expert in the
area. IMO this is something the WG should comment on.

Regards,
Reshad.

==== review of 06 ====

Going back to this comment on rev-05:

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

The only thing which seems to have been fixed is that the RPC statement was put
after groupings. But you should look at
https://tools.ietf.org/html/rfc8407#page-61:

     // extension statements
     // feature statements
     // identity statements
     // typedef statements
     // grouping statements
     // data definition statements
     // augment statements
     // rpc statements
     // notification statements
     // DO NOT put deviation statements in a published module

That means your data definition statements (container nsf-registrations) should
be after the groupings.

Also the indentation seems off on the YANG module in some places, for example
on P14.

Regards,
Reshad.

==== review of 05 ====
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.