Subject: [Teas] AD review of draft-ietf-teas-gmpls-signaling-smp-08

From: John Scudder <[email-masked]>
Date: Thu, 6 Jan 2022 21:50:00 +0000

Dear Authors,

I generally found this to be a very readable spec, thanks!

I’ve supplied my comments in the form of an edited copy of the draft. You can use your favorite diff tool to review them; I’ve attached a PDF of the rfcdiff output for your convenience if you’d like to use it. I’ve also pasted a traditional diff below in case you want to use it for in-line reply. I’d appreciate feedback regarding whether you found this a useful way to receive my comments as compared to a more traditional numbered list of comments with selective quotation from the draft.

In addition to the comments I’ve interspersed in the text, I do have one thornier issue I’d like to elevate. In §6.1 you define “shared mesh protection” as a value for the “LSP Flags” field. However, as defined in RFC 4872 §14.1, that is a field of six orthogonal bit flags as the name suggests, not a six-bit integer. The definitional text in 4872 confirms that this is what the WG intended, the text says “only one value SHOULD be set at a time”. You can’t set an integer to more than one value at a time; Q.E.D. the WG intended that the field is six orthogonal bit flags. (Or semi-orthogonal anyway, because 4872 goes on to say “only one value SHOULD be set at a time”.)

You’ve defined “shared mesh protection” as having value 0x11, in other words it’s 0x01|0x10 — the “(Full) Rerouting” and “1+1 Bidirectional Protection” bits are both set. This seems, on the face of it, like it must be wrong.

Now, I personally find it strange that 4872 wants to call this field a set of bit flags instead of an integer, given that it also says the values are mutually exclusive. I imagine the reason for it being specced this way is lost in the mists of time; I can think of several reasons it might have been done this way, some good, some not as good. But speculation is futile: the fact is that it says what it says.

I can think of a few ways forward:

1. If there’s already a standards-track document that updates 4872 by redefining the “LSP Flags” field as a six-bit integer, hooray. Please let me know the citation. (I did look for such, I don’t think it exists, sadly.)

2. You could make such an update yourself, presumably as a subsection of your specification although it could be a small standalone document. I think there would be two concerns arising from such a proposal. First, are there implementations in the field that faithfully implement RFC 4872 as written, and test bit values when determining the protection type? What would be the expected behavior of such an implementation when presented with two or more set flags? Even if you don’t know of any such implementations, can you prove there are none, i.e. can you prove that the redefinition would be safe? Second, since RFC 4872 makes the “one value at a time” restriction a SHOULD, not a MUST, the authors presumably had in mind a use case for legitimately setting two of the flags. By redefining the field as an integer, you would usurp such use cases. (It’s also possible the SHOULD is just sloppiness and there is no such use case, but that would have to be proven — presumptively, there is a legitimate use case even though I don’t know what it is.)

3. You could continue the practice of treating the field as bit flags. This raises the obvious problem that the flags are used up already (which makes me wonder what the authors of 4872 meant by “all other values are reserved”, since there aren’t any other values). But, you could get another bit from the adjacent five-bit Reserved field. So, instead of 0x11, “shared mesh protection” would be 0x20. One potential problem with this approach is that it looks to me as though, if a node in the path doesn’t implement SMP, if it receives 0x20 it would believe it’s received a PROTECTION object with the bottom Reserved bit set (and would ignore it, per RFC 4872), and with a Protection Type field of 0x00, Unprotected. So it would happily treat an SMP PROTECTION object as though it were an “Unprotected” PROTECTION object.

Let’s have a discussion about how to proceed on this point.

Regards,

—John

--- draft-ietf-teas-gmpls-signaling-smp-08.txt  2022-01-06 14:26:46.000000000 -0500
+++ draft-ietf-teas-gmpls-signaling-smp-08-jgs-markup.txt       2022-01-06 16:36:58.000000000 -0500
@@ -155,6 +155,23 @@
    support the control of the SMP mechanism.  Only the generic aspects
    for signaling SMP are addressed by this document.  The technology-
    specific aspects are expected to be addressed by other documents.
