ietf-wg-idr / draft-ietf-idr-sr-policy-safi

Repository for draft-ietf-idr-sr-policy-safi Issues
0 stars 0 forks source link

Shepherd's comments on -04 text #32

Open suehares opened 3 months ago

suehares commented 3 months ago

Shepherd's Comments on -04 Section 1 - Based on RTG-DIR

status: Editorial, closed Comments: Intro-1, Intro-2, Intro-3

SR04-Intro-1

Where: Introduction, paragraph 13 Github-1: RTG-DIR Issues 4 and 5 status-05: closed RTG-DIR Issue-4 https://github.com/ietf-wg-idr/draft-ietf-idr-sr-policy-safi/issues/6 RTG-DIR Issue-5 https://github.com/ietf-wg-idr/draft-ietf-idr-sr-policy-safi/issues/7 Shepherd's editorial preference is noted below but the -04 text is technically correct.

Suggested change: / Old text:/ An SR Policy intended only for the receiver will, in most cases, not traverse any Route Reflector (RR, [RFC4456]). / New text:/ An SR Policy intended only for the receiver will, in most cases, not traverse any Route Reflector (RR, [RFC4456]). (See Section 4.2.3). / status: fixed in -05.

SR04-Intro-2

Section: Introduction, paragraph 14 original issue: RTG-DIR Issue 6 https://github.com/ietf-wg-idr/draft-ietf-idr-sr-policy-safi/issues/8 Status-05: Editorial, closed

old text:/

New text: /

SR04-Intro-3

-04 text / The SR Policy SAFI route updates use the Tunnel Encapsulation Attribute to signal an SR Policy - i.e., a tunnel itself./

**New-text:/ The SR Policy SAFI route updates use the Tunnel Encapsulation Attribute to signal an SR Policy - which is a tunnel itself/ Status: Closed, -05

suehares commented 3 months ago

Shepherd's comments on -04.txt Section 2

only S2-02 (partial), S3-03 (partial, segment type validation)

S2-01: Section 2.4.2, paragraph 2

github issue: RTG-DIR-Issue-14, github 16 https://github.com/ietf-wg-idr/draft-ietf-idr-sr-policy-safi/issues/16 Status: Editorial, clarity of text, closed.

Original Text: / It is RECOMMENDED that the SRv6 Binding SID sub-TLV defined in Section 2.4.3, that enables the specification of the SRv6 Endpoint Behavior, be used for signaling of an SRv6 Binding SID for an SR Policy candidate path./

New text:/ It is RECOMMENDED that the SRv6 Binding SID sub-TLV (defined in Section 2.4.3) be used to signal an SRv6 Binding SID for an SR Policy candidate path./

Status: (Ketan) Retaining the original text since it explains why the SRv6 BSID sub-TLV is recommended.

S2-02: Section 2.4.2, I-Flag

github issue: RTG-DIR Issue-19, https://github.com/ietf-wg-idr/draft-ietf-idr-sr-policy-safi/issues/21 Reasons: Technical, RFC9256, Section 8.2 is confusing, CP undefined Status: closed with -06.txt release

Text:/

Text in RFC9256 section 8.2 states:

Specifically, the operator does not want the PW to be routed according to the IGP shortest path to the PW endpoint.

Technical comment: BGP passes to the SRPM a valid SR Policy Candidate Route a specific behavior (I-Flag). Validation of the SR Policy Candidate Route is based on NLRI and TEA checks. The general concept of the I-Flag is given in RFC9256 in Section 8.2 is reasonable, but matching the BGP-SRPM actions is difficult. (See RTG-DIR comments)

Alternate text:/

S2-03: Segment list validation, Section 2.4.4

github issue: RTG-DIR Issue-19 status: Technical issue: Validating Segment lists - fixed weight and empty list, need type validation status-05: partial fix to weight and zero length. Status-06: - issue closed with text addition in -06 that handles mixed SR-MPLS and SRv6 types. reason: RFC9256 in section 5.1 indicates that

Text-1:/ The Segment List sub-TLV contains zero or more Segment sub-TLVs and MAY contain a Weight sub-TLV./

Text-2:/ Validation of an explicit path encoded by the Segment List sub-TLV is beyond the scope of BGP and performed by the SRPM as described in section 5 of [RFC9256]./

The components of the explicit SR Candidate Policy path can be validated during the TEA processing for the following:

