Re: [Pce] WGLC for draft-ietf-pce-segment-routing-policy-cp-12

["Mike Koldychev (mkoldych)" <[e-mail]>]

Hi Cheng,

Thanks for your review! Comments inline with <MK></MK>.

Thanks,
Mike.


From: Cheng Li <c.l=[e-mail]>
Sent: Tuesday, January 16, 2024 11:09 AM
To: Dhruv Dhody <[e-mail]>; [e-mail]
Cc: pce-chairs <[e-mail]>; [e-mail]
Subject: RE: [Pce] WGLC for draft-ietf-pce-segment-routing-policy-cp-12

Hi WG,

I read the document and support the WGLC.
However, I also have some minor comments below.


1.
Abstract

A Segment Routing (SR) Policy [RFC9256<https://www.rfc-editor.org/info/rfc9256>] is a non-empty set of SR Candidate Paths, that share the same <headend, color, endpoint> tuple.
1.Nits: s/that/which.
<MK>
Sure.
</MK>

2.share the same <> tuple?
<MK>
Sorry, not clear what your comment is here? The text is referring to the 3-tuple that identifies the SR Policy, from here: https://www.rfc-editor.org/rfc/rfc9256.html#name-identification-of-an-sr-pol
</MK>

3.This document extends [RFC8664<https://www.rfc-editor.org/info/rfc8664>] to fully support the SR Policy construct.
fully? suggest to remove this world. The SR policy is developing, so it can not be fully supported now I guess.
<MK>
Sure, I will remove the “fully”.
</MK>


4.
Introduction

PCEP Extensions for Segment Routing [RFC8664<https://www.rfc-editor.org/info/rfc8664>] specifies extensions that allow PCEP to work with basic SR-TE paths.¶<https://datatracker.ietf.org/doc/html/draft-ietf-pce-segment-routing-policy-cp-12#section-1-1>
s/specifies/specify
<MK>
Sure, I can just use the RFC Number, instead of the full name.
</MK>

PCEP Extensions for Establishing Relationships Between Sets of LSPs [RFC8697<https://www.rfc-editor.org/info/rfc8697>] introduces
s/introduces/introduce because of extensions?  or you only wanto to list the name of the RFC here? anyway, all are nits. Normally, we use RFCXXXX as the subject directly.
<MK>
Sure, I can just use the RFC Number, instead of the full name.
</MK>

5. again. Suggest to delete 'fully' in the last paragraph of Introduction.
<MK>
Will do.
</MK>

6.
SR Policy Association. PCEP ASSOCATION that describes the SR Policy. Can refer to the PCEP object or to the group of LSPs that belong to the Association. This should be clear from the context.¶<https://datatracker.ietf.org/doc/html/draft-ietf-pce-segment-routing-policy-cp-12#section-2-2.4>

suggest to rephrase this description to be more formal? , too casual to me.
<MK>
Sure, will rephrase. There was a prior comment about this as well.
</MK>

7.When these rules are not satisfied, the PCE MUST send a PCErr message with Error-Type = 26 "Association Error", Error Value = TBD8

Only the PCE sends? do we have any case that a PCC may send this error?
<MK>
True, it should be “PCEP speaker”, not “PCE”. Thanks.
</MK>

8.

This Association Type is dynamic in nature, thus operator-configured Association Range MUST NOT be set for this Association type and MUST be ignored.¶<https://datatracker.ietf.org/doc/html/draft-ietf-pce-segment-routing-policy-cp-12#section-4-2>

Sorry I do not understand this paragraph. What do you mean this association type is dynamic in nature?
<MK>
It’s referring to this: https://www.rfc-editor.org/rfc/rfc8697.html#section-3.4 . The text is basically saying that this Association Type is fully dynamic. I’m not sure if this is necessary to say, or if we can rephase it to be clearer? Any suggestions?
</MK>

9.
·        Association ID (16-bit): set to "1".¶<https://datatracker.ietf.org/doc/html/draft-ietf-pce-segment-routing-policy-cp-12#section-4.1-3.3>
why the ID must be set as 1? do you mean a association is identified by association source, color, and endpoint in extended association ID TLV? so we do not need Association ID?
or you like to use This ID 1 to avoid the race case between multiple PCE?
<MK>
Yes, the association is identified by <Source, Color, Endpoint>, so this 16-bit field is not useful. We set it to “1” to avoid using “0”, which is a reserved value for that field. I can put a sentence to clarify this in the text.
</MK>


10.

   0                   1                   2                   3
   0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |             Type              |             Length            |
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
  |                                                               |
  ~                       SR Policy Name                          ~
  |                                                               |
  +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
Figure 2<https://datatracker.ietf.org/doc/html/draft-ietf-pce-segment-routing-policy-cp-12#figure-2>: The SRPOLICY-POL-NAME TLV format<https://datatracker.ietf.org/doc/html/draft-ietf-pce-segment-routing-policy-cp-12#name-the-srpolicy-pol-name-tlv-f>

Though I can not image what may be added to this TLV now. But I think reserving 0 bits for a new TLV is not a good decision.
same for "SRPOLICY-CPATH-NAME" TLV
<MK>
It’s not reserving 0 bits. If you read the description of the TLV, it encodes a policy name string.
</MK>


11. section 8

This document defines one new type for association, which do not add any new security concerns beyond those discussed
s/association/ASSOCIATION Object
s/do/does
<MK>
Ack, thanks.
</MK>

Thanks,
Cheng



From: Pce <[e-mail]<mailto:[e-mail]>> On Behalf Of Dhruv Dhody
Sent: Monday, January 8, 2024 11:29 AM
To: [e-mail]<mailto:[e-mail]>
Cc: pce-chairs <[e-mail]<mailto:[e-mail]>>; [e-mail]<mailto:[e-mail]>
Subject: [Pce] WGLC for draft-ietf-pce-segment-routing-policy-cp-12

Hi WG,

This email starts a 2-weeks working group last call for draft-ietf-pce-segment-routing-policy-cp-12.

https://datatracker.ietf.org/doc/draft-ietf-pce-segment-routing-policy-cp/

Please indicate your support or concern for this draft. If you are opposed to the progression of the draft to RFC, please articulate your concern. If you support it, please indicate that you have read the latest version and it is ready for publication in your opinion. As always, review comments and nits are most welcome.

The WG LC will end on Monday 22nd January 2024.

A general reminder to the WG to be more vocal during the last-call/adoption.

Thanks,
Dhruv & Julien