Re: [yang-doctors] [Lsr] Yangdoctors last call review of draft-ietf-ospf-yang-09

Rob Shakir <rjs@rob.sh> Mon, 19 March 2018 09:22 UTC

Return-Path: <rjs@rob.sh>
X-Original-To: yang-doctors@ietfa.amsl.com
Delivered-To: yang-doctors@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id B4FDD126DFB for <yang-doctors@ietfa.amsl.com>; Mon, 19 Mar 2018 02:22:30 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.898
X-Spam-Level:
X-Spam-Status: No, score=-1.898 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, URIBL_BLOCKED=0.001] autolearn=unavailable autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=rob-sh.20150623.gappssmtp.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ZqTl--LCvzni for <yang-doctors@ietfa.amsl.com>; Mon, 19 Mar 2018 02:22:29 -0700 (PDT)
Received: from mail-ot0-x22a.google.com (mail-ot0-x22a.google.com [IPv6:2607:f8b0:4003:c0f::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 91BA3124B18 for <yang-doctors@ietf.org>; Mon, 19 Mar 2018 02:22:25 -0700 (PDT)
Received: by mail-ot0-x22a.google.com with SMTP id 79-v6so16580452oth.11 for <yang-doctors@ietf.org>; Mon, 19 Mar 2018 02:22:25 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rob-sh.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Wxaz0O9aDhUjv78odxPGju6w/QCzmnFQ6g2wHEmIri0=; b=QSPoZCx+LhSpKQG2NDUXdIpdTroC5SrsGpZe2qHYxXuzTLnpKlyBjX54Pjorik8gc2 4QOhev2Ap4dLeX2+dmgyHbH40iUNCyE+hqo+Pp8zCWgqQKV9PGVZ38r3t3Hlugr5/eQB 5sgepqwiD5q8PWcX6qB20IZyEZHVw4g+WnU8WRJDUQXbhTrWhT9kXDJJPkTQxt/wC8WI zETKfFDbjekokumUtmqWE1ZIolQY6aisfs4wOlsJCngP0avkfrb6SwB/fq8Io4t7nkSN 9lcICE8jWWHnruqQvzUHSgF6okTzuanWKr64rkLdP7zwizLNjyd1ucHgHWFbhVHNNsj1 S1Ww==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Wxaz0O9aDhUjv78odxPGju6w/QCzmnFQ6g2wHEmIri0=; b=jMyTziOzEHV9S2Ksr7HseiTf5BazmQ+OYNJVw1ZW6x/+83iT1gYUmjsClbtAlpP8Q7 u6Vzk4b5KlnJWoF+Hy/vqg1A9aCe3maWulXVwDj9+SiIqleIJQ/9CG3TNNwITxq0NoH6 cOodMsHBNAMyZXc6Vq64k2ZRY1ux5DHvhOrGnTfBoRBmFZXhbN4HfRHZRw6Z9GhNXjoo eq5WFRo6GIp7RRI5lyg7EQlZonqgCEooK6X1+TpU827DPDDUztY6288E7eQe+K0/LuWD Sggrh2FO2Jd76awRID50D/HZqmEgrxraFUd2bsPOIaUNORNB2Q3ZM+T20WzRIggZgdas 0FBQ==
X-Gm-Message-State: AElRT7Ei+ZgDmX44MMXS7KVjnRsiTIbR+25QyrbjL9JkOegU6NHP0h9R KQ0kLfUYZckOQSChQorYKEyq4sirm2sayifJ4s0/Xw==
X-Google-Smtp-Source: AG47ELtKRkln4sIPCqgbn5Z3TbW55nbeUOaUKMagLzGKzGCUSHmNYADVRV15r/PD9heQkzSDsxoDZfOMRYO0+R+z8jc=
X-Received: by 2002:a9d:1c97:: with SMTP id l23-v6mr6609768ota.297.1521451344546; Mon, 19 Mar 2018 02:22:24 -0700 (PDT)
MIME-Version: 1.0
References: <20171206.124611.1073986528289329597.mbj@tail-f.com> <1F5FF6F1-1309-4A89-9A79-4607C2423CEE@cisco.com> <ED41D824-69F8-4A01-92E7-890E908BB0CE@cisco.com> <20180314.083035.1071848174448590900.mbj@tail-f.com> <2AA1D0A2-3FB5-4512-A038-FD7FA75A1AD0@cisco.com>
In-Reply-To: <2AA1D0A2-3FB5-4512-A038-FD7FA75A1AD0@cisco.com>
From: Rob Shakir <rjs@rob.sh>
Date: Mon, 19 Mar 2018 09:22:13 +0000
Message-ID: <CAHxMReZNGGtYF45==v2h7M0KWe3sb63EaQ0W039U_whB6pJmhg@mail.gmail.com>
To: "Acee Lindem (acee)" <acee@cisco.com>
Cc: Martin Bjorklund <mbj@tail-f.com>, "lsr@ietf.org" <lsr@ietf.org>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>, "lhotka@nic.cz" <lhotka@nic.cz>, "draft-ietf-ospf-yang.all@ietf.org" <draft-ietf-ospf-yang.all@ietf.org>
Content-Type: multipart/alternative; boundary="000000000000ef61260567c07eb2"
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/1duOVBJhE1S20ggAZjlSEUAAG48>
Subject: Re: [yang-doctors] [Lsr] Yangdoctors last call review of draft-ietf-ospf-yang-09
X-BeenThere: yang-doctors@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: Email list of the yang-doctors directorate <yang-doctors.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/yang-doctors/>
List-Post: <mailto:yang-doctors@ietf.org>
List-Help: <mailto:yang-doctors-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 19 Mar 2018 09:22:31 -0000

Hi Acee,

For the descriptions, there are tools that use these to build documentation
automatically for a YANG model. We have found that such documentation is
particularly useful for engineers that are actually consuming the models to
generate config.

Going forward, if we expect YANG modelled data to be the way that we
interact with network elements, then such generated documentation is likely
to replace a bunch of CLI manuals, so it is worth ensuring the descriptions
are useful in such a context IMHO.

Just my $0.02,
r.
On Fri, Mar 16, 2018 at 17:31 Acee Lindem (acee) <acee@cisco.com> wrote:

> Hi Martin,
>
> On 3/14/18, 3:30 AM, "Martin Bjorklund" <mbj@tail-f.com> wrote:
>
>     Hi,
>
>     "Acee Lindem (acee)" <acee@cisco.com> wrote:
>     > Hi Martin,
>     > We believe that
> https://datatracker.ietf.org/doc/draft-ietf-ospf-yang/
>     > satisfies your comments. Please take a look.
>     > Thanks,
>     > Acee
>     >
>     >     On 12/6/17, 6:46 AM, "Martin Bjorklund" <mbj@tail-f.com> wrote:
>     >
>     >         Ladislav Lhotka <lhotka@nic.cz> wrote:
>     >         > Reviewer: Ladislav Lhotka
>     >         > Review result: Ready with Issues
>     >         >
>     >         > The data model defined in this document is a massive piece
> of work:
>     >         > it
>     >         > consists of 11 YANG modules and defines around 1200 schema
> nodes. The
>     >
>     >         11 YANG modules?  Isn't there just one?
>     >
>     >
>     >         I would like to add two quick comments to Lada's review.
>     >
>     >         o  Remove all "revision" statements except the one that says
> "initial
>     >            revision".   The idea is to have one revision statement
> per
>     >            published version (i.e., RFC).
>
>     This is now addressed.
>
>
>     >
>     >         o  Many of the nodes have quite rudimentary descriptions,
> and the
>     >            "reference" statement is rarely used.  For example:
>     >
>     >                  leaf lsa-id {
>     >                    type yang:dotted-quad;
>     >                    mandatory true;
>     >                    description "LSA ID.";
>     >                  }
>     >
>     >            It seems as the description is put there just to keep the
> tools
>     >            quiet.  I know that it is painful to write good
> descriptions, but
>     >            it does help the consumer of the data model.
>
>     I don't think this was addressed; many descriptions are still on this
>     form (including the lsa-id).  But if the authors and the WG believe
>     that this is clear to the reader this is fine.
>
> Note that I did beef up the descriptions and references for a number of
> data leaves. This is a pretty fundamental concept in OSPF and one could
> argument that someone who doesn’t understand it won't benefit from looking
> at the YANG model. However, we will expand the abbreviation in the first
> occurrence and indicate that LSAs are uniquely identified by the <lsa-type,
> lsa-id, adv-router> tuple in the various database description text.
>
>
>     Just a few additional comments:
>
>     o  In section 3, before the module, add:
>
>        This module augments [I-D.ietf-netmod-rfc8022bis], imports a
>        grouping from [I-D.ietf-bfd-yang], and references [RFC1765],
>        [RFC4552], [RFC5082], [RFC5286], [RFC5329], [RFC5443], [RFC5631],
>        [RFC5714], [RFC5880], [RFC5881], [RFC6021], [RFC6860], [RFC6987],
>        [RFC7490], [RFC7684], [RFC7777], [RFC8291], and
>        [I-D.ietf-rtgwg-backoff-algo].
>
>
>       and add these documents to the references.
>
>       NOTE: the list above is probably not complete, please ensure that
>       all references in the YANG module are covered in this list, except
>       the ones that are already present in the text body of the document
>       (like RFC 8177).
>
> Thanks - actually I knew about these many were a result of the above done
> right before the IETF cut-off. I was fully expecting a comment from Tom
> Petch by now... We'll make sure we add all these in the next revision.
>
>
>       Also add RFC editor notes to the "RFC XXXX" strings you have that
>       refer to current I-Ds (as opposed to the current document).
>
> Right - a few of these are in the AUTH48 hours and will be updated to the
> actual numbers in the next revision.
>
>
>     o  Remove the statement
>
>          reference "RFC XXXX";
>
>        on the top-level (before the revision statement).
>
>        (The revision statement references this RFC).
>
> Sure.
>
>
>     o  I suggest you remove the "end comments" like:
>
>           } // list link-scope-lsas
>
>        It is not really clear why some end brackets have them and some do
>        not.  Also they are not consistent, and sometimes don't match the
>        "other end" (probably half of the ones I quickly checked were not
>        correct).
>
>        If you decide to keep them, please ensure that they are consistent
>        and correct.
>
> I agree - let me discuss with my co-authors.
>
> Thanks,
> Acee
>
>
>
>
>
>     /martin
>
>
>     >
>     >
>     >         /martin
>     >
>     >
>     >         > "ietf-ospf@2017-10-30" module is compatible with the NMDA
>     >         > architecture.
>     >         >
>     >         > **** Comments
>     >         >
>     >         > 1. Unless there is a really compelling reason not to do
> so, the
>     >         >    "ietf-ospf" should declare YANG version 1.1. For one,
>     >         >    "ietf-routing" that is being augmented by "ietf-ospf"
> already
>     >         >    declares this version. Some of my suggestions below
> also assume
>     >         >    version 1.1.
>     >         >
>     >         > 2. The "ietf-ospf" can work only with the new
> NMDA-compatible
>     >         >    revisions of some modules, such as "ietf-interfaces" and
>     >         >    "ietf-routing". I understand it is not desirable to
> import such
>     >         >    modules by revision, but at least it should be
> mentioned in a
>     >         >    description attached to every such import.
>     >         >
>     >         > 3. Maybe the draft could mention that implementations
> should supply a
>     >         >    default routing domain as a system-controlled resource.
>     >         >
>     >         > 4. In "when" expressions, the module uses literal strings
> for
>     >         >    identities. This is known to be problematic, the XPath
> functions
>     >         >    derived-from() or derived-from-or-self() should be used
> instead.
>     >         >
>     >         > 5. Some enumerations, such as "packet-type" and
> "if-state-type"
>     >         >    define enum identifiers with uppercase letters and/or
> underscores,
>     >         >    for example "Database-Description" or "LONG_WAIT".
> RFC6087bis
>     >         >    recommends that only lowercase letters, numbers and
> dashes. I
>     >         >    think
>     >         >    this convention should be observed despite the fact
> that the
>     >         >    current names are traditionally used in OSPF specs. The
>     >         >    "ietf-routing" module also defines "router-id" even
> though the
>     >         >    documents use "Router ID".
>     >         >
>     >         > 6. The types of LSA headers are modelled as integers.
> While OSPF
>     >         > gurus
>     >         >    probably know these numbers by heart, it is not very
>     >         >    reader-frienly. So at least some references to
> documents defining
>     >         >    these numbers should be provided, but my suggestion is
> to consider
>     >         >    implementing them with identities. It seems it might
> also be
>     >         >    useful
>     >         >    to define some "abstract" identities for these types.
> For example,
>     >         >    if "opaque-lsa" is defined, then the definition of
> container
>     >         >    "opaque" could simply use
>     >         >
>     >         >      when "derived-from(../../header/type,
> 'ospf:opaque-lsa')";
>     >         >
>     >         >    instead of
>     >         >
>     >         >       when "../../header/type = 9 or "
>     >         >               + "../../header/type = 10 or "
>     >         >               + "../../header/type = 11";
>     >         >
>     >         > 7. The title of sec. 2.9 should be "OSPF Notifications"
> rather than
>     >         >    "OSPF notification".
>     >         >
>     >         >
>     >         > _______________________________________________
>     >         > yang-doctors mailing list
>     >         > yang-doctors@ietf.org
>     >         > https://www.ietf.org/mailman/listinfo/yang-doctors
>     >         >
>     >
>     >
>     >
>     >
>
>
> _______________________________________________
> Lsr mailing list
> Lsr@ietf.org
> https://www.ietf.org/mailman/listinfo/lsr
>