Re: [Ntp] [Last-Call] Last Call: <draft-ietf-ntp-yang-data-model-10.txt> (A YANG Data Model for NTP) to Proposed Standard

Dhruv Dhody <dhruv.ietf@gmail.com> Wed, 17 February 2021 11:35 UTC

Return-Path: <dhruv.ietf@gmail.com>
X-Original-To: ntp@ietfa.amsl.com
Delivered-To: ntp@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id C6CA43A193F; Wed, 17 Feb 2021 03:35:25 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.098
X-Spam-Level:
X-Spam-Status: No, score=-2.098 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.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 zfIB2-QrKl9O; Wed, 17 Feb 2021 03:35:22 -0800 (PST)
Received: from mail-il1-x12c.google.com (mail-il1-x12c.google.com [IPv6:2607:f8b0:4864:20::12c]) (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 EDDF93A193C; Wed, 17 Feb 2021 03:35:21 -0800 (PST)
Received: by mail-il1-x12c.google.com with SMTP id m20so10930626ilj.13; Wed, 17 Feb 2021 03:35:21 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xxX+cTCBtup3DRlMuCaAuAB+LIpwFw7zQXns7rrPZo8=; b=Ud/jFJZqzUM2jk3hEE3XQBgqZYev0T1dXt35m71UAJ+x52LcKBkfHoKMkJLTcvZxdP Y6o17N0CiTyEcPIA+tRf2PGNDBBJ2vXsEqVfoiDmIF47kN+QbxaRRLSnqaJVo0oTMe7w pq1Jn6ElgGmDxjP5axXTtRR9Qme7E7wG7b5iQDxGpKdYdInj3Pa9TGGmfNaVA0h8RWo7 axj4IkorT6XvhonDuSMeQWOXMTNmvUiyEolpG78k1JX2m2WZ8Q4Fu7pMyEAwcAej5Tzh uwZRJQuMM627WEKfav5X4yq25ZHFUnarDGgkXHXyejpYefuzaKd9JHJsw5uZIUG1xUFn EFzw==
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=xxX+cTCBtup3DRlMuCaAuAB+LIpwFw7zQXns7rrPZo8=; b=AOj5tvaWiD67jKpeTpJ0oE+da6eks+9VyDt4EQHIF1Tcaem6QsUBNKwo4sPoCEY/l0 2Vm5p7Yu22a5NZ8kfjnhXV9Qn+f6Sx1o9odjIt1wr3x8UeGIbOxiFBrFjwbh1NqHPWxf wGJ1fbD7kidBBH+kjIH2pbooe2h3BCscOSu/PZ1CS561qGbspIKOSBY7ndXsa8RoZzZd HO49iu+WpfK03QrQKu+Kj0A0QNVZ2g+eyjjA3b5ck2E50oxd072I1uaK/OKLd4MAYbwB UCpc6XeroYbkBPouwXY3o2Zg0zLa7kK1GvTq8hS/lVq1/GNjKScJPctlfI7TASC5qBum POKg==
X-Gm-Message-State: AOAM5325UU8QQE6dq4Nnmtq+C/fhr8BbBJDpD4rpXkyKXwMhKLpbKsOn /H1xITHpnAK6mBvIfedUuAbeoyWemmJg1PnXggo=
X-Google-Smtp-Source: ABdhPJyxxsHybBJOMvhYS587RA7RX8NBb5VQBFvfJgynsmV89Q13HupvvpNgEg2T1pUkNszdph9r+GTw4/Jr7VqM8fE=
X-Received: by 2002:a92:512:: with SMTP id q18mr20536508ile.279.1613561720578; Wed, 17 Feb 2021 03:35:20 -0800 (PST)
MIME-Version: 1.0
References: <161195994417.2651.6499166797756243533@ietfa.amsl.com> <CAB75xn5CQr2yg7wWZHj-sJM7WaaTJK5NF0pzzLhqmx5hHf8GiQ@mail.gmail.com> <60266E12.6070207@btconnect.com> <602A611E.4020306@btconnect.com>
In-Reply-To: <602A611E.4020306@btconnect.com>
From: Dhruv Dhody <dhruv.ietf@gmail.com>
Date: Wed, 17 Feb 2021 17:04:43 +0530
Message-ID: <CAB75xn7QVL+F_5bQ8roZYakbADgQ06pChb0ei7Oaf0=eqLu7Mg@mail.gmail.com>
To: tom petch <daedulus@btconnect.com>
Cc: NTP WG <ntp@ietf.org>, last-call@ietf.org, Ankit Kumar Sinha <ankit.ietf@gmail.com>, ntp-chairs@ietf.org, draft-ietf-ntp-yang-data-model@ietf.org, ek.ietf@gmail.com, Dieter Sibold <dsibold.ietf@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Archived-At: <https://mailarchive.ietf.org/arch/msg/ntp/WseQX1gGQClq6nWL8lu_DdPu9j8>
Subject: Re: [Ntp] [Last-Call] Last Call: <draft-ietf-ntp-yang-data-model-10.txt> (A YANG Data Model for NTP) to Proposed Standard
X-BeenThere: ntp@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: <ntp.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ntp>, <mailto:ntp-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ntp/>
List-Post: <mailto:ntp@ietf.org>
List-Help: <mailto:ntp-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ntp>, <mailto:ntp-request@ietf.org?subject=subscribe>
X-List-Received-Date: Wed, 17 Feb 2021 11:35:26 -0000

Hi Tom,

We have posted another update.

Updated: https://datatracker.ietf.org/doc/draft-ietf-ntp-yang-data-model/
Diff: https://www.ietf.org/rfcdiff?url2=draft-ietf-ntp-yang-data-model-13

As suggested by Ben, added a YANG feature for MD5.

Further details -


> s.6
> /For this reason, this YANG module allow/
> For this reason, this YANG module allows/
>

Updated

> s.7
> This has lost its two character margin, needs insetting
>

Not sure where. I did run "pyang -f yang --keep-comments --yang-line-length 69"

> typedef refid I like, much clearer, but perhaps could do with a
> reference for MD5, RFC1321
>

I added the reference around the new MD5 feature (as suggested by Ben)
and not at refid.

> /Discountinuities in the value/Discontinuities in the value/
> in three places
>

Updated.

> when 'false() = boolean(/sys:system/sys:ntp)' {
> I am sure that this is valid but is not something I am used to seeing.
> An aspect of YANG that I have never understood is when you have to spell
> things out and when they are implicit and I think that all you need here is
> when not <presence container>
> but would want to verify that with a YANG doctor
>

I have used when presence before but not sure what is the best way for
"when not presence" and didn't see it before :)

> The definition of each possible values:/
> The definition of each possible value:/
>

Updated.

>        leaf poll {
>          type uint8;
> This seems wrong to me.  Looking at s.7.3
>     Poll: 8-bit signed integer representing the maximum interval between
>     successive messages, in log2 seconds.  Suggested default limits for
>     minimum and maximum poll intervals are 6 and 10, respectively.
> ie signed integer and not uint
>

int8 is better, updated.

> TTL good to have the reference - I was barking up the wrong tree
>
> I note
> #define TTLMAX          8       /* max ttl manycast */
> I assume that this is a matter of choice - I see 255 in examples which
> is at the other end of the spectrum
>

Updated example.

>            leaf beacon {
>              type uint8;
> I cannot find a definition of this in the RFC (which I think a defect in
> the RFC).  Reverse engineering the code I see beacon compared to
> unreach, which is defined, as
>     unreach: integer representing the number of seconds the server has
>     been unreachable
> which suggests that beacon is a number of seconds, and not a log base 2
> value or a counter; is this right? (The RFC should say IMHO)
>

Yes, it is not log2. I have added more text in the description.

> s.8
> /for illustration purporses./for illustration purposes./
>

Updated.

>
> I have run out of time but have made a second pass.  Some aspects still
> confuse me, where I cannot be sure of the correlation between YANG and
> RFC, notable time and mode; I want the words to be identical and they
> are not!
>
>    identity access-mode {
> I do not understand. The RFC has protocol, association and packet modes
> but not access mode.  This is perhaps section 3, but I am uncertain, the
> terminology is not quite the same, which is perhaps ok for an NTP
> expert, which I am not, but not in a technical specification IMHO.
>

The access-mode is related to the ACL. It is not related to
association and packet modes. The RFC does not go into detail but
mentions its use in Section 9.2.

> Thus you derive
>    identity query-access-mode {
> but searching the RFC only yields a match in Security.  I want the terms
> to be identical!
>

This is not mentioned, but the granularity comes from the existing ACL
configurations with NTP.

> In contrast, association mode is in the RFC and so I can see
>    identity broadcast-client {
> which lacks a value for mode - perhaps 6.  But I would still like a
> reference to section 3 for these.
>

I have made an update and aligned it as per the RFC

>
>    identity ntp-sync-state {
> is again troublesome.  The RFC has
> #define NSET     0       /* clock never set */
> #define FSET     1       /* frequency set from file */
> #define SPIK     2       /* spike detected */
> #define FREQ     3       /* frequency mode */
> #define SYNC     4       /* clock synchronized */
> whereas the I-D has
>    identity clock-not-set {
>    identity freq-set-by-cfg {
>    identity clock-set {
>    identity freq-not-determined {
>    identity clock-synchronized {
>    identity spike {
> which is close but I want them to be identical or a note saying how they
> are different.
>

Updated

>
> Then what is the difference between this and
>    identity clock-state {
> which has
>    identity synchronized {
>    identity unsynchronized {
> Why have both?
>

I have added the description to say one is a higher-level state and
another a granular one.

> Other thoughts
>
> I wonder about the role of
>    identity aes-cmac {
>      base key-chain:crypto-algorithm;
> What does it add that a direct reference to AES-CMAC does not have? Why
> import key-chain.  I am not saying it has no value but am unclear of
> that value.  And
>      leaf algorithm {
> could do with a reference for AES-CMAC.
>

We are no longer using key-chain to make sure we could use the YANG
feature for MD5.

> Some authors include the prefix on the module in the list of prefix;
> seems logical.
>
>     typedef refid
> MD5 could do with a reference.
>
>          leaf clock-stratum {
> What is syspeer? I do not see it in the RFC:-)
>

Updated.

>          leaf clock-precision {
> should be signed not unsigned I think
>

Updated.

> leaf poll
> is indeed poll in section 7.3 but I wonder why  13.1 uses hpoll?
> Probably a comment on the RFC and not on the I-D.
>

Hal provided the details.

> With ones from my previous post, below, that is it!
>
> Tom Petch
>

Thanks!
Dhruv & Ankit

On Mon, Feb 15, 2021 at 5:25 PM tom petch <daedulus@btconnect.com> wrote:
>
> On 12/02/2021 12:01, tom petch wrote:
> > On 11/02/2021 09:39, Dhruv Dhody wrote:
> >> Hi Tom,
> >>
> >> Thanks for your review. I have posted an updated version of the YANG.
> >>
> >> https://datatracker.ietf.org/doc/draft-ietf-ntp-yang-data-model/
> >> https://www.ietf.org/rfcdiff?url1=draft-ietf-ntp-yang-data-model-10&url2=draft-ietf-ntp-yang-data-model-12
> >>
> >
> > That was quick!  I have done a first pass but have yet to chase down the
> > references and one at least looks odd to me. I expect to complete the
> > process on Monday am GMT.
>
> Ian
>
> I have run out of time but have made a second pass.  Some aspects still
> confuse me, where I cannot be sure of the correlation between YANG and
> RFC, notable time and mode; I want the words to be identical and they
> are not!
>
>    identity access-mode {
> I do not understand. The RFC has protocol, association and packet modes
> but not access mode.  This is perhaps section 3, but I am uncertain, the
> terminology is not quite the same, which is perhaps ok for an NTP
> expert, which I am not, but not in a technical specification IMHO.
>
> Thus you derive
>    identity query-access-mode {
> but searching the RFC only yields a match in Security.  I want the terms
> to be identical!
>
> In contrast, association mode is in the RFC and so I can see
>    identity broadcast-client {
> which lacks a value for mode - perhaps 6.  But I would still like a
> reference to section 3 for these.
>
>
>    identity ntp-sync-state {
> is again troublesome.  The RFC has
> #define NSET     0       /* clock never set */
> #define FSET     1       /* frequency set from file */
> #define SPIK     2       /* spike detected */
> #define FREQ     3       /* frequency mode */
> #define SYNC     4       /* clock synchronized */
> whereas the I-D has
>    identity clock-not-set {
>    identity freq-set-by-cfg {
>    identity clock-set {
>    identity freq-not-determined {
>    identity clock-synchronized {
>    identity spike {
> which is close but I want them to be identical or a note saying how they
> are different.
>
>
> Then what is the difference between this and
>    identity clock-state {
> which has
>    identity synchronized {
>    identity unsynchronized {
> Why have both?
>
> Other thoughts
>
> I wonder about the role of
>    identity aes-cmac {
>      base key-chain:crypto-algorithm;
> What does it add that a direct reference to AES-CMAC does not have? Why
> import key-chain.  I am not saying it has no value but am unclear of
> that value.  And
>      leaf algorithm {
> could do with a reference for AES-CMAC.
>
> Some authors include the prefix on the module in the list of prefix;
> seems logical.
>
>     typedef refid
> MD5 could do with a reference.
>
>          leaf clock-stratum {
> What is syspeer? I do not see it in the RFC:-)
>
>          leaf clock-precision {
> should be signed not unsigned I think
>
> leaf poll
> is indeed poll in section 7.3 but I wonder why  13.1 uses hpoll?
> Probably a comment on the RFC and not on the I-D.
>
> With ones from my previous post, below, that is it!
>
> Tom Petch
>
> > Meanwhile,
> >
> > s.6
> > /For this reason, this YANG module allow/
> > For this reason, this YANG module allows/
> >
> > s.7
> > This has lost its two character margin, needs insetting
> >
> > typedef refid I like, much clearer, but perhaps could do with a
> > reference for MD5, RFC1321
> >
> > /Discountinuities in the value/Discontinuities in the value/
> > in three places
> >
> > when 'false() = boolean(/sys:system/sys:ntp)' {
> > I am sure that this is valid but is not something I am used to seeing.
> > An aspect of YANG that I have never understood is when you have to spell
> > things out and when they are implicit and I think that all you need here is
> > when not <presence container>
> > but would want to verify that with a YANG doctor
> >
> > The definition of each possible values:/
> > The definition of each possible value:/
> >
> >        leaf poll {
> >          type uint8;
> > This seems wrong to me.  Looking at s.7.3
> >     Poll: 8-bit signed integer representing the maximum interval between
> >     successive messages, in log2 seconds.  Suggested default limits for
> >     minimum and maximum poll intervals are 6 and 10, respectively.
> > ie signed integer and not uint
> >
> > TTL good to have the reference - I was barking up the wrong tree
> >
> > I note
> > #define TTLMAX          8       /* max ttl manycast */
> > I assume that this is a matter of choice - I see 255 in examples which
> > is at the other end of the spectrum
> >
> >            leaf beacon {
> >              type uint8;
> > I cannot find a definition of this in the RFC (which I think a defect in
> > the RFC).  Reverse engineering the code I see beacon compared to
> > unreach, which is defined, as
> >     unreach: integer representing the number of seconds the server has
> >     been unreachable
> > which suggests that beacon is a number of seconds, and not a log base 2
> > value or a counter; is this right? (The RFC should say IMHO)
> >
> > s.8
> > /for illustration purporses./for illustration purposes./
> >
> >
> > Back on Monday, all being well
> >
> > I got a bounce message last time from Yi saying on holiday until 22nd
> > February.
> >
> > Tom Petch.
> >
> >
> >> Please find my consolidated response below -
> >>
> >> Email 1:
> >> ========
> >>
> >> On Mon, Feb 8, 2021 at 5:52 PM tom petch <daedulus@btconnect.com> wrote:
> >>>
> >>> I have two main problems and a lot of lesser ones with this I-D; given
> >>> the number, about 50, I am not optimistic that a single cycle of
> >>> revision will see them addressed.
> >>>
> >>> I see a potential loophole in the security which I will post separately
> >>> since the audience is likely to be different.
> >>>
> >>> References are missing or not specific enough so when I try to compare
> >>> values in the I-D with those of the protocol, either I cannot find them
> >>> or they seem to be different. Giving values to enumerations is unusual
> >>> in YANG, since NETCONF does not transmit them, and their presence
> >>> suggests that they are protocol values, in which case I want to see what
> >>> the protocol says.  A reference to a 110 page I-D, with two updating
> >>> I-D, is inadequate IMO - section numbers are needed in every case.
> >>>
> >>
> >> I have added section numbers in most of the cases.
> >> I have changed enums to identity.
> >>
> >>> Introduction
> >>> should mention support, or lack thereof, for NMDA
> >>>
> >>
> >> Its present in Section 1.1 already, I have added keyword NMDA as well
> >> now.
> >>
> >>> 1.4.  Prefixes in Data Node Names
> >>>              | ianach    | iana-crypt-hash          | [RFC7317] |
> >>> the reference is wrong; this is an IANA- maintained module so the
> >>> reference must be to the IANA website
> >>>
> >>
> >> No longer using ianach in this update.
> >>
> >>> 1.5.  Refrences in the Model
> >>> /Refrences/References/
> >>> /refrenced in /referenced in /
> >>>
> >>
> >> Updated
> >>
> >>> 2.  NTP data model
> >>>
> >>> I do not see the value of a condensed model followed immediately by a
> >>> full model. Perhaps the full model should be an Appendix although at
> >>> less than three pages, this is quite small and would be ok on its own
> >>> IMHO.
> >>>
> >>
> >> Updated
> >>
> >>> 4.  Relationship with RFC 7317
> >>> /supports per-interface configurations /
> >>> support per-interface configuration/
> >>>
> >>
> >> Updated
> >>
> >>> 5.  Access Rules
> >>> /refer access-mode) and attach different acl-rule/
> >>> see access-mode) and attach a different acl-rule/
> >>>
> >>
> >> Updated
> >>
> >>> 6.  Key Management
> >>> /32-bits unsigned /32-bit unsigned/
> >>>
> >>> /this YANG modules/this YANG module/
> >>>
> >>
> >> Updated
> >>
> >>> NTP association (for example unicast-server),
> >>> /specefied/specified/
> >>>
> >>
> >> Updated
> >>
> >>> 7.  NTP YANG Module
> >>>
> >>>       import iana-crypt-hash {
> >>>        reference         "RFC 7317: A YANG Data Model for System
> >>> Management";
> >>> wrong reference - this module is IANA-maintained so the reference must
> >>> be to the IANA website
> >>>
> >>
> >> Not using iana-crypt-hash anymore!
> >>
> >>>       contact
> >>>          WG List:  <mailto: ntpwg@lists.ntp.org
> >>> this is not the address I see on the datatracker
> >>>
> >>> the I-D has five editors but there are only two here
> >>>
> >>
> >> There are 2 co-authors marked as editors. RFC 8407 says the document
> >> author or editor contact information needs to be present.
> >>
> >>>       typedef access-mode {
> >>> I cannot find this in RFC5905
> >>>
> >>
> >> Section 9.2 and Appendix A.5.4. I have added section number in reference.
> >>
> >>>       typedef association-mode {
> >>> this I can find but it ranges from 0 to 7 whereas the I-D has 0 to 4
> >>> - is
> >>> this intended?
> >>>
> >>
> >> Changed to identity.
> >>
> >>>       typedef ntp-sync-state {
> >>> this I cannot find; a search for 'spike' yields a value of 2 in the
> >>> RFC, 5 here - is this intended?
> >>>
> >>
> >> Instead of enum and values, changed to identity.
> >>
> >>>                effect in XXX seconds.";
> >>> for what value of XXX?
> >>>
> >>
> >> Removed.
> >>
> >>>         leaf packet-sent {
> >>>         leaf packet-received {
> >>>         leaf packet-dropped {
> >>>              discontinuities in the value of sysUpTime.";
> >>> those who have been involved with network management for ten years or
> >>> less will likely not recognise this object.  You could add a reference
> >>> but I suggest you replace it with a YANG-based approach; see for example
> >>> how draft-ietf-ospf-yang handles discontinuities
> >>>
> >>
> >> Updated as suggested.
> >>
> >>
> >>>             leaf access-mode {
> >>> /defination/definition/
> >>>
> >>
> >> Updated
> >>
> >>>             leaf clock-refid {
> >>> ...                         reference clock of the peer to
> >>>                  which clock is synchronized.";
> >>>
> >>> I do not understand this.  Presumably this corresponds to
> >>>                 type string {
> >>>                   length "4";
> >>> from the three type union but what object is this?
> >>>
> >>
> >> I created a typedef and cleaned it up. This is as described in Section
> >> 7.3 of RFC5905 under Reference ID.
> >>
> >>>             leaf clock-offset {
> >>> examples could do with units to make it clear that it is '1.232mS' and
> >>> not '1.232s'
> >>>
> >>
> >> Updated
> >>
> >>>           leaf address {
> >>>             type inet:host;
> >>> this includes the domain name, which I see no mention of in the RFC
> >>>
> >>
> >> Changed to inet:ip-address
> >>
> >>>         list associations {
> >>>              /and isconfigured is required/and isconfigured are
> >>> required/
> >>>
> >>>           leaf address {
> >>>             type inet:host;
> >>> as above, the description seems to ignore the option of the domain name
> >>>
> >>>           leaf refid {
> >>> same union as for leaf clock-refid, but a completely different
> >>> description, neither of which I understand.
> >>>
> >>
> >> Updated (all of the above).
> >>
> >>>                  '20.1.1.1'
> >>> this address would appear to be assigned to Microsoft, not an
> >>> affiliation I see among the authors.  Is the company ok with this?
> >>>
> >>
> >> Updated to 203.0.113.1
> >>
> >>>           leaf reach {
> >>>             type uint8;
> >>> is this the 8-bit p.reach shift register? reference needed (again:-)
> >>>
> >>>           leaf unreach {
> >>> ditto
> >>>
> >>
> >> Added.
> >>
> >>>           leaf poll {
> >>>             type uint8;
> >>>             units "seconds";
> >>>             description
> >>>               "The polling interval for current association";
> >>> is there a useful default?  2s appears in the RFC in places
> >>>
> >>
> >> No default in this case.
> >>
> >>>           leaf offset {
> >>> as above, the example values would be clearer with units
> >>>
> >>
> >> Updated
> >>
> >>>           leaf transmit-time {
> >>>             type yang:date-and-time;
> >>>             description
> >>>               "This is the local time, in timestamp format,
> >>>                at which the NTP packet departed the peer(T3).
> >>>                If the peer becomes unreachable the value is set to
> >>> zero.";
> >>> I think, but am not sure, that a yang:date-and-time can never be set to
> >>> zero, the syntax does not allow it; the usual approach with YANG is a
> >>> union with another type which can indicate a special condition - int,
> >>> boolean, etc
> >>>
> >>>           leaf input-time {
> >>>             type yang:date-and-time;
> >>> ditto
> >>>
> >>
> >> Thanks, updated as per your suggestion.
> >>
> >>>               leaf ttl {
> >>>                 type uint8;
> >>>                 description
> >>>                   "Specifies the time to live (TTL)
> >>> TTL does not exist in IPv6
> >>>
> >>
> >> This TTL is not related to IP. See section 3.1 for manycast and
> >> updated with reference.
> >>
> >>>               uses common-attributes {
> >>>                 description
> >>> /attribute like/attributes such as/
> >>>
> >>
> >> Updated
> >>
> >>>               leaf ttl {
> >>>                 type uint8;
> >>>                 description
> >>>                   "Specifies the maximum time to live (TTL) for
> >>> TTL does not exist in IPv6
> >>>
> >>
> >> As above.
> >>
> >>>               uses common-attributes {
> >>> /attributes like/attributes such as/
> >>>
> >>
> >> Updated
> >>
> >>>               leaf beacon {
> >>> what are the units and is there a default? Is there a maximum of 15?  As
> >>> ever, a reference could tell me.
> >>>
> >>
> >> Added reference
> >>
> >>> 8.  Usage Example
> >>> lots of examples but none for IPv6 or JSON
> >>>
> >>
> >> Added one IPv6. I dont think we need add one for JSON as well.
> >>
> >>> 8.1
> >>>        <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf-ntp">
> >>> sys: is a defined prefix and must not be re-used
> >>>
> >>
> >> Oops, updated here and all other instance!
> >>
> >>>            <refid>20.1.1.1</refid>
> >>> as above, is Microsoft ok with this?
> >>>
> >>
> >> Updated to 203.0.113.1
> >>
> >>> 8.2
> >>>          <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf-ntp">
> >>> sys: is a defined prefix and must not be re-used
> >>>
> >>> 8.3
> >>>          <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf-ntp">
> >>> sys: is a defined prefix and must not be re-used
> >>>
> >>> 8.4
> >>>          <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf-ntp">
> >>> sys: is a defined prefix and must not be re-used
> >>>
> >>> 8.5
> >>>    "224.1.1.1"
> >>> would appear to be a reserved address.  Other RFC used 224.0.1.1
> >>>
> >>
> >> Updated
> >>
> >>>          <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf-ntp">
> >>> and again, twice
> >>>
> >>>                <address>224.1.1.1</address>
> >>> as above
> >>>
> >>> 8.6
> >>>    "224.1.1.1"
> >>> as above
> >>>
> >>>           <sys:ntp xmlns:sys="urn:ietf:params:xml:ns:yang:ietf-ntp">
> >>> as above, twice
> >>>
> >>> 8.7
> >>>        <ntp xmlns="urn:ietf:params:xml:ns:yang:ietf-ntp">
> >>> as above
> >>>
> >>> 8.8
> >>>        <ntp xmlns="urn:ietf:params:xml:ns:yang:ietf-ntp">
> >>> as above
> >>>
> >>>            <refid>20.1.1.1</refid>
> >>> as above
> >>>
> >>> 8.9
> >>>        <ntp xmlns="urn:ietf:params:xml:ns:yang:ietf-ntp">
> >>> as above
> >>>
> >>
> >> Updated in all cases.
> >>
> >>> 12.2
> >>>      [RFC7317]  Bierman, A. and M. Bjorklund, "A YANG Data Model
> >>> wrong reference in the wrong place
> >>> this is an IANA-maintained module and so the reference must be to the
> >>> IANA website; and since the module is imported, the reference must be
> >>> Normative.
> >>>
> >>
> >> No longer using iana-crypt-hash.
> >>
> >>> Tom Petch
> >>>
> >>>
> >>> ----- Original Message -----
> >>> From: "The IESG" <iesg-secretary@ietf.org>
> >>> To: "IETF-Announce" <ietf-announce@ietf.org>
> >>> Cc: <ek.ietf@gmail.com>; <ntp-chairs@ietf.org>; <ntp@ietf.org>;
> >>> <dsibold.ietf@gmail.com>; <draft-ietf-ntp-yang-data-model@ietf.org>
> >>> Sent: Friday, January 29, 2021 10:39 PM
> >>> Subject: Last Call: <draft-ietf-ntp-yang-data-model-10.txt> (A YANG Data
> >>> Model for NTP) to Proposed Standard
> >>>
> >>
> >>
> >> Email 2:
> >> ========
> >>
> >> On Mon, Feb 8, 2021 at 6:07 PM tom petch <daedulus@btconnect.com> wrote:
> >>>
> >>> This is my second response to this Last Call, about a possible security
> >>> issue.
> >>>
> >>> RFC8573 seems clear that MD5 must not be used to effect security for NTP
> >>> but this I-D imports iana-crypt-hash which allows MD5 without any
> >>> restriction, so is MD5 allowed or not?
> >>>
> >>> There are features defined which allow the hash in iana-crypt-hash to be
> >>> restricted but this I-D does not use them.
> >>>
> >>> Probably iana-crypt-hash should be updated - I will raise that on the
> >>> NETMOD WG list.
> >>>
> >>
> >> No longer using iana-crypt-hash, instead made this change to allow all
> >> sort of key formats -
> >>
> >> OLD:
> >> |     +--rw key?         ianach:crypt-hash
> >> NEW:
> >> |     +--rw key
> >> |     |  +--rw (key-string-style)?
> >> |     |     +--:(keystring)
> >> |     |     |  +--rw keystring?            string
> >> |     |     +--:(hexadecimal) {hex-key-string}?
> >> |     |        +--rw hexadecimal-string?   yang:hex-string
> >> END
> >>
> >> The comment related to use of MD5 still applies. I have not yet added
> >> a check for the algorithm. I have updated the description and security
> >> consideration to highligh that MD5 is depricated as per RFC 8573. Lets
> >> see what the IESG suggests...
> >>
> >>> The I-D also uses MD5 in a way that would appear not to be security
> >>> related, to hash an IPv6 address.
> >>>
> >>
> >> This is as per RFC 5905 - "If using the IPv6 address family, it is the
> >> first four octets of the MD5 hash of the IPv6 address."
> >>
> >>> In passing, this I-D has three references to RFC7317.  This is wrong -
> >>> the module is IANA-maintained and so the references should be to the
> >>> IANA website.
> >>>
> >>
> >> No longer using iana-crypt-hash.
> >>
> >>> The secdir reviewer might be interested in my thoughts.
> >>>
> >>> Tom Petch
> >>
> >>
> >> Email 3:
> >> ========
> >>
> >> On Tue, Feb 9, 2021 at 4:40 PM tom petch <daedulus@btconnect.com> wrote:
> >>>
> >>> A separate thought to my previous two.
> >>>
> >>> Section 4 is very keen that this I-D and the system module which
> >>> configures ntp SHOULD NOT coexist but this is not enforced by the YANG.
> >>>
> >>> It could be.  Import the system module and make the presence container
> >>> in this I-D dependent on the absence of the presence container in
> >>> /system/ntp.
> >>>
> >>
> >> Updated as per your suggestion!
> >>
> >> ====
> >>
> >> Thanks again for your detailed review. It has improved the YANG model
> >> significantly!
> >>
> >> Thanks!
> >> Dhruv & Ankit
> >>
> >> On Sat, Jan 30, 2021 at 4:09 AM The IESG <iesg-secretary@ietf.org> wrote:
> >>>
> >>