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

[Ketan Talaulikar <[e-mail]>]

Hi Sue,

Thanks for your responses and clarifications.

Since the draft submission is closed, I've added the updated text and
diff files for your review. Once we conclude, I can submit when the
tool opens.

Please check inline below with KT2.

On Wed, Oct 23, 2024 at 6:24 PM Susan Hares <[e-mail]> wrote:
>
> 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.
>

KT2> All of those updated and responded inline below.

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

KT2> Also responded inline below.

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

KT2> The context here is the advertisement of a single SR Policy CP
within BGP since that is the "unit" of advertisement. Therefore, "the"
is referencing the CP being signaled and "a" does not seem appropriate
since we are not talking about any general CP but "the" one being
signaled. Whether that particular CP is or becomes active or not does
not matter.

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

KT2> Done

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

KT2> Updated

>
>
>
> 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:”.
>

KT2> Updated

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

KT2> Updated

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

KT2> Updated

>
> •   What does mutual exclusive mean between P-Flag and U-Flag.  (I think it means both cannot be set).

KT2> Yes

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

KT2> Updated

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

KT2> Updated.

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

KT2> Updated

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

KT2> Updated the text without needing to get into PCEP specifics.

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

KT2> Updated

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

KT2> This is some misunderstanding on this point. This document is
about the specification of the BGP-LS TLV encoding. What information
is carried inside it is opaque to BGP-LS and out of the scope of this
document. Therefore, the discussion of metric types is not for this
document.

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

KT2> Updated

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

KT2> Updated

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

KT2> Updated

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

KT2> Updated

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

KT2> The context is where this information comes from. One example is
configuration (when the SR Policy is configured as an explicit path on
the router). Another example is when this is signaled via PCEP (when
the segment list comes as EROs). In both of these cases, when
referring to p-t-p adjacencies, it is sufficient to use the local
address alone. Therefore, the remote address would not be available on
the router where the SR Policy is being advertised into BGP-LS. In
such cases, the guidance is to set 0 as the remote address. I hope
that clarifies.

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

KT2> Please see my response to your Issue no 18 - I hope that sets the context.

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

KT2> Updated

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

KT2> Responded at the top.

>
>
>
> 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/html/draft-ietf-idr-bgp-ls-sr-policy-06
>
>
>
> Please check inline below for responses.
>
>
>
>
>
> On Fri, Aug 30, 2024 at 2:16 AM Susan Hares <[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:
>
>
>
> Edits to address this shepherd report
> Report on 2 implementations on the BGP wiki
> 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:
>
> State at top of bit positions,
> 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
>
>
>
>
>
>