Hi, I am the assigned YANG doctor for draft-ietf-rtgwg-lne-model, and here are my review comments, based on -02: 1) Add reference to import statements. import ietf-interfaces { prefix if; reference "RFC 7223: A YANG Data Model for Interface Management"; } 2) IETF boilerplate Use IETF boilerplate with contact, description w/ copyright etc. 3) feature bind-lne-name feature bind-lne-name { description "Logical network element to which an interface is bound"; } This description doesn't make much sense. Also, this feature is not used in the model. Should the feature be removed or used? 4) leaf managed leaf managed { type boolean; description "True if the host can manage the LNE using the root mount point"; } This description needs to be expanded. This is a config leaf, so what happens if a client sets this to 'true' for a certain LNE? The text in the document says: In addition to standard management interfaces, a host device implementation may support accessing LNE configuration and operational YANG models directly from the host system. When supported, such access is accomplished through a yang-schema-mount mount point So are there three cases involved? 1. The host device does NOT support accessing LNE data from the host system => in this case no modules will be mounted. 2. The host device supports reading LNE data from the host system => in this case the modules are mounted read-only. 3. The host device supports reading/writing LNE data from the host system => in this case the modules are mounted read-write. I _think_ the idea is that if the server supports 2 or 3, a client can choose to set 'managed' to "false", in which case it turns into case 1. Is this correct? If so, would it be useful to allow a client to turn case 3 into 2? The text in the document in section 3 has more details on the "managed" leaf. I suggest you add text from the document to the YANG module. 5) leaf bind-lne-name This leaf is a string. Shouldn't it be a leafref? leaf bind-lne-name { type leafref { path "/logical-network-elements/logical-network-element/name"; } ... } 6) inconsistent formatting I suggest you run pyang -f yang [--keep-comments] and possibly edit the result add/remove comments. The following comment doesn't add much and should be removed: // namespace namespace "urn:ietf:params:xml:ns:yang:ietf-logical-network-element"; Also remove the comments about rpcs and notifications. Also add period ('.') at the end of all sentences in the descriptions. 7) YANG tree The document should explain the tree diagram syntax. Section 2 has this tree diagram: +--rw interfaces | +--rw interface* [name] | +--rw name string | +--rw lne:bind-lne-name? string | +--rw ethernet | | +--rw ni:bind-network-instance-name? string | | +--rw aggregates | | +--rw rstp | | +--rw lldp | | +--rw ptp | +--rw vlans | +--rw tunnels | +--rw ipv4 | | +--rw ni:bind-network-instance-name? string | | +--rw arp | | +--rw icmp | | +--rw vrrp | | +--rw dhcp-client | +--rw ipv6 | +--rw ni:bind-network-instance-name? string | +--rw vrrp | +--rw icmpv6 | +--rw nd | +--rw dhcpv6-client This diagram is not correct; it seems to indicate that there are nodes 'ethernet', 'vlans', 'ipv4', etc in the ietf-interfaces module. I suggest you remove the nodes that do not exist, and change 'ipv4' to 'ip:ipv4' (and add a reference to RFC 7277). 8) YANG tree (2) Section 3 has this diagram: module: ietf-logical-network-element +--rw logical-network-inventory +--rw logical-network-element* [name] +--rw name? string +--rw description? string +--rw managed? boolean +--rw root? yang-schema-mount augment /if:interfaces/if:interface: +--rw bind-lne-name? string It needs to be updated to match the YANG model (rename logical-network-inventory), and the 'root' should not be shown as a leaf. (Yes I know that there currently is no syntax defined for a mount point in a tree) 9) YANG tree (3) Section 3.1 uses a tree diagram to show instance data. I think this is confusing, and you should use XML or JSON instead. 10) typo s/The interface management model [RFC7223] is and existing model/ The interface management model [RFC7223] is an existing model/ /martin