+
+Thanks for briefly mentioning the updates in your abstract and
+introduction. Note that you are also supposed to have a "Summary
+of Changes" section that provides greater detail
+(see https://www.ietf.org/standards/ids/guidelines/). An appropriate
+level of detail would be something like,
+
+       This document updates RFC 4872 to blah blah blah. Specifically,
+       it:
+
+       - defines a new type, "Shared Mesh Protection" (SMP), for the LSP
+       Flags field of of the PROTECTION object [Section XX]
+       - updates the definition of the N field of the PROTECTION object
+       to take the new SMP type into account [Section YY]
+       - updates the definition of the O field of the PROTECTION object
+       to take the new SMP type into account [Section ZZ]
+       - and so on as needed

 2.  Conventions Used in This Document

@@ -306,6 +323,10 @@
    normal traffic and the secondary LSP.

    A new LSP Protection Type "Shared Mesh Protection" is introduced to
+
+Please insert a forward reference to §6.1 where the introduction is
+actually done, and importantly, where the type is defined.
+
    the LSP Flags of PROTECTION object (see [RFC4872]) to set up the two
    LSPs.  This LSP Protection Type value is applicable only to
    bidirectional LSPs as required in [G808.3].
@@ -346,9 +367,21 @@
    LSP SHOULD include at least one PRIMARY_PATH_ROUTE object (see
    [RFC4872]) that further allows for recovery resource sharing at each
    intermediate node along the secondary path.
+
+What's the consequence if the SHOULD isn't honored? For what legitimate
+reasons can the SHOULD be ignored? Please consider either discussing this
+in the document, or converting the SHOULD to a MUST.
+
+This is discussed further in the Guidelines for Authors document I
+provided a link for above. It says, "SHOULD is especially problematic:
+it needs to be clear why SHOULD is used rather than MUST, and what the
+implications are of varying from the recommendation."

    With this setting, the resources for the secondary LSP SHOULD be pre-
    reserved, but not committed at the data plane level, meaning that the
+
+Same comment re SHOULD.
+
    internals of the switch need not be established until explicit action
    is taken to activate this LSP.  Activation of a secondary LSP and
    protection switching to the activated protecting LSP is done using
@@ -356,13 +389,16 @@

    After protection switching completes the protecting LSP SHOULD be
    signaled with the S bit set to 0 and O bit set to 1 in the PROTECTION
+
+Same comment re SHOULD.
+
    object.  At this point, the link and node resources MUST be allocated
    for this LSP that becomes a primary LSP (ready to carry normal
    traffic).  The formerly working LSP MAY be signaled with the A bit
    set in the ADMIN_STATUS object (see [RFC3473]).

    Support for extra traffic in SMP is for further study.  Therefore,
-   mechanisms to setup LSPs for extra traffic are also for further
+   mechanisms to set up LSPs for extra traffic are also for further
    study.

 5.4.  SMP Preemption Priority
@@ -395,8 +431,14 @@

    In SMP, a preempted LSP SHOULD NOT be torn down.  Once the working
+
+Same comment re SHOULD.
+
    LSP and the protecting LSP are configured or pre-configured, the end
    node SHOULD keep refreshing both working and protecting LSPs
+
+Same comment re SHOULD.
+
    regardless of failure or preempted situation.

 5.5.  Notifying Availability of Shared Resources
@@ -427,6 +469,25 @@
    nodes of the protecting LSPs that have lower SMP preemption
    priorities than the one that does not need the shared resources any
    more.
+
+Is there any need to say what the receiver is expected to do when it
+gets a "Shared resources available" message? I mean, it's not hard to
+guess -- if it requires protection, it's allowed to try again. But it
+might be worth being more explicit?
+
+Since you mention lower preemption priorities, is there any benefit to
+sending out "available" messages staggered, in priority order? Suppose
+a high priority LSP was occupying the resources, and a medium and low
+priority one have been preempted and suppose both of them also wanted
+access to the resources. When the high priority one clears its use, if
+both the medium and low priorities are notified at roughly the same time,
+odds are even that the low priority one could claim the resources first,
+then have to unwind when preempted by the medium priority. This seems
+suboptimal, and could be avoided by notifying the medium priority client
+first. The counter-point of course, is that perhaps the medium one didn't
+want to claim the resources, in which case the low-priority one would
+have a longer-than-necessary wait to claim them, during which the
+resources would sit idle.

 5.6.  SMP APS Configuration

@@ -436,7 +497,7 @@
    In order to allow exchange of APS protocol messages, an APS channel
    has to be configured between adjacent nodes along the path of the SMP
    protecting LSP.  This should be done before any SMP protecting LSP
-   has been setup by other means than GMPL signaling which are therefore
+   has been set up by other means than GMPLS signaling which are therefore
    outside the scope of this document.

    Depending on the APS protocol message format, the APS protocol may
@@ -469,15 +530,36 @@
    LSP (Protection Type) Flags:

       0x11: Shared Mesh Protection
+
+See my earlier comments.

    According to the rules defined in Section 14.2 of [RFC4872], all the
    nodes along an SMP LSP MUST be SMP aware and therefore there are no
    backward compatibility issues.
+
+The MUST above seems misplaced. I think what you're doing is summarizing
+your understanding of the consequences of the procedures given in 4872
+§14.2, i.e. the "MUST" is expressing an expectation (that flows from the
+separate MUST in RFC 4872). Since you aren't, here, introducing a new
+requirement, I think you should remove this MUST and replace it with a
+word that just expresses expectation, such as "will".
+
+As a separate point, though, as far as I can tell your statement is wrong!
+As discussed previously, isn't it true that a given node along the LSP
+could be non-SMP aware, and could be parsing the PROTECTION object as
+having (supported) LSP flags 0x01|0x10?

 6.2.  Updates on Notification and Operational Bits

    N bit and O bit in the PROTECTION object as defined in [RFC4872] are
    also updated to include applicability to SMP.
+
+Different people follow different styles when updating previous RFCs.
+Here, you're providing replacement text. Please indicate this explicitly
+somehow. You could say something like "the definitions of the N and O
+bits in RFC 4872 Section 14.1 are replaced as follows", for example.
+You could also use "OLD/NEW" style, although I don't think that's
+required.

    Notification (N): 1 bit

@@ -487,10 +569,15 @@
       plane message exchanges are used for protection-switching
       purposes.  The N bit is only applicable when the LSP Protection
       Type Flag is set to either 0x04 (1:N Protection with Extra-
-      Traffic), or 0x08 (1+1 Unidirectional Protection), or 0x10 (1+1
-      Bidirectional Protection).  In SMP, N bit MUST be set to 1.  The N
+      Traffic), 0x08 (1+1 Unidirectional Protection), 0x10 (1+1
+      Bidirectional Protection), or 0x11 (Shared Mesh Protection).
+      If 0x11 (SMP), the N bit MUST be set to 1.  The N
       bit MUST be set to 0 in any other case.

+I suggested a bit of a rewrite for clarity above. However, this may have
+to change more depending on the outcome of our discussion of the 0x11
+value.
+
    Operational (O): 1 bit

       When set to 1, this bit indicates that the protecting LSP is
