Hi Julien,
On Tue, May 21, 2024 at 10:26 PM <julien.meu...@orange.com> wrote:
> Hi Dhruv,
>
> Thanks for addressing my comments. The new version looks good to me.
>
> Please note however that idnits shouts because of too long lines: have you
> tried circumvent it?
>
>
New version -25 posted where idnits runs clear!
Thanks,
Dhruv
> Cheers,
>
> Julien
>
>
> On 17/05/2024 08:51, Dhruv Dhody wrote:
>
>
> HI Julien,
>
> On Thu, May 16, 2024 at 7:57 PM <julien.meu...@orange.com> wrote:
>
>> Dear authors of draft-ietf-pce-pcep-yang,
>>
>> I've reviewed the aforementioned document to prepare its publication
>> request. The I-D is almost ready to move forward and only has minor
>> issues and nits that should be addressed before sending it to the IESG.
>>
>> Minor issues:
>> - The introduction doesn't mention the ietf-pcep-stats module though
>> it's defined in the body of the I-D; a brief additional sentence would
>> be welcome.
>>
>
> Dhruv: Added "Further, this document also includes the PCEP statistics
> YANG module "ietf-pcep-stats" which provides statistics, counters and
> telemetry
> data."
>
>
>> - For SR, the leaf "msd-limit" (page 45) is a boolean that should be
>> renamed to be understandable, e.g. into "no-msd-limit" or "ignore-msd".
>>
>
> Dhruv: changed to "no-msd-limit".
>
>
>
>> - On page 46, in the H-PCE section, there's a "if Stateful GMPLS is
>> enabled" left instead of "if H-PCE is enabled.
>>
>
> Dhruv: thanks for spotting that
>
>
>
>> - Section 7.1 about TLS should be deeply summarized and rather point to
>> the referenced document (pointer to be updated).
>>
>
> Dhruv: Changed to -- "The PCC acting as the TLS client opens the TLS
> connection and the PCE acting as the TLS server listens for incoming
> connections as per TLS specifications ([RFC8446] and [RFC5246]). [RFC8253]
> specifies the StartTLS procedure in PCEP that initiates the TLS connection
> before exchanging PCEP messages thus the identity verification is
> completed before the PCEP session is established."
>
>
>> - On page 51, in the description of the hexadecimal case, I don't think
>> the 2 long sentences about the rationale should be included there; if
>> the authors consider it necessary, it may be included in the text body
>> of the draft.
>>
>
> Dhruv: The text/approach is borrowed from RFC 8177. I prefer to keep it.
>
>
>
>> - On page 59, similar comment about the sync-timer description, which
>> I'd shorten into the following:
>> "The value of SyncTimer in seconds is used in the
>> case of synchronized path computation request
>> using the SVEC object. If after the expiration of
>> the SyncTimer all the path computation requests
>> have not been received, a protocol error is
>> triggered and the PCE must cancel the whole set
>> of path computation requests.
>> Zero means that the PCEP entity does not use the
>> SyncTimer."
>>
>
> Dhruv: Thanks. Updated.
>
>
>
>> - On page 69, about path key, the name of the leaf "pcc-original" feels
>> odd, how about "originator-pcc" instead?
>>
>>
> Dhruv: I changed it to pcc-requester and used "original" in the
> description to march the text in RFC 5520.
>
> leaf pcc-requester {
> type leafref {
> path "/pcep/entity/peers/peer/addr";
> }
> description
> "Reference to PCC peer address that
> issued the original request that led
> to the creation of the path-key.";
> }
>
>
>> Nits:
>> - Page 11: s/system generated entity index/system-generated entity index
>> - P.11: s/the local entity is PCE it/the local entity is a PCE, it
>> - P.11: s/dead-timer in YANG is called DeadTimer in the protocol
>> specification/DeadTimer in the protocol specification is called
>> dead-timer in YANG/
>> - P.16: s/learn PCE in the network via IGP discovery/learn a PCE address
>> in the network via the IGP discovery/
>> - P.28-30: There are several bullets points in the descriptions fields
>> that would benefit from semicolons at each line end.
>>
> Dhruv: I used a comma instead for readability
>
>> - P.40: s/maybe relevant/may be relevant/
>> - P.44: s/PCE triggered/PCE-triggered/ [twice]
>> - P.49: s/instance specific data/instance-specific data/
>> - P.105: s/this document also include/this document also includes/
>>
>>
> Dhruv: Ack!
>
> Thanks for your review! New version -24 is posted.
>
> Diff:
> https://author-tools.ietf.org/iddiff?url1=draft-ietf-pce-pcep-yang-23&url2=draft-ietf-pce-pcep-yang-24&difftype=--html
>
> Regards,
> Dhruv
>
>
>
>>
>> Best regards,
>>
>> Julien
>>
>> _______________________________________________
>> Pce mailing list -- pce@ietf.org
>> To unsubscribe send an email to pce-le...@ietf.org
>>
>
>
_______________________________________________
Pce mailing list -- pce@ietf.org
To unsubscribe send an email to pce-le...@ietf.org
