[Idr] Re: Shepherd's review of draft-ietf-idr-bgp-ls-sr-policy-05

[Susan Hares <[e-mail]>]

Ketan:

First, I’m so sorry you were unwell when this review first came out.   Your health is the priority.  IDR can wait.

Thank you for -06 which fixes a great deal of the issues.

The following technical issues still need to be resolved:

Issue 4e (definition of F-Flag when clear),
Issue 8a and 8b (bit definitions when bit is cleared)
Issue 10 (bit definitions when cleared, reference to 9252 specific section)
Issues 13, 14, 15, 16, 19, 20, 31 - all request clarity of on definition of bit (or bits) when cleared.
Issue 18 and 30:  Values for type of metric (consider values 6-120 in section 8.6)
Issue 28: Awaiting validation on p2p question, before I close the issue.

All NITs are closed, but I would like to you to look at NIT-8b following (I've copied it below).

The Two major issues are:
a) definition of Flags when the flag is cleared,
b) the metric types (section 8.6) being full defined when values 6-120 are not assigned.

Sue Hares


===================
NIT-8b, Section 5.6.4, "a" candidate path instead of "the" candidate path
Old text:/
   The SR Disjoint Group Constraint sub-TLV is an optional sub-TLV of
   the SR CP Constraints TLV that is used to carry the disjointness
   constraint associated with the candidate path. /
New text:/
    The SR Disjoint Group Constraint sub-TLV is an optional sub-TLV of
   the SR CP Constraints TLV that is used to carry the disjointness
   constraint associated with a candidate path. /


KT> I believe the use of "the" is correct since the reference is to the specific CP that is being advertised
Sue> What singular Candidate Path (CP) is being referenced? I thought many CPs were possible, and one becomes the active CP.

===============================
From: Ketan Talaulikar [e-mail]<mailto:[e-mail]>
Sent: Saturday, October 19, 2024 1:15 PM
To: Susan Hares [e-mail]<mailto:[e-mail]>
Cc: [e-mail]<mailto:[e-mail]>; Dongjie (Jimmy) [e-mail]<mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>; [e-mail]<mailto:[e-mail]>; Jeff Tantsura [e-mail]<mailto:[e-mail]>
Subject: Re: Shepherd's review of draft-ietf-idr-bgp-ls-sr-policy-05

Hi Sue, Thanks a lot for your detailed review of this document. Apologies for the delay in response.
We have also posted an update in line with the discussions below: https://datatracker.ietf.org/doc/
External ([e-mail]<mailto:[e-mail]>)


Hi Sue,

Thanks a lot for your detailed review of this document. Apologies for the delay in response.

We have also posted an update in line with the discussions below:
https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgp-ls-sr-policy-06

Please check inline below for responses.


On Fri, Aug 30, 2024 at Susan Hares [e-mail]<mailto:[e-mail]> wrote:
Ketan, Jie, Stefano, Hannes, and Jeff:
This shepherd review is in preparation for draft-ietf-idr-bgp-ls-sr-policy-05 being WG LC.

Status: My understanding is that Ketan requested WG LC.

KT> Yes, this is correct.

If you wish a WG LC of this draft during the time period 9/1 to 12/1,
 the following steps need to occur:
1.    Edits to address this shepherd report
2.    Report on 2 implementations on the BGP wiki
3.    A presentation at IDR SR interim (9/9/2024)

Please let me know if you wish to have a WG LC during 9/1/2024 to 12/1/2024.
I will add you to the list of presenters at the IDR SR interim on 9/9/2024.

KT> I was not able to present at that interim since I was unwell. The draft has been stable since the last presentation and updates have been editorial in nature. We can present to the WG at the IETF 121 if discussion is required on any aspects of the drafts and if time permits.

The shepherd’s review is below. I will post the review on the SR wiki as well.

Cheerily, Sue Hares

=================
Summary:
The technology in this specification appears to be consistent.
However, the text seems to have several technical issues that need to corrected in an editing pass.

Beyond the technical issues, there are editorial nits.


Technical issues:
Issue 1: Section 1, introduction

Does this draft cover explicit Candidate Policy (CP) information in BGP-LS or dynamic and explicit CP information?
Please specify whether this is appropriate for explicit, dynamic, or composite Candidate Paths.

KT> The draft covers explicit and dynamic CPs. Composite CPs are outside the scope of this document. Now clarified this in the introduction section.
Sue-2> Thank you for the clarification.  Technical issue 1 is closed.

Issue 2: Section 3 diagram:  “Node Descriptor TLV (for the Headend)” in the diagram in section 3 should be “Local Node Descriptor”

KT> Fixed.
Sue-2> Thank you for addressing this issue. Technical issue 2 is closed.

Issue 3: Section 4, *Flags – Why is “cleared” used for bits?  It would seem that “zero would be better”.
Old text:/Other bits MUST be cleared by the originator and
          and MUST be ignored by a receiver./
New text:/Other bits MUST be cleared (set to zero)
         by the originator and MUST be ignored
          by a receiver./
Note: You can simply indicate that you will be using “cleared”/”sets” for bits.
I am looking for consistency across the document.

KT> We've used the term "clear" to mean set to 0 and "set" to mean set to 1 for individual
bits throughout this document. We have not used "set to zero" anywhere for individual bits.
I believe there is consistency in this regard - please let me know if something has been missed.
Sue> You have maintained clear means set to zero and set means set to 1.  As a courtesy to the reader,
you might mention that in you first usage (4.3).  However, the major problem when a bit is cleared
what does it mean.  This larger technical issue will stay open until we address this point.

Issue 4: Section 5.1 – Bit Definition (D-Flag, B-Flag, U-Flag, L-Flag, and F-flag) link to section in RFC9256 are unclear.

KT> Updated the text to refer to section 6.2 of RFC9256 instead of only the RFC 9256 reference. This applies to all the bits.
Sue> Super!  Thank you.  Technical issue 4 closed.

4a) D-Flag:
Draft text:/
         D-Flag: Indicates the dataplane for the BSIDs and if they are
         16 octet SRv6 SID when set and are 4 octet SR/MPLS label value
         when clear./
New text:/
         D-Flag: Indicates the dataplane for the BSIDs. If D-Flag is set,
         the BSID is 16 octet SRv6 SID. If D-Flag is clear, the BSID is
         the 4-octet SR/MPLS label value.  [RFC9256, section 6.2]/


Text in RFC9256, section 6.2, text:/
  “When the active candidate path has a specified BSID,
   the SR Policy uses that BSID if this value
  (label in MPLS, IPv6 address in SRv6) is available.”/
4b) B-Flag
 Draft text:/
       B-Flag: Indicates the allocation of the value in the BSID field
       when set and indicates that BSID is not allocated when clear./
  Question: Does “B-Flag” set indicate a specified BSID-only case per
  [RFC9256, section 6.2.3].  Does B-Flag clear, indicate unspecified
  BSID (RFC9256, section 6.2.1)? Or just that the node has allocated (or not-allocated)
  the BSID value.  The problem with this definition is the linkage to
  RFC9252.

KT> It literally means what the text says - i.e. the node has allocated or not-allocated the BSID value.
Sue-2> Thanks for the validation that it means just what the text say.  Issue 4a and 4b closed.

4c) U-Flag – Does this U-Flag link to the Unspecified BSID (RFC9256, section 6.2.1)?
     Or is it just that the BSID is unavailable due to another cause?

KT> This also literally means what the text says "Indicates the specified BSID value is unavailable when set". It is not related to the "unspecified BSID" RFC9256 section 6.2.1.
Sue-2> Thanks for the validation that it means just what the text says.  Issue 4c closed.

4d) L-Flag – If this explanation references RFC9256 section 6.2, please add the section to the text.

KT> All flags reference section 6.2 of RFC 9256.
Sue-2> Thank you for the clarification.  Issue 4d (closed).

RFC9256 text:/Optionally, instead of only checking that the BSID of the active path is available,
a headend MAY check that it is available within the given SID range i.e.,
Segment Routing Local Block (SRLB) as specified in [RFC8402]./

4e) F-Flag – The explanation is clear, but the link to RFC9256 is not clear.
Is this a reference to section 9 in RFC9256?

KT> The reference is still to the text in section 6.2 which describes cases where the specified BSID is not available and the headend can fallback to using one from the dynamic label range.
Sue-2> What does it mean when the F-Flag is clear?

Issues 4a-4d are closed, 4e - but it is not clear what F-flag means when clear.

Issue 5: Section 5.1 text on length of BSID fields
Old text:/ The BSID fields above are 4-octet carrying the MPLS Label or 16-octet
   carrying the SRv6 SID based on the BSID D-flag.  When carrying the
   MPLS Label, as shown in the figure below, the TC, S, and TTL (total
   of 12 bits) are RESERVED and MUST be set to 0 by the originator and
   MUST be ignored by a receiver./

New text:/ The BSDI fields above depend on the dataplane (SRv6 or MPLS)
   indicated by the BSID D-Flag.  If D-Flag set (SRv6 dataplane), then
   the length of the BSID fields is 16 octets.  If the D-Flag is clear
   (MPLS dataplane), then the length of the BSDI Fields is 4 octets.
   When carrying the MPLS Label, as shown in the figure below, the TC, S, and TTL (total
   of 12 bits) are RESERVED and MUST be set to 0 by the originator and
   MUST be ignored by a receiver./