@@ -519,6 +606,23 @@
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
       |                   Reserved                    | Preempt Prio  |
       +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+
+There's a problem with the above, which is that RFC 4872 isn't the
+relevant authority for this field. The complete text from 4872 that
+defines this field is:
+
+       Encoding of this field is detailed in [RFC4873].
+
+So I found the definition in RFC 4873. Fortunately, it still leaves
+available the bits you want. Here’s how it’s defined in §6.1 of 4873:
+
+      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+      |I|R|   Reserved    | Seg.Flags |           Reserved            |
+      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+
+So, I think it’s still OK for you to take your bits, but you should
+refer to the correct RFC when doing so, and show the correct diagram.
+And you should add RFC 4873 to your Updates list.

    Preemption Priority (Preempt Prio): 8 bit

@@ -571,6 +675,16 @@
    important not only to use security mechanisms as discussed in
    [RFC4872] but also to preserve privacy of information about
    protecting LSPs within the network.
+
+Do I correctly understand that you're proposing that "don't tell the bad
+guys about your protecting LSP layout" will "prevent" the attack you
+rightly identify? I don't think that will stand up to review, security-
+through-obscurity is a notoriously weak "protection".
+
+If you don't have a better mitigation to offer, that's OK, I just would
+be careful not to overclaim about the efficacy of keeping your LSP layout
+secret. Perhaps something like "For this reason, it is important not only
+to use..."

 9.  Acknowledgements

---

From: "Adrian Farrel" <[email-masked]>
Date: Sun, 9 Jan 2022 17:34:27 -0000

Hi John,

This is stretching my memory, but as far as I can recall, the thinking in 4872 was that at some time in the future it might become meaningful to set move than one bit in the LSP Flags field at the same time.

I believe that this is exactly what the authors have done here. That is, they have deliberately set two bits that have independent meaning, with the result of a special case of both bits meaning “full rerouting on a bidirectional protected LSP”. 

What the authors have done is possibly a stretch except for the sentence in 6.1 about backward compatibility. Given that it is an explicitly routed TE LSP, all routers on the path will have been selected by the path computation algorithm to be “SMP aware” and so the assertion is right and your concern that a non-SMP-aware node will see this bit setting is not a problem.

However,

*	The backward compatibility paragraph could be clearer
*	I might suggest a fourth way forward which is simply to take another bit from one of the adjacent reserved fields (so that was a six bit field becomes a seven bit field and SMP = 0x20). Should a legacy node see one of these messages, it will parse as “unprotected” (just the six bits set to 0x00) and not do anything special. 

Ciao,

Adrian

From: Teas <[email-masked]> On Behalf Of John Scudder
Sent: 06 January 2022 21:50
To: [email-masked]
Cc: [email-masked]
Subject: [Teas] AD review of draft-ietf-teas-gmpls-signaling-smp-08

Dear Authors,

Regards,

—John 

 
@@ -306,6 +323,10 @@

 

 

@@ -436,7 +497,7 @@

 

 
    Notification (N): 1 bit
 
@@ -487,10 +569,15 @@

 
 
@@ -571,6 +675,16 @@
 
 9.  Acknowledgements

---

From: Jeong-dong Ryoo <[email-masked]>
Date: Mon, 10 Jan 2022 11:46:33 +0900 (KST)

Hi, all.

Thanks to John for his thorough review and those important comments. The PDF of the rfcdiff seems a bit more efficient for revision work, but I don't really have a preference. I think it would be good to give it in a more convenient form for your review work. And, thanks also to Adrian for the response email.
&nbsp;
I cannot tell why it was set at that value as it was there before I joined as a co-author. However, I admit that I overlooked the description of "LSP Flags" in RFC 4872. Anyway, in my opinion, we can just change the value to 0x20 without extending the LSP Flags field. The length of the LSP Flags field is currently defined as 6 bits, and 0x20 just fits into the 6-bit field.

All other comments from John should be reflected in the next revision. Accordingly, I will prepare a revision.&nbsp;

Best regards,

Jeong-dong

-----Original Message-----
From: "Adrian Farrel" &lt;[email-masked]&gt;
To: "'John Scudder'" &lt;jgs=[email-masked]&gt;; &lt;[email-masked]&gt;;
Cc: &lt;[email-masked]&gt;;
Sent: 2022-01-10 (월) 02:34:54 (UTC+09:00)
Subject: Re: [Teas] AD review of draft-ietf-teas-gmpls-signaling-smp-08

Hi John,
&nbsp;
This is stretching my memory, but as far as I can recall, the thinking in 4872 was that at some time in the future it might become meaningful to set move than one bit in the LSP Flags field at the same time.
&nbsp;
I believe that this is exactly what the authors have done here. That is, they have deliberately set two bits that have independent meaning, with the result of a special case of both bits meaning “full rerouting on a bidirectional protected LSP”.&nbsp;
&nbsp;
What the authors have done is possibly a stretch except for the sentence in 6.1 about backward compatibility. Given that it is an explicitly routed TE LSP, all routers on the path will have been selected by the path computation algorithm to be “SMP aware” and so the assertion is right and your concern that a non-SMP-aware node will see this bit setting is not a problem.
&nbsp;
However,
The backward compatibility paragraph could be clearerI might suggest a fourth way forward which is simply to take another bit from one of the adjacent reserved fields (so that was a six bit field becomes a seven bit field and SMP = 0x20). Should a legacy node see one of these messages, it will parse as “unprotected” (just the six bits set to 0x00) and not do anything special.&nbsp;&nbsp;
Ciao,
Adrian
&nbsp;
From:&nbsp;Teas &lt;[email-masked]&gt; On Behalf Of&nbsp;John ScudderSent: 06 January 2022 21:50To: [email-masked]: [email-masked]: [Teas] AD review of draft-ietf-teas-gmpls-signaling-smp-08

