Re: [Detnet] AD review of draft-ietf-detnet-bounded-latency-07

[Mohammadpour Ehsan <[e-mail]>]

Dear John,

Thank you so much for your invaluable comments! We’ll go through them and let you know as soon as we address them all!

Also, I find the PDF version pretty much illustrative that helps to easily understand the parts that require modification!



Best,
Ehsan



--
Ehsan Mohammadpour
PhD Candidate at Swiss Federal Institute of Technology (EPFL)
IC IINFCOM, LCA2, INF 011, Station 14, 1015 Lausanne, Switzerland
https://people.epfl.ch/ehsan.mohammadpour

On 3 Jan 2022, at 23:33, John Scudder <[e-mail]<mailto:[e-mail]>> wrote:

Dear Authors,

Here’s my review of your document. I don't think it will be too difficult to address my comments.

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.

Thanks,

—John



--- draft-ietf-detnet-bounded-latency-07.txt    2022-01-03 15:52:43.000000000 -0500
+++ draft-ietf-detnet-bounded-latency-07-jgs-markup.txt 2022-01-03 17:23:06.000000000 -0500
@@ -186,7 +186,7 @@

 3.1.  Flow admission

-   This document assumes that following paradigm is used to admit DetNet
+   This document assumes that the following paradigm is used to admit DetNet
    flows:

    1.  Perform any configuration required by the DetNet transit nodes in
@@ -214,6 +214,8 @@
    This paradigm can be implemented using peer-to-peer protocols or
    using a central controller.  In some situations, a lack of resources
    can require backtracking and recursing through this list.
+
+What is "this list" in the sentence above?



@@ -227,7 +229,7 @@


    Issues such as service preemption of a DetNet flow in favor of
-   another, when resources are scarce, are not considered, here.  Also
+   another, when resources are scarce, are not considered here.  Also
    not addressed is the question of how to choose the path to be taken
    by a DetNet flow.

@@ -249,14 +251,23 @@

    The static latency calculation is not limited only to static
    networks; the entire calculation for all DetNet flows can be repeated
+
+This is a surprising sentence and I wonder if it can be expressed some
+way that doesn't make the reader do the mental equivalent of slipping
+on a banana peel. I mean, here we have a subsection called "static
+latency calculation" and a preceding paragraph that says "this problem
+is of interest to relatively static networks", ok fine... and then in the
+very next paragraph you say "not limited only to static networks". The
+reader cries out, please make up your mind!
+
    each time a new DetNet flow is created or deleted.  If some already-
    established DetNet flow would be pushed beyond its latency
    requirements by the new DetNet flow, then the new DetNet flow can be
    refused, or some other suitable action taken.

-   This calculation may be more difficult to perform than that of the
+   This calculation may be more difficult to perform than the
    dynamic calculation (Section 3.1.2), because the DetNet flows passing
-   through one port on a DetNet transit node affect each others'
+   through one port on a DetNet transit node affect each other's
    latency.  The effects can even be circular, from a node A to B to C
    and back to A.  On the other hand, the static calculation can often
    accommodate queuing methods, such as transmission selection by strict
@@ -366,6 +377,11 @@
    4.  Processing delay
       This delay covers the time from the reception of the last bit of
       the packet to the time the packet is enqueued in the regulator
+
+Is "regulator" a term defined in one of the normative references, or a
+term of art so common that the reader can be assumed to be familiar with
+it?
+
       (Queuing subsystem, if there is no regulation).  This delay can be
       variable, and depends on the details of the operation of the
       forwarding node.
@@ -402,6 +418,12 @@
    The initial and final measurement point in this analysis (that is,
    the definition of a "hop") is the point at which a packet is selected
    for output.  In general, any queue selection method that is suitable
+
+The first sentence above is challenging for me to make sense of.  My
+problem is with "the initial and final" (which implies, two) "measurement
+point" (which implies, one).  That is, there's a disagreement in number
+in how the sentence is written.  A rewrite may be in order?
+
    for use in a DetNet network includes a detailed specification as to
    exactly when packets are selected for transmission.  Any variations
    in any of the delay times 1-4 result in a need for additional buffers
@@ -476,10 +498,19 @@

    For other queuing mechanisms the only available value of
    queuing_delay_bound is the sum of the per-hop queuing delay bounds.
+
+So far so good...
+
    In such cases, the computation of per-hop queuing delay bounds must
    account for the fact that the T-SPEC of a DetNet flow is no longer
    satisfied at the ingress of a hop, since burstiness increases as one
    flow traverses one DetNet transit node.
