Re: [mpls] Updated rev -08 posted [Re: Benjamin Kaduk's Discuss on draft-ietf-mpls-ldp-yang-07: (with DISCUSS and COMMENT)]

["Rob Wilton (rwilton)" <[e-mail]>]

Hi Ben, Kamran, 

> -----Original Message-----
> From: iesg <[e-mail]> On Behalf Of Benjamin Kaduk
> Sent: 17 March 2020 20:56
> To: Kamran Raza (skraza) <[e-mail]>
> Cc: [e-mail]; [e-mail]; Nicolai Leymann
> <[e-mail]>; The IESG <[e-mail]>; draft-ietf-mpls-ldp-
> [e-mail]; Tarek Saad <[e-mail]>
> Subject: Re: Updated rev -08 posted [Re: Benjamin Kaduk's Discuss on
> draft-ietf-mpls-ldp-yang-07: (with DISCUSS and COMMENT)]
> 
> On Fri, Feb 28, 2020 at 04:39:14AM +0000, Kamran Raza (skraza) wrote:
> > Thanks for your detailed review, Ben.
> > We have posted a new rev -08 that takes care almost all of your comments
> and comments from other IESG reviewers.
> > On behalf of authors, Please see inline [skraza] for each of your
> comment/Q:
> >
> > 
> >     --------------------------------------------------------------------
> --
> >     DISCUSS:
> >
> > ----------------------------------------------------------------------
> >
> >     The YANG doctor last call review of the -06 notes that these modules
> are
> >     in violation of the expectations of RFC 8349 (at least w.r.t.
> defining a
> >     new identity for the control-plane protocol and possibly more).
> This
> >     document should either be brought into compliance or explain why it
> >     diverges.
> >
> > [skraza]: Fixed.
> >
> >     The YANG doctor's comments about default values for "local"
> >     configuration (e.g., graceful-restart configuration) causing the
> global
> >     configuration to never take effect should also be addressed.
> >
> > [skraza]: Fixed.
> 
> These both look addressed, to my eye, but I am very much not a YANG
> expert.
> I'd recommend trying to get a dedicated YANG review, perhaps from the
> incoming Management AD Rob Wilton.
[RW] 
I've checked the graceful-restart configuration and it looks fine. 

Is there further hierarchical configuration that needs to be checked?

> 
> >     Please include some justification for why LDP IPv6 is considered an
> >     "extended feature" (which is particularly surprising given that
> Section
> >     2 classifies "IP" to refer to both IPv4 and IPv6 together).
> >
> > [skraza]:  This was an early decision taken and presented to WG and WG
> had agreed.
> > Moreover, Sec 1.1 does points out why LDP IPv6 is considered as an
> extended feature. The "base" features are the one most commonly
> implemented and deployed features.
> > The idea was that base must not contain any if-feature - given that
> LDPv6 is not widely implemented and was branded as if-feature, it is
> classified in the extended category then.
> > BTW, note that LDP has separate RFC for LDP IPv6.
> 
> It's not really clear to me that just because the protocol is described in
> separate RFCs for IPv4 and IPv6 we cannot present a unified management
> interface on top of it.  I note that several other ADs expressed concern
> about this topic, so I've moved this document onto the agenda for this
> week's IESG telechat to get a better sense on what the right thing to do
> is here.
[RW] 
I note that IPv6 isn't under if-feature in the extension module.  This would imply that implementations that wish to implement optional features in the extension module must also support LDP IPv6 (or deviate).  Is that intentional?


> 
> >
> >     We need to define the format/semantics of the md5-key string (e.g.,
> is
> >     it hex? base64{url,}?) either directly or by reference (as the YANG
> >     doctor notes).  Using a crypto-type would probably be appropriate,
> as
> >     would adding a note that tcp-md5 is obsoleted by TCP-AO.
> >
> > [skraza]: It is beyond our control as we are using it as-is from the
> RFC. Made the change anyways to add crypto-algo to widen the scope/type of
> key beyond MD5.
> 
> I'm still concerned that there isn't enough text here for me to be able to
> implement something and have confidence it will interoperate.  For
> example, the recommendations in RFC 3562 include the ability to use a
> direct binary key for TCP-md5.  Does this YANG stanza allow for doing so?
> If yes, what mechanism is used for converting input into "raw" binary, if
> needed?
> 
> That is, even if the actual protocol semantics for how TCP-MD5 uses a
> given key/password are out of your control, there is still ambiguity as to
> what mapping (if any) occurs between the YANG-configured value and what is
> passed to the TCP-MD5 implementation.
> 
> Furthermore, the authentication-type still only has a "string" leaf for
> the key, with (if I am reading the YANG correctly) the recommended key-
> chain functionality for protected key storage being left as an extension.
> It's not clear to me why we should be leaving the best-practice
> formulation as an extension.
> 
> >     In the peer-af-policy-container grouping's
> >     label-policy/advertise/prefix-list, we need to say if the filter is
> for
> >     inclusion or exclusion of outgoing label advertisements.
> >     Similarly for the incoming label advertisements in the 'accept'
> >     container (and most of the <foo>-list-ref usages?).
> >
> >     In the policy-container grouping's
> >     label-policy/assign/independent-mode/prefix-list, the description
> >     suggests that the contents will provide not just a list of prefixes
> that
> >     act as a filter, but also a map from prefix to label.  This is a
> >     qualitatively different usage than the previous occurrences of
> >     prefix-list-ref, and it seems like a different type may be needed
> for
> >     it.  Similarly for ordered-mode.
> >
> > [skraza]: Fixed as per latest changes that use rtgwg policy model
> constructs.
> 
> Ah, this took me a little more work to find, since the changes were in the
> prefix-list-ref typedef.  Thanks.
> 
> >     (pro-forma) This document has 6 authors, and per RFC 7322, as this
> is
> >     more than five, we are supposed to consider whether that's
> appropriate.
> >     Seeing nothing in the shepherd writeup or similar, I mention it
> here.
> >
> > [skraza]: We had taken an advise from Loa (MPLS WG chair) and would like
> to keep it to 6.
> 
> Sure; this was presumed to just be "pro forma" but I put it in since the
> shepherd writeup didn't mention it specifically.
> 
> >     --------------------------------------------------------------------
> --
> >     COMMENT:
> >
> > ----------------------------------------------------------------------
> >
> >     Please also respond to the other YANG doctor, rtg-dir, and other
> >     directorate reviewers' comments.
> >
> >     I support Roman's Discuss.
> >
> >     I suggest making a reference to RFC 5920 at some point, perhaps from
> the
> >     security considerations.
> > [skraza]: Done.
> >
> >     We have several instances of a "container ipv4" that's a "container
> with
> >     presence" in the jargon of RFC 7950.  As SEC AD one thing I look for
> is
> >     edge cases, such as when a subset of these containers are present
> but
> >     not all (and not none).  While for this specific case, it seems
> fairly
> >     natural to assume that IPv4 is enabled if any are present, it does
> make
> >     me wonder if the modeling is at an optimal state if there's even the
> >     possibility for such skewed signals.  (I didn't specifically audit
> for
> >     any other cases of "redundant" containers-with-presence.)
> >
> > [skraza]: This is the same structure as RFC 8344. The edge cases are OK.
> The ipv4/ipv6 container may contain none, one, or more nodes in the
> container. ipv4/ipv6 address family is enabled as long as the container is
> “present”. Your assumption is correct, and the modeling (and the protocol
> as well) is in a good state, even with such possibilities. The signals
> should be clear enough.
[RW] 

Intuitively, I don't like the combination of a presence container + enable leaf below it.  I've asked Martin (author of RFC 8344) why the construct is used in this way, since he must have had a good reason.  It isn't immediately obvious to me why what is written isn't a convoluted way of making it a non-presence container.

Regards,
Rob


> >
> >     It's not entirely clear to me why we need (all of) the
> specialization of
> >     structures into using inet:ipv4-address and inet:ipv6-address as
> opposed
> >     to just inet:ip-address.  I do see that the latter is used in
> several
> >     places already, and so assume that this was already considered;
> perhaps
> >     some of the discussion/motivation could be included in the document,
> >     though.
> >
> > [skraza]: When the type of an attribute is for a specific address family
> (ipv4 or ipv6), we use the address family specific type (inet:ipv4-address
> or inet:ipv6-address), so that user cannot provide a value of the wrong
> format.  inet:ip-address is simply a union of net:ipv4-address and
> inet:ipv6-address, and is used when the address family is not restricted.
> This is particular convenient when the user makes a query on a particular
> peer when the user does not have a particular address family in mind. The
> system will provide the address in a proper format.
> >
> >     Section 1
> >
> >        The data model is defined for following constructs that are used
> for
> >        managing the protocol:
> >
> >     nit: "the following".
> > [skraza]: Fixed.
> >
> >     I share the YANG doctor's concern that separating the configuration
> and
> >     operational state constructs is not consistent with the NMDA model.
> > [skraza]: Fixed.
> >
> >     Section 1.1
> >
> >        The "base" category contains the basic and fundamental features
> that
> >        are covered in LDP base specification [RFC5036] and constitute
> the
> >        minumum requirements for a typical base LDP deployment.  Whereas,
> the
> >        "extended" category contains all other non-base features.  All
> > the
> >
> >     This statement ("all other") is not future-proof.
> > [skraza]:  Removed the "all".
> >
> >     Section 3
> >
> >        o  This module aims to address only the core LDP parameters as
> per
> >           RFC specification, as well as some widely deployed non-RFC
> >           features (such as label policies, session authentication etc).
> >           Any vendor specific feature should be defined in a vendor-
> specific
> >           augmentation of this model.
> >
> >     [Even for widely-deployed non-RFC features we still need to provide
> a
> >     clear description of what semantics are being covered, whether
> >     in-document or via an external reference.]
> >
> > [skraza]: The only non-RFC config was related to label policies -
> Reworked/enhanced the text.
> >
> >        o  This model is a VPN Forwarding and Routing (VRF)-centric
> model.
> >           It is important to note that [RFC4364] defines VRF tables
> > and
> >
> >     side note: I would like the IETF to someday get to a point where
> "VPN"
> >     (the 'P' is for "private") is used only for protocols/deployments
> that
> >     provide confidentiality protection ("privacy") for the data being
> >     conveyed.  We are, of course, not there now, and I cannot insist on
> any
> >     change here.  But please consider whether an alternate phrasing
> would
> >     suffice, that includes just the "virtual" part but not "VPN".  Since
> we
> >     are mostly using "VRF" in the rest of the document, this is not as
> >     daunting as it sometimes is...
> >
> >     Section 4
> >
> >     It might be nice to have the tree show the ipv4 and ipv6 children in
> >     as similar an order as is possible (with the understanding that the
> >     semantics are not affected by the order of appearance, of course).
> >
> >            |  +--rw address-families
> >            |  |  +--rw ipv4!
> >            |  |  |  +--rw enable?                           boolean
> >            |  |  |  +--ro label-distribution-controlmode?   enumeration
> >            |  |  |  +--ro bindings
> >            |  |  |  |  +--ro address* [address]
> >            |  |  |  |  |  +--ro address               inet:ipv4-address
> >            |  |  |  |  |  +--ro advertisement-type?   advertised-
> received
> >            |  |  |  |  |  +--ro peer
> >            |  |  |  |  |     +--ro lsr-id?           leafref
> >            |  |  |  |  |     +--ro label-space-id?   leafref
> >
> >     Does this imply that there is only one peer per address?  Is that
> going
> >     to be a limitation for any deployment cases?
> >
> > [skraza]: Not an issue. Note that a peer may have one or more bound
> addresses.
> >
> >               +--rw ldp-ext:session-downstream-on-demand
> >               |       {session-downstream-on-demand-config}?
> >               |  +--rw ldp-ext:enable?      boolean
> >
> >     nit(?): missing '|' to connect down to ldp-ext:enable?
> >     Similarly for ldp-ext:max-wait a few lines later, and some other
> >     occurrences later in the document.
> >
> >
> > [skraza]: Some of these instances/typos were due to hand picked sub-
> tree.
> > Not an issue anymore as we are printing the entire tree as generated by
> the tool.
> >
> >
> >     Section 5.1.2
> >
> >     Am I reading this right that (e.g.) per-interface hello-holdtime is
> >     configurable as an extension and only the global hello-holdtime is
> >     present in the "base" module?
> >
> > [skraza]: Yes.
> >
> >     Section 7
> >
> >     "NHLFE" is used only once in this document (and is not listed as
> >     "well-known" at
> >     https://www.rfc-editor.org/materials/abbrev.expansion.txt) please
> >     expand it out.
> >
> > [skraza]: It is well known and listed in the above url. Please x-chk.
> 
> To be clear, the linked content uses the convention of an "*" marking to
> indicate that the expansion is "well-known" and does not need to be
> provided in a given document.  The entry for "Next Hop Label Forwarding
> Entry" does not include such a marking; perhaps the responsible AD should
> request it to be added, if it is indeed well-known?
> 
> >     Section 9
> >
> >     Is 2018 still the right copyright year for the modules themselves?
> >
> >
> > [skraza]: 2020-02-25 rev now.
> >
> >
> >     Section 9.1
> >
> >          typedef ldp-address-family {
> >            type identityref {
> >              base rt:address-family;
> >            }
> >
> >     Why can't we use rt:address-family directly in the one container
> that
> >     uses this?
> >
> > [skraza]: Fixed now.
> >
> >            description
> >              "Duration represented as 32 bit seconds with infinite.";
> >
> >     nits: "32-bit" (hyphenated), "with infinity".
> > [skraza]:  Fixed.
> >
> >          typedef downstream-upstream {
> >            type enumeration {
> >              enum downstream {
> >                description "Downstream information.";
> >              }
> >              enum upstream {
> >                description "Upstream information.";
> >              }
> >            }
> >            description
> >              "Received or advertised.";
> >          }
> >
> >     nit: copy/paste on the description?
> > [skraza]:  Fixed.
> >
> >              leaf used-in-forwarding {
> >                type boolean;
> >                description
> >                  "'true' if the lable is used in forwarding.";
> >
> >     nit: s/lable/label/
> > [skraza]:  Fixed.
> >
> >          grouping graceful-restart-attributes-per-peer {
> >            description
> >              "Per peer graceful restart attributes.
> >               On the local side, these attributes are configuration and
> >               operational state data. One the peer side, these
> attributes
> >               are operational state data reveiced from the peer.";
> >
> >     nit: s/reveiced/received/
> > [skraza]:  Fixed.
> >
> >          grouping peer-state-derived {
> >            description
> >              "The peer state information derived from the LDP protocol
> >               operatoins.";
> >
> >     nit: s/operatoins/operations/
> > [skraza]:  Fixed.
> >
> >          grouping peer-state-derived {
> >          [...]
> >            leaf next-keep-alive {
> >              type uint16;
> >              units seconds;
> >              config false;
> >              description "Time to send the next KeepAlive message.";
> >            }
> >
> >     nit: this description text sounds like it's giving an absolute time,
> but
> >     given that it's a uint16 I presume it's "time [from now] until
> sending".
> > [skraza]:  Clarified.
> >
> >            container received-peer-state {
> >              config false;
> >              description
> >                "Operational state information learned from the peer.";
> >
> >              uses graceful-restart-attributes-per-peer;
> >
> >              container capability {
> >                description "Configure capability.";
> >
> >     This is config-false; "Configure" in the description cannot be
> correct,
> >     though I suppose "Configured" could be.
> >     (Also applies to several of the child containers' descriptions.)
> >
> > [skraza]:  Fixed all these.
> >
> >            leaf up-time {
> >              type string;
> >              config false;
> >              description "Up time. The interval format in ISO 8601.";
> >
> >     We probably should have some sort of reference for ISO 8601 (though
> of
> >     course it cannot actually *be* a <xref> in the YANG module itself)
> >
> > [skraza]:  Changed to use timeticks64 now.
> >
> >            leaf notification {
> >              type yang:counter64;
> >              description
> >                "The number of messages sent or received.";
> >
> >     Is this the same as or different than "leaf total-messages"?
> > [skraza]:  Fixed.
> >
> >                    leaf label-distribution-controlmode {
> >
> >     nit: any reason to not hyphenate control-mode as well?
> > [skraza]:  Fixed.
> >
> >                container interfaces {
> >                  description
> >                    "A list of interfaces for LDP Basic Descovery.";
> >
> >     nit: s/Descovery/Discovery/
> > [skraza]:  Fixed.
> >
> >              container discovery {
> >                description
> >                  "Neibgbor discovery configuration and operational
> > state.";
> >
> >     nit: s/Neibgbor/Neighbor/
> > [skraza]:  Fixed.
> >
> >                        // ipv4
> >                        container hello-adjacencies {
> >                          config false;
> >                          description
> >                            "Containing a list of hello adjacencies.";
> >
> >                          list hello-adjacency {
> >                            key "adjacent-address";
> >                            config false;
> >                            description "List of hello adjacencies.";
> >
> >                            leaf adjacent-address {
> >                              type inet:ipv4-address;
> >                              description
> >                                "Neighbor address of the hello
> adjacency.";
> >                            }
> >
> >     I'm not an expert here, but I'm not really seeing why we need to
> >     specialize this container for v4 vs v6 as opposed to just using
> >     inet:ip-address and having something that's applicable to both.
> >
> > [skraza]: We could. There are pros and cons either way. The reasons to
> use separate containers in this model are:
> > 1) Operators usually plan and configure IPv4 and IPv6 separately, rather
> than mixing the address family together. The separate containers provide
> more focus and clear separations.
> > 2) Often times, there are attributes with different types for different
> address families. Mixing two together brings up some modeling difficulties
> to specifies differently for different address families.
> > 3) Sometimes, there are attributes that are applicable to one address
> family but not the other. Mixing the two containers together would require
> some difficult conditions, which would complicate the model and make the
> model less usable.
> > 4) Mixing the two containers can make the YANG file more compact, but
> not the configuration data and operational state data.
> > 5) The existing RFCs use the style of separate containers, such as RFC
> 8344 and RFC 8349.
> 
> Thanks for writing this out; it helps me understand the situation better.
> 
> >                        leaf adjacent-address {
> >                          type inet:ipv4-address;
> >                          description
> >                            "Configures a remote LDP neighbor and enables
> >                             extended LDP discovery of the specified
> >                             neighbor.";
> >
> >     Given that there's a sibling leaf "enable", do we still want "and
> >     enables" here?
> > [skraza]: We don’t need “enables” here. Removed it.
> >
> >
> >     Should peer/capability have a comment in its description that the
> >     container exists to be augmented by other (extension) modules?
> >
> > [skraza]: Yes. It is better. Added as suggested.
> >
> >          notification mpls-ldp-fec-event {
> >
> >     Am I reading this right that we only generate notifications on 0->1
> and
> >     1->0 transitions of "number of prefixes active for a given FEC"?
> Would
> >     we ever want to know about addition/removal of prefixes that do
> leave
> >     the FEC up?
> >
> > [skraza]: Here, the prefix is the FEC. Addition/removal of a prefix is
> equivalent to the addition/removal of the FEC. Changed the leaf name from
> prefix to fec to avoid the confusion.
> >
> >     Section 9.2
> >
> >     I can't find a pattern into which if-feature statements go into
> >     containers used to augment things vs. the augment directives
> themselves.
> >     Can we be more consistent (or am I missing the pattern)?
> >
> > [skraza]: It is not intended to set up a strict pattern between feature
> name and containers, while the purpose is to provide more descriptive
> feature names. That said, the parent container names are often used to
> provide the contexts for the feature name. e.g. In the case of “dual-
> stack-transport-pereference”, it seems that the parent container name
> “peers” is also beneficial, so we have the feature name “dual-stack-
> transport-preference” to “peers-dual-stack-transport-preference” to make
> the feature name fit better with the pattern and more descriptive.
> 
> Thanks for the explanation
> 
> >          feature dual-stack-transport-pereference {
> >            description
> >              "This feature indicates that the system allows to configure
> >               the transport connection pereference in a dual-stack
> > setup.";
> >
> >     nit: s/pereference/preference/ (twice)
> > [skraza]: Fixed.
> >
> >          feature policy-targeted-discovery-config {
> >            description
> >              "This feature indicates that the system allows to configure
> >               policies to control the acceptance of targeted neighbor
> >               discovery hello messages.";
> >
> >     nit: s/hello/Hello/
> >     [skraza]: Fixed.
> >
> >          typedef neighbor-list-ref {
> >            type string;
> >            description
> >              "A type for a reference to a neighbor address list.
> >               The string value is the name identifier for uniquely
> >               identifying the referenced address list, which contains a
> list
> >               of addresses that a routing policy can applied. The
> definition
> >               of such an address list is outside the scope of this
> >               document.";
> >
> >     If I'm reading correctly between here and where this is used, the
> >     semantics of this are better described as "implementation-specific"
> --
> >     that is, this is a label that refers to some local configured state,
> >     specifically, a list of neighbor addresses used as an ACL.
> (Emphasis on
> >     *local* -- it seems like it does not need to interact with anything
> >     else.)
> >     I do sympathize with the YANG doctor, though -- as-is, this feels
> >     underspecified.
> >     (BTW, IP ACLs are not considered to be a security measure by the
> broader
> >     security community.)
> >     Similarly for the subsequent <foo>-list-refs.
> >
> > [skraza]: We do understand the concerns, and have fixed these two types
> by using the https://tools.ietf.org/html/draft-ietf-rtgwg-policy-model,
> which was not available when we initially started to work on this model.
> 
> Great!
> 
> >              container interfaces {
> >                description
> >                  "A list of interfaces on which forwarding is
> > disabled.";
> >
> >     nit: given the separate 'ldp-disable' leaf, perhaps s/is/can be/ is
> in
> >     order.
> > [skraza]: Understand the confusion. Changed as suggested.
> >
> >     Also, the description of the list itself implies additional
> >     functionality, of listing the interfaces used for basic discovery.
> >
> > [skraza]: The description was wrong. We don’t intend to use this list
> for discovery. Fixed.
> >
> >          grouping graceful-restart-augment {
> >            description "Augmentation to graceful restart.";
> >
> >            leaf helper-enable {
> >              if-feature graceful-restart-helper-mode;
> >              type boolean;
> >              default false;
> >              description
> >                "Enable or disable graceful restart helper mode.";
> >
> >     Where is this "helper mode" documented?
> > [skraza]: This is not an RFC defined term but commonly known and used
> when GR timer is set to 0.
> >   Fixed the description in yang to highlight this.
> 
> Is this just staged for the -09?  I am not seeing it in the -08.
> 
> >          augment "/rt:routing/rt:control-plane-protocols/ldp:mpls-ldp/"
> >            + "ldp:global" {
> >            description "Graceful forwarding nexthop augmentation.";
> >            uses global-forwarding-nexthop-augment;
> >          }
> >
> >     nit: I don't think "Graceful" is correct, in the description.
> >     [skraza]: Removed it.
> >
> >     Why is (the augmented)
> >     /rt:routing/rt:control-plane-protocols/ldp:mpls-
> ldp/ldp:global/ldp:address-families/ipv6/label-distribution-controlmode
> >     'config false'?  Doesn't RFC 5036 say that an LSR might support
> either
> >     as a configuration option?
> >
> > [skraza]: Yes, but no implementation supports such a cfg as change of
> such a mode is a massive disruptive operation - all implementation pick
> one mode as the default and only supported mode and stick to it.
> >   The LDP YANG design team had discussed this early enough and agreed to
> not provide this a cfg option.
> 
> Ah, understood.
> 
> >                leaf adjacent-address {
> >                  type inet:ipv6-address;
> >                  description
> >                    "Configures a remote LDP neighbor and enables
> >                     extended LDP discovery of the specified
> >                     neighbor.";
> >
> >     As for the ipv4 case, do we still want "and enables" here?
> > [skraza]: Fixed by removing it.
> >
> >          augment "/rt:routing/rt:control-plane-protocols/ldp:mpls-ldp/"
> >            + "ldp:discovery/ldp:interfaces/ldp:interface/"
> >            + "ldp:address-families/ldp-ext:ipv6" {
> >            description "Interface address family IPv6 augmentation.";
> >            uses interface-address-family-ipv6-augment;
> >
> >     Didn't we just create .../ldp:address-families/ldp-ext:ipv6 with an
> >     earlier augmentation in this module?  (Similarly for
> >     /rt:routing/rt:control-plane-protocols/ldp:mpls-
> ldp/ldp:peers/ldp:peer/ldp:address-families/ldp-ext:ipv6
> >     which we augment with "uses peer-af-policy-container".)
> >
> > [skraza]: Yes. It is ok to augment a node which is part of another
> augmentation. The result is to add them all together. Doing so is to avoid
> too much irregular patterns, so that we have more similar construction
> between IPv4 and IPv6.
> >
> >     Appendix A
> >
> >                      "is-router": [null],
> >
> >     This looks weird to me ("array containing one element with value
> literal
> >     null") -- why does the array come into play?
> >
> >     [Xufeng]: Here is a little explanation to the seemingly confusing
> notation: the pair of square brackets does not indicate a normal array.
> Instead, it is a special notation to indicate that the value of a “empty”
> type leaf is “present” (Section 6.9. of RFC 7951). Semantically the line
> above says that “(the neighbor) is a router.”
> 
> Ah, thanks for the explanations.
> 
> -Ben