Re: Genart telechat review of draft-ietf-ippm-twamp-yang-11

Pete Resnick <presnick@qti.qualcomm.com> Wed, 20 June 2018 12:18 UTC

Return-Path: <presnick@qti.qualcomm.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 AAC5E130E53; Wed, 20 Jun 2018 05:18:12 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.701
X-Spam-Level:
X-Spam-Status: No, score=-2.701 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=qti.qualcomm.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 iFvjw15vAh1n; Wed, 20 Jun 2018 05:18:10 -0700 (PDT)
Received: from alexa-out-sd-01.qualcomm.com (alexa-out-sd-01.qualcomm.com [199.106.114.38]) (using TLSv1.2 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 7221D1277CC; Wed, 20 Jun 2018 05:18:10 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=qti.qualcomm.com; i=@qti.qualcomm.com; q=dns/txt; s=qcdkim; t=1529497090; x=1561033090; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=XB2eEEWZxdqL4UJmbeHvh0yjEmFRcRnkQatf6O+j2v8=; b=UjFDCArR6NNrBNDGTU3bbP6mijluL8mIBmC5CoDe+HqBZmsN9aCy8yeH WLM39bap0UzUTEo+Wd0K8b9lRJAGJEFPvXFpYjrQ8glfMSqP0vQzVwp5+ /bEXET+3kWs342WuoMOv7qmIFmYi6s0vGSlMXpDDEMD/hCqtMHCP8HpVp 8=;
Received: from unknown (HELO ironmsg01-sd.qualcomm.com) ([10.53.140.141]) by alexa-out-sd-01.qualcomm.com with ESMTP; 20 Jun 2018 05:18:10 -0700
Received: from nasanexm01f.na.qualcomm.com ([10.85.0.32]) by ironmsg01-sd.qualcomm.com with ESMTP/TLS/AES256-SHA; 20 Jun 2018 05:18:10 -0700
Received: from [172.16.1.204] (10.80.80.8) by NASANEXM01F.na.qualcomm.com (10.85.0.32) with Microsoft SMTP Server (TLS) id 15.0.1365.1; Wed, 20 Jun 2018 05:18:09 -0700
From: Pete Resnick <presnick@qti.qualcomm.com>
To: Mahesh Jethanandani <mjethanandani@gmail.com>
CC: gen-art@ietf.org, ippm@ietf.org, ietf@ietf.org, draft-ietf-ippm-twamp-yang.all@ietf.org
Subject: Re: Genart telechat review of draft-ietf-ippm-twamp-yang-11
Date: Wed, 20 Jun 2018 07:18:07 -0500
X-Mailer: MailMate (1.11.2r5479)
Message-ID: <1EF72E31-8D35-4CB8-BEC7-692E1B0C72B6@qti.qualcomm.com>
In-Reply-To: <C4F60DFB-4D29-4D9B-86E9-0CD5D91B61A0@gmail.com>
References: <152946732769.32402.15646758857363676488@ietfa.amsl.com> <C4F60DFB-4D29-4D9B-86E9-0CD5D91B61A0@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"; format="flowed"
Content-Transfer-Encoding: 8bit
X-Originating-IP: [10.80.80.8]
X-ClientProxiedBy: NASANEXM01G.na.qualcomm.com (10.85.0.33) To NASANEXM01F.na.qualcomm.com (10.85.0.32)
Archived-At: <https://mailarchive.ietf.org/arch/msg/ietf/KpJKrx2DE1ZAEQfBoVu5m1LSQAE>
X-BeenThere: ietf@ietf.org
X-Mailman-Version: 2.1.26
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: Wed, 20 Jun 2018 12:18:13 -0000

Hi Mahesh,

Trimming a bit:

On 20 Jun 2018, at 0:36, Mahesh Jethanandani wrote:


>> 3.1 - s/The test session name that MUST be identical/The test session 
>> name,
>> which MUST be identical (Unless you mean something really weird that 
>> I don't
>> think you mean. If you don't see the difference, then trust me, you 
>> mean
>> "which", not "that”.)
>
> You mean in Section 3.3.

Yes, sorry about that. Section 3.1 has a similar problem:

s/The test session name that uniquely identifies/The test session name, 
which uniquely identifies

and I forgot to note that one.

> How about s/The test session name that MUST be identical with the/The 
> test session name MUST be identical to the/?

That's not quite right. You are giving a list of fields (as you say, 
"Primary configuration fields include:"), so you don't want something in 
that list that is a rule. The field is "the test session name", and that 
field MUST be identical to the client name.

When you say, "the test session name that MUST be identical with...", it 
sounds like there is more than one test session name, and you're talking 
about the one that MUST be identical with the client name. Similarly 
with the above, it sounds like there's one test session name which 
uniquely identifies it, and one that doesn't uniquely identify it. 
That's not what you mean.

>> 4.1 -
>>
>> OLD
>>   Specifically, mode-preference-chain lists the
>>   mode and its corresponding priority, expressed as a 16-bit unsigned
>>   integer, where zero is the highest priority and subsequent integers
>>   increase by one.
>>
>> This is a bit confusing. I think you mean:
>>
>> NEW
>>   Specifically, mode-preference-chain lists the mode and its
>>   corresponding priority, expressed as a 16-bit unsigned integer.
>>   Values for the priority start with zero, the highest priority, and
>>   subsequent priority value increases by one.
>
> I can see why this can be confusing. How about ...
>
> NEW
>    Specifically, mode-preference-chain lists the
>    mode and its corresponding priority as a 16-bit unsigned
>    integer. Values for the priority start with zero, the highest 
> priority, and
>    decreasing priority value is indicated by every increase of value 
> by one.

That's fine. It's a little verbose, but the RFC Editor can suggest any 
wordsmithing if necessary during their edit.

>> OLD
>>   In turn, each ctrl-connection holds a list of test-session-request.
>>   test-session-request holds information associated with the Control-
>>   Client for this test session.
>>
>> A bit awkward. I suggest:
>>
>> NEW
>>   In turn, each ctrl-connection holds a test-session-request list. 
>> Each
>>   test-session-request holds information associated with the
>>   Control-Client for this test session.
>
> Ok.

Thanks.

>> OLD
>>   The Control-Client is also responsible for scheduling TWAMP-Test
>>   sessions so test-session-request holds information related to these
>>   actions (e.g. pm-index, repeat-interval).
>>
>> The word "so" in there is weird. Do you mean "therefore", or "such 
>> that", or
>> something else? I just had a bit of trouble understanding what you 
>> meant.
>
> We meant “therefore”. Will make the change.

Ah, good. The other solution is to put a comma before "so".

>> 4.2 - In the penultimate paragraph, change "key-id" to either "The 
>> key-id" or
>> "The KeyID”.
>
> Will change it to “The key-id”.

Sounds good.

>> Please note: I did not thoroughly review the YANG in section 5.2 or 
>> the
>> examples in Section 6 or Appendix A. I gave them a quick run through, 
>> but did
>> not check for complete consistency with the rest of the text. The 
>> below two
>> items are simply things I happened to spot because I was looking at 
>> particular
>> pieces of the module.
>>
>> 5.2 -
>>
>>           leaf priority {
>>             type uint16;
>>             description
>>               "Indicates the Control-Client Mode preference priority
>>                expressed as a 16-bit unsigned integer, where zero is
>>                the highest priority and subsequent values
>>                monotonically increasing.";
>>           }
>>
>> I am almost positive that you don't mean "monotonically increasing". 
>> I'm
>> guessing you mean "increase by one”.
>
> Will update this description to match the comment you made above or 
> whatever we agree to.

Thanks.

>>              Depending on the Modes available in the TWAMP Server
>>              Greeting message (see Fig. 2 of RFC 7717), the
>>              this Control-Client MUST choose the highest priority
>>              Mode from the configured mode-preference-chain list.";
>>
>> Typo: "the this Control-Client”
>
> Will fix it to say “the Control-Client”.

Perfect.

> Thanks.

Thanks for your speedy reply.

pr