+
+This appears to me as though it's a disconnected thought from the
+previous sentence. Does it somehow logically follow? Also, it's the case
+that you're not telling the reader how to compute those bounds, right?
+Just saying "my goodness it's a PITA to compute them" and providing a
+tiny hint as to why?

 4.2.1.  Per-flow queuing mechanisms

@@ -588,8 +619,8 @@

    This ingress conditioning typically consists of a FIFO with an output
    regulator that is compatible with the queuing employed by the DetNet
-   transit node on its output port(s).  For some queuing methods, simply
-   requires added extra buffer space in the queuing subsystem.  Ingress
+   transit node on its output port(s).  For some queuing methods, this simply
+   requires added buffer space in the queuing subsystem.  Ingress
    conditioning requirements for different queuing methods are mentioned
    in the sections, below, describing those queuing methods.

@@ -711,6 +742,16 @@
    queues of a DetNet transit node.  The DetNet packets are mapped to a
    number of regulators.  Here, we assume that the PREOF (Packet
    Replication, Elimination and Ordering Functions) functions are
+
+This is just a nit, but "... Functions) functions" above scans kind of
+funny. It's an "ATM machine" problem -- PREOF contains the word
+"functions" within itself but normally we just use it as a noun,
+"pre-off". When used that way, sure, "PREOF functions" doesn't seem
+wrong. But when expanding it out, as you've done here (thank you for
+expanding it by the way) then it's a little funny.
+
+Change it or not, as you prefer.
+
    performed before the DetNet packets enter the regulators.  All
    Packets are assigned to a set of queues.  Packets compete for the
    selection to be passed to queues in the queuing subsystem.  Packets
@@ -815,7 +856,12 @@
    DetNet flows typically pass through the interrupting MAC.  For those
    DetNet flows with T-SPEC, latency bound can be calculated by the
    methods provided in the following sections that accounts for the
-   affect of frame preemption, according to the specific queuing
+
+"methods ... accounts" seems like a disagreement in number. Probably should
+be "methods ... account", as in "methods provided in the following sections
+that account for the"?
+
+   effect of frame preemption, according to the specific queuing
    mechanism that is used in DetNet nodes.  Best-effort queues pass
    through the interruptible MAC, and can thus be preempted.

@@ -825,12 +871,16 @@
    described in section 8.6.8.4.  On each node, the transmission
    selection for packets is controlled by time-synchronized gates; each
    output queue is associated with a gate.  The gates can be either open
-   or close.  The states of the gates are determined by the gate control
+   or closed.  The states of the gates are determined by the gate control
    list (GCL).  The GCL specifies the opening and closing times of the
    gates.  The design of GCL should satisfy the requirement of latency
-   upper bounds of all DetNet flows; therefore, those DetNet flows
+   upper bounds of all DetNet flows; therefore, those DetNet flows that
    traverse a network should have bounded latency, if the traffic and
    nodes are conformant.
+
+Would it be right to insert "that uses this kind of shaper" or similar? As in,
+"therefore, those DetNet flows that traverse a network that uses this kind
+of shaper should have a bounded latency".



@@ -853,14 +903,14 @@

 6.4.  Credit-Based Shaper with Asynchronous Traffic Shaping

-   In the considered queuing model, we considered the four traffic
+   In the queuing model, we considered the four traffic
    classes (Definition 3.268 of [IEEE8021Q]): control-data traffic
    (CDT), class A, class B, and best effort (BE) in decreasing order of
    priority.  Flows of classes A and B are together referred as AVB
    flows.  This model is a subset of Time-Sensitive Networking as
    described next.

-   Based on the timing model described in Figure 1, the contention
+   Based on the timing model described in Figure 1, contention
    occurs only at the output port of a DetNet transit node; therefore,
    the focus of the rest of this subsection is on the regulator and
    queuing subsystem in the output port of a DetNet transit node.  The
@@ -882,6 +932,10 @@
    respective interleaved regulator at the output port.  Then, the
    packets from all the flows, including CDT and BE flows, are enqueued
    in queuing subsytem; there is no regulator for such classes.
+
+I assume by "such classes" you mean CDT and BE flows? If so please be
+more clear, as in "there is no regulator for the latter" or clearer still
+just name them, as in "there is no regulator for CDT or BE flows".



@@ -924,11 +978,18 @@
    Figure 4: The architecture of an output port inside a relay node with
          interleaved regulators (IRs) and credit-based shaper (CBS)

