[Detnet] Yangdoctors early review of draft-ietf-detnet-yang-12

[Xufeng Liu via Datatracker <[e-mail]>]

Reviewer: Xufeng Liu
Review result: On the Right Track

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