KT> Thanks. I've incorporated this text.
Sue-2> Thank you for adding the text change, Issue 5 is closed.

Issue 6: Section 5.2 – bit definitions link to RFC9256 is not clear.
  The following text describing the “bit positions” does not give a clear and
  specific reference to sections in RFC9256:
   Draft-text:/“The following bit positions are defined and the semantics are described in detail
     in [RFC9256]. /

   See my comments on section 5.1 regarding my questions on each bit.

KT> Fixed same as for section 5.1.
Sue> Thank you for fixing this in section 5.2, issue 6 is closed.


Issue 7: Section 5.2 – Text on which sub-TLVs can be used in the SR Binding SID TLV
    Note: This is a technical issue because the unclear text could impact interoperability
    Two points are being made in the following paragraph:
1.    SRv6 Endpoint Behavior TLV (1250) and SRv6 SID Structure TLV (1252) are defined in RFC9514
2.    These two sub TLVs may optionally be used in the SRv6 Binding SID

Old text:/
   The SRv6 Endpoint Behavior TLV (1250) and the SRv6 SID Structure TLV
   (1252) defined in [RFC9514] are used as sub-TLVs of the SRv6 Binding
   SID TLV to optionally indicate the SRv6 Endpoint behavior and SID
   structure for the Binding SID value in the TLV./
New text:/
   The SRv6 Endpoint Behavior TLV (1250) and the SRv6 SID Structure TLV
   (1252) MAY optionally be used as sub-TLVs of the SRv6 Binding SID TLV
   to indicate the SRv6 Endpoint behavior and SID structure for the
   Binding SID value in the TLV. [RFC9514] defines SRv6 Endpoint Behavior TLV
   And SRv6 SID Structure TLV. /

KT> Incorporated your suggestion.
Sue-2> Thank you for addressing this issue. Issue 7 is closed.

Issue 8: Section 5.3 – Flag bits definitions
•     S-Flag – Please define what it means when S-Flag is clear (zero), and give RFC9256 reference.
•     A-Flag – Please define what it means when A-Flag is clear (zero), and give RFC9256 reference.
•     E-Flag – Please define what it means when B-Flag is clear (zero), and give RFC9256 reference.
•     V-Flag – Please give RFC9256 reference
•     O-Flag – Please define what it means when O-Flag is clear (zero). [Here the RFC9256 reference is given.]
•     D-Flag – Please define what it means when D-Flag is clear (zero), and give RFC9256 reference.
•     C-Flag – Please define what it means when C-Flag is clear (zero), and give RFC9256 reference.
•     I-Flag – Please define what it means when I-Flag is set (one) or cleared (zero).
•     T-flag – Please define what it means when the T-Flag is clear (zero)
•     U-Flag – Please define what it means when the U-Flag is clear (clear), and give RFC9256 reference.

KT> About the use of the term "clear and set" and "what happens when not set".
To me, the clear and set for a bit are well known operations and the set indicates
presence of a state or condition while clear indicates an absence.
Let me take an example: "Indicates the CP is in an administrative shut state
when set and not shut when clear". The explanation of the "clear" part seems redundant and obvious.
That said, there are places where the clear state has also been described.

Sue-2> (Issue 8-a) What a “clear” bit means is not clear in this context.
In section 4.3, you use Clear and Set to mean IPv4 or IPv6 in E-Flag.
If you simply mean that for all these flags in section 5.3 clear means the state
described does not exist, then say that before the bit allocation start under “where:”.


Please give references to specific sections in RFC9256 since you state in the bit descriptions that:
“The following bit positions are defined and the semantics are described in detail in [RFC9256].”

KT> The references to RFC9256 sections have been given where necessary -
e.g. where there is some special behavior described and we have a section for it in the RFC 9256.
The expectation is any implementor or reader of this document is fully conversant with RFC9256
and the SR Policy Architecture.

Sue-2: (Issue 8-b) Interoperability is about tightening specification so people know what sections
you refer to in the RFC9256 and SR Policy Architecture document.  Bugs happen in new code
less frequently when drafts specify the sections.  Section 5.3  does not specify the section
for S-Flag, A-Flag, B-Flag, E-flag, V-flag, D-flag, and C-flag.   In previous sections,
you mentioned section 6.2 for B-Flag, D-Flag, and U-Flag.   It would seem easy to add this text.
In previous sections, you have not mentioned S-Flag, A-Flag, E-Flag, D-Flag, and C-flag.
It would be good to add this text.  If all these flags come from section 6.2 of RFC9256,
then add one sentence at the top that says “flags come from section 6.2 of RFC9256
unless otherwise specified.

Status: Issues 8a and 8b remain.

Issue 9: Section 5.6 – Only Explicit CP and Dynamic CP discussion

This section describes usage by:
•     explicit candidate paths (tunnels), and
•     dynamic candidate paths (on-demand tunnels).
It does not discuss usage by a composite path.  Is it valid to apply to composite?

KT> Composite CP is outside the scope.
Sue> Thank you for the confirmation and adding the comment in the introduction.

Issue 10a: Section 5.6 – Bit flags definitions
P-Flag, U-Flag, A-Flag, T-Flag, S-flag, F-Flag, H-Flag – do not indicate what a cleared flag means.

KT> Same as a similar previous comment ...
Sue-2> (Issue 10b) See my previous comment as well.
If the clear state for P-Flag, U-Flag, A-Flag, T-Flag, S-Flag, F-Flag, and H-Flag,
simply indicates the state is not present – state it at the top.
•   What does mutual exclusive mean between P-Flag and U-Flag.  (I think it means both cannot be set).

KT> Yes
Sue-2> (Issue 10c) That is not clear enough in the specification.
Suggested new text:/ This flag is mutually exclusive with the U-Flag .
This means the P-Flag and U-flag cannot both be set. /

Issue 11: Section 5.6, What sub-TLVs can be included
concern:  The text does not clearly state which sub-TLVs MAY be included.

Draft-Text:/
*  sub-TLVs: optional sub-TLVs MAY be included in this TLV to describe other constraints.
   The following constraint sub-TLVs are defined for the SR CP Constraints TLV./
New text:/
*  sub-TLVs: A sequence of optional sub-TLVs MAY be included in this TLV
             to describe other constraints. The optional sub-TLVs that can be
              included are: SR Affinity Constraint, SR SRLG Constraint, SR Bandwidth
              Constraint, SR Disjoint Group Constraint, SR Bidirectional Group Constraint,
              and SR Metric Constraint. These constraint Sub-TLVs are defined below./

KT> Thanks. Incorporated your suggestion.
Sue> Thank you for addressing this issue. Issue 11 closed.

Issue-12: Section 5.6.3, text description of SR Bandwidth Sub-TLv

old text:/  Only a single instance of
            this sub-TLV is advertised for a given CP./

problem: I think you mean for a given CP Constraint TLV.
         Since the link is unclear, I have added this as a technical issues.
             I suspect the issues is mostly editorial.

New text:/ Only  a single instance of
           this sub-TLV is advertised for a given
               CP Constraint TLV, and CP.  Recall that only one
               CP constraint TLV is allowed per CP./

KT> This is editorial and this is already clear for each level of TLV/sub-TLV.
Sue-2> I think the text needs the editorial fix as a technical issues, but I will clear this issue.
Issue 12 is closed.

Issue-13: Section 5.6.4, clear of Request Flag Bits when cleared

The descriptions of the request flags (S-Flag, N-Flag, L-Flag, F-Flag, and I-Flag)
do specify a behavior when the bit is set.  What is the behavior when the bit is cleared?
This needs to be described either in the introduction the request bits or on each bit.

KT> Same as previous response.
Sue-2>  See my earlier comments that you need to fix this point.


Issue-14, Section 5.6.4, clear meaning of Status Flag bits when cleared
The descriptions of the status flags (S-Flag, N-Flag, L-Flag, F-Flag, I-Flag, and X-flag)
do specify the status meaning when the bit is set.   What is the behavior when the bit is cleared?
This needs to be described either in the introduction the status bits or on each bit.

KT> Same as previous response ...
Sue-2> Same as my previous comments.

Issue-15: Section 5.6.5, length maximum
The minimum length is clearly stated.  Is there a maximum length for this field?

KT> We cannot specify a maximum here since the size is coming from PCEP protocol encoding which may be extended from time to time.
Sue-2> Of course, PCE (and IDR) specification change and grow.  However, specify the limits based on PCEP protocol version, and indicate may change if PCE changes.

Issue-16: Section 5.6.5, C-Flag, what does C-Flag mean if clear

Current text:
      -  C-Flag: Indicates that the bidirectional path is co-routed when
         set/

Problem: What does it mean when the C-Flag is clear?

KT> Same as the previous comment ...
Sue-2> Again, if you mean the C-Flag when clear means the path is not co-routed, please state that fact.
Or, indicated the clear bit means the absence of the state.

Issue-17: Section 5.6.6

The SR Metric Constraint Sub-TLV is used for a dynamic path and an explicit path.
Is this sub-TLV supported for a composite path?

KT> Composite path is out of scope.
Sue> Thank you for including this in the introduction. Issue 17 is closed.


