Hi Ben,
On 8/22/19, 1:46 AM, "Benjamin Kaduk via Datatracker" <[email protected]> wrote:
Benjamin Kaduk has entered the following ballot position for
draft-ietf-ospf-yang-26: Discuss
When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)
Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.
The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-ospf-yang/
----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------
(1) Can we check whether it's okay to use the yang "string" type for raw
cryptographic keys (e.g., ospfv2-key, ospfv3-key)? My understanding was
that yang strings were limited to human-readable, but that the crypto
keys could be raw binary values.
It does only include human-readable keys. Here is the RFC 7950 YANG ABNF
definition.
;; any Unicode or ISO/IEC 10646 character, including tab, carriage
;; return, and line feed but excluding the other C0 control
;; characters, the surrogate blocks, and the noncharacters
yang-char = %x09 / %x0A / %x0D / %x20-D7FF /
; exclude surrogate blocks %xD800-DFFF
%xE000-FDCF / ; exclude noncharacters %xFDD0-FDEF
%xFDF0-FFFD / ; exclude noncharacters %xFFFE-FFFF
%x10000-1FFFD / ; exclude noncharacters %x1FFFE-1FFFF
%x20000-2FFFD / ; exclude noncharacters %x2FFFE-2FFFF
%x30000-3FFFD / ; exclude noncharacters %x3FFFE-3FFFF
%x40000-4FFFD / ; exclude noncharacters %x4FFFE-4FFFF
%x50000-5FFFD / ; exclude noncharacters %x5FFFE-5FFFF
%x60000-6FFFD / ; exclude noncharacters %x6FFFE-6FFFF
%x70000-7FFFD / ; exclude noncharacters %x7FFFE-7FFFF
%x80000-8FFFD / ; exclude noncharacters %x8FFFE-8FFFF
%x90000-9FFFD / ; exclude noncharacters %x9FFFE-9FFFF
%xA0000-AFFFD / ; exclude noncharacters %xAFFFE-AFFFF
%xB0000-BFFFD / ; exclude noncharacters %xBFFFE-BFFFF
%xC0000-CFFFD / ; exclude noncharacters %xCFFFE-CFFFF
%xD0000-DFFFD / ; exclude noncharacters %xDFFFE-DFFFF
%xE0000-EFFFD / ; exclude noncharacters %xEFFFE-EFFFF
%xF0000-FFFFD / ; exclude noncharacters %xFFFFE-FFFFF
%x100000-10FFFD ; exclude noncharacters %x10FFFE-10FFFF
However, this is the intent as this is support for implementations that don't
support key-chains (RFC 8177). The "Security Considerations" recommend key
chains. We'll add the hexadecimal specification as another reason for
preferring key-chains.
(2) Do we need to say anything about how to indicate when there are
discontinuities for the various "counter" types?
I've never heard any mention of counter discontinuities in the context of OSPF
control plane counters.
----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------
I'm happy to see the discussion around Roman's Discuss.
Section 1
YANG [RFC6020][RFC7950] is a data definition language used to define
the contents of a conceptual data store that allows networked devices
to be managed using NETCONF [RFC6241]. YANG is proving relevant
beyond its initial confines, as bindings to other interfaces (e.g.,
ReST) and encodings other than XML (e.g., JSON) are being defined.
This text feels a bit stale at this point.
Updated in -27.
Section 2
model varies among router vendors. Differences are observed in terms
of how the protocol instance is tied to the routing domain, how
multiple protocol instances are be instantiated among others.
nit: the grammar here is a bit odd, with the comma suggesting the start
of a list but no "and" present.
Fixed in -27.
Section 2.2
The ospf module is intended to match to the vendor specific OSPF
configuration construct that is identified by the local identifier
'name'.
I don't really understand what this is intended to mean.
I removed this paragraph and augmented the next one.
Section 2.7
hello-timer in the module claims to be a rt-types:timer-value-seconds16
but shows up in the tree(s) as a uint32.
Similarly, the wait-itmer is a rt-types:timer-value-seconds32, which
also shows up in the tree(s) as a uint32, which is perhaps more
reasonable but perhaps not entirely accurate.
Other 'timer' leafs seem to have similar issues.
Fixed.
Section 3
feature ospfv2-authentication-trailer {
description
"Use OSPFv2 authentication trailer for OSPFv2
authentication.";
Is the feature for "use" or "support for"?
(Similarly for the ospfv3 authentication features.)
The latter - fixed.
identity ospfv2-lsa-option {
description
"Baes idenity for OSPFv2 LSA option flags.";
nit: "Base"
Fixed.
identity v2-p-bit {
base ospfv2-lsa-option;
description
"Only used in type-7 LSA. When set, an NSSA
border router should translate the type-7 LSA
to a type-5 LSA.";
There seem to be a few "identity <foo>-bit" stanzas whose description do
not mention the named bit specifically (but many that do). Do we want
to be consistent about doing so? (Likewise for <foo>-flag.)
Made these consistent. This was added very late when we realized that YANG type
bit is not extendable.
grouping tlv {
description
"Type-Length-Value (TLV)";
leaf type {
type uint16;
description "TLV type.";
}
leaf length {
type uint16;
description "TLV length (octets).";
}
leaf value {
type yang:hex-string;
description "TLV value.";
Is there a way to apply a constraint so the 'length' matches the
hext-string's length?
Since this is returned operational data, it doesn't make a lot of sense to put
constraints on it. It is basically what the router returns. If it is wrong, it
wouldn't have been parsed correctly in the first place.
grouping router-capabilities-tlv {
The various descriptions hereunder could perhaps benefit from section
references, as, e.g., two nodes named "informational-capabilities" may
otherwise require some effort to distinguish. Well, aside from the fact
that one is currently listed as "capabilitiess" with two esses, which
seems unlikely to be intended.
I fixed the description on these to differential between the list uint32 flags
and the identities.
grouping ospf-router-lsa-bits {
container rputer-bits {
s/rputer/router/?
Fixed.
container te-opaque {
[...]
container link-tlv {
description "Describes a singel link, and it is constructed
of a set of Sub-TLVs.";
s/singel/single/
Fixed.
grouping ospfv3-lsa-external {
[...]
leaf referenced-link-state-id {
type yang:dotted-quad;
RFC 5340 section 2.2 implies that the Link State ID is going to be a
32-bit identifier that need not be represented as dotted-quad, as it
does not have addressing semantics. (dotted-quad seems to be used for
Link-State-ID-shaped things elsewhere, too, though the preferred form
may be the union of dotted-quad and uint32.)
Good catch. This should be unit32 for OSPFv3. There were two instances of this.
grouping lsa-common {
description
"Common fields for OSPF LSA representation.";
leaf decode-completed {
type boolean;
description
"The OSPF LSA body was successfully decoded other than
unknown TLVs. Unknown LSAs types and OSPFv2 unknown
opaque LSA types are not decoded. Additionally,
malformed LSAs are generally not accepted and are
not be in the Link State Database.";
nit: "are not be" is not grammatical.
Fixed - "will not be".
grouping lsa-key {
description
"OSPF LSA key.";
This could maybe benefit from a more descriptive description; is this a
sort or lookup key, for example?
Yes - I've improved.
container database {
description "Container for per AS-scope LSA statistics.";
list as-scope-lsa-type {
[...]
leaf lsa-cksum-sum {
type uint32;
description
"The sum of the LSA checksums of the LSA type.";
[It's not entirely clear to me why this sum-of-checksums is a useful
thing to track, but it may not be this document's role to do so.
Though, perhaps we do need to say if the sum is computed as integers
modulo 2**32.]
This is from the OSPF MIB (RFC 4750). It is not used functionally as one cannot
guarantee differing LSDBs will not yield the same checksum. However, if the
checksums are different, you can assure the databases are different.
leaf transmit-delay {
type uint16;
units seconds;
description
"Estimated time needed to transmit Link State Update
(LSU) packets on the interface (seconds). LSAs have
their age incremented by this amount on advertised
on the interface. A sample value would be 1 second.";
nit: "on advertised on" does not seem grammatical.
Fixed - "when advertised on".
leaf lls {
[...]
container ttl-security {
Should these have a default value?
I think 1 would be appropriate since only virtual-links and sham-links require
more.
case ospfv3-auth-ipsec {
when "derived-from-or-self(../../../../../../rt:type, "
+ "'ospfv3')" {
description "Applied to OSPFv3 only.";
}
if-feature ospfv3-authentication-ipsec;
leaf sa {
type string;
I don't see RFC 4552 talking about names for SAs; where would this be
discussed (and, are they guaranteed to be human-readable)?
If it is not human readable, how would it be configured?
leaf poll-interval {
type uint16;
units seconds;
description
"Neighbor poll interval (seconds) for sending OSPF
hello packets to discover the neighbor on NBMA
networks. This interval dictates the granularity for
discovery of new neighbors. A sample would be 2 minutes
for a legacy Packet Data Network (PDN) X.25 network.";
Maybe "2 minutes (120 seconds)" since the units are seconds?
Fixed.
container preference {
description
"Route preference configuration In many
implementations, preference is referred to as
administrative distance.";
nit: missing sentence break?
Fixed.
For the spf- and lsa-logs, do we require that the 'id' is assigned in
any particular order, or do we just rely on the included timestamp(s)
for any time-ordering required by the consumer?
I added ordering to the description of the spf-log and lsa-log. However, the id
is a purely internal value to provide a uniqueness key for the YANG list.
grouping notification-neighbor {
[...]
leaf neighbor-ip-addr {
type yang:dotted-quad;
description "Neighbor address.";
neighbors can only be identified by IPv4 addresses?
Changed to IP address type.
leaf packet-source {
type yang:dotted-quad;
packet sources, too? (multiple times)
Changed to IP address type.
description
"This notification is sent when aa neighbor
state change is detected.";
nit: s/aa/a/
Fixed.
Section 4
Additionally, local specificationn of OSPF authentication keys and
the associated authentication algorithm is supported for legacy
implementations that do not support key-chains [RFC8177] for legacy
implementations that do not support key-chains. It is RECOMMENDED
that implementations migrate to key-chains due the seamless support
of key and algorithm rollover, as well as, the encryption of key
using the Advanced Encryption Standard (AES) Key Wrap Padding
Algorithm [RFC5649].
(Roman caught the nits, so I won't duplicate that.)
I expected to see something about keeping the actual key material
secret, as well.
Can you suggest some text?
Thanks,
Acee
_______________________________________________
Lsr mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/lsr
