Re: [mpls] draft-ietf-mpls-spring-lsp-ping: resolution of issues raised in my RTG-DIR review

[Alexander Vainshtein <[e-mail]>]

Nagendra and all,
Apologies for a long delayed response. I was not checking my email for the last week or so.
Please see responses to your comments inline below.

Regards,
Sasha

Office: [phone]
Cell:      [phone]
Email:   [e-mail]

From: Nagendra Kumar Nainar (naikumar) [mailto:[e-mail]]
Sent: Monday, October 9, 2017 12:32 PM
To: Alexander Vainshtein <[e-mail]>; BRUNGARD, DEBORAH A <[e-mail]>
Cc: [e-mail]; [e-mail]; [e-mail]; Jonathan Hardwick ([e-mail]) <[e-mail]>; [e-mail]; [e-mail]
Subject: Re: draft-ietf-mpls-spring-lsp-ping: resolution of issues raised in my RTG-DIR review

Hi Sasha,

Please see inline..

Deborah and all,

I have read the latest (-11) version of the draft in order to check to which extent my RTG-DIR review comments (that referred to the -06 version of the draft) have been addressed.

Here are the results of this check:

Major Issue #1 in the -06 version of the draft:
Section 7.4 “Segment ID Check” of the draft claims to update “the procedure defined in Step 6 of Section 4.4  of [RFC8029]”. However, I have failed to integrate the logic defined by the pseudocode in this section with the logic defined by the referenced pseudocode.
-          I have suspected that the newly introduced logic should be part of the Egress FEC Validation logic defined in Section 4.4.1 of RFC 8029.  This suspicion has been acknowledged by the authors, and they plan to fix the wrong reference in the next revision of the draft.
-          From my POV the best way to avoid possible misinterpretation by the implementers would be inclusion of the updated version of the pseudocode (with explicit marking of the new logic) directly in the document. I have suggested this to the authors, but I did not get their response so far.

Status in the -11 version of the draft:
The problematic text has been replaced with pseudocode that integrates the logic that has been defined in Section 4.4.1 of RFC 8029 with the new, SR-specific pseudocode. So, in a way, the original issue has been resolved. However, I have several issues with the current text:
-          From my POV, the pseudocode could be made much more readable if it were better structured. One possible way to do so could be defining how IGP extensions for SR can be treated as distributing if they distributed labels (including Implicit Null) for Node Segment IDs, and using the results in the original pseudocode of RFC 8029
<Nagendra> I couldn’t understand what you meant by “SR can be treated as distributing if they distributed labels”.  Can you clarify this?
[[Sasha]] Oops! I should have said “IGP extensions to SR can be treated as if they were distributing labels for IGP Prefix/Node and IGP adjacency SIDs”.

-          The proposed pseudocode seems to lack the check of Label-L matching the Node Segment ID in the SRGB of the responder
<Nagendra>The Segment ID Sub-TLV defined in draft-ietf-mpls-spring-lsp-ping does not carry the SID value.
[[Sasha]] There is no need for that, each node in the SR domain can map each IGP prefix to its SID (index) and vice versa
Incorrect SRGB range or incorrect index will result in wrong segment/label to FEC mapping and this will be detected by the defined machinery. When a node is receiving LSP Ping packet with incoming label as L which is not within the SRGB, I can think of 2 possible scenarios:
                Scenario 1 – The incoming label is not in ILM. In this case, the node will reply with a return code of 11 (No label entry in ILM)
                Scenario 2 – The incoming label is not mapped to the right FEC. In this case, the FEC mapping will not be validated.

The same is applicable for label in DDMAP.
[[Sasha]] The pseudocode in Step 4 says:
      4.  If the label mapping for FEC is Implicit Null, set FEC-status to
           2 and proceed to step 4a.  Otherwise, if the label mapping for FEC
           is Label-L, proceed to step 4a.
But I do not see an explicit definition of the label mapping rules for the new sub-TLVs anywhere in the draft. Did I miss something?
Please note also that one of the sub-steps in (4a) mentions checking that the Segment ID is advertised with PHP – but should not this check be part of checking whether the label mapping for FEC is Implicit NULL?

-          It is not obvious how the proposed pseudocode treats previously defined FECs in the target FEC stack

<Nagendra> By defining the procedure in this draft as Step 4a, we execute Step 4a if FEC==SR and skip step 5. When FEC!=SR, we execute step 5. I think we discussed and clarified this already. Are you pointing to a different issue?
[[Sasha]] The modified pseudocode in Step 4 does not differentiate between “old” and SR-related FECs, it sends all FECs that match certain conditions to 4a. It is by only after checking all the conditions in Step 4a (which fail for “old” FECs”) that execution will reach Step 5. This is what I call “not obvious”.

-          Interface-I (the ingress interface of the received LSP Ping request) is only mentioned in the check of Adjacency SID. It is also not clear to me why the check expects its match with the Remote Interface ID in the Adjacency SID TLV.