Issue-18: Section 5.6.6, Metric-Type

Are all other values except the values in section 8.6 invalid?

KT> Why would they be invalid? New metric types may be defined by other documents and this encoding does not need to change.
Sue> If this is the case, then specify which groups of values an implementation supporting this draft must implement.
Here’s what I understand: 0-5 (required), all other optional.  Do I understand what you are stating?


Issue-19: Section 5.6.6, Flags

a) What is the meaning of the O-Flag when the bit is cleared?
b) What is the meaning of the M-Flag when the bit is cleared?
c) What is the meaning of the B-Flag when the bit is cleared?

KT> Same as previous comment
Sue> Same as previous comments.  Please specify.  If you simply mean the state does not exist, please state at bit header.


Issue-20: Section 5.7, Flags clarity

The link between RFC9256 and the Flag bits (D-Flag, E-Flag, C-Flag, V-Flag,
R-Flag, F-Flag, A-Flag, T-Flag, M-Flag)is vague.  Please give a
section reference per flag bit in the form (RFC9256, section x.x (or x.x.x)).

KT> The descriptions should be clear enough. Same as a previous response.
Sue> I do not find the clear state clear.   Again, two options:
a)    State at top of bit positions,
b)    State at introduction of text, and deal with section 4.3 in a different manner (1/0 – being IPv6 / IPv4).


Please indicate what action occurs when the following flags are clear: C-Flag,
A-Flag, and T-Flag,

KT> Same as previous comment
Sue> I do not think the clear bit position is clear.  See previous comments.


Issue-21: Section 5.7, Algorithm values
Please indicate what values can be set in the Algorithm octet.

KT> Clarified.
Sue-2> thank you for clarification. Issue 21 is closed.

Issue-22: Section 5.7, Methodology paragraph clarity, ";" issue
The ";" causes the methodology to be unclear.

 old text:/
  A SID-List may be empty in certain cases
   (e.g. for a dynamic path) where the headend has not yet performed the
   computation and hence not derived the segments required for the path;
   in such cases, the SR Segment List TLV SHOULD NOT include any SR
   Segment sub-TLVs. /

  New text (suggested):
    A SID-List may be empty in certain situations
      (e.g. for a dynamic path) where the headend has not yet performed
      the computation and hence not derived the segments required for the path.
    In such cases where the SID-LIST is empty, the SR Segment List TLV
      SHOULD NOT include any SR Segment sub-TLVs. /

KT> Thanks for your suggestion. Incorporated it.
Sue-2> Thank for fixing this issues. Issue 22 is closed.

Issue-23, Section 5.8, Segment type

  Other proposed specifications give other segment types.
  The following text does not take this into consideration:

 Old text:/
   *  Segment Type: 1 octet which indicates the type of segment (refer
      Section 5.8.1 for details)/

A suggestion for the new text is:
New text:/
   *  Segment Type: 1 octet which indicates the type of segment. Initial
      values are specified by this document (See Section 5.8.1 for details).
        Additional segment types are possible, but out of scope for this document./

KT> Thanks for your suggestion. Incorporated it.
Sue-2: Thank you for addressing this issue. Issue 23 is closed.

Issue-24, Section 5.8, Flag bits

The link between RFC9256 and the flag bits is unclear for S-Flag, E-Flag, V-Flag,
R-Flag, and A-Flag.  Please give a reference for a section in RFC9256 for each bit type.


KT> Same as a previous comment.
Sue-2: What I like about section 5.8 in the flag section is that you provide definitions for set and cleared.  The V-Flag and R-Flag are much clearer.
If all the definitions are from section 6.2 of RFC9256, could you state this in the flags section?
V-Flag grammar makes the meaning unclear.
Old text:/
      -  V-Flag: Indicates the SID has passed verification or did not
         require verification when set and failed verification when
         clear./

Suggested new text:/
      -  V-Flag: Indicates the SID has passed verification or did not
         require verification when set.  When V-Flag is cleared, it
         indicates the SID has failed verification. /
R-Flag and A-Flag might als benefit from a similar rewrite.

KT> Ack. Changed accordingly.
Sue: Thank you for addressing this issue. Issue 24 is closed.


Issue-25: Section 5.8, methodology paragraph at end

Text:/
   The SRv6 Endpoint Behavior TLV (1250) and the SRv6 SID Structure TLV
   (1252) defined in [RFC9514] are used as sub-sub-TLVs of the SR
   Segment sub-TLV to optionally indicate the SRv6 Endpoint behavior and
   SID structure when advertising the SRv6 specific segment types./

problem: Too much compression of meaning as method is squished into 1 sentence.

alternative text:/
   The SRv6 Endpoint Behavior TLV (1250) and the SRv6 SID Structure TLV
   (1252) defined in [RFC9514] are used as sub-sub-TLVs of the SR
   Segment sub-TLV.  These twoi sub-sub-TLVs are used to optionally
   indicate the SRv6 Endpoint behavior and SID structure when
   advertising the SRv6 specific segment types./

KT> Updated accordingly.
Sue: Thank you for adding this change. Issue 25 is closed.

Issue-26: Section 5.8.1.1, 5.8.1.2, 5.8.1.3, and 5.8.1.4

Algorithm: What are the valid values for Algorithm.
Is there a Minimum and maximum value for the algorithm?

KT> Provided pointer to the IANA registry for where Algorithm values are taken.
Sue: Excellent resolution to the comment.  Issue 26 closed.

Issue-27: Section 5.8.1.3, 5.8.1.4, 5.8.1.5, 5.8.1.6, 5.8.1.7

Are there any invalid IPv4 Node addresses or IPv6 Node addresses.
If so, please indicate this in the text.

KT> We are not doing validation of this information in BGP-LS.
Sue-2:  Thank you for confirming that the BGP-LS consumer is required to validate the 4-octet IPv4 Node address and 16-octet IPv6 node address is valid.
That is the agreement for BGP-LS. Issue 27 closed.


Issue-28: Section 5.8.1.6, 5.8.1.7, 5.8.1.10
Please indicate why point-to-point link might not have a remote address or interface ID.

KT> The text does not say "point-to-point link might not have a remote address" - it says it is not necessary to provide a remote address as well as a local address to identify the link.
Sue-2> The text in -06  states
 “This is optional and MAY be set to 0 when not used (e.g. when identifying point-to-point links).”

The point is that some point-to-point (p2p) links may have local and remote addresses.   And some p2p links may only have local address.
 Is this what the text is saying? Is this why there is a “MAY” at this point?   And if this is true, the operator pulling the information will compare it against the configuration.
If this is the context, please let me know.    I will close the issue with your response.

Issue-29: Section 5.8.1.8, 5.8.1.9, 5.8.1.1
Are there any invalid Global IPv6 addresses? If so, please indicate what addresses are invalid.

KT> We are not doing validation of this information in BGP-LS.
Sue> OK.  Issue 29 is closed.

Issue-30: Section 5.9, Metric type

Metric type:  Are values not listed in Section 8.6 valid?
I believe you are allowing additional values in the future, but
those values are outside the scope of this work.

KT> Yes, they are valid. They may be defined in the future.
Sue-2> No, this specification does not specify how these metric types (6-120) will be generated or used.
Private use 121-127 signals private usage so two consenting BGP Peers could do this in their own network.
128-255 requires that you see user defined metrics (per draft-ietf-lsr-flex-algo-bw-con).
You can indicate future specifications may define values for 6-120.

Issue-31: Section 5.9, Flags
What is the meaning of the following flags when they are not set:
M-Flag, A-Flag, B-Flag, and V-Flag.  Please provide this information.

KT> Same as a previous comment.
Sue-2> We can take A-Flag off my above list.  If A-Flag = 0 (cleared), it is a percentage of minimum metric.
M-Flag, B-Flag, and V-flag do not indicate what the meaning is if these flags are cleared.

Issue-32: Section 6, Does this text apply to all protocol origins?
Current text:/ Then
   the SR Policy Candidate Path's state and attributes are encoded in
   the BGP-LS Attribute field as SR Policy State TLVs and sub-TLVs as
   described in Section 5.  The SR Candidate Path State TLV as defined
   in Section 5.3 is included to report the state of the CP.  The SR
   BSID TLV as defined in Section 5.1 or Section 5.2 is included to
   report the BSID of the CP when one is either specified or allocated
   by the headend.  The constraints and the optimization metric for the
   SR Policy Candidate Path are reported using the SR Candidate Path
   Constraints TLV and its sub-TLVs as described in Section 5.6.  The SR
   Segment List TLV is included for each of the SID-List(s) associated
   with the CP.  Each SR Segment List TLV in turn includes SR Segment
   sub-TLV(s) to report the segment(s) and their status.  The SR Segment
   List Metric sub-TLV is used to report the metric values at an
   individual SID List level./

This text come after the PCEP discussion, so it is unclear if this applies
to information originated by all BGP-LS producers (see table 8.4).

KT> Thanks for catching that. I've replaced "Then the" with "The" since this text is for all origins.
Sue-2> I’m glad I could catch a simple error. Issue 32 is closed.

==============================
Editorial comments:

