Subject: Re: [Idr] AD Review of draft-ietf-idr-bgp-prefix-sid-07

From: "Acee Lindem (acee)" <[email-masked]>
Date: Wed, 3 Jan 2018 02:35:34 +0000

Hi Alvaro,

From: Alvaro Retana <[email-masked]<mailto:[email-masked]>>
Date: Thursday, December 21, 2017 at 4:52 PM
To: "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>
Cc: "[email-masked]<mailto:[email-masked]>" <[email-masked]<mailto:[email-masked]>>, Susan Hares <[email-masked]<mailto:[email-masked]>>, IDR List <[email-masked]<mailto:[email-masked]>>
Subject: AD Review of draft-ietf-idr-bgp-prefix-sid-07
Resent-From: <[email-masked]<mailto:[email-masked]>>
Resent-To: Stefano Previdi <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>, Acee Lindem <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>, <[email-masked]<mailto:[email-masked]>>
Resent-Date: Thursday, December 21, 2017 at 4:52 PM

Dear authors:

Hi!  I just finished reading this document.

I have a significant number of comments (please see below).  Two of them I would like to highlight up here.

(1) References to draft-ietf-spring-segment-routing-msdc

In the current text, draft-ietf-spring-segment-routing-msdc is not only referenced as an example of the use of the BGP Prefix-SID attribute, but in a Normative way to indicate how the attribute should be treated.  While there’s nothing wrong with draft-ietf-spring-segment-routing-msdc (it already passed IETF LC and is in IESG Evaluation), I’m sure there are other applications for the new attribute, right?  The importance given to it in this document, which should elevate it to a Normative reference, is probably more than it deserves since it just focuses on describing "the design to deploy segment routing in [large-scale] data-centers” and it doesn’t contain a list of requirements nor it mandates how the new attribute should be used.  Note also that draft-ietf-spring-segment-routing-msdc refers normatively to this document to explain the use case, so pointing Normatively back to it creates a loop....  Please treat draft-ietf-spring-segment-routing-msdc as what is should be: a good Informative reference — I put specific comments above about the text below (see M1).

I agree. However, I think this is fixable by removing some of the unnecessary “As described in” references and cite the others purely as examples. Please see the –08 version.

(2) Error Handling

The error handling (in Section 6) specifies that “attribute discard” (rfc7606) should be used if the attribute is malformed.  I think that behavior has a direct effect on the path used to forward the traffic, which puts it then in conflict with rfc7606 because it clearly says that "Attribute discard...MUST NOT be used except in the case of an attribute that has no effect on route selection or installation.”  Please see M9 below.

Well, everybody has implemented “Attribute discard”. I’d argue that the statement in RFC 7606 applies to the BGP speaker encountering the malformed attribute and not BGP speakers further upstream. Since the advertised RFC 8277 label is installed in any case, discarding the attribute really only impacts the local label selection and not path selection or route installation (strictly speaking).

Thanks,
Acee

I will wait for the Major comments to be addressed before starting the IETF LC.

Thanks!

Alvaro.

Major:

M1. References to draft-ietf-spring-segment-routing-msdc.  As I mentioned above, I believe that this document gives a lot more importance to draft-ietf-spring-segment-routing-msdc than it deserves — for one, I think that draft-ietf-spring-segment-routing-msdc should not be considered as a Normative source for this document, but as it stands right now its reference would have to in fact be Normative.

M1.1 Section 1: "As described in [I-D.ietf-spring-segment-routing-msdc], the BGP Prefix-SID attribute defined in this document can be attached to prefixes from AFI/SAFI...labeled IPv4/IPv6 Unicast...unlabeled IPv6 Unicast”.  draft-ietf-spring-segment-routing-msdc does describe that, but within the DC application — IOW, just because one use case uses the BGP Prefix SID in that way doesn’t mean that it determines it’s use…just an example.  s/As described in [I-D.ietf-spring-segment-routing-msdc]/

M1.2. s/[I-D.ietf-spring-segment-routing-msdc] describes use cases where the Prefix-SID is used for the above AFI/SAFI./[I-D.ietf-spring-segment-routing-msdc] describes use cases where the Prefix-SID is used for the above AFI/SAFI in a MSDC.

