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
_______________________________________________
rtgwg mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/rtgwg