Nit-1.      Section 1, paragraph 1, English language usage of “;”
Old text:/
   Each CP in turn may have one or
   more SID-List of which one or more may be active; when multiple are
   active then traffic is load balanced over them. This document covers
   the advertisement of state information at the individual SR Policy CP
   level./

New text:/
   Each CP may have one or more SID-List and one or
   more of these SID-LIST may be active when traffic is
   load-balanced over them. This document covers
    the advertisement of state information at the individual SR Policy CP
   level via BGP [RFC9552].

KT> I've clarified but with different text.
Sue-2: Ok, issue closed.

----
Nit-2.      Section 1, paragraph 2, unclear sentence

Old text:/
   SR Policies are generally instantiated at the head-end and are based
   on either local configuration or controller-based programming of the
   node using various APIs and protocols, e.g., PCEP or BGP./
New text:/
   SR Policies are generally instantiated at the head-end and are based
   on either local configuration or controller-based programming of the
   node using various APIs and protocols (e.g., PCEP or BGP). /

KT> Fixed.
Sue-2: Thank you for addressing the issue. Nit-2 is closed.

----
Nit-3.      Section 1, paragraph 3 run-on sentence, broken into two statements.

Old text:/
   In many network environments, the configuration, and state of each SR
   Policy that is available in the network is required by a controller
   which allows the network operator to optimize several functions and
   operations through the use of a controller aware of both topology and
   state information./
New text:/
   In many network environments,  the network operator uses a controller
   to optimize operations in the network.  The controller needs information
   regarding the configuration and the state of each available SR Policy
   to calculate the optimized topologies.  A description of five controllers
   that can benefit from using BGP [RFC9552] to collect SR Policy state is given below.
   (management-based PCE, composite PCE, parent-child PCE deployments,
    centralized controller, and  NMS visualization of SR Policy (tunnels)). /

KT> Rephrased existing text for clarity without adding "a description of five controllers ...".
Sue-2:  Ok, this issue is closed.

Nit-4.      Section 1, paragraph 4, plural/singular issues and clarity of text

Old text:/
   One example of a controller is the stateful Path Computation Element
   (PCE) [RFC8231], which could provide benefits in path optimization.
   While some extensions are proposed in the Path Computation Element
   Communication Protocol (PCEP) for the Path Computation Clients (PCCs)
   to report the LSP states to the PCE, this mechanism may not be
   applicable in a management-based PCE architecture as specified in
   section 5.5 of [RFC4655]./

New text:/
   One example of a controller is the stateful Path Computatdion Element
   (PCE) [RFC8231].  The stateful PCE controller could be useful in calculating
   optimized paths if Path Computation Clients (PCCs) use PCE Communication
   Protocol (PCEP) to report LSP states to the PCE.  However, this mechanism
   may not be applicable in a management-based PCE architecture as specified in
   section 5.5 of [RFC4655]./

KT> I see no issue in the old text.
Sue-2: This is a grammar /clarity issue and not a technical issue.
I will answer why there is a grammar issue in the text,  but it is a NIT so it is your choice.

The current phrasing is passive withs
Old text:
   One example of a controller is the stateful Path Computation Element
   (PCE) [RFC8231], which could provide benefits in path optimization.

What’s wrong?: This is a passive sentence which hints rather than states the benefits the document is leveraging.
Old text 2:
   While some extensions are proposed in the Path Computation Element
   Communication Protocol (PCEP) for the Path Computation Clients (PCCs)
   to report the LSP states to the PCE, this mechanism may not be
   applicable in a management-based PCE architecture as specified in
   section 5.5 of [RFC4655]./

What’s wrong: This passive statement is difficult read since: a) there are PCEP extensions that may help, but b) it is not guaranteed. D
This writing does not take the active verbs recommended by English grammar experts for clarity in technical writing.

Old text:/
  This document proposes using the BGP-LS protocol [RFC9552] to collect SR Policy state
  In a mechanism complementary to the mechanism defined in [RFC8231]./
New text:/
  This document proposes SR Policy state collection
   mechanism complementary to the mechanism defined in [RFC8231]./

KT> I see no issue in the old text.
Sue:   The text below is in the document.  NIT-4 closed.

Old text of: /  This document proposes a SR Policy state collection
   mechanism complementary to the mechanism defined in [RFC8231]./

Nit-5.      Section 1, paragraph 5
Old text:/ An external
   component may also need to collect the SR Policy information from all
   the PCEs in the network to obtain a global view of the SR Policy
   paths' state in the network./
   New text:/  An external
   component may also need to collect the SR Policy information from all
   the PCEs in the network to obtain a global view of the state of all SR Policy
   paths (tunnels) in the network./

KT> Rephrased.
Sue-2> NIT-5 closed.


Nit-6.      Section 3, diagram


Current diagram:/
      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
     +-+-+-+-+-+-+-+-+
     |  Protocol-ID  |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |                        Identifier                             |
     |                        (64 bits)                              |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     //            Node Descriptor TLV (for the Headend)            //
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     //           SR Policy Candidate Path Descriptor TLV           //
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
New diagram:
      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
     +-+-+-+-+-+-+-+-+
     |  Protocol-ID  |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |                        Identifier                             |
     |                        (64 bits)                              |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     //      Local Node Descriptor TLV (for the Headend)            //
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     //           SR Policy Candidate Path Descriptor TLV           //
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+


KT> Fixed
Sue: NIT-6 Closed.


Nit-7. Section 5.6,  repeat the abbreviation (CP) before reusing it in this section.

Why: The refresh of the CP abbreviation provides easier reading.
Otherwise, the reader must go back and find where CP was defined.
Old text: / The SR Candidate Path Constraints TLV is an optional TLV that is used
   to report the constraints associated with the candidate path./
New text:/ The SR Candidate Path Constraints TLV is an optional TLV that is used
   to report the constraints associated with the candidate path (CP)./

KT> If the reader is not aware of "CP" until they have come to this point in the document
then that indicates they are not familiar with the very base spec that this document depends on.
Sue-2: This repeating of an abbreviation is suggested best practice in technical writing.
It is not because the author is not familiar, but aids quick reading of text.
It is an editorial NIT so it is not required.  NIT-7 is losed.

Nit-8. Section 5.6.3, lack of clarity of length of SRLG value.

old text:/
  *  SRLG Values: One or more SRLG values (each of 4 octets)./

new text:/
*SRLG VAlues: One or more SRLG values.  Each SRLG value is 4 octets.
/

KT> Fixed.
Sue: Thank you for addressing the issue.  Nit-8a is closed.

NIT-8b, Section 5.6.4, "a" candidate path instead of "the" candidate path
Old text:/
   The SR Disjoint Group Constraint sub-TLV is an optional sub-TLV of
   the SR CP Constraints TLV that is used to carry the disjointness
   constraint associated with the candidate path. /
New text:/
    The SR Disjoint Group Constraint sub-TLV is an optional sub-TLV of
   the SR CP Constraints TLV that is used to carry the disjointness
   constraint associated with a candidate path. /


KT> I believe the use of "the" is correct since the reference is to the specific CP that is being advertised
Sue> What singular Candidate Path is being referenced?

NIT-9, Section 5.6.5, clarity of R-Flag description

 Old text:/
      -  R-Flag: Indicates that this CP of the SR Policy forms the
         reverse path when set and otherwise it is the forward path when
         clear/

 New text:/
      -  R-Flag: Indicates that this CP of the SR Policy forms the
         reverse path when the R-Flag is set.  If the R-Flag is clear,
         this CP forms the forward path./

KT> Incorporated this suggestion.
Sue: Thank you for addressing this issue. Nit-9 is closed.

NIT-10: Section 6, sentence clarity can be improved by breaking long sentence into two sentences.

Original text:/
   For the reporting of SR Policy Candidate Paths, the NLRI descriptor
   TLV as specified in Section 4 is used.  An SR Policy candidate path
   (CP) may be instantiated on the headend node via a local
   configuration, PCEP, or BGP SR Policy signaling and this is indicated
   via the SR Protocol Origin.  When a PCE node is the BGP-LS Producer,
   it uses the "reported via PCE" variants of the SR Protocol Origin so
   as to distinguish them from advertisements by headend nodes.  Then
   the SR Policy Candidate Path's state and attributes are encoded in
   the BGP-LS Attribute field as SR Policy State TLVs and sub-TLVs as
   described in Section 5.

new text:/
    An SR Policy candidate path
   (CP) may be instantiated on the headend node via a local
   configuration, PCEP, or BGP SR Policy signaling.  The protocol
   that instantiates the SR Policy candidate path is indicated
   via the SR Protocol Origin.

   When a PCE node is the BGP-LS Producer, the PCE node uses the
   "reported via PCE" variants of the SR Protocol Origin to
   distinguish them from advertisements by headend nodes.
   These "report via PCE" variants include "PCEP reported via PCE/PCEP" (10),
   "BGP SR Plicy reported via PCE/PCEP" (20), Configuration (CLI, Yang Model
    via NETCONF, etc.) reported via PCE/PCEP).    Then
   the SR Policy Candidate Path's state and attributes are encoded in
   the BGP-LS Attribute field as SR Policy State TLVs and sub-TLVs as
   described in Section 5. /

KT> I think the original text is clear as well.
Sue> I disagree, but this is a NIT based on my understanding of grammar
and sentence clarity.  All Editorial nits may be ignored.  Nit-10 is closed.

 NIT-11: Security considerations, paragraph 3, unclear use of ";"