M1.3. Section 1: "As described in [I-D.ietf-spring-segment-routing-msdc], a BGP Prefix-SID MAY be attached to a prefix.”

M1.3.1. draft-ietf-spring-segment-routing-msdc doesn’t even use Normative language

M.1.3.2. ...even if it did, what is the option?  Why is “MAY” used?  What else can the BGP Prefix-SID be attached to?  Everywhere else in this document, the text talks about attachment to a prefix.

M1.3.3. Section 5. (Announcing BGP-Prefix-SID Attribute) makes a similar statement: "The BGP Prefix-SID attribute MAY be attached to labeled BGP prefixes (IPv4/IPv6) [RFC3107] or to IPv6 prefixes [RFC4760].”  Same questions: is there another option?  In this case, did you mean “…MUST only be attached to…”?

M1.4. Section 2.1: "As described in [I-D.ietf-spring-segment-routing-msdc] the operator assigns a globally unique “index”, L_I…”.  Again, that’s an example…. s/As described in [I-D.ietf-spring-segment-routing-msdc]/

M1.4.1. [nit] BTW, is there a reason for “index” to be in “”??  It isn’t anywhere else.

M1.4.2. [minor] Also, please be consistent.  In some places you use “Label Index” (as in the packet format), but also “label index”, label_index, label-index, Label-Index, simply index or even the (redundant?) "index L_I".  Some of those may be ok, but many seem to talk about the same thing while calling it by slightly different names.

M1.5. Section 2.2: "As illustrated in [I-D.ietf-spring-segment-routing-msdc]...the BGP Prefix-SID consists of an IPv6 address…”. s/As illustrated in [I-D.ietf-spring-segment-routing-msdc]/

M1.6. Section 3.3: "It is used to build segment routing policies when different SRGB's are used in the fabric ([I-D.ietf-spring-segment-routing-msdc]).”  I’m assuming that the fabric is not the only use case of having different SRGBs, right?  Make the statement general and not dependent on I-D.ietf-spring-segment-routing-msdc.

M1.7. Section 8: "This document defines a new BGP attribute in order to address the use case described in [I-D.ietf-spring-segment-routing-msdc].”  I’m sure that is not the only use case…reword to show it is an example.

M1.8. Section 9: "The BGP Prefix-SID attribute addresses the requirements introduced in [I-D.ietf-spring-segment-routing-msdc]…”. draft-ietf-spring-segment-routing-msdc doesn’t actually present requirements…but an application in the DC...

M2. TLV Definitions:

M2.1. [minor] what is the unit used in the Length?  Bytes, octets, bits

M2.2. Please define a registry and a registration policy for the Flag fields.

M3. What should be done if a Label-Index TLV is received if an unlabeled IPv6 prefix or if the IPv6 SID TLV is received with a labeled prefix?  I assume they MUST be ignored…but please include that in the text.

M4. IPv6 SID TLV

