This is a review of the YANG module in draft-ietf-detnet-yang-12.

1) Missing diagrams in the PDF and HTML versions of the document. 
Sec A.1. says “Please consult the PDF or HTML versions for the Case A-1 Diagram”, but the this diagram and other diagrams are not present.
Because of the missing diagrams, it is unclear how these examples work, so the following comments are based on some assumptions that may or may not be what the authors intend to hold.

2) Implementation locations and the users of the model.
    Is this a network model implemented on a controller or a device model implemented on a network element? The abstract section states “The model allows for provisioning of end-to-end DetNet service”, but the YANG module and all examples use “ietf-interfaces”, which is a device model. The interface name such as “eth0” is unique within the scope of a device. There is no explanation in the YANG module and any of these examples. 
   If the model is intended to be used on a network-wide controller, any device module should not be imported. A network-wide module may be used on a device, and a device module may then be imported. 

3) In Sec 5., in the diagram, “TR” is used, but not defined. Amusing that “Ref” means “referencing”, the abbreviation also needs to be defined, especially if this is using the YANG leafref mechanism.

4) In Sec 5., the last paragraph states “four parts of DetNet functions defined in section 3”, but “section 3” does not define the “four” parts.

5) In Sec 5., the last paragraph describes “three instances in DetNet YANG Model”. The term “instance” is not clear here. A YANG data model does not usually consist of “instances”. RFC7950 uses “instance” to indicate the configuration data or operational data, so “instance” means “data instance”, which is not part of a YANG data model that “describes how data is represented and accessed”.

In Sec A.1, Example A-1 provides one data instance that contains all three “parts” (or components) of the DetNet system, so there is no such a concept of “App-flow instance”, “service sub-layer instance”, or “forwarding sub-layer instance”, because one single “data instance” may contain all three parts.

6) In Sec 6. in the sub-tree app-flows,
    6.1) Does this sub-tree model App-flow or NetNet flow, or both? draft-ietf-detnet-flow-information-model-14 defines both terms and describes the distinctions between them.

    6.2) Except the leaf "name", all other leaves are optional. When a user creates an instance of app-flow with only the name specified, what does the system do?

    6.3) Containers “ingress” and “egress” are specified. However, based on Sec 2.1 of draft-ietf-detnet-flow-information-model-14, An app-flow consists of source and destination, not ingress and egress. “DN Ingress” and “DN Egress” are parts of a “DetNet flow”, not app-flow. 

    6.4) Container “ingress” should not contain both source and destination. Sec 2.1. of draft-ietf-detnet-flow-information-model-14 defines the “ingress” as the location where DetNet flow starts, so the ingress is related only to the source, not the destination.

    6.5) Based on Sec 5.4. of draft-ietf-detnet-flow-information-model-14, the choice data-flow-type under the container “ingress” is how a DN flow is identified, so this choice is not just related to the ingress of the flow, and should be outside of the container “ingress”.

    6.6) Based on Sec 5.6. of draft-ietf-detnet-flow-information-model-14, the ingress section of the model should identify the ingress node(s) and/or starting reference points. It would be good to have some description on how the ingress node(s) is (are) identified, especially in the case of mpls-app-flow. The description on the leaf name currently is "Ingress DetNet application", which is not consistent with the definition in draft-ietf-detnet-flow-information-model-14.

    6.7) Why is app-flow-status under the container ingress? app-flow-status is related to an instance of app-flow, not only the “ingress” part of the app-flow, right?

    6.8) The leaf interface in the container ingress would only work when this model is implemented on a device (but not on a controller) because ietf-interfaces is a device model. Based on my previous assumption that this model may be used as a network-wide model on a controller, the referencing to ietf-interfaces would not be appropriate.

    6.9) The relationship between app-flow and service-sub-layer needs better described. When an app-flow is served by a DN service, the packets of this app-flow are processed by this particular ND service, so there is only one supporting service. Why are there the “incoming-service” and “outgoing-service”? Are the terms “incoming-service” and “outgoing-service” defined and/or described in any document?

7) In Sec 6. in the  sub-tree traffic-profile:
   The container flow-spec is not consistent with draft-ietf-detnet-flow-information-model-14, which defines the leaves under this container as TrafficSpecification in Sec 4.1.

8) Before Sec 6, the document does not have an informative reference to the YANG tree diagrams specification RFC8340. Refer to Section 2.2 of [RFC8349] for an example of such a reference.

9) In Sec 7, the document text does not have the references to the RFCs cited in the YANG module. These references also need to be listed in the document. Sec 5 of RFC7317 provides a good example.

10) In Sec 9, the Security section is missing.

11) In Sec 8, the IANA Considerations section is incorrect, with missing IANA requests.

12) For the IP address or prefix examples in the Appendix,  a mix of either IPv4 and IPv6 addresses or IPv6 addresses exclusively SHOULD be used.

13) The module prefix ietf-detnet is better to be changed to detnet. IETF-defined YANG modules usually do not use “ietf-” as part of the prefix name.

14) As described in Sec 4.3.1. of RFC8407, child nodes within a container or list SHOULD NOT replicate the parent identifier. There are several occasions in the YANG module, for example, profile-name, app-flow-bidir-congruent, service-protection, and service-operation-type.

15) The node name in the list or leaf-list statements should not contain the suffix “-list”, or in the plural format. For example, service-sub-layer-list is better to be changed to service-sub-layer; member-services should be member-service. The structure of app-flows/app-flow* is the common convention used in IETF. It would be good to be consistent within this module, so it will be a good idea to add a container "traffic-profiles" to the list "traffic-profile".

16) Most of the leaves in this module are defined as optional, but only one leaf has a default statement in the entire module. It is generally needed to either provide a default value or an explanation in the description for such an optional leaf.

17) There is no configuration node and operational state node to indicate whether DetNet is enabled on the system. Please confirm that this is the intention. The fact that this module is supported on a system may not mean that DetNet is enabled.

Thanks,
- Xufeng