&nbsp;
Dear Authors,I generally found this to be a very readable spec, thanks!I’ve supplied my comments in the form of an edited copy of the draft. You can use your favorite diff tool to review them; I’ve attached a PDF of the rfcdiff output for your convenience if you’d like to use it. I’ve also pasted a traditional diff below in case you want to use it for in-line reply. I’d appreciate feedback regarding whether you found this a useful way to receive my comments as compared to a more traditional numbered list of comments with selective quotation from the draft.In addition to the comments I’ve interspersed in the text, I do have one thornier issue I’d like to elevate. In §6.1 you define “shared mesh protection” as a value for the “LSP Flags” field. However, as defined in RFC 4872 §14.1, that is a field of six orthogonal bit flags as the name suggests, not a six-bit integer. The definitional text in 4872 confirms that this is what the WG intended, the text says “only one value SHOULD be set at a time”. You can’t set an integer to more than one value at a time; Q.E.D. the WG intended that the field is six orthogonal bit flags. (Or semi-orthogonal anyway, because 4872 goes on to say “only one value SHOULD be set at a time”.)You’ve defined “shared mesh protection” as having value 0x11, in other words it’s 0x01|0x10 — the “(Full) Rerouting” and “1+1 Bidirectional Protection” bits are both set. This seems, on the face of it, like it must be wrong.Now, I personally find it strange that 4872 wants to call this field a set of bit flags instead of an integer, given that it also says the values are mutually exclusive. I imagine the reason for it being specced this way is lost in the mists of time; I can think of several reasons it might have been done this way, some good, some not as good. But speculation is futile: the fact is that it says what it says.I can think of a few ways forward:1. If there’s already a standards-track document that updates 4872 by redefining the “LSP Flags” field as a six-bit integer, hooray. Please let me know the citation. (I did look for such, I don’t think it exists, sadly.)2. You could make such an update yourself, presumably as a subsection of your specification although it could be a small standalone document. I think there would be two concerns arising from such a proposal. First, are there implementations in the field that faithfully implement RFC 4872 as written, and test bit values when determining the protection type? What would be the expected behavior of such an implementation when presented with two or more set flags? Even if you don’t know of any such implementations, can you prove there are none, i.e. can you prove that the redefinition would be safe? Second, since RFC 4872 makes the “one value at a time” restriction a SHOULD, not a MUST, the authors presumably had in mind a use case for legitimately setting two of the flags. By redefining the field as an integer, you would usurp such use cases. (It’s also possible the SHOULD is just sloppiness and there is no such use case, but that would have to be proven — presumptively, there is a legitimate use case even though I don’t know what it is.)3. You could continue the practice of treating the field as bit flags. This raises the obvious problem that the flags are used up already (which makes me wonder what the authors of 4872 meant by “all other values are reserved”, since there aren’t any other values). But, you could get another bit from the adjacent five-bit Reserved field. So, instead of 0x11, “shared mesh protection” would be 0x20. One potential problem with this approach is that it looks to me as though, if a node in the path doesn’t implement SMP, if it receives 0x20 it would believe it’s received a PROTECTION object with the bottom Reserved bit set (and would ignore it, per RFC 4872), and with a Protection Type field of 0x00, Unprotected. So it would happily treat an SMP PROTECTION object as though it were an “Unprotected” PROTECTION object.Let’s have a discussion about how to proceed on this point.Regards,—John--- draft-ietf-teas-gmpls-signaling-smp-08.txt 2022-01-06 14:26:46.000000000 -0500+++ draft-ietf-teas-gmpls-signaling-smp-08-jgs-markup.txt 2022-01-06 16:36:58.000000000 -0500@@ -155,6 +155,23 @@support the control of the SMP mechanism. Only the generic aspectsfor signaling SMP are addressed by this document. The technology-specific aspects are expected to be addressed by other documents.++Thanks for briefly mentioning the updates in your abstract and+introduction. Note that you are also supposed to have a "Summary+of Changes" section that provides greater detail+(see https://www.ietf.org/standards/ids/guidelines/) https://www.ietf.org/standards/ids/guidelines/). An appropriate+level of detail would be something like,++ This document updates RFC 4872 to blah blah blah. Specifically,+ it:++ - defines a new type, "Shared Mesh Protection" (SMP), for the LSP+ Flags field of of the PROTECTION object [Section XX]+ - updates the definition of the N field of the PROTECTION object+ to take the new SMP type into account [Section YY]+ - updates the definition of the O field of the PROTECTION object+ to take the new SMP type into account [Section ZZ]+ - and so on as needed2. Conventions Used in This Document@@ -306,6 +323,10 @@normal traffic and the secondary LSP.A new LSP Protection Type "Shared Mesh Protection" is introduced to++Please insert a forward reference to §6.1 where the introduction is+actually done, and importantly, where the type is defined.+the LSP Flags of PROTECTION object (see [RFC4872]) to set up the twoLSPs. This LSP Protection Type value is applicable only tobidirectional LSPs as required in [G808.3].@@ -346,9 +367,21 @@LSP SHOULD include at least one PRIMARY_PATH_ROUTE object (see[RFC4872]) that further allows for recovery resource sharing at eachintermediate node along the secondary path.++What's the consequence if the SHOULD isn't honored? For what legitimate+reasons can the SHOULD be ignored? Please consider either discussing this+in the document, or converting the SHOULD to a MUST.++This is discussed further in the Guidelines for Authors document I+provided a link for above. It says, "SHOULD is especially problematic:+it needs to be clear why SHOULD is used rather than MUST, and what the+implications are of varying from the recommendation."With this setting, the resources for the secondary LSP SHOULD be pre-reserved, but not committed at the data plane level, meaning that the++Same comment re SHOULD.+internals of the switch need not be established until explicit actionis taken to activate this LSP. Activation of a secondary LSP andprotection switching to the activated protecting LSP is done using@@ -356,13 +389,16 @@After protection switching completes the protecting LSP SHOULD besignaled with the S bit set to 0 and O bit set to 1 in the PROTECTION++Same comment re SHOULD.+object. At this point, the link and node resources MUST be allocatedfor this LSP that becomes a primary LSP (ready to carry normaltraffic). The formerly working LSP MAY be signaled with the A bitset in the ADMIN_STATUS object (see [RFC3473]).Support for extra traffic in SMP is for further study. Therefore,- mechanisms to setup LSPs for extra traffic are also for further+ mechanisms to set up LSPs for extra traffic are also for furtherstudy.5.4. SMP Preemption Priority@@ -395,8 +431,14 @@In SMP, a preempted LSP SHOULD NOT be torn down. Once the working++Same comment re SHOULD.+LSP and the protecting LSP are configured or pre-configured, the endnode SHOULD keep refreshing both working and protecting LSPs++Same comment re SHOULD.+regardless of failure or preempted situation.5.5. Notifying Availability of Shared Resources@@ -427,6 +469,25 @@nodes of the protecting LSPs that have lower SMP preemptionpriorities than the one that does not need the shared resources anymore.++Is there any need to say what the receiver is expected to do when it+gets a "Shared resources available" message? I mean, it's not hard to+guess -- if it requires protection, it's allowed to try again. But it+might be worth being more explicit?++Since you mention lower preemption priorities, is there any benefit to+sending out "available" messages staggered, in priority order? Suppose+a high priority LSP was occupying the resources, and a medium and low+priority one have been preempted and suppose both of them also wanted+access to the resources. When the high priority one clears its use, if+both the medium and low priorities are notified at roughly the same time,+odds are even that the low priority one could claim the resources first,+then have to unwind when preempted by the medium priority. This seems+suboptimal, and could be avoided by notifying the medium priority client+first. The counter-point of course, is that perhaps the medium one didn't+want to claim the resources, in which case the low-priority one would+have a longer-than-necessary wait to claim them, during which the+resources would sit idle.5.6. SMP APS Configuration@@ -436,7 +497,7 @@In order to allow exchange of APS protocol messages, an APS channelhas to be configured between adjacent nodes along the path of the SMPprotecting LSP. This should be done before any SMP protecting LSP- has been setup by other means than GMPL signaling which are therefore+ has been set up by other means than GMPLS signaling which are thereforeoutside the scope of this document.Depending on the APS protocol message format, the APS protocol may@@ -469,15 +530,36 @@LSP (Protection Type) Flags:0x11: Shared Mesh Protection++See my earlier comments.According to the rules defined in Section 14.2 of [RFC4872], all thenodes along an SMP LSP MUST be SMP aware and therefore there are nobackward compatibility issues.++The MUST above seems misplaced. I think what you're doing is summarizing+your understanding of the consequences of the procedures given in 4872+§14.2, i.e. the "MUST" is expressing an expectation (that flows from the+separate MUST in RFC 4872). Since you aren't, here, introducing a new+requirement, I think you should remove this MUST and replace it with a+word that just expresses expectation, such as "will".++As a separate point, though, as far as I can tell your statement is wrong!+As discussed previously, isn't it true that a given node along the LSP+could be non-SMP aware, and could be parsing the PROTECTION object as+having (supported) LSP flags 0x01|0x10?6.2. Updates on Notification and Operational BitsN bit and O bit in the PROTECTION object as defined in [RFC4872] arealso updated to include applicability to SMP.++Different people follow different styles when updating previous RFCs.+Here, you're providing replacement text. Please indicate this explicitly+somehow. You could say something like "the definitions of the N and O+bits in RFC 4872 Section 14.1 are replaced as follows", for example.+You could also use "OLD/NEW" style, although I don't think that's+required.Notification (N): 1 bit@@ -487,10 +569,15 @@plane message exchanges are used for protection-switchingpurposes. The N bit is only applicable when the LSP ProtectionType Flag is set to either 0x04 (1:N Protection with Extra-- Traffic), or 0x08 (1+1 Unidirectional Protection), or 0x10 (1+1- Bidirectional Protection). In SMP, N bit MUST be set to 1. The N+ Traffic), 0x08 (1+1 Unidirectional Protection), 0x10 (1+1+ Bidirectional Protection), or 0x11 (Shared Mesh Protection).+ If 0x11 (SMP), the N bit MUST be set to 1. The Nbit MUST be set to 0 in any other case.+I suggested a bit of a rewrite for clarity above. However, this may have+to change more depending on the outcome of our discussion of the 0x11+value.+Operational (O): 1 bitWhen set to 1, this bit indicates that the protecting LSP is@@ -519,6 +606,23 @@+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+| Reserved | Preempt Prio |+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+++There's a problem with the above, which is that RFC 4872 isn't the+relevant authority for this field. The complete text from 4872 that+defines this field is:++ Encoding of this field is detailed in [RFC4873].++So I found the definition in RFC 4873. Fortunately, it still leaves+available the bits you want. Here’s how it’s defined in §6.1 of 4873:++ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-++ |I|R| Reserved | Seg.Flags | Reserved |+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+++So, I think it’s still OK for you to take your bits, but you should+refer to the correct RFC when doing so, and show the correct diagram.+And you should add RFC 4873 to your Updates list.Preemption Priority (Preempt Prio): 8 bit@@ -571,6 +675,16 @@important not only to use security mechanisms as discussed in[RFC4872] but also to preserve privacy of information aboutprotecting LSPs within the network.++Do I correctly understand that you're proposing that "don't tell the bad+guys about your protecting LSP layout" will "prevent" the attack you+rightly identify? I don't think that will stand up to review, security-+through-obscurity is a notoriously weak "protection".++If you don't have a better mitigation to offer, that's OK, I just would+be careful not to overclaim about the efficacy of keeping your LSP layout+secret. Perhaps something like "For this reason, it is important not only+to use..."9. Acknowledgements&nbsp;

&nbsp;

_______________________________________________
Teas mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/teas

---

From: John Scudder <[email-masked]>
Date: Mon, 10 Jan 2022 22:03:21 +0000

> On Jan 9, 2022, at 9:46 PM, Jeong-dong Ryoo <[email-masked]> wrote:
> 

Oh right, silly me, I had an off-by-one in my binary arithmetic. As you say, you can simply use 0x20 and still fit within the field. A happy ending! I will look forward to reviewing your next version.

Thanks,

—John

---

From: Jeong-dong Ryoo <[email-masked]>
Date: Tue, 11 Jan 2022 16:01:26 +0900 (KST)

John,

Yes, we were lucky this time. Since there are already 6 values defined in the 6-bit field, we can be easily deluded. :-) Anyway, I will be working with other co-authors for a revision.

