Re: [Pce] WG Adoption of draft-peng-pce-entropy-label-position-10

[[e-mail]]

Hi Samuel,

Thanks for your reply!
I am glad that most of your comments are addressed.
The reason why I use the "SHOULD" is that "MUST" may be too strong which mentioned by Andrew's comment.
I think it may be better to add PCErr message associated with "MUST" in case the PCE can not set it.
So I use the "SHOULD" and could change to "MUST" in the future with a PCErr code if necessary. Thanks!


Best Regards,
Quan






Original


From: SamuelSidor(ssidor) <[e-mail]>
To: 熊泉00091065;
Cc: [e-mail] <[e-mail]>;[e-mail] <[e-mail]>;[e-mail] <[e-mail]>;[e-mail] <[e-mail]>;
Date: 2024年02月20日 17:08
Subject: RE: [Pce] WG Adoption of draft-peng-pce-entropy-label-position-10



Thanks Quan,
 
Ack, I’m fine with your proposals + see inline <S> (I skipped most of them as you already responded and there was not much to discuss 😊 ).
 
Regards,
Samuel
 

From: [e-mail] <[e-mail]> 
 Sent: Sunday, February 18, 2024 3:23 AM
 To: Samuel Sidor (ssidor) <[e-mail]>
 Cc: [e-mail]; [e-mail]; [e-mail]; [e-mail]
 Subject: Re: [Pce] WG Adoption of draft-peng-pce-entropy-label-position-10


 
 
Hi Samuel,
 
Thanks for your detailed review and comments! Sorry for the delay due to the holiday.LoL. Happy Chinese New Year!
 
Please see inline with [Quan].
 
Abstract:
“…Label Indicator (ELI)/EL pairs SHOULD be inserted in the SR-MPLS label stack as per RFC8662…”.
 
Is it good to start using requirements in draft abstract like this (I consider it a bit non-standard looking into abstracts used in other RFCs)? I would use more generic wording to just say that RFC8662 is already explaining how Els how are supposed to be used in SR-MPLS label stack. Requirements language (including “SHOULD” is really defined in Section 2.2 only).
 
[Quan]: Thanks for your remind. I checked the RFC8662 section 7.1, it mentioned "In order for the EL to occur within the ERLD of LSRs along the path corresponding to a SPRING label stack, multiple <ELI, EL> pairs MAY be inserted in this label stack."  Would it be better to change it to "multiple Entropy Label Indicator (ELI)/EL pairs may be inserted in the SR-MPLS label stack. stack as per RFC8662." without using Requirements language?
 
<S> Sure, that should work.
Introduction:
“[RFC5440] describes the Path Computation Element Computation Protocol (PCEP) which is …“
 
Do you need to expand “PCEP” acronym again even if you expanded it already in draft title and abstract? Same potentially applies to EL/ELI/ELP
[Quan]: I think this is required when processing RFC. (And I also checked the existing RFC.)
 
“In some cases, the the controller(e.g. PCE)”
Double “the” and maybe add space after “controller”.
[Quan]:Thanks, I will fix it in next version.
 
Section 4.2:
“A PCE would also set this bit to 1 to indicate that the ELP information is included by PCE and encoded in the Path Computation Reply (PCRep) message as per [RFC5440]. And in a stateful PCE model, it also can be carried in Path Computation Update Request (PCUpd) message as per [RFC8231] or LSP Initiate Request (PCInitiate) message as per [RFC8281].”
 
It seems that for stateless it is “MUST” requirement and for stateful, it is “CAN” requirement. Isn’t it better to explicitly indicate for both whether PCE MUST do it or it is just optional and PCE may decide to do not set that flag at all?
 [Quan]: Thanks for your comment. Would it be better to add that " The PCE SHOULD set this bit to 1 ... And in a stateful PCE model, it also CAN be carried in .... "
<S> Is it just “SHOULD” requirement for stateful and not MUST same way like it is done for stateless?
Section 4.3:
“E (ELP Configuration) : If this flag is set, the PCC SHOULD insert <ELI, EL> into the position after this SR-ERO subobject, otherwise it SHOULD not insert <ELI, EL> after this segment.”
 
You are indicating what PCC should do if flag is set (so I assume that PCE should set it, but it may be better to be explicit and say if “If this flag is set by PCE,…”). I can see that you are explaining briefly in next section, but it is still better to be clear. Maybe explain also whether PCC can set it by itself (e.g. if it wants to report state of existing LSP).
 [Quan]: Thanks for your comment. I think the ELP can only be computed by PCE, so the flag can only  be set by PCE. 
<S> Ack, I was just pointing out that it may be better to add some explicit statement to clarify that.
Section 5:
“And the SR path can also be initiated by PCE with PCInitiate or PCUpd message in stateful PCE mode.” 
It seems a bit misleading to say that SR path can be initiated by PCUpd.
“The SR path being received by PCC with SR-ERO segment list…”
Maybe “…received by PCC encoded in SR-ERO…” (without segment-list as you are already talking about SR path)
 [Quan]:Thanks, I will fix it in next version.
 
You defined 3 new flags, but what I’m missing a bit is any details about interaction between those flags. E.g. what will happen if capability was not advertised by E flag in LSP object is set? Is that allowed? Is it valid to include E flag in SR-ERO if it was not requested by PCC? If any of those will happen, should we have some PCError to reject them?
Is E flag from SR-ERO applicable to any SID type or are there cases, when it is not valid to include it (e.g. L2 Adj SID?) Should PCC just ignore that flag in such case?
Is E flag from SR-ERO applicable to RRO as well?
(I’m not pushing you to solve everything before adoption, just throwing ideas for improving 😊 )
  [Quan]:Yes, thanks for your great point. So far we only consider the normal use of the E bit and will consider other abnomal case in the future.