-   Each of the queuing subsystems for classes A and B, contains Credit-
+   Each of the queuing subsystems for classes A and B, contains a Credit-
    Based Shaper (CBS).  The CBS serves a packet from a class according
    to the available credit for that class.  The credit for each class A
    or B increases based on the idleslope (as guaranteed rate), and
    decreases based on the sendslope (typically equal to the difference
+
+You use "idleslope" and "sendslope" here (with no space between words).
+Later in the document you use "idle slope" (my grep didn't find any
+instances of "send slope"; this is evidently the only time you reference
+it).  Unless you have a reason to prefer the two different terms, please
+settle on one way of writing it (I prefer "idle slope" with the space).
+
    between the guaranteed and the output link rates), both of which are
    parameters of the CBS (Section 8.6.8.2 of [IEEE8021Q]).  The CDT and
    BE0-BE4 flows are served by separate queuing subsystems.  Then,
@@ -936,14 +997,14 @@
    subsystem that serves packets from each class based on its priority.
    All subsystems are non-preemptive.  Guarantees for AVB traffic can be
    provided only if CDT traffic is bounded; it is assumed that the CDT
-   traffic has leaky bucket arrival curve with two parameters r_h as
+   traffic has a leaky bucket arrival curve with two parameters r_h as
    rate and b_h as bucket size, i.e., the amount of bits entering a node
    within a time interval t is bounded by r_h * t + b_h.

    Additionally, it is assumed that the AVB flows are also regulated at
-   their source according to leaky bucket arrival curve.  At the source,
+   their source according to a leaky bucket arrival curve.  At the source,
    the traffic satisfies its regulation constraint, i.e. the delay due
-   to interleaved regulator at source is ignored.
+   to interleaved regulator at the source is ignored.



@@ -962,7 +1023,7 @@

    The regulation parameters for a flow (leaky bucket rate and bucket
    size) are the same at its source and at all DetNet transit nodes
-   along its path in the case of that all clocks are perfect.  However,
+   along its path in the case where all clocks are perfect.  However,
    in reality there is clock nonideality thoughout the DetNet domain
    even with clock synchronization.  This phenomenon causes inaccuracy
    in the rates configured at the regulators that may lead to network
@@ -975,6 +1036,16 @@
    A delay bound of the queuing subsystem ((4) in Figure 1) for an AVB
    flow of classes A or B can be computed if the following condition
    holds:
+
+I guess this section (or rather, this part of this section!) is talking
+about the delay bound on an individual node. I suppose that's obvious,
+ish, from the lead-in where you reference the queueing subsystem -- so
+the delay bound is in the context of a given node, which is where a
+given queueing subsystem is instantiated.
+
+Nevertheless, it would have helped me if there had been some explicit
+text reminding the reader of this. My later comment will hopefully
+illuminate why this was problematic to my understanding.

       sum of leaky bucket rates of all flows of this class at this
       transit node <= R, where R is given below for every class.
@@ -996,9 +1067,17 @@
       T_A = L_nA + b_h + r_h * L_n/c)/(c-r_h)

    where I_A is the idle slope for class A; L_nA is the maximum packet
+
+Is "idle slope" a well-known term of art? You do sort-of define it in
+§6.4, that might be sufficient in any case, especially once you rationalize
+"idleslope" vs. "idle slope".
+
    length of class B and BE packets; L_n is the maximum packet length of
    classes A,B, and BE; r_h as rate and b_h as bucket size of CDT
    traffic leaky bucket arrival curve.
