Hi Stewart,
Thanks for your review. Please see some comments inline. From: rtgwg <[email protected]> On Behalf Of Stewart Bryant Sent: vendredi 7 mai 2021 21:21 To: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected] Subject: Shepherd review of draft-ietf-rtgwg-segment-routing-ti-lfa I have been asked to Shepherd draft-ietf-rtgwg-segment-routing-ti-lfa and base this review on -06 I have a number of comments that I think we can usefully resolve before the WGLC process, two of which (points 1 & 5) the Chairs need to comments on. I will start the IPR call in parallel with the process of resolution of these comments. Overall the draft is a significant improvement on the version that I reviewed some years ago, but there are a number of issues that we need to consider in this version. 1) You have six authors when the policy as I understand it is five. Please either change one of the authors to a contributor, or provide text for inclusion in the shepherd’s report that the IESG is likely to find a satisfactory reason for agreeing to an exception. [SLI] Unfortunately, this draft has a long history and changed editors multiple times. This is the reason we have six co-authors now and taking into account the contribution of each individual, it’s hard to remove one. We can see with ADs later, but I think we have a good case to keep the six. 2) In section 5.1 the text says: "When a direct neighbor is in P(S,X) and Q(D,x) and on the post-convergence path, the outgoing interface is set to that neighbor and the repair segment list MUST be empty." I agree that normally the repair list would be empty, but MUST is a very strong term. I am thus wondering if the MUST is correct and whether this is not a local matter. I think that it would not break anything PHP was not enabled and the packet was sent with a label to Q. Also an implementation might I suppose create a repair segment list with Adj(Q) and PHP it. MUST is a very strong term and I am therefore concerned as to whether it is correct. [SLI] Valid point, the empty stack is what is likely to be expected. I would then propose to use “SHOULD”. Same applies to the cases you mentioned below. 3) Again in 5.2 "When a remote node R is in P(S,X) and Q(D,x) and on the post-convergence path, the repair list MUST be made of a single node segment to R and the outgoing interface MUST be set to the outgoing interface used to reach R.” Again I agree that the repair list is likely to just contain “R” but I am worried if the strong term MUST is required. 4) In section 5.3, again worried that the strong term MUST is used. 5) In 6.1.1. The active segment is a node segment "The active segment MUST be kept on the SR header unchanged and the repair list MUST be inserted at the head of the list. The active segment becomes the first segment of the inserted repair list." This text applies to both MPLS and SRv6, but as I understand it, and I have asked one of the SPRING chairs, in network SRH modification is not yet agreed in 6man? The text says that it MUST be done as opposed to adding a layer of encapsulation. If this text remains, the document will stall until the matter is resolved in 6man. I think we need to seek the chairs and possibly ADs position on how hard to progress this draft in the absence of a resolution of this issue. Of course if there is an RFC that permits SRv6 header insertion, it might be useful to include a reference to stop this issue being raised again at a later stage in the publication process. [SLI] You are right, the text is simply ambiguous I think. It was written in a way to be abstracted from the dataplane and “insert” may not mean a real insertion for the SRv6 dataplane. I’ll clarify the difference and point to RFC8986 Sec 5 for the SRv6 behavior. 6) 6.2.1. Protecting {F, T, D} or {S->F, T, D} SB> I cannot see what {F, T, D} or {S->F, T, D} refers to though can glean this from later text. A more readable header would help the reader. [SLI] I’ll clarify the title with something like to highlight the difference cases : next SID is a Node SID, or next SID is an Adj-SID. 7) The 6.2.1 and 6.2.2 will be impenetrable to a lot of future readers who are not both SR and IPRFF specialist. I think to would be of benefit if an English text summary was provided to support the pseudo-math approach used in the draft. [SLI] You may be right, however it also make the text more clear for implementors. People implementing must be aware of IPFRR and SRR as they are normative references. Would an example provide the necessary help ? 8) " 2. Confirm that the next segment is in the SRGB of F, meaning that” - SRGB needs to be expanded on first use. [SLI] Agree. BTW it is missing in acronyms, will add it. 9) Point 3 on page 14 "3. Otherwise the failure is not a midpoint failure and hence the node "S" may apply other protection techniques that are beyond the scope of this document or simply drop the packet and wait for normal protocol convergence.” I do not understand what is happening here, so cannot judge whether this conflicts with the complete coverage claim made earlier. Please could you clarify. [SLI] This is the case where the next segment is neither a Node-SID or Adj-SID. For instance, in MPLS case, next label is a service label. To protect it, MPLS egress protection mechanism has to kick in which is not considered anymore as an SRTE midpoint failure. 10) Section 8 It is well known that multiple repairs can form loops. How are such loops avoided in the case of a double repair? Are you gleaning the original failure from the adj(R7-R8) SID? Also what is you AAH strategy when supporting multiple repairs? [SLI] Could you clarify “AAH” ? If I’m not mistaken, the text does not claim that we can avoid loops. As you mention, when multiple failures are happening loops can be formed. Could you clarify your overall comment ? Note that either when using Adj-SIDs in the repair list, if the repair list uses Node-SIDs, we cannot prevent double FRR. 11) Table 3B Line T3, Shouldn’t “3 SIDs” be 100% in this cumulative table? Line T8 SID should be 100.000% However a general comment on all of the tables. Three decimal places, i.e. five places of precision seems unlikely to add value. You might consider reducing the number of decimal places. That is not a required change, but some may consider this a false precision in terms extrapolating to other network environments. [SLI] Let me fix that. 12 )Security Section We can wait to see what the security directorate say, but I doubt that this will past muster. You probably need to at least refer to the security considerations in previous FRR RFCs and in SR-MPLS and SRv6 and assure the reader that no new security issues are introduced - assuming of course that is the case. [SLI] Right, will improve the text. 13) Conclusion Whilst common in the research literature conclusions after the boilerplate sections (Security and IANA) are rare in RFCs. I am not sure that the conclusion adds any value and I think that it could safely be removed. 14) [I-D.ietf-spring-srv6-network-programming] ID-NITS is reporting that this is included but not called up in the text. Best regards Stewart
_______________________________________________ rtgwg mailing list [email protected] https://www.ietf.org/mailman/listinfo/rtgwg