<S> Ack.
Section 7:
You should explicitly mention registry name from which you want to allocate
Consider if you need to add “Manageability Considerations” Section.
   [Quan]: Thanks, will consider to add in next version.
 
Best Regards,
Quan
 

Original

From: SamuelSidor(ssidor) <[e-mail]>



To: 熊泉00091065;



Cc: pce-chairs <[e-mail]>;Dhruv Dhody <[e-mail]>;[e-mail] <[e-mail]>;[e-mail] <[e-mail]>;



Date: 2024年02月07日 18:38



Subject: RE: [Pce] WG Adoption of draft-peng-pce-entropy-label-position-10




Hi Quan and PCE WG,
 
I support adoption of this draft with a few minor/not blocking comments (I reviewed v11 as a lot of comments were addressed, so to avoid commenting same thing again even if v10 is being adoption).
 
Abstract:
“…Label Indicator (ELI)/EL pairs SHOULD be inserted in the SR-MPLS label stack as per RFC8662…”.
 
Is it good to start using requirements in draft abstract like this (I consider it a bit non-standard looking into abstracts used in other RFCs)? I would use more generic wording to just say that RFC8662 is already explaining how Els how are supposed to be used in SR-MPLS label stack. Requirements language (including “SHOULD” is really defined in Section 2.2 only).
 
Introduction:
“[RFC5440] describes the Path Computation Element Computation Protocol (PCEP) which is …“
 
Do you need to expand “PCEP” acronym again even if you expanded it already in draft title and abstract? Same potentially applies to EL/ELI/ELP
 
“In some cases, the the controller(e.g. PCE)”
 
Double “the” and maybe add space after “controller”.
 
Terminology
You are just pointing to terminology in other RFCs, but in the same time you are introducing a lot of new acronyms and expanding them directly in the text. For example in section 3:
“The Entropy Readable Label Depth (ERLD) is defined as the number of labels which m…”
“… MSD (e.g. Base MPLS Imposition MSD (BMI-MSD) or ERLD-MSD) through Interior Gateway Protocol (IGP) and…”
Why not use this section to explain them?
 
Section 3:
“The PCEs could get the information of all nodes such as … through Interior Gateway Protocol (IGP)”
Isn’t it better to refer to BGP-LS as well? At least my understanding is that usage of BGP-LS as a source for topology for PCE is more standard than directly learning topology from IGP (but maybe that is me).
 
Section 4.1:
“…multiple ELI/EL pairs and and supports the results of SR path with ELP from PCE”
 
Double “and”. I also assume that PCC really “supports the results of SR path-computation”  or it “supports SR path with ELP received from PCE”.
 
Section 4.2:
“A PCE would also set this bit to 1 to indicate that the ELP information is included by PCE and encoded in the Path Computation Reply (PCRep) message as per [RFC5440]. And in a stateful PCE model, it also can be carried in Path Computation Update Request (PCUpd) message as per [RFC8231] or LSP Initiate Request (PCInitiate) message as per [RFC8281].”
 
It seems that for stateless it is “MUST” requirement and for stateful, it is “CAN” requirement. Isn’t it better to explicitly indicate for both whether PCE MUST do it or it is just optional and PCE may decide to do not set that flag at all?
 
Section 4.3:
“E (ELP Configuration) : If this flag is set, the PCC SHOULD insert <ELI, EL> into the position after this SR-ERO subobject, otherwise it SHOULD not insert <ELI, EL> after this segment.”
 
You are indicating what PCC should do if flag is set (so I assume that PCE should set it, but it may be better to be explicit and say if “If this flag is set by PCE,…”). I can see that you are explaining briefly in next section, but it is still better to be clear. Maybe explain also whether PCC can set it by itself (e.g. if it wants to report state of existing LSP).
 
Section 5:
“And the SR path can also be initiated by PCE with PCInitiate or PCUpd message in stateful PCE mode.”
 
It seems a bit misleading to say that SR path can be initiated by PCUpd.
 
“The SR path being received by PCC with SR-ERO segment list…”
 
Maybe “…received by PCC encoded in SR-ERO…” (without segment-list as you are already talking about SR path)
 
You defined 3 new flags, but what I’m missing a bit is any details about interaction between those flags. E.g. what will happen if capability was not advertised by E flag in LSP object is set? Is that allowed? Is it valid to include E flag in SR-ERO if it was not requested by PCC? If any of those will happen, should we have some PCError to reject them?
 
Is E flag from SR-ERO applicable to any SID type or are there cases, when it is not valid to include it (e.g. L2 Adj SID?) Should PCC just ignore that flag in such case?
 
Is E flag from SR-ERO applicable to RRO as well?
 
(I’m not pushing you to solve everything before adoption, just throwing ideas for improving 😊 )
 
Section 7:
You should explicitly mention registry name from which you want to allocate
 
Consider if you need to add “Manageability Considerations” Section.
 
Thanks,
 Samuel
 

From: Pce <[e-mail]> On Behalf Of Dhruv Dhody
 Sent: Friday, January 26, 2024 5:49 PM
 To: [e-mail]
 Cc: pce-chairs <[e-mail]>; [e-mail]
 Subject: [Pce] WG Adoption of draft-peng-pce-entropy-label-position-10


 

Hi WG,
 
 This email begins the WG adoption poll for draft-peng-pce-entropy-label-position-10
 
 https://datatracker.ietf.org/doc/draft-peng-pce-entropy-label-position/
 
 Should this draft be adopted by the PCE WG? Please state your reasons - Why / Why not? What needs to be fixed before or after adoption? Are you willing to work on this draft? Review comments should be posted to the list.
 
 Please respond by Monday 12th Feb 2024.
 
 Please be more vocal during WG polls!
 
 Thanks!
 Dhruv & Julien