M4.1. Section 3.2 defines this TLV as optional ("IPv6-SID TLV MAY be present”), ok.  Section 4.2 then says that "If present, then the receiver assumes that the originator supports SR on the IPv6 dataplane.”  This seems to indicate that the purpose of this TLV is not only to advertise an SID, but to communicate support for SR, is that correct?  Later, 5.2 again says that the sender "MAY include the IPv6 SID TLV”.  If this TLV is not present, can the receiver assume that the originator doesn’t support SR?  I’m wondering about the optional nature of the TLV, it seems to be mandatory if the sender wants the receiver to know that it supports SR, and whenever an IPv6 SID needs to be advertised (which seems like always when using the IPv6 dataplane).  What am I missing?

M4.2. [nit] s/IPv6-SID/IPv6 SID

M5. Section 4: "A BGP speaker receiving a BGP Prefix-SID attribute from an EBGP neighbor residing outside the boundaries of the SR domain, SHOULD discard the attribute unless it is configured…”. If that is the only exception, then s/SHOULD/MUST

M5.1. The same text is present in 4.2.  If the text in 4 applies to both dataplanes, then the text in 4.2 is redundant.

M6. In 4.1: "A BGP speaker MAY be locally configured with an SRGB=[SRGB_Start, SRGB_End].  The preferred method for deriving the SRGB is a matter of local node configuration.”  Given that the "method for deriving the SRGB is a matter of local node configuration”, that “MAY” is out of place since it has no Normative value.  s/MAY/may

M7. Section 4.1 explains the conditions under which the BGP Prefix-SID attribute should be considered unacceptable, and it says that the receiver of “an unacceptable BGP Prefix-SID attribute...MUST treat the path as if it came without a Prefix-SID attribute.”  However, Section 5.1 says that a "speaker that advertises a path received from one of its neighbors SHOULD advertise the Prefix-SID received with the path without modification regardless of whether the Prefix-SID was acceptable”.  I think there is a Normative contradiction here because the MUST seems to imply that the attribute is to be dropped (“as if it came without”), but the SHOULD indicates the opposite.  I can also see how the text could be interpreted as “just ignore the attribute but forward it”.  Please clarify so there is no confusion.

M7.1. I assume that the acceptable condition doesn’t just apply to the MPLS data plane.  It may be clearer if the text in 4.1 was moved to a common section.

M7.2. Note that the same text (from 5.1) is repeated in 5.2.  It may be clearer if the specification was in a common place instead.

M8. Section 6: "If the BGP Prefix-SID attribute appears more than once in an BGP Update message, then, according to [RFC7606], all the occurrences of the attribute other than the first one SHALL be discarded and the BGP Update message SHALL continue to be processed.”  Note that rfc7606 doesn’t say exactly that, instead it says "all the occurrences of the attribute other than the first one SHALL be discarded and the UPDATE message will continue to be processed”.  Yes, it’s a slight difference, but it’s not the same.  In this case, the Normative language is already in rfc7606, so there’s no real need to repeat it here (if you do, please use “”).  Suggestion:  “…according to rfc7606, only the first occurrence will be considered.”

M9.  If the attribute is malformed then the index or SID in it won’t be used by the receiver (attribute discard) as specified in Section 6.  Section 4.1 explains how a local label is assigned if the BGP Prefix-SID attribute is unacceptable.

M9.1. I’m assuming that the local label assignment in 4.1 includes the case when the attribute is malformed (i.e. not just the unacceptable case).  Is that true?   Using a local label would result in successfully delivering the traffic to the destination, but not using the intended SR path, right?  If that is true, then the malformed attribute case would have an effect on route selection/installation and could result (among other things) in the traffic taking an unwanted path due to policy, a path that is congested, etc.…and that is explicitly the case where rfc7606 specifies that attribute discard MUST NOT be used.  It seems that any action beyond attribute discard may be too harsh given that a local label can be assigned…but the current specification ("MUST treat the path as if it came without a Prefix-SID attribute...MUST assign a local (also called dynamic) label”) is in conflict with rfc7606.   I would be ok if the document explained the potential issues and made the local label behavior optional (pending a discussion in the WG and maybe an update to rfc7606).  BTW, was this discussed in the WG (I couldn’t find a related discussion in the archive)?

M9.2. What about the IPv6 case?  If the attribute is malformed, then the receiver won’t have an IPv6 SID to use — again, there seems to be a clear effect on route selection and installation.

M10. Section 8. (Manageability Considerations).  There’s a Normative conflict on these two sentences: "By default, a BGP Prefix-SID attribute SHOULD NOT be originated and attached to a prefix.  The operator MUST be capable of explicitly enabling the BGP Prefix-SID origination.”  The “MUST" indicates that the operator has to be involved, but the "SHOULD NOT" implies that there are cases where it is ok for the operator not to be involved.

M11. Security Considerations

M11.1. You should mention the Security Considerations of the overall SR architecture.

M11.2. While I agree with the rest of this section, I think there’s a new risk that is introduced by the attribute: the modification of the TLVs in transit.  For example, if one of the transit BGP speakers modifies the Label Index to make it an unacceptable TLV (according to 4.1) then it would have the effect of potentially causing the traffic to take a different path.  I don’t think there’s a mitigation mechanism (specially because the document describes upfront the unacceptable criteria — which is what opens this door), but recognizing the modification risk and at least mentioning that all the routers are part of the same domain (trusted) would be a good idea.

M12. The Reference to rfc4760 should be Normative.

Minor:

P1. Please update the Requirements Language to use the new template in rfc8174.

P2. rfc3107 hs been obsoleted by rfc8277

P3. s/as_path/AS_PATH

P4. s/is assumed to be done through/is done through

P5. "As defined in [I-D.ietf-spring-segment-routing-mpls], the index L_I is an offset in the SRGB.”  The reference should be I-D.ietf-spring-segment-routing (that is where the index and the SRGB are explained).

P6. Please be consistent when referring to the new attribute: s/Prefix-SID attribute/BGP Prefix-SID attribute

Nits:

N1. s/It i assumed/It is assumed

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Fri, 5 Jan 2018 16:16:06 +0000

Hi Alvaro,

Thanks for your comments. I’ve added you to the acknowledgements. I believe I’ve satisfied or obviated the need for resolution for all of them in versions –08 and -09. Please see inline.

Dear authors:

Hopefully, I’ve completely decoupled dependence on this draft as a normative reference.

(2) Error Handling

While a BGP Prefix-SID for MPLS is still takes the “Attribute discard” action, I’m indicated that the “Treat-as-withdrawl” action should be taken for SRv6.

Thanks!

Alvaro.

Major:

See above. Hopefully, I’ve completely removed this problematic dependency.

Fixed.

Fixed.

M2. TLV Definitions:

Octets – fixed.

Added although registries are created empty.

Yes, they are ignored. Fixed.

M4. IPv6 SID TLV

Removed. This has been reworded to not imply this connotation to the BGP Prefix-SID attribute.

Fixed.

Fixed.

Moved to section 4.

Changed – a major comment?

This has been removed. Implementations simply don’t re-advertise BGP attributes that found to be unacceptable.

Fixed.

As previously stated and discussed on the list, an unacceptable BGP Prefix-SID attribute for MPLS will retain the “Attribute discard” action while an unacceptable BGP Prefix-SID attribute for SRv6 will have the “Treat-as-withdraw” action.

I don’t really see this as a contradiction but have reworded it to be clear.

Fixed.

I believe I’ve obviated the need for this by changing the SRv6 error handling.

Fixed.

Minor:

Fixed.

Fixed.

P3. s/as_path/AS_PATH

Fixed.

Fixed.

Fixed.

Fixed.

Nits:

Fixed.

Thanks,
Acee

---

From: Alvaro Retana <[email-masked]>
Date: Fri, 12 Jan 2018 08:52:31 -0800

Thanks Acee!

I have started the IETF Last Call.

Alvaro.

On January 5, 2018 at 11:16:09 AM, Acee Lindem (acee) ([email-masked])
wrote:

Hi Alvaro,

Thanks for your comments. I’ve added you to the acknowledgements. I believe
I’ve satisfied or obviated the need for resolution for all of them in
versions –08 and -09. Please see inline.

From: Alvaro Retana <[email-masked]>
Date: Thursday, December 21, 2017 at 4:52 PM
To: "[email-masked]" <
[email-masked]>
Cc: "[email-masked]" <[email-masked]>, Susan Hares <
[email-masked]>, IDR List <[email-masked]>
Subject: AD Review of draft-ietf-idr-bgp-prefix-sid-07
Resent-From: <[email-masked]>
Resent-To: Stefano Previdi <[email-masked]>, <[email-masked]>, Acee
Lindem <[email-masked]>, <[email-masked]>, <[email-masked]>
Resent-Date: Thursday, December 21, 2017 at 4:52 PM

Dear authors:

I have a significant number of comments (please see below).  Two of them I
would like to highlight up here.

In the current text, draft-ietf-spring-segment-routing-msdc is not only
referenced as an example of the use of the BGP Prefix-SID attribute, but in
a Normative way to indicate how the attribute should be treated.  While
there’s nothing wrong with draft-ietf-spring-segment-routing-msdc (it
already passed IETF LC and is in IESG Evaluation), I’m sure there are other
applications for the new attribute, right?  The importance given to it in
this document, which should elevate it to a Normative reference, is
probably more than it deserves since it just focuses on describing "the
design to deploy segment routing in [large-scale] data-centers” and it
doesn’t contain a list of requirements nor it mandates how the new
attribute should be used.  Note also
that draft-ietf-spring-segment-routing-msdc refers normatively to this
document to explain the use case, so pointing Normatively back to it
creates a loop....  Please treat draft-ietf-spring-segment-routing-msdc as
what is should be: a good Informative reference — I put specific comments
above about the text below (see M1).

Hopefully, I’ve completely decoupled dependence on this draft as a
normative reference.

(2) Error Handling

The error handling (in Section 6) specifies that “attribute discard”
(rfc7606) should be used if the attribute is malformed.  I think that
behavior has a direct effect on the path used to forward the traffic, which
puts it then in conflict with rfc7606 because it clearly says that
"Attribute discard...MUST NOT be used except in the case of an attribute that
has no effect on route selection or installation.”  Please see M9 below.

While a BGP Prefix-SID for MPLS is still takes the “Attribute discard”
action, I’m indicated that the “Treat-as-withdrawl” action should be taken
for SRv6.

I will wait for the Major comments to be addressed before starting the IETF
LC.

Thanks!

Alvaro.

Major:

M1. References to draft-ietf-spring-segment-routing-msdc.  As I mentioned
above, I believe that this document gives a lot more importance
to draft-ietf-spring-segment-routing-msdc than it deserves — for one, I
think that draft-ietf-spring-segment-routing-msdc should not be considered
as a Normative source for this document, but as it stands right now its
reference would have to in fact be Normative.

M1.1 Section 1: "As described in [I-D.ietf-spring-segment-routing-msdc],
the BGP Prefix-SID attribute defined in this document can be attached to
prefixes from AFI/SAFI...labeled IPv4/IPv6 Unicast...unlabeled IPv6
Unicast”.  draft-ietf-spring-segment-routing-msdc does describe that, but
within the DC application — IOW, just because one use case uses the BGP
Prefix SID in that way doesn’t mean that it determines it’s use…just an
example.  s/As described in [I-D.ietf-spring-segment-routing-msdc]/

M1.2. s/[I-D.ietf-spring-segment-routing-msdc] describes use cases where
the Prefix-SID is used for the above
AFI/SAFI./[I-D.ietf-spring-segment-routing-msdc] describes use cases where
the Prefix-SID is used for the above AFI/SAFI in a MSDC.

M1.3. Section 1: "As described in [I-D.ietf-spring-segment-routing-msdc], a
BGP Prefix-SID MAY be attached to a prefix.”

M1.3.1. draft-ietf-spring-segment-routing-msdc doesn’t even use Normative
language

M.1.3.2. ...even if it did, what is the option?  Why is “MAY” used?  What
else can the BGP Prefix-SID be attached to?  Everywhere else in this
document, the text talks about attachment to a prefix.

M1.3.3. Section 5. (Announcing BGP-Prefix-SID Attribute) makes a similar
statement: "The BGP Prefix-SID attribute MAY be attached to labeled BGP
prefixes (IPv4/IPv6) [RFC3107] or to IPv6 prefixes [RFC4760].”  Same
questions: is there another option?  In this case, did you mean “…MUST only
be attached to…”?

M1.4. Section 2.1: "As described in [I-D.ietf-spring-segment-routing-msdc]
the operator assigns a globally unique “index”, L_I…”.  Again, that’s an
example…. s/As described in [I-D.ietf-spring-segment-routing-msdc]/

M1.4.1. [nit] BTW, is there a reason for “index” to be in “”??  It isn’t
anywhere else.

Fixed.

M1.4.2. [minor] Also, please be consistent.  In some places you use “Label
Index” (as in the packet format), but also “label index”, label_index,
label-index, Label-Index, simply index or even the (redundant?) "index
L_I".  Some of those may be ok, but many seem to talk about the same thing
while calling it by slightly different names.

Fixed.

M1.5. Section 2.2: "As illustrated in
[I-D.ietf-spring-segment-routing-msdc]...the BGP Prefix-SID consists of an
IPv6 address…”. s/As illustrated in [I-D.ietf-spring-segment-routing-msdc]/

M1.6. Section 3.3: "It is used to build segment routing policies when
different SRGB's are used in the fabric
([I-D.ietf-spring-segment-routing-msdc]).”
 I’m assuming that the fabric is not the only use case of having different
SRGBs, right?  Make the statement general and not dependent
on I-D.ietf-spring-segment-routing-msdc.

M1.7. Section 8: "This document defines a new BGP attribute in order to
address the use case described in [I-D.ietf-spring-segment-routing-msdc].”
 I’m sure that is not the only use case…reword to show it is an example.

M1.8. Section 9: "The BGP Prefix-SID attribute addresses the requirements
introduced in [I-D.ietf-spring-segment-routing-msdc]…”.
draft-ietf-spring-segment-routing-msdc
doesn’t actually present requirements…but an application in the DC...

M2. TLV Definitions:

Octets – fixed.

M2.2. Please define a registry and a registration policy for the Flag
fields.

M3. What should be done if a Label-Index TLV is received if an unlabeled
IPv6 prefix or if the IPv6 SID TLV is received with a labeled prefix?  I
assume they MUST be ignored…but please include that in the text.

M4. IPv6 SID TLV

M4.1. Section 3.2 defines this TLV as optional ("IPv6-SID TLV MAY be
present”), ok.  Section 4.2 then says that "If present, then the
receiver assumes
that the originator supports SR on the IPv6 dataplane.”  This seems to
indicate that the purpose of this TLV is not only to advertise an SID, but
to communicate support for SR, is that correct?  Later, 5.2 again says that
the sender "MAY include the IPv6 SID TLV”.  If this TLV is not present, can
the receiver assume that the originator doesn’t support SR?  I’m wondering
about the optional nature of the TLV, it seems to be mandatory if the
sender wants the receiver to know that it supports SR, and whenever an IPv6
SID needs to be advertised (which seems like always when using the IPv6
dataplane).  What am I missing?

Removed. This has been reworded to not imply this connotation to the BGP
Prefix-SID attribute.

Fixed.

M5. Section 4: "A BGP speaker receiving a BGP Prefix-SID attribute from an
EBGP neighbor residing outside the boundaries of the SR domain, SHOULD discard
the attribute unless it is configured…”. If that is the only exception,
then s/SHOULD/MUST

Fixed.

M5.1. The same text is present in 4.2.  If the text in 4 applies to both
dataplanes, then the text in 4.2 is redundant.

Moved to section 4.

M6. In 4.1: "A BGP speaker MAY be locally configured with an
SRGB=[SRGB_Start, SRGB_End].  The preferred method for deriving the SRGB is
a matter of local node configuration.”  Given that the "method for deriving
the SRGB is a matter of local node configuration”, that “MAY” is out of
place since it has no Normative value.  s/MAY/may

M7. Section 4.1 explains the conditions under which the BGP Prefix-SID
attribute should be considered unacceptable, and it says that the receiver
of “an unacceptable BGP Prefix-SID attribute...MUST treat the path as if it
came without a Prefix-SID attribute.”  However, Section 5.1 says that a
"speaker that advertises a path received from one of its neighbors SHOULD
advertise the Prefix-SID received with the path without modification
regardless of whether the Prefix-SID was acceptable”.  I think there is a
Normative contradiction here because the MUST seems to imply that the
attribute is to be dropped (“as if it came without”), but the SHOULD
indicates the opposite.  I can also see how the text could be interpreted
as “just ignore the attribute but forward it”.  Please clarify so there is
no confusion.

This has been removed. Implementations simply don’t re-advertise BGP
attributes that found to be unacceptable.

M7.1. I assume that the acceptable condition doesn’t just apply to the MPLS
data plane.  It may be clearer if the text in 4.1 was moved to a common
section.

M7.2. Note that the same text (from 5.1) is repeated in 5.2.  It may be
clearer if the specification was in a common place instead.

M8. Section 6: "If the BGP Prefix-SID attribute appears more than once in
an BGP Update message, then, according to [RFC7606], all the occurrences of
the attribute other than the first one SHALL be discarded and the BGP Update
message SHALL continue to be processed.”  Note that rfc7606 doesn’t say
exactly that, instead it says "all the occurrences of the attribute other
than the first one SHALL be discarded and the UPDATE message will continue to
be processed”.  Yes, it’s a slight difference, but it’s not the same.  In
this case, the Normative language is already in rfc7606, so there’s no real
need to repeat it here (if you do, please use “”).  Suggestion:
 “…according to rfc7606, only the first occurrence will be considered.”

Fixed.

M9.  If the attribute is malformed then the index or SID in it won’t be
used by the receiver (attribute discard) as specified in Section 6.
Section 4.1 explains how a local label is assigned if the BGP Prefix-SID
attribute is unacceptable.

As previously stated and discussed on the list, an unacceptable BGP
Prefix-SID attribute for MPLS will retain the “Attribute discard” action
while an unacceptable BGP Prefix-SID attribute for SRv6 will have the
“Treat-as-withdraw” action.

M9.1. I’m assuming that the local label assignment in 4.1 includes the case
when the attribute is malformed (i.e. not just the unacceptable case).  Is
that true?   Using a local label would result in successfully delivering
the traffic to the destination, but not using the intended SR path, right?
If that is true, then the malformed attribute case would have an effect on
route selection/installation and could result (among other things) in the
traffic taking an unwanted path due to policy, a path that is congested,
etc.…and that is explicitly the case where rfc7606 specifies that attribute
discard MUST NOT be used.  It seems that any action beyond attribute
discard may be too harsh given that a local label can be assigned…but the
current specification ("MUST treat the path as if it came without a
Prefix-SID attribute...MUST assign a local (also called dynamic) label”) is
in conflict with rfc7606.   I would be ok if the document explained the
potential issues and made the local label behavior optional (pending a
discussion in the WG and maybe an update to rfc7606).  BTW, was this
discussed in the WG (I couldn’t find a related discussion in the archive)?

M9.2. What about the IPv6 case?  If the attribute is malformed, then the
receiver won’t have an IPv6 SID to use — again, there seems to be a clear
effect on route selection and installation.

M10. Section 8. (Manageability Considerations).  There’s a Normative
conflict on these two sentences: "By default, a BGP Prefix-SID attribute
SHOULD NOT be originated and attached to a prefix.  The operator MUST be
capable of explicitly enabling the BGP Prefix-SID origination.”  The “MUST"
indicates that the operator has to be involved, but the "SHOULD NOT"
implies that there are cases where it is ok for the operator not to be
involved.

I don’t really see this as a contradiction but have reworded it to be
clear.

M11.1. You should mention the Security Considerations of the overall SR
architecture.

Fixed.

M11.2. While I agree with the rest of this section, I think there’s a new
risk that is introduced by the attribute: the modification of the TLVs in
transit.  For example, if one of the transit BGP speakers modifies the
Label Index to make it an unacceptable TLV (according to 4.1) then it would
have the effect of potentially causing the traffic to take a different
path.  I don’t think there’s a mitigation mechanism (specially because the
document describes upfront the unacceptable criteria — which is what opens
this door), but recognizing the modification risk and at least mentioning
that all the routers are part of the same domain (trusted) would be a good
idea.

I believe I’ve obviated the need for this by changing the SRv6 error
handling.

Fixed.

Minor:

P1. Please update the Requirements Language to use the new template in
rfc8174.

Fixed.

Fixed.

P3. s/as_path/AS_PATH

Fixed.

Fixed.

P5. "As defined in [I-D.ietf-spring-segment-routing-mpls], the index L_I is
an offset in the SRGB.”  The reference should
be I-D.ietf-spring-segment-routing (that is where the index and the SRGB
are explained).

Fixed.

P6. Please be consistent when referring to the new attribute: s/Prefix-SID
attribute/BGP Prefix-SID attribute

Fixed.

Nits:

Fixed.

Thanks,
Acee