Re: [Lsr] AD review of draft-ietf-lsr-isis-fast-flooding-05

[[e-mail]]

[+Acee as I'm proposing a change which could be seen as a technical change. Please looks for //Acee ]

Hi John, 

Many thanks for your careful review, your comments and your propositions.
I'll upload -06 within minutes.

Please see inline [Bruno]
 
 
> From: John Scudder <[e-mail]> 
> Sent: Thursday, January 25, 2024 6:52 PM
> 
> Hi Authors, WG,
> 
> Thanks for this document.
> 
> I’ve supplied my questions and comments in the form of an edited copy of the draft. Minor editorial suggestions I’ve made in place without further comment, more substantive questions and comments are done in-line and prefixed with “jgs:”. You can use your favorite diff tool to review them; I’ve attached the iddiff 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.

[Bruno] Ack. Minor editorial comments should be included -06
 
> I’ve also requested a TSV early review of the document.

[Bruno] Thank you.
 
> —John
> 
> --- draft-ietf-lsr-isis-fast-flooding-05.txt	2024-01-24 19:46:21.000000000 -0500
> +++ draft-ietf-lsr-isis-fast-flooding-05-jgs-comments.txt	2024-01-25 12:43:39.000000000 -0500
> @@ -285,6 +285,9 @@
 > 4.1.  LSP Burst Window sub-TLV
>  
    > The LSP Burst Window sub-TLV advertises the maximum number of LSPs
> +--
> +jgs: should this say "maximum number of un-acknowledged LSPs"?


[Bruno] Thinking about this, I don't believe so.
The receiver is receiving LSPs. It does not matter for the receiver whether they have been acknowledged not.
Also the number of un-acknowledged LSPs is tracked by the flow control algorithm. The LSP Burst Window is more general and may be used by an implementation not implementing flow control.

That being said, your comment is logical given some errors in the draft and the use of a sub-optimal name.
So instead of applying your proposed resolution, we are proposing the following resolution:



OLD:  4.1 LSP Burst Window sub-TLV
NEW: 4.1 LSP Burst Size sub-TLV
Motivation: the term "Window" is being used by the flow control algorithm. Re-using the same term is incorrect in this case and a source of incomprehension for the reader. The error is coming from an historical artefact before the addition of the "4.6 Receive Window sub-TLV" in draft version -01

Obviously, the change needs to be replicated in the whole document.


In §4.2
OLD:  The LSP Transmission Interval sub-TLV advertises the minimum interval, in micro-seconds, between LSPs arrivals which can be received on this interface, 
after the maximum number of un-acknowledged LSPs have been sent.
NEW:The LSP Transmission Interval sub-TLV advertises the minimum interval, in micro-seconds, between LSPs arrivals which can be sustained on this receiving interface.
.


In §6.2.1.1
OLD:  After having sent a full burst of un-acknowledged LSPs, it MUST send the subsequent LSPs with an LSP Transmission Interval between LSP transmissions.
NEW: After having sent a full burst of LSPs, it MUST send the subsequent LSPs with a minimum of LSP Transmission Interval between LSP transmissions..

(I would assume that the above text was the main reason for your comment, as indeed " un-acknowledged LSPs " was explicitly stated 

---
//Acee

A bigger issue from that historical artifact is that some text refers to "LSP Burst Window" when they should refer to the "new" (from -01) "Receive Window sub-TLV". That's a bigger issue as "in letter" this may be seen as technical change (even if "in spirit" the Flow Control Receive Window logically refers to the Receive Window sub-TLV)
This requires the following changes in the 6.2.1 "Flow control" section:

§6.2.1 Flow control
No change required. The text correctly refers to " Receive Window sub-TLV"

§6.2.1.1 Operation on a point to point interface
" By sending the LSP Burst Window sub-TLV, a node advertises to its neighbor its ability to receive that many un-acknowledged LSPs from the neighbor, without an intervening delay. This is akin to a receive window or sliding window in flow control. In some implementations, this value should reflect the IS-IS socket buffer size. Special care must be taken to leave space for CSNP and PSNP (SNP) PDUs and IIHs if they share the same input queue. In this case, this document suggests advertising an LSP Burst Window corresponding to half the size of the IS-IS input queue."

:s/ LSP Burst Window/ Receive Window
(*2)

§6.2.1.2 Operation on a broadcast LAN interface
"In order for the LSP Burst Window to be a useful parameter, an LSP transmitter needs to be able to keep track of the number of un-acknowledged LSPs it has sent to a given LSP receiver."
:s/ LSP Burst Window/ Receive Window

OLD:  The LSP Burst Window is still very useful for the first burst of LSPs sent, especially in the case of a single node failure that requires the flooding of a relatively small number of LSPs.
NEW: The Receive Window is still very useful for the first set of LSPs sent, especially in the case of a single node failure that requires the flooding of a relatively small number of LSPs.

In §6.2.2.5 Remarks
"If an LSP Burst Window is advertised, LPP SHOULD be lower and the best performance is achieved when LPP is an integer fraction of the LSP Burst Window."

:s/ LSP Burst Window/ Receive Window
(*2)

> +--
    > that the node can receive without an intervening delay between LSP
    > transmissions.
>  
> @@ -293,6 +296,16 @@
    > Length: 4 octets
>  
    > Value: number of LSPs that can be sent back-to-back.
> +--
> +jgs: in this subsection and the following one, it seems like you're 
> +switching between writing from the perspective of the advertising IS in 
> +the first paragraph, to writing from the perspective of the receiving 
> +IS, in the "value" description. In this case, first you tell me it's 
> +the maximum number that the node can receive, but then you tell me the 
> +value means the maximum number that "can be sent". Which is it, sent, 
> +or received? I think you need to clarify this somehow, for example you 
> +could clarify by saying "can be sent" *by whom*.
> +--

[Bruno] Good point.
NEW: Value: number of LSPs that can be received back-to-back.
  
 > 4.2.  LSP Transmission Interval sub-TLV
>  
> @@ -300,6 +313,9 @@
    > interval, in micro-seconds, between LSPs arrivals which can be
    > received on this interface, after the maximum number of un-
    > acknowledged LSPs have been sent.
> +--
> +jgs: here you're mixing sent and received in the same paragraph.
> +--

[Bruno]
OLD:  The LSP Transmission Interval sub-TLV advertises the minimum interval, in micro-seconds, between LSPs arrivals which can be received on this interface, after the "LSP Burst Size" of LSPs have been sent.
NEW: The LSP Transmission Interval sub-TLV advertises the minimum interval, in micro-seconds, between LSPs arrivals which can be received on this interface, following the reception of "LSP Burst Size".
 
    > Type: 2
>  
> @@ -307,6 +323,9 @@
>  
    > Value: minimum interval, in micro-seconds, between two consecutive
    > LSPs sent after LSP Burst Window LSPs have been sent
> +--
> +jgs: and above
> +--

[Bruno]
OLD:  Value: minimum interval, in micro-seconds, between two consecutive LSPs sent after LSP Burst Size LSPs have been sent
NEW: Value: minimum interval, in micro-seconds, between two consecutive LSPs received after LSP Burst Size LSPs have been received

 
    > The LSP Transmission Interval is an advertisement of the receiver's
    > steady-state LSP reception rate.
> @@ -351,6 +370,9 @@
    > When the O-flag (Ordered acknowledgement) is set, the LSPs will be
    > acknowledged in the order they are received: a PSNP acknowledging N
    > LSPs is acknowledging the N oldest LSPs received.  The order inside
> +--
> +jgs: should that be "the N oldest unacknowledged LSPs"?

[Bruno] After discussion between co-authors, we would rather keep the original text.
One reason is that we don't want to imply that we are changing the way IS-IS acknowledge LSP (i.e. the one "unacknowledged" vs the one "received"). It's very unlikely that the ISO spec is referring to unacknowledged LSP.
From the POV of the receiver, every LSP received is considered "unacknowledged" i.e., the receiver does not keep track as to whether the LSP was previously received and acknowledged or not. If it was received then it MUST be acknowledged. "Collapse" of acknowledgements is only possible if the receiver receives multiple copies of the same LSP before it has sent any acknowledgement. 

> +--
    > the PSNP is meaningless.  If the sender keeps track of the order of
    > LSPs sent, this indication allows a fast detection of the loss of an
    > LSP.  This MUST NOT be used to alter the retransmission timer for any @@ -363,7 +385,7 @@
    > Sequence Number PDUs.  This time will trigger the sending of a PSNP
    > even if the number of unacknowledged LSPs received on a given
    > interface does not exceed LPP (Section 4.3).  The time is measured
> -   from the reception of the first unacknowldeged LSP.
> +   from the reception of the first unacknowledged LSP.
>  
    > Type: 5
>  
> @@ -453,6 +475,14 @@
 > 5.1.  Rate of LSP Acknowledgments
>  
    > On point-to-point networks, PSNP PDUs provide acknowledgments for
> +--
> +jgs: up to you whether to fix this or not, but since PSNP is an 
> +abbreviation for "Partial Sequence Number PDU", the above expands as 
> +"Partial Sequence Number PDU PDUs". So pedantically, just "PSNPs" or if 
> +you want to get funky, "PSN PDUs".

[Bruno] PSNPs works for me.
(I wouldn't try being funky with IS-IS or ISO)

> +
> +See also, ATM machine.
> +--
    > received LSPs.  [ISO10589] suggests that some delay be used when
    > sending PSNPs.  This provides some optimization as multiple LSPs can
    > be acknowledged by a single PSNP.
> @@ -541,6 +571,12 @@
    > as per the OSI model.  A typical example is the TCP protocol defined
    > in [RFC9293] that relies on the flow control, congestion control, and
    > reliability mechanisms of the protocol.
> +--
> +jgs: this doesn't quite make sense as written. TCP doesn't "rely on"
> +TCP, that would be a tautology. I'm not sure what the best way to 
> +rewrite it is, perhaps "... that provides flow control, congestion 
> +control, and reliability"?
> +--

[Bruno] Thank you for the suggestion. Applied.
As per above "ATM" comment:
OLD :  A typical example is the TCP protocol defined in <xref target="RFC9293"></xref>
NEW:  A typical example is TCP <xref target="RFC9293"></xref>
  
    > Flow control creates a control loop between a transmiter and a
    > receiver so that the transmitter does not overwhelm the receiver.
> @@ -551,7 +587,7 @@
    > multiple transmitters and multiple receivers to prevent the
    > transmitters from overwhelming the overall network.  For an IS-IS
    > adjacency, the network between two IS-IS neighbors is relatively
> -   limited in scope and consist of a link that is typically over-sized
> +   limited in scope and consists of a link that is typically over-sized
    > compared to the capability of the IS-IS speakers, but may also
    > include components inside both routers such as a switching fabric,
>  
> @@ -608,6 +644,9 @@
    > implementations, this value should reflect the IS-IS socket buffer
    > size.  Special care must be taken to leave space for CSNP and PSNP
    > (SNP) PDUs and IIHs if they share the same input queue.  In this
> +--
> +jgs: Same "ATM machine" nit applies here.
> +--

[Bruno] same correction. 

    > case, this document suggests advertising an LSP Burst Window
    > corresponding to half the size of the IS-IS input queue.
>  
> @@ -623,9 +662,13 @@
    > advertised value, outside of LSP bursts.
>  
    > The LSP transmitter MUST NOT exceed these parameters.  After having
> -   sent a full burst of un-acknowledged LSPs, it MUST send the following
> +   sent a full burst of un-acknowledged LSPs, it MUST send the 
> + subsequent
    > LSPs with an LSP Transmission Interval between LSP transmissions.
    > For CPU scheduling reasons, this rate may be averaged over a small
> +--
> +jgs: this is one of those rare times when I suggest replacing "may" 
> +with "MAY".

[Bruno] works for me.

> +--
    > period, e.g., 10-30ms.
>  
    > If either the LSP transmitter or receiver does not adhere to these @@ -733,7 +776,7 @@  6.2.2.2.  Congestion signals
>  
    > The congestion signal can take various forms.  The more reactive the
> -   congestion signals, the less LSPs will be lost due to congestion.
> +   congestion signals, the fewer LSPs will be lost due to congestion.
    > However, overly aggressive congestion signals will cause a sender to
    > keep a very low sending rate even without actual congestion on the
    > path.
> @@ -755,7 +798,7 @@
>  
    > There are multiple strategies to set the timeout value t1.  It should
    > be based on measures of the maximum acknowledgement time (MAT) of
> -   each PSNP.  The simplest one is to use a exponential moving average
> +   each PSNP.  The simplest one is to use an exponential moving average
    > of the MATs, like [RFC6298].  A more elaborate one is to take a
    > running maximum of the MATs over a period of a few seconds.  This
    > value should include a margin of error to avoid false positives @@ -765,7 +808,7 @@
    > Reordering: a sender can record its sending order and check that
    > acknowledgements arrive in the same order as LSPs.  This makes an
    > additional assumption and should ideally be backed up by a
> -   confirmation by the receiver that this assumption stands.  The O flag
> +   confirmation by the receiver that this assumption holds.  The O flag
    > defined in Section 4.4 serves this purpose.
>  
 > 6.2.2.3.  Refinement 1
> @@ -788,6 +831,9 @@
>  
    > It is possible to use a fast recovery phase once congestion is
    > detected, to avoid going through this linear rate of growth from
> +--
> +jgs: Surely cwin += 1/cwin is sub-linear?

[Bruno] 
May be what's missing to improve clarity is to mention the variable used by the (linear) function. Here the variable is the time (t) which is probably what matters for the user and the discussion. In which case, for each RTT, cwin LSPs are in flight and will be acknowledged. So cwin += 1/cwin per LSP will translate into cwin += 1/cwin * cwin per RTT (hence for "t") i.e., cwin += 1 per RTT which looks more linear.
https://en.wikipedia.org/wiki/TCP_congestion_control seems to refer to linear increase but https://datatracker.ietf.org/doc/html/rfc5681 does not.
https://en.wikipedia.org/wiki/Additive_increase/multiplicative_decrease has the linear formular https://wikimedia.org/api/rest_v1/media/math/render/svg/55a5e0b0be92bbdfd774092e0da9a41e4450c607

We have applied to following change to try to improve clarity on this aspect:

OLD:  The algorithm starts with cwin = LPP + 1. In the congestion avoidance phase, cwin increases as LSPs are acked: for every acked LSP, cwin += 1 / cwin. Thus, the sending rate is inversely proportional to the RTT. Since the RTT is low in many IS-IS deployments, the sending rate can reach fast rates in short periods of time.

NEW: The algorithm starts with cwin = cwin0 = LPP + 1. In the congestion avoidance phase, cwin increases as LSPs are acked: for every acked LSP, cwin += 1 / cwin. When LSPs are exchanged, cwin LSPs will be acknowledged in 1 RTT, meaning cwin(t) = t/RTT + cwin0. Since the RTT is low in many IS-IS deployments, the sending rate can reach fast rates in short periods of time.

Hope this clarify. Additional feedback and proposal is welcomed.


> +--
    > scratch.  When congestion is detected, a fast recovery threshold
    > frthresh is set to frthresh = cwin / 2.  In this fast recovery phase,
    > for every acked LSP, cwin += 1.  Once cwin reaches frthresh, the @@ -870,7 +916,12 @@
    > Expressed as an inter-packet interval in units of time:
>  
    > interval = (smoothed_rtt / congestion_window) / N
> -
> +--
> +jgs: You haven't defined smoothed_rtt or congestion_window. It's 
> +readily apparent that congestion_window is cwin, but please be 
> +consistent in your terminology. As for smoothed_rtt, this is the only occurrence of "smooth"
> +in the document.

[Bruno]  Correct.
Regarding smoothed_rtt, one option may be to refer to [RFC6298] "Computing TCP's Retransmission Timer".
On the pro side, we don't duplicate text but point to the reference. If the reference gets updated, this is tracked.
On the con side, the routing community reading this isis document may not be familiar with this RFC from the transport community. So this may not be the easiest path for the typical reader.

So far, I have done this:
NEW: interval = (SRTT / cwin) / N 
  SRTT is the smoothed round-trip time [RFC6298]


Another option would be to copy the relevant text from [RFC6298]. Something like
As defined in [RFC6298], the SRTT (smoothed round-trip time) is computed as below:
When the first RTT measurement R is made, SRTT <- R
When a subsequent RTT measurement R' is made, SRTT <- (1 - alpha) * SRTT + alpha * R'
The above SHOULD be computed using alpha=1/8 (as suggested in [JK88]).


As this point, I don't think I have a preference. I've picked the easiest change for me.
Please advise or suggest.

> +--
    > Using a value for N that is small, but at least 1 (for example, 1.25)
    > ensures that variations in RTT do not result in underutilization of
    > the congestion window.
> @@ -908,6 +959,11 @@
    > loss will reduce the usable receive window and the protocol
    > mechanisms will allow the adjacency to recover.  Flooding several
    > orders of magnitude slower than both nodes can support will hurt
> +--
> +jgs: is "several orders of magnitude" doing any useful work here? I 
> +would Think that simply flooding slower than both nodes can support 
> +will hurt performance, and the reader can figure out the rest.
> +--

[Bruno] OK. Change applied. (the goal was to give an overview of the possible gain compared to the use of historic values. But I agree that such number may not age well.)

    > performance, as will consistently overloading the receiver.
>  
    > The values advertised need not be dynamic as feedback is provided by @@ -918,12 +974,19 @@
    > advertising relatively static parameters, we expect to produce
    > overall flooding behavior similar to what might be achieved by
    > manually configuring per-interface LSP rate-limiting on all
> -   interfaces in the network.  The advertised values may be based, for
> -   example, on an offline tests of the overall LSP-processing speed for
> +   interfaces in the network.  The advertised values could be based, for
> +   example, on offline tests of the overall LSP-processing speed for
    > a particular set of hardware and the number of interfaces configured
    > for IS-IS.  With such a formula, the values advertised in the
    > Flooding Parameters TLV would only change when additional IS-IS
    > interfaces are configured.
> +--
> +jgs: It took me a while to understand the above, I proposed some 
> +clarifying/correcting edits.

[Bruno] I'm seeing 2 words changed. This works for me. Thank you.
Given your comment, I would have assumed more changes. So please come back if I've missed some clarifying/correcting edits

> I also wonder if "would only change" 
> +should more properly be "would only be changed" since I think the 
> +assumption is the parameters are being configured by system management 
> +(i.e., manually).

[Bruno] Ah...
On my side, my preference would be for the IS-IS implementation to send the "right" value that fits its capability. And if necessary, updates them if needed.(FYI, on our FRR implementation, dynamic change of value was not needed). E.g. reduce some parameters shared on a per box basis, as the number of IS-IS neighbor increase.
Now some vendor may find easier that these parameters be chosen by the network operator.
So YMMV, I guess. But I'd rather hope for the best and keep existing wording.

> +--
>  
    > The values may be updated dynamically, to reflect the relative change
    > of load on the receiver, by improving the values when the receiver @@ -935,6 +998,11 @@
>  
    > The values may also be absolute value reflecting relevant average
    > hardware resources that are been monitored, typically the amount of
> +--
> +jgs: "that are been" is definitely wrong, but I'm not sure what you're 
> +trying to say so can't propose a fix, other than perhaps you could 
> +delete "that are been monitored".
> +--

[Bruno] Would the below change works?
OLD:  that are been monitored
NEW: that are monitored

    > buffer space used by incoming LSPs.  In this case, care must be taken
    > when choosing the parameters influencing the values in order to avoid
    > undesirable or unstable feedback loops.  It would be undesirable to @@ -983,7 +1051,7 @@
    > packets will be punted.
>  
    > An input queue in the control plane may then be used to assemble PDUs
> -   from multiple linecards, separate the IS-ISs PDU from other types of
> +   from multiple linecards, separate the IS-IS PDUs from other types of
    > packets, and place the IS-IS PDUs on an input queue dedicated to the
    > IS-IS protocol.
>  
> @@ -1014,7 +1082,7 @@
>  
    > The congestion control algorithm described in this section does not
    > depend upon direct signaling from the receiver.  Instead it adapts
> -   the tranmsmission rate based on measurement of the actual rate of
> +   the transmission rate based on measurement of the actual rate of
    > acknowledgments received.
>  
    > When flow control is necessary, it can be implemented based on @@ -1040,7 +1108,7 @@
    > LSPTxRate.
>  
    > The flow control algorithm MUST NOT assume the receive performance of
> -   a neighbor are static, i.e., it MUST handle transient conditions
> +   a neighbor is static, i.e., it MUST handle transient conditions
    > which result in a slower or faster receive rate on the part of a
    > neighbor.
>  
> @@ -1056,7 +1124,8 @@
 > 7.1.  Flooding Parameters TLV
>  
    > IANA has made the following temporary allocation from the IS-IS TLV
> -   codepoint registry.
> +   codepoint registry. This document requests the allocation be made
> +   permanent.

[Bruno] change applied.  
  
>  
> @@ -1075,6 +1144,10 @@
 > 7.2.  Registry: IS-IS Sub-TLV for Flooding Parameters TLV
>  
    > This document creates the following sub-TLV Registry:
> +--
> +jgs: we've already discussed the need to say what group the registry is 
> +under, for this and the following.
> +--

[Bruno]. Yes. 
On a side note, for naming consistency, I'm wondering whether we should change "Receive Window" into "LSP Receive Window" to be consistent with "LSP Burst Size" and "LSP Transmission Interval"??

Thank you for your time and your comments.

--Bruno
  
    > Name: IS-IS Sub-TLVs for Flooding Parameters TLV.
>  
> @@ -1155,7 +1228,7 @@
 > 8.  Security Considerations
>  
    > Security concerns for IS-IS are addressed in [ISO10589] , [RFC5304] ,
> -   and [RFC5310] .  These documents describe mechanisms that provide the
> +   and [RFC5310] .  These documents describe mechanisms that provide 
> + for the
    > authentication and integrity of IS-IS PDUs, including SNPs and IIHs.
    > These authentication mechanisms are not altered by this document.
>  
> @@ -1165,7 +1238,7 @@
>  
    > In the absence of cryptographic authentication, as IS-IS does not run
    > over IP but directly over the link layer, it's considered difficult
> -   to inject false SNP/IHH without having access to the link layer.
> +   to inject false SNP/IIH without having access to the link layer.
>  
    > If a false SNP/IIH is sent with a Flooding Parameters TLV set to
    > conservative values, the attacker can reduce the flooding speed
> 
> 
>
____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.