Best regards,

Jeong-dong

-----Original Message-----
From:  "John Scudder" <[email-masked]>
To:     "Jeong-dong Ryoo" <[email-masked]>; 
Cc:     "[email-masked]" <[email-masked]>;  "[email-masked]" <[email-masked]>;  "[email-masked]" <[email-masked]>; 
Sent:  2022-01-11 (화) 07:03:42 (UTC+09:00)
Subject: Re: [Teas] AD review of draft-ietf-teas-gmpls-signaling-smp-08

Thanks,

—John

---

From: Jeong-dong Ryoo <[email-masked]>
Date: Fri, 14 Jan 2022 12:17:05 +0900 (KST)

Hi, John, et all.

We, co-authors of draft-ietf-teas-gmpls-signaling-smp, uploaded a new version, which is available at:
https://datatracker.ietf.org/doc/html/draft-ietf-teas-gmpls-signaling-smp-09

A diff from the previous version is available at:&nbsp;
https://www.ietf.org/rfcdiff?url2=draft-ietf-teas-gmpls-signaling-smp-09&nbsp;

We think that all the issues have been resolved in this version. If you find any additional comments or suggestions, please let us know. &nbsp;We will be happy to work to improve the quality of the document.&nbsp;

Best regards,

Jeong-dong (on behalf of co-auhors)

