Hi Roman,

Thanks for taking on the AD role for this I-D.

On Sat, Sep 21, 2024 at 12:35 AM Roman Danyliw <r...@cert.org> wrote:

> Hi!
>
> I performed an AD review of draft-ietf-pce-segment-routing-policy-cp-17.
> To help load-balance AD documents queues, I am assuming the role of
> responsible AD.
>
> Thanks for this document.  I have the following feedback and questions:
>
> ** Section 4.2.1
>    SR Policy Name: SR Policy name, as defined in [RFC9256].  It SHOULD
>    be a string of printable ASCII characters, without a NULL terminator.
>
> The use of SHOULD here implies that the encoding could be something other
> than printable ASCII.  That would be prohibited by the text in Section 2.1
> of RFC9256.  Perhaps s/SHOULD/must/.
>
> Same comment for nearly identical text in Section 4.2.3
>
>
Dhruv: FWIW past RFCs that encode strings in PCEP have used a similar
language - RFC 8231, RFC 9358! That said, I don't mind making a change
going forward.



> ** Section 4.2.2
>    If the PCE has its AS number, then it SHOULD set it,
>    otherwise the AS number can be set to 0.
>
> What would be the circumstance where the PCE has an AS, but still sets it
> to 0.  Why is this not a MUST?
>
>
Dhruv: With the qualifier text, it makes sense to make it a MUST.



> ** Section 5.2
>    Priority: Numerical priority with which this LSP is to be recomputed
>    by the PCE upon topology change.
>
> Can this text clarify whether a higher number suggests a higher priority?
>
>
Dhruv: Makes sense to add esp because RFC 9256 says "where the lowest value
is the highest priority"



> ** Section 5.3
>    Therefore the PCEP speaker SHOULD ignore the presence of this TLV for
>    SRv6 Policies.
>
> What are the circumstances where this TLV would NOT be ignored?
>
>
Dhruv: None, make it a MUST!



> ** Section 5.4
>    This field can be set to non-
>    zero values only by the PCC, it MUST be set to 0 by the PCE and
>    SHOULD be ignored by the PCC.
>
> What are the circumstances where it would not be ignored by the PCC?
>
>
Dhruv: None, make it a MUST!



> ** Section 6.5
>    The subregistry contains the following codepoints, with
>    initial values, to be assigned by IANA with the reference set to this
>    document:
>
> Consider explicitly stating there is a reference column in this table.
>
> Same feedback for Section 6.6
>
>
Dhruv: Yes, always a good practice when RFC and the iana registry match.



> ** Section 6.7 and 6.8.  Should the new registries defined here be created
> under the
> https://www.iana.org/assignments/segment-routing/segment-routing.xhtml?
>
>
Dhruv: Yes, authors please add under the "Segment Routing Parameters"
registry.

Also note that these days IANA has asked us to. stop using subregistry and
top registry. The preferred terms are registry and registry group
respectively. From a recent email -

NOTE: In the IANA Considerations section, please replace all occurrences of
"sub-registry" with "registry" and all occurrences of '"Path Computation
Element Protocol (PCEP) Numbers" registry' or "PCEP Numbers registry" with
'Path Computation Element Protocol (PCEP) Numbers" registry group.



> ** Section 8.  Should a statement to the effect of “the Security
> Considerations of RFC8231 continue to apply” be reiterated here?
>
>
Dhruv: Perhaps,

   The security considerations described in [RFC5440], [RFC8231],
   [RFC8281], [RFC8664], and [RFC9256] are applicable to this
   specification.

Authors, if you disagree, please feel free to jump in!


Thanks!

Dhruv (Document Shepherd)



> Regards,
> Roman
>
>
> _______________________________________________
> 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

Reply via email to