<Nagendra> Adj-SID requires strict incoming interface check and so is the reason we are checking Interface-I for Adj-SID. Remote Interface ID in TLV is the interface identifier assigned by the downstream node for which the Adj-SID is assigned by upstream node. In example R1-R2, assume R1 assigns Ad-SID of A1 for R2. The “Remote Interface ID” field of Adj-SID FEC Sub-TLV will carry the interface ID assigned by R2 for interface connected to R1.
[[Sasha]] Does the node that has advertised an Adjacency-SID (and a label that was assigned to it) perform any specific checks? Or are these checks covered by the generic “label mapping for FEC is Label-L” check in Step 4?
In addition, I wonder if reception of an LSP Ping request packet that carries SR-Related target FEC stack entries from an interface on which no IGP is enabled would be detected by the machinery defined in the draft? For “old” FECs such a check would be done at Step 5 of the validation algorithm, but this step is bypassed for SR-related FECs.


I did not raise these issues in my original review exactly because it was impossible to integrate the new validation logic with the logic defined in RFC 8029. Please consider these issues as my IESG LC comments.

Minor Issue #3 in the -06 version of the draft:
I am not sure if “OSPF” and ISIS” (without any reference to Segment Routing) are the proper names for the label distribution protocols in the proposed new IANA registry in Section 10.2. From my POV “OSPF/ISIS SR Extensions” would be more appropriate. The authors disagree with this proposal claiming that OSPF and ISIS are not used for label distribution in any other way. (RFC 8029 that encodes the label distribution protocol in the Downstream  Detailed Mapping TLV did not request a dedicated registry for this purpose, and I think that the authors have been right to request such a registry).
Status in the -11 version of the draft:
The new version uses the same protocol names as the previous one. Meanwhile, I have found that OSPF and IS-IS can be used for distriburing label ranges for BIER (see OSPF Extensions for BIER<https://datatracker.ietf.org/doc/draft-ietf-bier-ospf-bier-extensions/?include_text=1> and a similar draft for ISIS). From my POV, this supports my recommendation to change the names of protocols to differentiate between multiple extensions to OSPF and IS-IS that distribute label ranges. Please consider this as my IESG LC WG comment.
<Nagendra>I will leave it to other authors and WG. To me, It still appears that OSPF/ISIS will be appropriate and re-use same protocol number instead of defining another protocol number for OSPF with “x“extension ☺. If the consensus is to change it as OSPF with SR extension, I will update the draft accordingly.
[[Sasha]] We can agree to disagree on this point.

Minor Issue #4 in the -06 version of the draft:
I doubt the draft needs address any FRR issues even in future (as mentioned in the last statement in Section 5), since, to the best of my understanding, any form of FRR:
-          Is operated locally by the node that is upstream to the failure without passing any information to the initiator of LSP-Ping Request packets
-          In any case is just a transient state in the (presumably short) interval between the failure being detected by the upstream node and re-convergence of IGP
I also think that references to “future versions” are not quite appropriate in the document that is going to the IETF LC. I recommend removing any such references from the document.
Status in the -11 version of the draft:
All mention of FRR has been removed from the new version, as well as any references to future versions.

Minor Issue #5 in the -06 version of the draft:
Section 9 “Backward Compatibility with non-Segment Routing devices” defines the behavior for the two slightly different use cases:
-              LSP Ping Echo Request packets are initiated by an SR-capable node (and therefore their target FEC stack contains SR-related FECs), while the responder is legacy device that is not SR-capable. In this case the proposed solution (respond with the FEC-return-code “Replying router has no mapping for the FEC at stack-depth”) looks as the only possibility since the legacy device cannot parse SR FECs in any case
-              LSP Ping Echo Request packets are initiated by a legacy device while the responder is SR-capable. The draft defines the same behavior in this case – but, IMHO and FWIW, the responder could provide slightly better diagnostics since it can parse the “old” target FEC and recognize the equivalent Node ID. An additional return code would be required to implement this behavior.
This issue has not been discussed with the authors due to lack of time.
Status in the -11 version of the draft:
The authors have responded that a legacy (LDP-only) initiator of LSP Pings request messages in any case would not be able to parse a new return code. I agree with this comment, and withdraw the issue.

All the nits I’ve noticed also have been resolved. In particular, early IANA allocations that have expired on 15.09.17, have been extended to one more year.

Thanks,
Nagendra

Regards,
Sasha

Office: [phone]
Cell:      [phone]
Email:   [e-mail]<mailto:[e-mail]>


___________________________________________________________________________

This e-mail message is intended for the recipient only and contains information which is
CONFIDENTIAL and which may be proprietary to ECI Telecom. If you have received this
transmission in error, please inform us by e-mail, phone or fax, and then delete the original
and all copies thereof.
___________________________________________________________________________



___________________________________________________________________________

This e-mail message is intended for the recipient only and contains information which is 
CONFIDENTIAL and which may be proprietary to ECI Telecom. If you have received this 
transmission in error, please inform us by e-mail, phone or fax, and then delete the original 
and all copies thereof.
___________________________________________________________________________