Original text:/  BGP peerings are
   not automatic and require configuration; thus, it is the
   responsibility of the network operator to ensure that only trusted
   nodes (that include both routers and controller applications) within
   the SR domain are configured to receive such information./

 Suggested revision:/ BGP peer connections are
   not automatic and require configuration. Thus, it is the
   responsibility of the network operator to ensure that only trusted
   nodes (that include both routers and controller applications) within
   the SR domain are configured to receive such information./


KT> Fixed.
Sue-2: Thank you. Nit-11 is closed.

Thanks,
Ketan



From: Ketan Talaulikar <[e-mail]>
Sent: Saturday, October 19, 2024 1:15 PM
To: Susan Hares <[e-mail]>
Cc: [e-mail]; Dongjie (Jimmy) <[e-mail]>; [e-mail]; [e-mail]; Jeff Tantsura <[e-mail]>
Subject: Re: Shepherd's review of draft-ietf-idr-bgp-ls-sr-policy-05

Hi Sue, Thanks a lot for your detailed review of this document. Apologies for the delay in response. We have also posted an update in line with the discussions below: https://datatracker.ietf.org/doc/
External ([e-mail]<mailto:[e-mail]>)
  Report This Email<https://protection.inkyphishfence.com/report?id=bmV0b3JnMTA1ODY5MTIvc2hhcmVzQG5kemguY29tL2RlNmI3ODBkODVhMDA2NDIzZmYwOTE5ZmYyODZmM2UyLzE3MjkzNTgxOTYuNDM=#key=98240ed6b2c7b699d44fa1c7ad2727d1>  FAQ<https://www.godaddy.com/help/report-email-with-advanced-email-security-40813>  GoDaddy Advanced Email Security, Powered by INKY<https://www.inky.com/protection-by-inky>

Hi Sue,

Thanks a lot for your detailed review of this document. Apologies for the delay in response.

We have also posted an update in line with the discussions below:
https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgp-ls-sr-policy-06<https://shared.outlook.inky.com/link?domain=datatracker.ietf.org&t=h.eJxFjssOgyAUBX_FsC7yUgRX_gryUCOKgdtF2_TfW7rpdiZncl7oniMaG7QCXGUkxBkwkI3dfW43D6FNeSEuWbLCEYnLJgCuHG8u43m5cCy4ZHyluNkHphLdGrTX4OnhO2W0V1IzTspqsi_T6Z5ra9NBnJfzoKhTvaFUdlyEQDXTIXAlg_CcsIFr0SumZduJWvW1unswJ_yeTcthtlhj1bpq_-T9ARhsQ5k.MEUCIFMRhuW9Ol9DnidXamdLsKPuCRm5uTtcMh5zxKkTspnDAiEAzw2MRuWOQR4iIEKQcUzotkT_f0Y8x4JT6r1WRk4YQ2Y>

Please check inline below for responses.


On Fri, Aug 30, 2024 at 2:16 AM Susan Hares <[e-mail]<mailto:[e-mail]>> wrote:
Ketan, Jie, Stefano, Hannes, and Jeff:

This shepherd review is in preparation for draft-ietf-idr-bgp-ls-sr-policy-05 being WG LC.

Status: My understanding is that Ketan requested WG LC.

KT> Yes, this is correct.


If you wish a WG LC of this draft during the time period 9/1 to 12/1,
 the following steps need to occur:


  1.  Edits to address this shepherd report
  2.  Report on 2 implementations on the BGP wiki
  3.  A presentation at IDR SR interim (9/9/2024)

Please let me know if you wish to have a WG LC during 9/1/2024 to 12/1/2024.
I will add you to the list of presenters at the IDR SR interim on 9/9/2024.

KT> I was not able to present at that interim since I was unwell. The draft has been stable since the last presentation and updates have been editorial in nature. We can present to the WG at the IETF 121 if discussion is required on any aspects of the drafts and if time permits.


The shepherd’s review is below. I will post the review on the SR wiki as well.

Cheerily, Sue Hares

=================

Summary:

The technology in this specification appears to be consistent.
However, the text seems to have several technical issues that need to corrected in an editing pass.

Beyond the technical issues, there are editorial nits.


Technical issues:
Issue 1: Section 1, introduction

Does this draft cover explicit Candidate Policy (CP) information in BGP-LS or dynamic and explicit CP information?
Please specify whether this is appropriate for explicit, dynamic, or composite Candidate Paths.

KT> The draft covers explicit and dynamic CPs. Composite CPs are outside the scope of this document. Now clarified this in the introduction section.
 Sue-2> Thank you for the clarification.
Issue 2: Section 3 diagram:  “Node Descriptor TLV (for the Headend)” in the diagram in section 3 should be “Local Node Descriptor”

KT> Fixed.
Sue-2> Thank you.

Issue 3: Section 4, *Flags – Why is “cleared” used for bits?  It would seem that “zero would be better”.
Old text:/Other bits MUST be cleared by the originator and
          and MUST be ignored by a receiver./
New text:/Other bits MUST be cleared (set to zero)
         by the originator and MUST be ignored
          by a receiver./
Note: You can simply indicate that you will be using “cleared”/”sets” for bits. I am looking for consistency across the document.

KT> We've used the term "clear" to mean set to 0 and "set" to mean set to 1 for individual bits throughout this document. We have not used "set to zero" anywhere for individual bits. I believe there is consistency in this regard - please let me know if something has been missed.

Sue> I think you are consistent in your actions.  In this first instance in section 4 - it might be good to add a note that “clear means set to 0 and set means set to 1”.
We’ve had IESG review that commented on this type of nit.  I suggest a note in the first instance to be clear.

Issue 4: Section 5.1 – Bit Definition (D-Flag, B-Flag, U-Flag, L-Flag, and F-flag) link to section in RFC9256 are unclear.

KT> Updated the text to refer to section 6.2 of RFC9256 instead of only the RFC 9256 reference. This applies to all the bits.
Sue> Super!  Thank you.

4a) D-Flag:
Draft text:/
         D-Flag: Indicates the dataplane for the BSIDs and if they are
         16 octet SRv6 SID when set and are 4 octet SR/MPLS label value
         when clear./

New text:/
         D-Flag: Indicates the dataplane for the BSIDs. If D-Flag is set,
         the BSID is 16 octet SRv6 SID. If D-Flag is clear, the BSID is
         the 4-octet SR/MPLS label value.  [RFC9256, section 6.2]/


Text in RFC9256, section 6.2, text:/
  “When the active candidate path has a specified BSID,
   the SR Policy uses that BSID if this value
  (label in MPLS, IPv6 address in SRv6) is available.”/

4b) B-Flag
 Draft text:/
       B-Flag: Indicates the allocation of the value in the BSID field
       when set and indicates that BSID is not allocated when clear./

  Question: Does “B-Flag” set indicate a specified BSID-only case per
  [RFC9256, section 6.2.3].  Does B-Flag clear, indicate unspecified
  BSID (RFC9256, section 6.2.1)? Or just that the node has allocated (or not-allocated)
  the BSID value.  The problem with this definition is the linkage to
  RFC9252.

KT> It literally means what the text says - i.e. the node has allocated or not-allocated the BSID value.
 Sue-2> Thanks for the validation that it means just what the text says.

4c) U-Flag – Does this U-Flag link to the Unspecified BSID (RFC9256, section 6.2.1)?
     Or is it just that the BSID is unavailable due to another cause?

KT> This also literally means what the text says "Indicates the specified BSID value is unavailable when set". It is not related to the "unspecified BSID" RFC9256 section 6.2.1.
 Sue-2> Thanks for the validation that it means just what the text says.

4d) L-Flag – If this explanation references RFC9256 section 6.2, please add the section to the text.

KT> All flags reference section 6.2 of RFC 9256.
 Sue-2> Thank you for the clarification.

RFC9256 text:/Optionally, instead of only checking that the BSID of the active path is available,
a headend MAY check that it is available within the given SID range i.e.,
Segment Routing Local Block (SRLB) as specified in [RFC8402]./

4e) F-Flag – The explanation is clear, but the link to RFC9256 is not clear.
    Is this a reference to section 9 in RFC9256.

KT> The reference is still to the text in section 6.2 which describes cases where the specified BSID is not available and the headend can fallback to using one from the dynamic label range.
Sue-2> What does it mean when the F-Flag is clear?


Issue 5: Section 5.1 text on length of BSID fields

Old text:/ The BSID fields above are 4-octet carrying the MPLS Label or 16-octet
   carrying the SRv6 SID based on the BSID D-flag.  When carrying the
   MPLS Label, as shown in the figure below, the TC, S, and TTL (total
   of 12 bits) are RESERVED and MUST be set to 0 by the originator and
   MUST be ignored by a receiver./

New text:/ The BSDI fields above depend on the dataplane (SRv6 or MPLS)
   indicated by the BSID D-Flag.  If D-Flag set (SRv6 dataplane), then
   the length of the BSID fields is 16 octets.  If the D-Flag is clear
   (MPLS dataplane), then the length of the BSDI Fields is 4 octets.
   When carrying the MPLS Label, as shown in the figure below, the TC, S, and TTL (total
   of 12 bits) are RESERVED and MUST be set to 0 by the originator and
   MUST be ignored by a receiver./