+
+When you write "as rate" and "as bucket size" do you mean "is rate" and
+"is bucket size"? (If you really mean "as" that doesn't make sense to me.)

    If the flow is of class B:

@@ -1015,13 +1094,32 @@
       T_B = (L_BE + L_A + L_nA * I_A/(c_h-I_A) + b_h + r_h * L_n/
       c)/(c-r_h)

-   I_B is the idle slope for class B; L_A is the maximum packet length
-   of class A; L_BE is the maximum packet length of class BE.  where
+   where I_B is the idle slope for class B; L_A is the maximum packet length
+   of class A; L_BE is the maximum packet length of class BE.

    Then, an end-to-end delay bound of class X (A or B)is calculated by
-   the formula Section 4.2.2, where for Cij:
+   the formula from Section 4.2.2, where for Cij:

       Cij = d_X
+
+A few comments on the above. First, "the formula from Section 4.2.2"
+doesn't seem like a good cite, since what's presented in §4.2.2 is
+only an example, specific to a network of five nodes.  I mean, it's
+probably not hard for the reader to extrapolate what you mean, but I
+think it's worth going to the small additional effort to state it as
+rigorously as the rest of your material.
+
+Second, shouldn't you be parameterizing d_X in some fashion similar
+to the way you parameterize C?  That is, if you are talking about Cij,
+then wouldn't you be talking about d_Xij?  Pick your own terminology
+of course, or even explain it in prose, but the real point is that for
+the casual reader (me) it's not obvious until re-examination that d_X
+is specific to a given node.
+
+In the end I THINK what you are saying here is along the lines of, the
+e2e delay bound for class X is given by the sum of the individual
+delay bounds d_X computed for each node along that path.  Yes, no?
+(This is similar to how you write the delay bound in §6.5.)

    More information of delay analysis in such a DetNet transit node is
    described in [TSNwithATS].
@@ -1035,6 +1133,13 @@
    information on each class, i.e. maximum packet length of classes A,
    B, and BE.  Moreover, the leaky bucket parameters of CDT (r_h,b_h)
    should be known.  To admit a flow/flows of classes A and B, their
+
+I realize this isn't a Standards Track document and you're not using
+RFC 2119 language. Still, your use of "should" above, and following,
+is potentially a little problematic. I suggest you search through the
+document for instances of "should" and for each, consider whether you
+can replace it with "must" or something similarly unambiguous.
+
    delay requirements should be guaranteed not to be violated.  As
    described in Section 3.1, the two problems, static and dynamic, are
    addressed separately.  In either of the problems, the rate and delay
@@ -1093,6 +1198,11 @@
    allocated to this class at this node, b_t affects the delay bound and
    the buffer requirement.  R must satisfy the constraints given in
    Annex L.1 of [IEEE8021Q].
+
+I've tried to be relaxed about your citation of the various IEEE documents
+as Informational, but I can't see how the above citation isn't Normative.
+There is no way for me to know what the constraints on R are without
+referring to the reference, that's the essence of Normative.

 6.5.  Guaranteed-Service IntServ

@@ -1155,6 +1265,11 @@
    experienced by a given packet is from the beginning of cycle (i) to
    the end of cycle (i+1), or 2T_c; also, the minimum delay is from the
    end of cycle (i) to the beginning of cycle (i+1), i.e., zero.  Then,
+
+2T_c and zero as the maximum and minimum delays are relatively
+obvious. (Well, zero is not so obvious actually and I have doubts about
+it.) However, what follows is not so obvious:
+
    if the packet traverses h hops, the maximum delay is:

       (h+1) T_c
@@ -1164,6 +1279,24 @@
       (h-1) T_c

    which gives a latency variation of 2T_c.
+
+The naïve reader (me again) would assume that if a packet traverses h
+hops, and each hop has maximum delay 2T_c, then the per-path maximum
+delay would be the sum of the per-hop delays, i.e. (h) 2T_c.  Likewise,
+I would have thought that if the minimum per-node delay is zero then
+the per-path minimum delay is trivially zero. (Presumably link latency
+isn't accounted for here, or else "zero" is just wrong.)
+
+But this isn't what you say at all. Possibly there's something about
+CQF that provides the per-path property you state, e.g. something along
+the lines of, once I'm scheduled into a given cycle, the properties of
+the algorithm provide that I'll never encounter contention as I
+proceed through the network and thus at worst I'll endure 2T_c delay
+to be aligned to my cycle. But, this isn't stated, and your prose jumps
+without motivation from stating one set of properties for per-node
+delays to a surprising and non-obvious different set of properties for
+per-path delays. The "Then" in your prose implies that you've connected
+the two. AFAICT, you haven't.

    The cycle length T_c should be carefully chosen; it needs to be large
    enough to accomodate all the DetNet traffic, plus at least one
@@ -1279,6 +1412,9 @@
    the delay bound requirement of the flow.  If there is no such path,
    the control plane may compute new set of valid paths and redo the
    delay bound computation or do not admit the DetNet flow.
+
+"do not admit" is ungrammatical as used. I suggest "reject" or "choose
+not to admit".



@@ -1314,7 +1450,7 @@
    domain.

    A security consideration for this document is to secure the resource
-   reservation signaling for DetNet flows.  Any forge or manipulation of
+   reservation signaling for DetNet flows.  Any forgery or manipulation of
    packets during reservation may lead the flow not to be admitted or
    face delay bound violation.  Security mitigation for this issue is
    describedd in Section 7.6 of [RFC9055].
@@ -1432,6 +1568,8 @@
               J.-Y. Le Boudec and P. Thiran, "Network calculus: a theory
               of deterministic queuing systems for the internet", 2001,
               <https://ica1www.epfl.ch/PS_files/NetCal.htm>.
+
+This URL has changed, it redirects to https://leboudec.github.io/netcal/

    [RFC2697]  Heinanen, J. and R. Guerin, "A Single Rate Three Color
               Marker", RFC 2697, DOI 10.17487/RFC2697, September 1999,

<Diff- draft-ietf-detnet-bounded-latency-07.txt - draft-ietf-detnet-bounded-latency-07-jgs-markup.txt.pdf><draft-ietf-detnet-bounded-latency-07-jgs-markup.txt>