As stated, Section 5 of [RFC9256] does not provide a clear reason why the normal checks within BGP are being ignored for an explicit candidate path. If the "explicit path encoded by the Segment List SubTLV" is either an explicit SR candidate Policy path (tunnel), or a dynamic SR candidate Policy path (tunnel), or a composite SR Candidate Policy path, the text needs to state:
1) what type of segment lists are being passed by BGP, 2) why an empty segment list is not being checked, and 3) why a weight SubTLV with a value of zero is accepted for a valid SR Candidate Policy.

Suggested change for weight subTLV check: Change: Technical, validation of explicit SR Candidate Policy.

Current text:/

Suggested text for checking segment types: / New text- appended after: /The following sub-sections specify the sub-TLVs used for Segment Types A and B. The other segment types are specified in [I-D.ietf-idr-bgp-sr-segtypes-ext]./

New text/ Section 6 of [RFC9256] indicates that SRv6 and SR-MPLS segment types cannot be mixed in the same segment list. The following segment types are SRv6 segment types (A, C, E, F, I, J, and K). The following segment types are SR-MPLS segment types (B, D, G, and H). An implementation should check whether these types are mixed in the same segment list./

S3-04: Section 2.4.4.2.2,

github issue: RTG-DIR-20, github-22 https://github.com/ietf-wg-idr/draft-ietf-idr-sr-policy-safi/issues/22 Why: Editorial, Clarity and grammar Status:Rephrased and acceptable to shepherd:

-04-txt: / The TLV 2 defined for the advertisement of Segment Type B in the earlier versions of this document has been deprecated to avoid backward compatibility issues./

new text:/ A Segment Type B subTLV with a SubTLV type value of 2 was defined for the advertisement of Segment Type B in earlier versions of this document but it was deprecated to avoid backward compatibility issues./

rephrased: / The Sub-TLV code point 2 defined for the advertisement of Segment Type B in the earlier versions of this document has been deprecated to avoid backward compatibility issues./

S2-05: Section 2.4.4.2.2-2.4.4.2.3, Significance of Flags, V Flag

github issue: RTG-DIR-19 https://github.com/ietf-wg-idr/draft-ietf-idr-sr-policy-safi/issues/21 status: Technical issue 1: Validity of SR Candidate Routes, closed

Text:/ V-Flag: This flag, when set, is used by SRPM for "SID verification" as described in Section 5.1 of [RFC9256]./

The "V-Flag" applies to all segment types. However, since a segment list is only used for an explicit SR Candidate Policy, this phrase is acceptable in the text.

suehares commented 3 months ago

Shepherd's comments on Section 3

No issues from RTG-DIR Mail thread

RTG-DIR Mail thread: https://mailarchive.ietf.org/arch/msg/rtg-dir/E9qyJKHjRpMTNOAXnJo9jVi0nQg/

suehares commented 3 months ago

RTG-DIR Review: Issues with Section 4

SR04-01 - Section 4.2.1 - Lack of RT for IPv6

github issue: RTG-DIR Issue-24, github: https://github.com/ietf-wg-idr/draft-ietf-idr-sr-policy-safi/issues/26 status-06: closed.

Jeffrey Zhang's question: What about IPv6-address specific RT? https://mailarchive.ietf.org/arch/msg/rtg-dir/E9qyJKHjRpMTNOAXnJo9jVi0nQg/

Shepherd's question: NLRI specified are: SR Policy IPv4 (1/73) and SR Policy IPv6 (2/73). Why is the RT target not needed fro IPv6 NLRI (2/73)? Ketan answer: KT> The context is that the controller has done the path computation and is sending the computed path (i.e. computed segment list) down to the router. In that sense, they are only explicit paths - however in the broader SR Policy architecture context (controller + routers) they could be explicit paths (provided by operator) or dynamic paths (computed by controller). We have the following text in section 2.4.4 and also clarified the same in the introduction.