KT> Thanks. I've incorporated this text.
 Sue-2> Thank you for adding the text change.

Issue 6: Section 5.2 – bit definitions link to RFC9256 is not clear.
    The following text describing the “bit positions” does not give a clear and specific reference to sections in RFC9256:
   Draft-text:/“The following bit
     positions are defined and the semantics are described in detail
     in [RFC9256]. /

   See my comments on section 5.1 regarding my questions on each bit.

KT> Fixed same as for section 5.1.
Sue> Thank you for fixing this in section 5.2.


Issue 7: Section 5.2 – Text on which sub-TLVs can be used in the SR Binding SID TLV
    Note: This is a technical issue because the unclear text could impact interoperability
    Two points are being made in the following paragraph:
1.    SRv6 Endpoint Behavior TLV (1250) and SRv6 SID Structure TLV (1252) are defined in RFC9514
2.    These two sub TLVs may optionally be used in the SRv6 Binding SID

Old text:/
   The SRv6 Endpoint Behavior TLV (1250) and the SRv6 SID Structure TLV
   (1252) defined in [RFC9514] are used as sub-TLVs of the SRv6 Binding
   SID TLV to optionally indicate the SRv6 Endpoint behavior and SID
   structure for the Binding SID value in the TLV./
New text:/
   The SRv6 Endpoint Behavior TLV (1250) and the SRv6 SID Structure TLV
   (1252) MAY optionally be used as sub-TLVs of the SRv6 Binding SID TLV
   to indicate the SRv6 Endpoint behavior and SID structure for the
   Binding SID value in the TLV. [RFC9514] defines SRv6 Endpoint Behavior TLV
   And SRv6 SID Structure TLV. /

KT> Incorporated your suggestion.
 Sue-2> Thank you for addressing this issue.

Issue 8: Section 5.3 – Flag bits definitions
•     S-Flag – Please define what it means when S-Flag is clear (zero), and give RFC9256 reference.
•     A-Flag – Please define what it means when A-Flag is clear (zero), and give RFC9256 reference.
•     E-Flag – Please define what it means when B-Flag is clear (zero), and give RFC9256 reference.
•     V-Flag – Please give RFC9256 reference
•     O-Flag – Please define what it means when O-Flag is clear (zero). [Here the RFC9256 reference is given.]
•     D-Flag – Please define what it means when D-Flag is clear (zero), and give RFC9256 reference.
•     C-Flag – Please define what it means when C-Flag is clear (zero), and give RFC9256 reference.
•     I-Flag – Please define what it means when I-Flag is set (one) or cleared (zero).
•     T-flag – Please define what it means when the T-Flag is clear (zero)
•     U-Flag – Please define what it means when the U-Flag is clear (clear), and give RFC9256 reference.

KT> About the use of the term "clear and set" and "what happens when not set". To me, the clear and set for a bit are well known operations and the set indicates presence of a state or condition while clear indicates an absence. Let me take an example: "Indicates the CP is in an administrative shut state when set and not shut when clear". The explanation of the "clear" part seems redundant and obvious. That said, there are places where the clear state has also been described.

Sue-2> What a “clear” bit means is not clear in this context.   In section 4.3, you use Clear and Set to mean IPv4 or IPv6 in E-Flag.   If you simply mean that for all these flags, clear means the state described does not exist, then say that before the bit allocation start under “where:”.   This fix is required.


Please give references to specific sections in RFC9256 since you state in the bit descriptions that:
“The following bit positions are defined and the semantics are described in detail in [RFC9256].”

KT> The references to RFC9256 sections have been given where necessary - e.g. where there is some special behavior described and we have a section for it in the RFC 9256. The expectation is any implementor or reader of this document is fully conversant with RFC9256 and the SR Policy Architecture.

Sue-2:  Interoperability is about tightening specification so people know what sections you refer to in the RFC9256 and SR Policy Architecture document.  Bugs happen in new code less frequently when drafts specify the sections.  Section 5.3  does not specify the section for S-Flag, A-Flag, B-Flag, E-flag, V-flag, D-flag, and C-flag.   In previous sections, you mentioned section 6.2 for B-Flag, D-Flag, and U-Flag.   It would seem easy to add this text.  In previous sections, you have not mentioned S-Flag, A-Flag, E-Flag, D-Flag, and C-flag.   It would be good to add this text.  If all these flags come from section 6.2 of RFC9256, then add one sentence at the top that says “flags come from section 6.2 of RFC9256 unless otherwise specified.


Issue 9: Section 5.6 – Only Explicit CP and Dynamic CP discussion

This section describes usage by:
•     explicit candidate paths (tunnels), and
•     dynamic candidate paths (on-demand tunnels).
It does not discuss usage by a composite path.  Is it valid to apply to composite?

KT> Composite CP is outside the scope.
Sue> Thank you for the confirmation and adding the comment in the introduction.
Issue 10: Section 5.6 – Bit flags definitions
•     P-Flag, U-Flag, A-Flag, T-Flag, S-flag, F-Flag, H-Flag – do not indicate what a cleared flag means.

KT> Same as a similar previous comment ...
Sue-2> See my previous comment as well.  If the clear state for P-Flag, U-Flag, A-Flag, T-Flag, S-Flag, F-Flag, and H-Flag, simply indicates the state is not present – state it at the top.
•     What does mutual exclusive mean between P-Flag and U-Flag.  (I think it means both cannot be set).

KT> Yes
Sue-2> That is not clear enough in the specification.
Suggested new text:/ This flag is mutually exclusive with the U-Flag .  This means the P-Flag and U-flag cannot both be set. /



Issue 11: Section 5.6, What sub-TLVs can be included
concern:  The text does not clearly state which sub-TLVs MAY be included.

Draft-Text:/
*  sub-TLVs: optional sub-TLVs MAY be included in this TLV to describe other constraints.
   The following constraint sub-TLVs are defined for the SR CP Constraints TLV./
New text:/
*  sub-TLVs: A sequence of optional sub-TLVs MAY be included in this TLV
             to describe other constraints. The optional sub-TLVs that can be
              included are: SR Affinity Constraint, SR SRLG Constraint, SR Bandwidth
              Constraint, SR Disjoint Group Constraint, SR Bidirectional Group Constraint,
              and SR Metric Constraint. These constraint Sub-TLVs are defined below./

KT> Thanks. Incorporated your suggestion.
 Sue> Thank you for addressing this issue.

Issue-12: Section 5.6.3, text description of SR Bandwidth Sub-TLv

old text:/  Only a single instance of
            this sub-TLV is advertised for a given CP./

problem: I think you mean for a given CP Constraint TLV.
         Since the link is unclear, I have added this as a technical issues.
             I suspect the issues is mostly editorial.

New text:/ Only  a single instance of
           this sub-TLV is advertised for a given
               CP Constraint TLV, and CP.  Recall that only one
               CP constraint TLV is allowed per CP./

KT> This is editorial and this is already clear for each level of TLV/sub-TLV.
Sue-2> I think the text needs the editorial fix, but it is an optional fix.
Issue-13: Section 5.6.4, clear of Request Flag Bits when cleared

The descriptions of the request flags (S-Flag, N-Flag, L-Flag, F-Flag, and I-Flag)
do specify a behavior when the bit is set.  What is the behavior when the bit is cleared?
This needs to be described either in the introduction the request bits or on each bit.

KT> Same as previous response.
Sue-2>  Sue-2, see my earlier comments that you need to fix this point.


Issue-14, Section 5.6.4, clear meaning of Status Flag bits when cleared
The descriptions of the status flags (S-Flag, N-Flag, L-Flag, F-Flag, I-Flag, and X-flag)
do specify the status meaning when the bit is set.   What is the behavior when the bit is cleared?
This needs to be described either in the introduction the status bits or on each bit.

KT> Same as previous response ...
Sue-2> Same as my previous comments.
Issue-15: Section 5.6.5, length maximum
The minimum length is clearly stated.  Is there a maximum length for this field?

KT> We cannot specify a maximum here since the size is coming from PCEP protocol encoding which may be extended from time to time.
Sue-2> Of course, PCE (and IDR) specification change and grow.  However, specify the limits based on PCEP protocol version, and indicate may change if PCE changes.

Issue-16: Section 5.6.5, C-Flag, what does C-Flag mean if clear

Current text:
      -  C-Flag: Indicates that the bidirectional path is co-routed when
         set/

Problem: What does it mean when the C-Flag is clear?

KT> Same as the previous comment ...
Sue-2> Again, if you mean the C-Flag when clear means the path is not co-routed, please state that fact.  Or, indicated the clear bit means the absence of the state.

Issue-17: Section 5.6.6

The SR Metric Constraint Sub-TLV is used for a dynamic path and an explicit path.
Is this sub-TLV supported for a composite path?

KT> Composite path is out of scope.
Sue> Thank you for including this in the introduction.


Issue-18: Section 5.6.6, Metric-Type

Are all other values except the values in section 8.6 invalid?

KT> Why would they be invalid? New metric types may be defined by other documents and this encoding does not need to change.
Sue> If this is the case, then specify which groups of values an implementation supporting this draft must implement.
Here’s what I understand: 0-5 (required), all other optional.  Do I understand what you are stating?


