Mahesh
Yes, your responses look good.
Tom Petch
----- Original Message -----
From: "Mahesh Jethanandani" <mjethanandani@xxxxxxxxx>
Sent: Friday, April 13, 2018 1:13 AM
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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> To: <ietf@xxxxxxxx>
> Cc: <ippm-chairs@xxxxxxxx>; <draft-ietf-ippm-twamp-yang@xxxxxxxx>;
> <ippm@xxxxxxxx>
> 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@xxxxxxxx>
>> To: "IETF-Announce" <ietf-announce@xxxxxxxx>
>> Cc: <ippm-chairs@xxxxxxxx>; <draft-ietf-ippm-twamp-yang@xxxxxxxx>;
>> <ippm@xxxxxxxx>
>> Sent: Monday, April 09, 2018 3:57 PM