The Segment List sub-TLV encodes a single explicit path towards the endpoint as described in section 5.1 of [[RFC9256].

SR04-02 - Section 4.2.2 - Eligibility for Local Use of an SR Policy NLRI

github issue: RTG-DIR Issue-25: https://github.com/ietf-wg-idr/draft-ietf-idr-sr-policy-safi/issues/27

text: /

If one or more route targets are present and none matches the local BGP Identifier, then, while the SR Policy NLRI is valid, it is not usable on the receiver node./

Discussion: Jeffrey: As long as the receiver is configured with a local RT that matches one of the advertised RTs, it should be fine, right? That is how VPN RT works and I suppose the same can be used here. Ketan (KT): The semantics are different here. Shepherd's question: How is the -04 text above different than Jeffrey's assertion? Ketan (Kt): To clarify the local router's BGP Identifier matches one of the RTs on the route. Please let me know if I've misunderstood your point.

SR04-03 - Section 4.2.2 - WG discussion Questioned

Status: Simply Reporting the RTG-DIR hit the WG Discussion on RTs text:/

If one or more route targets are present and none matches the local BGP Identifier, then, while the SR Policy NLRI is valid, it is not usable on the receiver node./

Short Summary: Jeffrey: I think it SHOULD remove the RT that matches its BGP identifier and stop propagating if there are no more RTs left, w/o relying on configuration. (reference: https://mailarchive.ietf.org/arch/msg/rtg-dir/pzT0P9XId3qV3iCd-mgpZUvul7A/)

Ketan: I can understand where you are coming from and IIRC this option was discussed at some point of time during the draft's life as a WG document. The decision at that point was to keep it as an explicit policy option and to not create an exception in the base BGP propagation mechanism. The way it works today is that the BGP Identifier matching is done during "import" into SRPM that is specific to the BGP SR Policy SAFI.

Shepherd: restating this question: Are you using configured IBGP sessions rather than RT to reduce the flooding of information?

Ketan: Yes, most deployment designs that I am aware of use direct iBGP sessions from the controller to the BGP routers.

suehares commented 3 months ago

Shepherd's Review of RTG-DIR Comments on Section 5

S04-Section 5 - Issue-1

Status: Technical, closed Issue: Validation of the Tunnel, see RTG-DIR Issue-12 in github. https://github.com/ietf-wg-idr/draft-ietf-idr-sr-policy-safi/issues/14

Suggested text addition to section 5 in the following paragraph. There are changes to the original paragraph (editorial) and an additional set of text.

Paragraph revised (changed text in bold):/ The validation of the TLVs/sub-TLVs introduced in this document and defined in their respective sub-sections of Section 2.4 MUST be performed to determine if they are malformed or invalid. The validation of the Tunnel Encapsulation Attribute itself and the other TLVs/sub-TLVs specified in Section 13 of [RFC9012] MUST be done as described in that document. In case any error is detected, either at the attribute or its TLV/sub-TLV level, the "treat-as-withdraw" strategy MUST be applied. This is because an SR Policy update without a valid Tunnel Encapsulation Attribute (comprised of all valid TLVs/sub-TLVs) is not usable.

Section 6 of [RFC9012] referred to by section 13 of [RFC9012] does not apply to a TLV associated with the SR Policy NRLI so the Tunnel Egress Endpoint does not have to exist in the Tunnel Encapsulation Attribute with the tunnel type SR Policy. As noted in section 2.3, the Tunnel Egress Endpoint SubTLV is ignored for tunnel Type SR Policy. Only the SRPM validates the viability of the tunnel signaled through BGP./

SR04 Section 5 - Issue 2

Status: Technical, Reporting concern, Section 5 github: RTG-DIR Issue 27, closed merged into SR02-03 https://github.com/ietf-wg-idr/draft-ietf-idr-sr-policy-safi/issues/29 text:/

The validation of the individual fields of the TLVs/sub-TLVs defined in Section 2.4 are beyond the scope of BGP as they are handled by the SRPM as described in the individual TLV/sub-TLV sub-sections. A BGP implementation MUST NOT perform semantic verification of such fields nor consider the SR Policy update to be invalid or not usable based on such validation./

Comment from RTG-DIR: Jeffrey: The above says the validation of those in Section 2.4 may lead to "treat-as-withdraw" - I assume this is BGP handling. Does that not conflict with the following paragraph? Ketan: No, it does not. There is a line between what validation is done by BGP and what is done by SRPM. Shepherd: The RTG-DIR questions the wisdom of not doing minimal checks (like empty segment list or Weight = 0). The Concept of MUST NOT goes against the normal sanity checks in SR Routing toward BGP-LS.

suehares commented 1 month ago

Open issues for -05: S2-02 (partial), S3-03 (partial, segment type validation) Open issues for-06: None