Issue-19: Section 5.6.6, Flags

a) What is the meaning of the O-Flag when the bit is cleared?
b) What is the meaning of the M-Flag when the bit is cleared?
c) What is the meaning of the B-Flag when the bit is cleared?

KT> Same as previous comment
Sue> Same as previous comments.  Please specify.  If you simply mean the state does not exist, please state at bit header.


Issue-20: Section 5.7, Flags clarity

The link between RFC9256 and the Flag bits (D-Flag, E-Flag, C-Flag, V-Flag,
R-Flag, F-Flag, A-Flag, T-Flag, M-Flag)is vague.  Please give a
section reference per flag bit in the form (RFC9256, section x.x (or x.x.x)).

KT> The descriptions should be clear enough. Same as a previous response.
Sue> I do not find the clear state clear.   Again, two options:

  1.  State at top of bit positions,
  2.  State at introduction of text, and deal with section 4.3 in a different manner (1/0 – being IPv6 / IPv4).


Please indicate what action occurs when the following flags are clear: C-Flag,
A-Flag, and T-Flag,

KT> Same as previous comment
Sue> I do not think the clear bit position is clear.  See previous comments.


Issue-21: Section 5.7, Algorithm values
Please indicate what values can be set in the Algorithm octet.

KT> Clarified.
 Sue-2> thank you for clarification.

Issue-22: Section 5.7, Methodology paragraph clarity, ";" issue
The ";" causes the methodology to be unclear.

 old text:/
  A SID-List may be empty in certain cases
   (e.g. for a dynamic path) where the headend has not yet performed the
   computation and hence not derived the segments required for the path;
   in such cases, the SR Segment List TLV SHOULD NOT include any SR
   Segment sub-TLVs. /

  New text (suggested):
    A SID-List may be empty in certain situations
      (e.g. for a dynamic path) where the headend has not yet performed
      the computation and hence not derived the segments required for the path.
    In such cases where the SID-LIST is empty, the SR Segment List TLV
      SHOULD NOT include any SR Segment sub-TLVs. /

KT> Thanks for your suggestion. Incorporated it.
 Sue-2> Thank for fixing this issues.

Issue-23, Section 5.8, Segment type

  Other proposed specifications give other segment types.
  The following text does not take this into consideration:

 Old text:/
   *  Segment Type: 1 octet which indicates the type of segment (refer
      Section 5.8.1 for details)/

A suggestion for the new text is:
New text:/
   *  Segment Type: 1 octet which indicates the type of segment. Initial
      values are specified by this document (See Section 5.8.1 for details).
        Additional segment types are possible, but out of scope for this document./

KT> Thanks for your suggestion. Incorporated it.
Sue-2: Thank you for addressing this issue.

Issue-24, Section 5.8, Flag bits

The link between RFC9256 and the flag bits is unclear for S-Flag, E-Flag, V-Flag,
R-Flag, and A-Flag.  Please give a reference for a section in RFC9256 for each bit type.


KT> Same as a previous comment.
Sue-2: What I like about section 5.8 in the flag section is that you provide definitions for set and cleared.  The V-Flag and R-Flag are much clearer.
If all the definitions are from section 6.2 of RFC9256, could you state this in the flags section?
V-Flag grammar makes the meaning unclear.
Old text:/
      -  V-Flag: Indicates the SID has passed verification or did not
         require verification when set and failed verification when
         clear./

Suggested new text:/
      -  V-Flag: Indicates the SID has passed verification or did not
         require verification when set.  When V-Flag is cleared, it
         indicates the SID has failed verification. /

R-Flag and A-Flag might als benefit from a similar rewrite.

KT> Ack. Changed accordingly.
Sue: Thank you for addressing this issue.


Issue-25: Section 5.8, methodology paragraph at end

Text:/
   The SRv6 Endpoint Behavior TLV (1250) and the SRv6 SID Structure TLV
   (1252) defined in [RFC9514] are used as sub-sub-TLVs of the SR
   Segment sub-TLV to optionally indicate the SRv6 Endpoint behavior and
   SID structure when advertising the SRv6 specific segment types./

problem: Too much compression of meaning as method is squished into 1 sentence.

alternative text:/
   The SRv6 Endpoint Behavior TLV (1250) and the SRv6 SID Structure TLV
   (1252) defined in [RFC9514] are used as sub-sub-TLVs of the SR
   Segment sub-TLV.  These twoi sub-sub-TLVs are used to optionally
   indicate the SRv6 Endpoint behavior and SID structure when
   advertising the SRv6 specific segment types./

KT> Updated accordingly.
 Sue: Thank you for adding this change.

Issue-26: Section 5.8.1.1, 5.8.1.2, 5.8.1.3, and 5.8.1.4

Algorithm: What are the valid values for Algorithm.
Is there a Minimum and maximum value for the algorithm?

KT> Provided pointer to the IANA registry for where Algorithm values are taken.
Sue: Excellent resolution to the comment.

Issue-27: Section 5.8.1.3, 5.8.1.4, 5.8.1.5, 5.8.1.6, 5.8.1.7

Are there any invalid IPv4 Node addresses or IPv6 Node addresses.
If so, please indicate this in the text.

KT> We are not doing validation of this information in BGP-LS.
Sue-2:  Thank you for confirming that the BGP-LS consumer is required to validate the 4-octet IPv4 Node address and 16-octet IPv6 node address is valid.
That is the agreement for BGP-LS.


Issue-28: Section 5.8.1.6, 5.8.1.7, 5.8.1.10
Please indicate why point-to-point link might not have a remote address or interface ID.

KT> The text does not say "point-to-point link might not have a remote address" - it says it is not necessary to provide a remote address as well as a local address to identify the link.
 Sue-2> The text in -06  states
 “This is optional and MAY be set to 0 when not used (e.g. when identifying point-to-point links).”

The point is that some point-to-point (p2p) links may have local and remote addresses.   And some p2p links may only have local address.
 Is this what the text is saying? Is this why there is a “MAY” at this point?   And if this is true, the operator pulling the information will compare it against the configuration.
If this is the context, please let me know.    I will close the issue with your response.

Issue-29: Section 5.8.1.8, 5.8.1.9, 5.8.1.1

Are there any invalid Global IPv6 addresses? If so, please indicate what addresses are invalid.

KT> We are not doing validation of this information in BGP-LS.
Sue> OK.  Close the issue.

Issue-30: Section 5.9, Metric type

Metric type:  Are values not listed in Section 8.6 valid?
I believe you are allowing additional values in the future, but
those values are outside the scope of this work.

KT> Yes, they are valid. They may be defined in the future.
Sue-2> No, this specification does not specify how these metric types (6-120) will be generated or used.  Private use 121-127 signals private usage so two consenting BGP Peers could do this in their own network.
128-255 requires that you see user defined metrics (per draft-ietf-lsr-flex-algo-bw-con).  You can indicate future specifications may define values for 6-120.

Issue-31: Section 5.9, Flags
What is the meaning of the following flags when they are not set:
M-Flag, A-Flag, B-Flag, and V-Flag.  Please provide this information.

KT> Same as a previous comment.
Sue-2> We can take A-Flag off my above list.  If A-Flag = 0 (cleared), it is a percentage of minimum metric.
M-Flag, B-Flag, and V-flag do not indicate what the meaning is if these flags are cleared.

Issue-32: Section 6, Does this text apply to all protocol origins?
Current text:/ Then
   the SR Policy Candidate Path's state and attributes are encoded in
   the BGP-LS Attribute field as SR Policy State TLVs and sub-TLVs as
   described in Section 5.  The SR Candidate Path State TLV as defined
   in Section 5.3 is included to report the state of the CP.  The SR
   BSID TLV as defined in Section 5.1 or Section 5.2 is included to
   report the BSID of the CP when one is either specified or allocated
   by the headend.  The constraints and the optimization metric for the
   SR Policy Candidate Path are reported using the SR Candidate Path
   Constraints TLV and its sub-TLVs as described in Section 5.6.  The SR
   Segment List TLV is included for each of the SID-List(s) associated
   with the CP.  Each SR Segment List TLV in turn includes SR Segment
   sub-TLV(s) to report the segment(s) and their status.  The SR Segment
   List Metric sub-TLV is used to report the metric values at an
   individual SID List level./

This text come after the PCEP discussion, so it is unclear if this applies
to information originated by all BGP-LS producers (see table 8.4).

KT> Thanks for catching that. I've replaced "Then the" with "The" since this text is for all origins.
Sue-2> I’m glad I could catch a simple error.

==============================
Editorial comments:

Nit-1.      Section 1, paragraph 1, English language usage of “;”

Old text:/
   Each CP in turn may have one or
   more SID-List of which one or more may be active; when multiple are
   active then traffic is load balanced over them. This document covers
   the advertisement of state information at the individual SR Policy CP
   level./

New text:/
   Each CP may have one or more SID-List and one or
   more of these SID-LIST may be active when traffic is
   load-balanced over them. This document covers
    the advertisement of state information at the individual SR Policy CP
   level via BGP [RFC9552].

KT> I've clarified but with different text.
Sue-2: Ok, issue closed.

