**** Summary

This document defines a YANG data model for the IETF Network Slice service. It builds upon the suite of modules for network topology [RFC 8345] and traffic engineering topology [RFC 8795].

**** General comments

- The data model resulting from the YANG module is impressively large, which also makes it difficult to review it in its entirety. With this caveat, I think the schema is well designed and best practices of YANG modelling. I haven't found any serious issues, minor problems are mentioned below.
  
- Despite its size, the I-D is quite readable, also because it contains numerous examples illustrating various use cases.
  
- Many names of data nodes contain their parent's (partial) name as prefix, such as "template-description", "qos-policy-name" or "cc-id". This is unnecessary in YANG and (if there is no special reason) should be avoided because it just makes names and paths longer. See sec. 4.3.1 in RFC 8407.
  
- Observation: The module itself contains no "must" or "when" statements, which is quite extraordinary for a module of this size. Does it mean there are no (formally representable) semantic constraints, or that the data model is deliberately lax?
    
**** Specific comments

***** Section 2

- "IETF network slice", "customer", and possibly also "customer higher-level operation system" should be included in the list of terms defined in draft-ietf-teas-ietf-network-slices.
  
- The reference to RFC 8340 would be better placed next to the tree diagram in section 5.
  
***** Section 5

- The purpose of including a tree diagram in an I-D main text is to give a high-level overview of the schema. The full ASCII tree of the ietf-network-slice-service module is too long for this. Please adjust the diagram using the recommendations of sections 2.4 and 3.3 in RFC 8340. If it is useful to have the complete tree diagram in the I-D, it should go to an appendix.
  
- sec. 5.2: the first sentence in the definition of "service-tags" (with the quoted text "Customer higher level operation system") doesn't make sense.
  
***** Section 6 (the YANG module)

- If each slice requires at least 2 SDPs (sec. 5.2.1), why not have "min-elements 2;" for list "sdp"?
  
- Various tags, attributes etc. have their value modelled as a leaf-list. I haven't found any explanation why it is a sequence rather than a single value, and the descriptions mostly indicate the opposite. For example:

    leaf-list value {
      type string;
      description
        "The tag value";
    }

    I suspect these values started as leaves and were later changed to leaf-lists because some of the example in appendix A.4 also doesn't use JSON lists for these values (see below).

***** Appendix A

- All examples, many places: Module prefix "ietf-nss" cannot be used in JSON representation for identity values - full module name has to be used instead. See sections 4 and 6 in RFC 7951.
  
- A.4: in four places, "value" should be a JSON list, because it is defined as leaf-list.