-----Original Message-----
From: "Jeong-dong Ryoo" &lt;[email-masked]&gt;
To: "'John Scudder'" &lt;jgs=[email-masked]&gt;; &lt;[email-masked]&gt;; &lt;[email-masked]&gt;;
Cc: &lt;[email-masked]&gt;;
Sent: 2022-01-10 (월) 11:46:33 (UTC+09:00)
Subject: RE: Re: [Teas] AD review of draft-ietf-teas-gmpls-signaling-smp-08

Hi, all.

Best regards,

Jeong-dong

-----Original Message-----
From: "Adrian Farrel" &lt;[email-masked]&gt;
To: "'John Scudder'" &lt;jgs=[email-masked]&gt;; &lt;[email-masked]&gt;;
Cc: &lt;[email-masked]&gt;;
Sent: 2022-01-10 (월) 02:34:54 (UTC+09:00)
Subject: Re: [Teas] AD review of draft-ietf-teas-gmpls-signaling-smp-08

Hi John,
&nbsp;

&nbsp;

&nbsp;

---

From: John Scudder <[email-masked]>
Date: Fri, 14 Jan 2022 15:48:59 +0000

Thanks! This looks good. I did notice two nits while reviewing your updates. First, §5.1 now says

   A new LSP Protection Type "Shared Mesh Protection" is defined (see
   Section 6.1) to the LSP Flags of PROTECTION object (see [RFC4872]) to
   set up the two LSPs.

With this change the sentence isn’t quite grammatical (“type… is defined to”). Probably the “to” should be “for”, the type is defined *for* the flags, right?

Second, §5.6 still says “GMPL” where I assume you mean “GMPLS”.

If you feel like pushing a new revision before I send it for IETF Last Call that’s fine, but you can also leave it for later, up to you. I’ll hold off a bit on pressing the button to initiate IETF Last Call so you can tell me your preference.

—John

On Jan 13, 2022, at 10:17 PM, Jeong-dong Ryoo <[email-masked]<mailto:[email-masked]>> wrote:

Hi, John, et all.

A diff from the previous version is available at:
https://www.ietf.org/rfcdiff?url2=draft-ietf-teas-gmpls-signaling-smp-09

We think that all the issues have been resolved in this version. If you find any additional comments or suggestions, please let us know.  We will be happy to work to improve the quality of the document.

Best regards,

-----Original Message-----
From: "Jeong-dong Ryoo" <[email-masked]<mailto:[email-masked]>>
To: "'John Scudder'" <jgs=[email-masked]<mailto:jgs=[email-masked]>>; <[email-masked]<mailto:[email-masked]>>; <[email-masked]<mailto:[email-masked]>>;
Cc: <[email-masked]<mailto:[email-masked]>>;
Sent: 2022-01-10 (월) 11:46:33 (UTC+09:00)
Subject: RE: Re: [Teas] AD review of draft-ietf-teas-gmpls-signaling-smp-08

