[Pce] Re: Shepherd Review of draft-ietf-pce-pcep-yang

[julien . meuric]

Hi Dhruv,

Thanks for addressing my comments. The new version looks good to me.



Cheers,

Julien


On 17/05/2024 08:51, Dhruv Dhody wrote:

Please note however that idnits shouts because of too long lines: have you tried circumvent it?

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.




    - 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).




    - 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?




          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.

Dhruv: Added "Further, this document also includes the PCEP statistics YANGmodule "ietf-pcep-stats" which provides statistics, counters andtelemetry data." 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 messagesthus the identity verification is completed before the PCEP sessionis established." Dhruv: I changed it to pcc-requester and used "original" in the description to march the text in RFC 5520. Diff: https://author-tools.ietf.org/iddiff?url1=draft-ietf-pce-pcep-yang-23&url2=draft-ietf-pce-pcep-yang-24&difftype=--html <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

smime.p7s
Description: S/MIME Cryptographic Signature

_______________________________________________
Pce mailing list -- pce@ietf.org
To unsubscribe send an email to pce-le...@ietf.org