----
Nit-2.      Section 1, paragraph 2, unclear sentence

Old text:/
   SR Policies are generally instantiated at the head-end and are based
   on either local configuration or controller-based programming of the
   node using various APIs and protocols, e.g., PCEP or BGP./

New text:/
   SR Policies are generally instantiated at the head-end and are based
   on either local configuration or controller-based programming of the
   node using various APIs and protocols (e.g., PCEP or BGP). /

KT> Fixed.
 Sue-2: Thank you for addressing the issue.

----
Nit-3.      Section 1, paragraph 3 run-on sentence, broken into two statements.

Old text:/
   In many network environments, the configuration, and state of each SR
   Policy that is available in the network is required by a controller
   which allows the network operator to optimize several functions and
   operations through the use of a controller aware of both topology and
   state information./

New text:/
   In many network environments,  the network operator uses a controller
   to optimize operations in the network.  The controller needs information
   regarding the configuration and the state of each available SR Policy
   to calculate the optimized topologies.  A description of five controllers
   that can benefit from using BGP [RFC9552] to collect SR Policy state is given below.
   (management-based PCE, composite PCE, parent-child PCE deployments,
    centralized controller, and  NMS visualization of SR Policy (tunnels)). /

KT> Rephrased existing text for clarity without adding "a description of five controllers ...".
Sue-2:  Ok, this issue is closed.

Nit-4.      Section 1, paragraph 4, plural/singular issues and clarity of text

Old text:/
   One example of a controller is the stateful Path Computation Element
   (PCE) [RFC8231], which could provide benefits in path optimization.
   While some extensions are proposed in the Path Computation Element
   Communication Protocol (PCEP) for the Path Computation Clients (PCCs)
   to report the LSP states to the PCE, this mechanism may not be
   applicable in a management-based PCE architecture as specified in
   section 5.5 of [RFC4655]./

New text:/
   One example of a controller is the stateful Path Computation Element
   (PCE) [RFC8231].  The stateful PCE controller could be useful in calculating
   optimized paths if Path Computation Clients (PCCs) use PCE Communication
   Protocol (PCEP) to report LSP states to the PCE.  However, this mechanism
   may not be applicable in a management-based PCE architecture as specified in
   section 5.5 of [RFC4655]./

KT> I see no issue in the old text.
Sue-2: This is a grammar /clarity issue and not a technical issue.   I will answer your question, but it is a NIT so it is your choice.

The current phrasing is passive withs
Old text:
   One example of a controller is the stateful Path Computation Element
   (PCE) [RFC8231], which could provide benefits in path optimization.

What’s wrong?: This is a passive sentence which hints rather than states the benefits the document is leveraging.
Old text 2:
   While some extensions are proposed in the Path Computation Element
   Communication Protocol (PCEP) for the Path Computation Clients (PCCs)
   to report the LSP states to the PCE, this mechanism may not be
   applicable in a management-based PCE architecture as specified in
   section 5.5 of [RFC4655]./

What’s wrong: This passive statement is difficult read since: a) there are PCEP extensions that may help, but b) it is not guaranteed. D
This writing does not take the active verbs recommended by English grammar experts for clarity in technical writing.

Old text:/
  This document proposes using the BGP-LS protocol [RFC9552] to collect SR Policy state
  In a mechanism complementary to the mechanism defined in [RFC8231]./

New text:/
  This document proposes SR Policy state collection
   mechanism complementary to the mechanism defined in [RFC8231]./

KT> I see no issue in the old text.
Sue:   The text below is in the document.   I’m closing this issue.

Old text of: /  This document proposes a SR Policy state collection
   mechanism complementary to the mechanism defined in [RFC8231]./

Nit-5.      Section 1, paragraph 5
Old text:/ An external
   component may also need to collect the SR Policy information from all
   the PCEs in the network to obtain a global view of the SR Policy
   paths' state in the network./

   New text:/  An external
   component may also need to collect the SR Policy information from all
   the PCEs in the network to obtain a global view of the state of all SR Policy
   paths (tunnels) in the network./

KT> Rephrased.
Sue-2> issue closed.


Nit-6.      Section 3, diagram


Current diagram:/
      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
     +-+-+-+-+-+-+-+-+
     |  Protocol-ID  |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |                        Identifier                             |
     |                        (64 bits)                              |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     //            Node Descriptor TLV (for the Headend)            //
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     //           SR Policy Candidate Path Descriptor TLV           //
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

New diagram:
      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
     +-+-+-+-+-+-+-+-+
     |  Protocol-ID  |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |                        Identifier                             |
     |                        (64 bits)                              |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     //      Local Node Descriptor TLV (for the Headend)            //
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     //           SR Policy Candidate Path Descriptor TLV           //
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+


KT> Fixed
Sue: Issue closed



Nit-7. Section 5.6,  repeat the abbreviation (CP) before reusing it in this section.

Why: The refresh of the CP abbreviation provides easier reading.
Otherwise, the reader must go back and find where CP was defined.

Old text: / The SR Candidate Path Constraints TLV is an optional TLV that is used
   to report the constraints associated with the candidate path./

New text:/ The SR Candidate Path Constraints TLV is an optional TLV that is used
   to report the constraints associated with the candidate path (CP)./

KT> If the reader is not aware of "CP" until they have come to this point in the document then that indicates they are not familiar with the very base spec that this document depends on.
Sue-2: This repeating of an abbreviation is suggested best practice in scholarly writing.  It is not because the author is not familiar, but aids quick reading of text.
It is an editorial NIT so it is not required.


Nit-8. Section 5.6.3, lack of clarity of length of SRLG value.

old text:/
  *  SRLG Values: One or more SRLG values (each of 4 octets)./

new text:/
*SRLG VAlues: One or more SRLG values.  Each SRLG value is 4 octets.
/

KT> Fixed.
Sue: Thank you for addressing the issue.

 NIT-8, Section 5.6.4, "a" candidate path instead of "the" candidate path

Old text:/
   The SR Disjoint Group Constraint sub-TLV is an optional sub-TLV of
   the SR CP Constraints TLV that is used to carry the disjointness
   constraint associated with the candidate path. /
New text:/
    The SR Disjoint Group Constraint sub-TLV is an optional sub-TLV of
   the SR CP Constraints TLV that is used to carry the disjointness
   constraint associated with a candidate path. /


KT> I believe the use of "the" is correct since the reference is to the specific CP that is being advertised
Sue> What singular Candidate Path is being referenced?

 NIT-9, Section 5.6.5, clarity of R-Flag description

 Old text:/
      -  R-Flag: Indicates that this CP of the SR Policy forms the
         reverse path when set and otherwise it is the forward path when
         clear/

 New text:/
      -  R-Flag: Indicates that this CP of the SR Policy forms the
         reverse path when the R-Flag is set.  If the R-Flag is clear,
         this CP forms the forward path./

KT> Incorporated this suggestion.
Sue: Thank you for addressing this issue.

NIT-10: Section 6, sentence clarity can be improved by breaking long sentence into two sentences.

Original text:/
   For the reporting of SR Policy Candidate Paths, the NLRI descriptor
   TLV as specified in Section 4 is used.  An SR Policy candidate path
   (CP) may be instantiated on the headend node via a local
   configuration, PCEP, or BGP SR Policy signaling and this is indicated
   via the SR Protocol Origin.  When a PCE node is the BGP-LS Producer,
   it uses the "reported via PCE" variants of the SR Protocol Origin so
   as to distinguish them from advertisements by headend nodes.  Then
   the SR Policy Candidate Path's state and attributes are encoded in
   the BGP-LS Attribute field as SR Policy State TLVs and sub-TLVs as
   described in Section 5.

new text:/
    An SR Policy candidate path
   (CP) may be instantiated on the headend node via a local
   configuration, PCEP, or BGP SR Policy signaling.  The protocol
   that instantiates the SR Policy candidate path is indicated
   via the SR Protocol Origin.

   When a PCE node is the BGP-LS Producer, the PCE node uses the
   "reported via PCE" variants of the SR Protocol Origin to
   distinguish them from advertisements by headend nodes.
   These "report via PCE" variants include "PCEP reported via PCE/PCEP" (10),
   "BGP SR Plicy reported via PCE/PCEP" (20), Configuration (CLI, Yang Model
    via NETCONF, etc.) reported via PCE/PCEP).    Then
   the SR Policy Candidate Path's state and attributes are encoded in
   the BGP-LS Attribute field as SR Policy State TLVs and sub-TLVs as
   described in Section 5. /

KT> I think the original text is clear as well.
Sue> I disagree, but this is a NIT based on my understanding of grammar and sentence clarity.  All Editorial nits may be ignored.

 NIT-11: Security considerations, paragraph 3, unclear use of ";"
 Original text:/  BGP peerings are
   not automatic and require configuration; thus, it is the
   responsibility of the network operator to ensure that only trusted
   nodes (that include both routers and controller applications) within
   the SR domain are configured to receive such information./

 Suggested revision:/ BGP peer connections are
   not automatic and require configuration. Thus, it is the
   responsibility of the network operator to ensure that only trusted
   nodes (that include both routers and controller applications) within
   the SR domain are configured to receive such information./


KT> Fixed.
Sue-2: Thank you.

Thanks,
Ketan