Hi, all.

All other comments from John should be reflected in the next revision. Accordingly, I will prepare a revision.

Best regards,

Jeong-dong

-----Original Message-----
From: "Adrian Farrel" <[email-masked]<mailto:[email-masked]>>
To: "'John Scudder'" <jgs=[email-masked]<mailto:jgs=[email-masked]>>; <[email-masked]<mailto:[email-masked]>>;
Cc: <[email-masked]<mailto:[email-masked]>>;
Sent: 2022-01-10 (월) 02:34:54 (UTC+09:00)
Subject: Re: [Teas] AD review of draft-ietf-teas-gmpls-signaling-smp-08

Hi John,

However,

Ciao,

Adrian

From: Teas <[email-masked]<mailto:[email-masked]>> On Behalf Of John Scudder
Sent: 06 January 2022 21:50
To: [email-masked]<mailto:[email-masked]>
Cc: [email-masked]
Subject: [Teas] AD review of draft-ietf-teas-gmpls-signaling-smp-08

Dear Authors,

Regards,

—John

+(see https://www.ietf.org/standards/ids/guidelines/)<https://urldefense.com/v3/__https://www.ietf.org/standards/ids/guidelines/)__;!!NEt6yMaO-gk!QCa7NR3jF4Q-dBEUCV5VgJQ9exyJ2uI77oQjNCrukC112JkD4Y7FQ55UetS_Nw$>. An appropriate

@@ -306,6 +323,10 @@

@@ -436,7 +497,7 @@

Notification (N): 1 bit

@@ -487,10 +569,15 @@

@@ -571,6 +675,16 @@

9. Acknowledgements

_______________________________________________ Teas mailing list [email-masked]<mailto:[email-masked]> https://www.ietf.org/mailman/listinfo/teas
[https://gov-dooray.com/mail-receipts?img=2f4f555936636369-255f338911948417-2c37b3bc3e4b3b50-2c37b420ee737299.gif]
_______________________________________________
Teas mailing list
[email-masked]<mailto:[email-masked]>
https://urldefense.com/v3/__https://www.ietf.org/mailman/listinfo/teas__;!!NEt6yMaO-gk!QCa7NR3jF4Q-dBEUCV5VgJQ9exyJ2uI77oQjNCrukC112JkD4Y7FQ56REYPPqg$

---

From: Jeong-dong Ryoo <[email-masked]>
Date: Mon, 17 Jan 2022 11:51:28 +0900 (KST)

John, thank you for the nits.&nbsp;

I have uploaded a new version. Please, go ahead with this new version.

Best regards,

Jeong-dong

-----Original Message-----
From: "John Scudder" &lt;jgs=[email-masked]&gt;
To: "Jeong-dong Ryoo" &lt;[email-masked]&gt;;
Cc: "John Scudder" &lt;jgs=[email-masked]&gt;; "[email-masked]" &lt;[email-masked]&gt;; "[email-masked]" &lt;[email-masked]&gt;; "[email-masked]" &lt;[email-masked]&gt;;
Sent: 2022-01-15 (토) 01:39:11 (UTC+09:00)
Subject: Re: [Teas] AD review of draft-ietf-teas-gmpls-signaling-smp-08

&nbsp; &nbsp;A new LSP Protection Type "Shared Mesh Protection" is defined (see
&nbsp; &nbsp;Section 6.1) to the LSP Flags of PROTECTION object (see [RFC4872]) to
&nbsp; &nbsp;set up the two LSPs.

—John
On Jan 13, 2022, at 10:17 PM, Jeong-dong Ryoo &lt;[email-masked] mailto:[email-masked]&gt; wrote:

Hi, John, et all.

We, co-authors of draft-ietf-teas-gmpls-signaling-smp, uploaded a new version, which is available at:
https://datatracker.ietf.org/doc/html/draft-ietf-teas-gmpls-signaling-smp-09 https://datatracker.ietf.org/doc/html/draft-ietf-teas-gmpls-signaling-smp-09

A diff from the previous version is available at:&nbsp;
https://www.ietf.org/rfcdiff?url2=draft-ietf-teas-gmpls-signaling-smp-09 https://www.ietf.org/rfcdiff?url2=draft-ietf-teas-gmpls-signaling-smp-09&nbsp;

Best regards,

-----Original Message-----
From: "Jeong-dong Ryoo" &lt;[email-masked] mailto:[email-masked]&gt;
To: "'John Scudder'" &lt;jgs=[email-masked] mailto:jgs=[email-masked]&gt;; &lt;[email-masked] mailto:[email-masked]&gt;; &lt;[email-masked] mailto:[email-masked]&gt;;
Cc: &lt;[email-masked] mailto:[email-masked]&gt;;
Sent: 2022-01-10 (월) 11:46:33 (UTC+09:00)
Subject: RE: Re: [Teas] AD review of draft-ietf-teas-gmpls-signaling-smp-08

Hi, all.

Best regards,

Jeong-dong

-----Original Message-----
From: "Adrian Farrel" &lt;[email-masked] mailto:[email-masked]&gt;

To: "'John Scudder'" &lt;jgs=[email-masked] mailto:jgs=[email-masked]&gt;; &lt;[email-masked] mailto:[email-masked]&gt;;

Cc: &lt;[email-masked] mailto:[email-masked]&gt;;

Hi John,

&nbsp;

&nbsp;

I believe that this is exactly what the authors have done here. That is, they have deliberately set two bits that have independent meaning, with the result of a special case of both bits meaning “full rerouting on a bidirectional
 protected LSP”.&nbsp;

&nbsp;

What the authors have done is possibly a stretch except for the sentence in 6.1 about backward compatibility. Given that it is an explicitly routed TE LSP, all routers on the path will have been selected by the path computation algorithm
 to be “SMP aware” and so the assertion is right and your concern that a non-SMP-aware node will see this bit setting is not a problem.

&nbsp;

However,

The backward compatibility paragraph could be clearerI might suggest a fourth way forward which is simply to take another bit from one of the adjacent reserved fields (so that was a six bit field becomes a seven bit field and SMP = 0x20). Should a legacy node
 see one of these messages, it will parse as “unprotected” (just the six bits set to 0x00) and not do anything special.&nbsp;
&nbsp;

Ciao,

Adrian

&nbsp;

From:&nbsp;Teas &lt;[email-masked] mailto:[email-masked]&gt;&nbsp;On Behalf
 Of&nbsp;John Scudder
Sent:&nbsp;06 January 2022 21:50
To:&nbsp;[email-masked] mailto:[email-masked]
Cc:&nbsp;[email-masked]
Subject:&nbsp;[Teas] AD review of draft-ietf-teas-gmpls-signaling-smp-08

&nbsp;

Dear Authors,

I’ve supplied my comments in the form of an edited copy of the draft. You can use your favorite diff tool to review them; I’ve attached a PDF of the rfcdiff output for your convenience if you’d like to use it. I’ve also pasted a traditional diff below in case
 you want to use it for in-line reply. I’d appreciate feedback regarding whether you found this a useful way to receive my comments as compared to a more traditional numbered list of comments with selective quotation from the draft.

In addition to the comments I’ve interspersed in the text, I do have one thornier issue I’d like to elevate. In §6.1 you define “shared mesh protection” as a value for the “LSP Flags” field. However, as defined in RFC 4872 §14.1, that is a field of six orthogonal
 bit flags as the name suggests, not a six-bit integer. The definitional text in 4872 confirms that this is what the WG intended, the text says “only one value SHOULD be set at a time”. You can’t set an integer to more than one value at a time; Q.E.D. the WG
 intended that the field is six orthogonal bit flags. (Or semi-orthogonal anyway, because 4872 goes on to say “only one value SHOULD be set at a time”.)

Now, I personally find it strange that 4872 wants to call this field a set of bit flags instead of an integer, given that it also says the values are mutually exclusive. I imagine the reason for it being specced this way is lost in the mists of time; I can
 think of several reasons it might have been done this way, some good, some not as good. But speculation is futile: the fact is that it says what it says.

2. You could make such an update yourself, presumably as a subsection of your specification although it could be a small standalone document. I think there would be two concerns arising from such a proposal. First, are there implementations in the field that
 faithfully implement RFC 4872 as written, and test bit values when determining the protection type? What would be the expected behavior of such an implementation when presented with two or more set flags? Even if you don’t know of any such implementations,
 can you prove there are none, i.e. can you prove that the redefinition would be safe? Second, since RFC 4872 makes the “one value at a time” restriction a SHOULD, not a MUST, the authors presumably had in mind a use case for legitimately setting two of the
 flags. By redefining the field as an integer, you would usurp such use cases. (It’s also possible the SHOULD is just sloppiness and there is no such use case, but that would have to be proven — presumptively, there is a legitimate use case even though I don’t
 know what it is.)

3. You could continue the practice of treating the field as bit flags. This raises the obvious problem that the flags are used up already (which makes me wonder what the authors of 4872 meant by “all other values are reserved”, since there aren’t any other
 values). But, you could get another bit from the adjacent five-bit Reserved field. So, instead of 0x11, “shared mesh protection” would be 0x20. One potential problem with this approach is that it looks to me as though, if a node in the path doesn’t implement
 SMP, if it receives 0x20 it would believe it’s received a PROTECTION object with the bottom Reserved bit set (and would ignore it, per RFC 4872), and with a Protection Type field of 0x00, Unprotected. So it would happily treat an SMP PROTECTION object as though
 it were an “Unprotected” PROTECTION object.

Regards,

—John

+(see&nbsp;https://www.ietf.org/standards/ids/guidelines/) https://urldefense.com/v3/__https://www.ietf.org/standards/ids/guidelines/)__;!!NEt6yMaO-gk!QCa7NR3jF4Q-dBEUCV5VgJQ9exyJ2uI77oQjNCrukC112JkD4Y7FQ55UetS_Nw$.
 An appropriate

@@ -306,6 +323,10 @@

@@ -436,7 +497,7 @@

Notification (N): 1 bit

@@ -487,10 +569,15 @@

@@ -571,6 +675,16 @@

9. Acknowledgements
&nbsp;

&nbsp;

_______________________________________________ Teas mailing list 
[email-masked] mailto:[email-masked] https://www.ietf.org/mailman/listinfo/teas https://www.ietf.org/mailman/listinfo/teas

_______________________________________________
Teas
 mailing list
[email-masked] mailto:[email-masked]
https://urldefense.com/v3/__https://www.ietf.org/mailman/listinfo/teas__;!!NEt6yMaO-gk!QCa7NR3jF4Q-dBEUCV5VgJQ9exyJ2uI77oQjNCrukC112JkD4Y7FQ56REYPPqg$ https://urldefense.com/v3/__https://www.ietf.org/mailman/listinfo/teas__;!!NEt6yMaO-gk!QCa7NR3jF4Q-dBEUCV5VgJQ9exyJ2uI77oQjNCrukC112JkD4Y7FQ56REYPPqg$