[Lsr] Yangdoctors last call review of draft-ietf-lsr-ospfv3-extended-lsa-yang-14

[Mahesh Jethanandani via Datatracker <[e-mail]>]

Reviewer: Mahesh Jethanandani
Review result: On the Right Track

Document reviewed: draft-ietf-lsr-ospfv3-extended-lsa-yang

Status: On the right track

I have marked it as On the Right Track, because of some of the points discussed
below.

Summary:

This document defines a YANG data model augmenting the IETF OSPF YANG model to
provide support for OSPFv3 Link State Advertisement (LSA) Extensibility as
defined in RFC 8362. OSPFv3 Extended LSAs provide extensible TLV-based LSAs for
the base LSA types defined in RFC 5340.

Nits

Please add a section on Instructions to RFC editors stating what they should do
with references such as RFC XXXX.

It would be nice to have some consistency between having description and
reference statements start on a new line or on the same line as the statement.
Right now, they are all over the place.

Some of the descriptions are very cryptic. E.g.

      leaf forwarding-address {
        type inet:ipv4-address;
        description
          "Forwarding address";

s/Description/description in the YANG model. Actually, I was surprised that
pyang did not complain, but yanglint did.

libyang err : Invalid character sequence "Description", expected a keyword.
(Line number 318.) libyang err : Parsing module "ietf-ospfv3-extended-lsa"
failed. YANGLINT[E]: Parsing schema module
"[e-mail]" failed.

s/Addrss/Address/

s/E-/Extended / in all descriptions.

Comments:

The grouping such as ospfv3-e-lsa-as, ospfv3-e-lsa-area, ipv6-fwd-addr-sub-tlv
etc. are used in one place only. Is there a reason why this has not been pulled
inline where it is used? Did not check for all groupings, but if there is only
one use of them, ideally they should be inlined.

No need to repeat parent name in the child. Just length will do in the
following. See Section 4.3.1 of RFC 8407. E.g.

    container route-tag-sub-tlv {
      description
        "Route Tag Sub-TLV";
      leaf route-tag-sub-tlv-length {

Why a double -- in  container unknown--tlv {?

A pyang compilation of the model with —ietf and —lint option was clean.

There are no examples of configuration instance data in the draft. It would be
helpful not only to validate the model, but also help folks who want to use the
model.

A idnits run of the draft reveals a few issues. Please address them.

   Checking boilerplate required by RFC 5378 and the IETF Trust (see
  https://trustee.ietf.org/license-info):
  ---------------------------------------------------------------------

     No issues found here.

  Checking nits according to
  https://www.ietf.org/id-info/1id-guidelines.txt:
  ---------------------------------------------------------------------

     No issues found here.

  Checking nits according to https://www.ietf.org/id-info/checklist :
  ---------------------------------------------------------------------

     No issues found here.

  Miscellaneous warnings:
  ---------------------------------------------------------------------

  == The copyright year in the IETF Trust and authors Copyright Line
     does not match the current year

  == Line 1266 has weird spacing: '... allows  a rou...'

  -- The document date (October 17, 2019) is 1337 days in the past.
     Is this intentional?

  Checking references for intended status: Proposed Standard
  ---------------------------------------------------------------------

     (See RFCs 3967 and 4897 for information about using normative
     references to lower-maturity documents in RFCs)

  == Outdated reference: draft-ietf-bfd-yang has been published as RFC
     9127

  ** Downref: Normative reference to an Experimental RFC: RFC 1765

  ** Downref: Normative reference to an Experimental RFC: RFC 4973

  ** Downref: Normative reference to an Informational RFC: RFC 5309

  ** Downref: Normative reference to an Informational RFC: RFC 5714

  ** Downref: Normative reference to an Informational RFC: RFC 6987

     Summary: 5 errors (**), 0 flaws (~~), 3 warnings (==), 1 comment
     (--).