Re: Last Call: <draft-ietf-ippm-twamp-yang-07.txt> (Two-Way Active Measurement Protocol (TWAMP) Data Model) to Proposed Standard

Mahesh Jethanandani <mjethanandani@gmail.com> Fri, 13 April 2018 00:13 UTC

Return-Path: <mjethanandani@gmail.com>
X-Original-To: ietf@ietfa.amsl.com
Delivered-To: ietf@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 91C8B127876; Thu, 12 Apr 2018 17:13:39 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.999
X-Spam-Level:
X-Spam-Status: No, score=-1.999 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_NONE=-0.0001, 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 4CVeNv8RALJ0; Thu, 12 Apr 2018 17:13:37 -0700 (PDT)
Received: from mail-pg0-x242.google.com (mail-pg0-x242.google.com [IPv6:2607:f8b0:400e:c05::242]) (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 3524412D7ED; Thu, 12 Apr 2018 17:13:23 -0700 (PDT)
Received: by mail-pg0-x242.google.com with SMTP id i194so1104211pgd.0; Thu, 12 Apr 2018 17:13:23 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=QDnkjIoFj6GS7IAQIKgcgHXf5b3ipeotL5Rs5sMmLgc=; b=UvJzDNdNIFM/zn2GY85N2Oio44Cc0LLWHvQqnmKPINUetMvYCnDoNmEdU90EHjF1Tv OQbXEXBydk6oytZq1JEA2I22pb89jcGJ6DTElLuJTXoyJxwD2GTZ6VQk6m5r4UJv4G9m bTB3qKBVdHrFbi/FL5DKF0dxLoqD+DuGDnK+QoWMRREBIaOgNOK/C9nPKZlt0u4adHsn 9NwZP+r4ZJfbjeT1Kdn2LfqOznLiTE9LEDtb+WUHFWATefVqLCzNa5vbi2iOnFroqAYe JwoiQcllDvF96m/RhmIco4sP5CfhvjxbVOtqczLZU3VkWrBu+9J5o/K1n2/HZ6N1up8Q vSZg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=QDnkjIoFj6GS7IAQIKgcgHXf5b3ipeotL5Rs5sMmLgc=; b=seDhFqGY8y7T4iMAE6dKOsvcDZr5d7qlLmYT+jqrpKo3eKM9Twcvl3iIiiMhNFYNak 5EwTNRmFokTb8tIpzRda00l5gvkQASUcv2ybXsAwsVUg1nhg94+mlC0EN2B++ee0c825 14AoLnYnTr5MjyjXD9qtKwqpLibeflDvuX/kJ2DuHdgBfZ0axLssShAVcvgdLk2AA/gC 94gSSXy5ncB/k53Cr8iT1a+dnZoDrTCF2mX1ZcdKeof8M/4ksMCsu9G7vxviPSEoi0AT GtUpEYfKKCpOXe1v2hxuBVAdzgI1A+5o1yAjC3Fx3qF629n9GVV9JgiRf97IGhj+uDKI Hs3w==
X-Gm-Message-State: ALQs6tDEzpkW9svfEEDvKChNUmCj0iCf7lSLn7MKhHXPnr4QIv+w0+dW iFKZ4HCFxVRVRT6QAe0ar6E=
X-Google-Smtp-Source: AIpwx4/d2rXGkg0ojAx7WVJ8FKjj1OKZtq+7JyPUlbbDNfkWBcDbLtcsx6h9xGlbUv+Jy8Yu+QunWw==
X-Received: by 10.99.120.5 with SMTP id t5mr2345957pgc.30.1523578402661; Thu, 12 Apr 2018 17:13:22 -0700 (PDT)
Received: from ?IPv6:2601:647:4700:1280:545e:76e1:a2c4:28a6? ([2601:647:4700:1280:545e:76e1:a2c4:28a6]) by smtp.gmail.com with ESMTPSA id p73sm10288725pfa.43.2018.04.12.17.13.21 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 12 Apr 2018 17:13:22 -0700 (PDT)
Content-Type: text/plain; charset="utf-8"
Mime-Version: 1.0 (Mac OS X Mail 11.3 \(3445.6.18\))
Subject: Re: Last Call: <draft-ietf-ippm-twamp-yang-07.txt> (Two-Way Active Measurement Protocol (TWAMP) Data Model) to Proposed Standard
From: Mahesh Jethanandani <mjethanandani@gmail.com>
X-Priority: 3
In-Reply-To: <008401d3d1ab$2ee8d520$4001a8c0@gateway.2wire.net>
Date: Thu, 12 Apr 2018 17:13:20 -0700
Cc: ietf@ietf.org, ippm-chairs@ietf.org, draft-ietf-ippm-twamp-yang@ietf.org, ippm@ietf.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <E8B358C4-39EC-4EC2-A1C0-55F292A2177B@gmail.com>
References: <008401d3d1ab$2ee8d520$4001a8c0@gateway.2wire.net>
To: "tom p." <daedulus@btconnect.com>
X-Mailer: Apple Mail (2.3445.6.18)
Archived-At: <https://mailarchive.ietf.org/arch/msg/ietf/D0P0unwwpHfxRRsNW6fiZTw6vGk>
X-BeenThere: ietf@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: IETF-Discussion <ietf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ietf>, <mailto:ietf-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/ietf/>
List-Post: <mailto:ietf@ietf.org>
List-Help: <mailto:ietf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ietf>, <mailto:ietf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 13 Apr 2018 00:13:40 -0000

Tom,

First of all, thanks for reviewing the document. See my comments inline with [mj].


> On Apr 11, 2018, at 8:35 AM, tom p. <daedulus@btconnect.com> wrote:
> 
> Some further thoughts on this I-D; I have included the earlier ones (as
> yet unanswered) below.
> 
> typedef test-session-state {
>     .....
>             "Indicates that accepted TWAMP-Test session request.";
> 
> something seems to have been omitted.

[mj] Yes. Should be “Indicates an accepted TWAMP-Test session request.”

> 
> 
> leaf reflector-udp-port {
>               type uint32 {
>                 range "862 | 49152..65535";
> I am puzzled as to why uint32 is used and inet:port-number is not used
> here (happens in two places)

[mj] Ok. Will make the change.

> 
> 
> "Reusable data structure for count which is used both in the Server
> container.";
> 
> 'both' suggests to me that there should be a second clause.

[mj] Yes, made it "Reusable data structure for count, which is used both in the Server and the Control-Client.”

> 
> 
> "grouping max-count {
>       leaf max-count {
>         type uint8 {
>           range 10..31;
>         }
>         default 15;"
> contradicts
> " ...The default max-count value SHOULD be 32768.'
> 32768 is outside the range!

> 
> I think that your problem here is that other TWAMP documentation, such
> as the RFC that you quote and reference uses 'max-count' to mean a 32
> bit value but here you are reusing the term with a different semantic
> and using it to mean an exponent; rename the object to make it clear
> that it is an exponent and not a count e.g. max-count-exp. This occurs
> in several places.

[mj] Ok. How about we call it 'max-count-exponent’, just to be explicit?

> 
> "container session-sender {
>         if-feature session-sender;
>         presence  "Enables TWAMP Session-Sender functionality.";
>         description
>           "Configuration of the TWAMP Session-Sender logical entity";
>         leaf admin-state {
>           type boolean;
>           mandatory true;
>           description
>             "Indicates whether the device is allowed to operate
>              as a TWAMP Session-Sender.";
> "
> 
> A presence container is a boolean so I am unclear what it is that the
> admin-state boolean adds here since the presence container "Enables
> TWAMP Session-Sender functionality.”;

[mj] Good catch. Will remove the presence statement.

> 
> "     container session-reflector {
> "
> same story here

[mj] Ditto.

> 
> Tom Petch
> 
> ----- Original Message -----
> From: "tom p." <daedulus@btconnect.com>
> To: <ietf@ietf.org>
> Cc: <ippm-chairs@ietf.org>; <draft-ietf-ippm-twamp-yang@ietf.org>;
> <ippm@ietf.org>
> Sent: Wednesday, April 11, 2018 12:40 PM
> 
> 
>> Some mostly administrative points on this I-D
>> 
>> [I-D.ietf-ippm-metric-registry] is an Informative Reference so would
> not
>> hold up the production of this as an RFC yet you ask that the text be
>> replaced by the RFC number which implies that you do want it held up.
> I
>> did ask the RFC Editor about this and yes, they would expect to catch
> it
>> but it would seem simpler if this could be a Normative Reference.  You
>> do have the statement up front about replacing the text with the RFC
>> number and yes, the RFC Editor like that.

[mj] I will drop the editor’s note to replace the RFC number for I-D.ietf-ippm-metric-registry.

>> 
>> There is no reference, no legend,  for Tree Diagrams in 5.1 .  There
> is
>> now an RFC on this, RFC 8340 while RFC8343 s.1.3 is an example of how
> to
>> reference this RFC.

[mj] Will add reference to RFC8340.

>> 
>> There is no copyright statement in the YANG module as is required by
>> 6087bis (please, IESG, make this an RFC soon:-)

[mj] Ok. Will add it.

>> 
>> leaf server-start-time {
>> includes
>>                The timestamp format follows RFC 1305
>> but I see no RFC 1305 in the references of the I-D

[mj] Ok. Will add it in the references, and also in the beginning of section 5.2. Will do the same for all the other references in the YANG module.

>> 
>> leaf reflector-udp-port
>> " The default number is
>>                  within to the dynamic port range and  .. "
>> which is not quite English.

[mj] Yes. How about “The default number is within the dynamic port range and …”?

>> 
>> "The new well-known port (862) MAY be used.";
>> This was allocated in 2008 which seems to stretch the meaning of ‘new'

[mj] Al, do you want to comment on this?

>> 
>> leaf secret-key {
>>             type binary;
>> I wonder about the choice of binary; elsewhere, e.g. RFC8177,
>> hexadecimal is used.
>> Are there, should there be, any length constraints on this key?

[mj] RFC8177 says that not all vendors support the definition of key-string as a hexadecimal. Just to be safe, I will make it string.

>> 
>> case poisson {
>> ...
>>   reference
>>                  "RFC 2330: Framework for IP Performance Metrics";
>> RFC2330 I cannot see in the references for the I-D

[mj] Will add.

>> 
>> The Security Considerations are not as per the current template e.g.
> no
>> mention of RESTCONF

[mj] Agree. Will update.

Cheers.

>> 
>> Tom Petch
>> 
>> ----- Original Message -----
>> From: "The IESG" <iesg-secretary@ietf.org>
>> To: "IETF-Announce" <ietf-announce@ietf.org>
>> Cc: <ippm-chairs@ietf.org>; <draft-ietf-ippm-twamp-yang@ietf.org>;
>> <ippm@ietf.org>
>> Sent: Monday, April 09, 2018 3:57 PM
>> 
>>> The IESG has received a request from the IP Performance Measurement
> WG
>> (ippm)
>>> to consider the following document: - 'Two-Way Active Measurement
>> Protocol
>>> (TWAMP) Data Model'
>>>  <draft-ietf-ippm-twamp-yang-07.txt> as Proposed Standard
>>> 
>>> The IESG plans to make a decision in the next few weeks, and
> solicits
>> final
>>> comments on this action. Please send substantive comments to the
>>> ietf@ietf.org mailing lists by 2018-04-27. Exceptionally, comments
> may
>> be
>>> sent to iesg@ietf.org instead. In either case, please retain the
>> beginning of
>>> the Subject line to allow automated sorting.
>>> 
>>> Abstract
>>> 
>>> 
>>>   This document specifies a data model for client and server
>>>   implementations of the Two-Way Active Measurement Protocol
> (TWAMP).
>>>   We define the TWAMP data model through Unified Modeling Language
>>>   (UML) class diagrams and formally specify it using YANG.
>>> 
>>> 
>>> 
>>> 
>>> The file can be obtained via
>>> https://datatracker.ietf.org/doc/draft-ietf-ippm-twamp-yang/
>>> 
>>> IESG discussion can be tracked via
>>> https://datatracker.ietf.org/doc/draft-ietf-ippm-twamp-yang/ballot/
>>> 
>>> 
>>> No IPR declarations have been submitted directly on this I-D.
>>> 
>>> 
>>> 
>>> 
>> 
> 

Mahesh Jethanandani
mjethanandani@gmail.com