Subject: Re: [tsvwg] Comments to draft-finzi-priority-switching-scheduler-02

From: Emmanuel Lochin <[email-masked]>
Date: Thu, 18 Oct 2018 15:27:44 +0200

Dear Vincent,

Thanks for your precious review. Answers and comments in-line:

On 10/09/2018 15:17, Vincent Roca wrote:
> Dear authors,
>
> Please find below my comments after reading your I-D
> 	draft-finzi-priority-switching-scheduler-02
> Those comments are not from a CC specialist as you know
> and I cannot judge the accuracy of the claims.
>
> Regards,
>
>    Vincent
>
> ————
> Abstract:
>
> ** the text first opposes "time constrained and elastic flows" from BE flows.
> Then, just after "Usual implementations of rate scheduler schemes", it opposes the AF and BE flows.
>  From a logical point of view, are the elastic flows  part of AF flows? I don't think so but there's
> an ambiguity here: does this proposal consider the same flow classes as the "usual schedulers"?

This is correct. Historically, the terms "elastic" and "real-time" have 
been defined in RFC1633. Then "inelsatic traffic" has been used in 
RFC4594 which provides guidelines to configure DiffServ service classes. 
As explained in RFC4594 section 1.5.2, the AF class "(...) is an 
enhanced best-effort service; traffic is expected to be "elastic" in 
nature."

So we effectively consider the same flow classes and kind of traffic as 
usual rate schedulers. But I confess this sentence is not 
self-contained. So we added clarification laying on RFC4594 for this 
terminology.

>
> ** The text could also be clarified WRT its use of "queues",  "traffic flow", and "traffic class".
> Since this is just an abstract that needs to be kept small and easy to understand, may be selecting
> one term would be sufficient.
ok we update it
>
> ** Finally the AF and BE acronyms are used but not expanded (I don't think it's appropriate to also
> expand WRR and DRR though, leave it as it is).
ok
>
>
> Introduction:
>
> ** Section 1.1: you say: "... depends on the amount of traffic crossing other queues".
> Do you really mean crossing or arriving at? Does "crossing" have the same meaning as "output rate"?
I see, may be a better sentence would be : "depends on the queuing 
occupancy/load of all other queues.".
>
> ** Section 1.1: I don't understand where's the difficulty in having controlled queues: if I always
> immediately schedule packets arriving at a "controlled queue", it's output rate will not depend on
> other BE traffic and uncertainty of the output rate will be kept minimum (once again WRT BE traffic).
> The scientific problem must be better stated. We understand what is expected by reading Section 1.3,
> but that's a section that describes the technical proposal, not the problem.

That's right and there is no "difficulty" here. As the system is not 
preemptive, if the BE traffic load from the DE class is constant, the 
next AF packet to serve will be penalized at worse by the service of the 
maximum size of a DE packet (which can also have different size). The 
difficulty comes from when you are chaining a rate scheduler with a 
static priority as proposed in RFC5865. In that case, blocking can be 
also due to the service of an EF packet.

But I see what is missing, I will add as suggested supplementary 
information concerning the problem tackled.

>
> ** Section 1.2: why do you put this section here? 1.3 should immediately follow the context/motivation
> section.
Ok
>
> ** Section 1.3: once again, I'd like to know if "queue" and "flow" are synonymous or not.
> Can two flows share the same queue? What's the mapping between them?

I see. A queue is managing an aggregate of flows belonging to the same 
traffic class. The rate scheduler is working on flows' aggregate and the 
change in priority is not aware of the number of micro-flows inside the 
queue.

> ** Section 1.3: I'd suggest simplifying a bit the sentence:
> s/each priority denoted p[i],/each priority p[i]/
> and
> s/credit counter, i.e., credit[i]/credit counter credit[i]/
ok
>
> ** Section 1.3: In sentence "Then a Priority Scheduler is used", you use "e.g., ".
> Is it really an example, i.e., could a different approach be used as suggested?
Oups ! It should be i.e. !
>
> ** Section 1.3, last paragraph: I don't understand what "queues scheduled with a simple Priority Scheduler"
> refers to, especially when the authors add, concerning this simple PS: "(when only one priority is set)".
> Could you clarify your point?

Having in mind RFC5865 core scheduling scheme where you have 3 classes : 
EF, AF, DE (corresponding to 3 queues) and 2 schedulers : one PS and one 
rate scheduler (WRR); with PSS : you still keep 3 queues/classes but you 
only have one scheduler.

> ** Is there a reference (e.g., article) for this proposal, beyond the [BLS] mentioned earlier?
AFAIK, there is one study published in 2016 called "Formal worst-case 
timing analysis of Ethernet TSN's burst-limiting shaper" in IEEE DATE. 
No further investigation has been done as this proposal seems to have 
been abandoned by the TSN group.
>
>
> Section 2.1 "Specifications":
>
> ** The relation: "p_high[q] < p_low[q]" is strange... Usually high > low, not the opposite.
Ok :) actually "high" refers to "first or premium" priority while "low" 
to "less/second" prio.
>
> ** the "maximum level" associated acronym should be ML and not "LMs[q]". Same comment for "LRs[q]"
> unless there's something I've missed.
We described here a general algorithm where several controlled queues 
are enabled. So LMs[q] is a table containing all the maximum level of 
each controlled queue while LM refers to one maximum level of one given 
queue. However, we figure out that this would be simpler to describe the 
algorithm for one controlled queue which enables a 3 classes scheduling 
scheme first and then to give the general formula later as it is much 
more complex to handle. We change that.
> What's the "C" parameter in the idle/sending slopes formulas? The output link capacity?
> Reading the formulas and assuming C is a birate, I understand that "BWs[q]" could a ratio. Is that
> the case? More generally, for the various parameters the unit (and possibly a quick definition)
> should be provided.

Damned ! You're right it misses this parameter definition. Indeed, the 
available capacity C is the output link capacity.

> ** In the paragraph describing the impacts of LMs[q], the authors use "non-premption" as a synonymous
> to "maximum sending windows", or at least this is the way I understand it since this is not clarified.
> I suggest removing "non-preemption" and sticking to "maximum sending windows" (or the opposite) to
> avoid confusion.
>
> ** s/premption/preemption/ (3 occurences).
>
> ** s/However, there are two cases keeping track of the transmission raises an issue:/However, two cases raise an issue:/
> or something else.
>
> ** In: "LRs[q]= Lmax(..." it is not clear to me what Lmax is.
Yes, once a gain we will change this part to simplify the algorithm 
description and give the general PSS for n controlled queues later in 
the draft.
>
> ** Globally: you should add a section for acronyms (e.g., section 1.4 to create).
>
> ** s/until the priority change/until the priority changes/
>
> ** s/This highlight/This highlights/
>
> ** About Figure 2 and Figure 3: I don't understand. It shows (y-axis) the evolution of credit[q] over
> time (x-axis). I don't understand where's the priority change. There's no such axis.
> I don't understand what the @@@ and ooo refer to (different periods?).
> I guess that the vertical lines refer to a network condition change, but which one?
Following David Black advice, we pushed a draft with SVG figs following 
RFC7996 recommendations which allows us to suppress these unclear ASCII 
drawings.
>
> Section 2.2 "Implementation":
>
> ** In "The new dequeuing algorithm is presented in the PSS Algorithm." please an explicit reference to Fig. 4.
Ok
>
> ** In Fig. 4, instead of "credits" (and all other input arguments where a final "s" is added), use
> credit[] to remind it's a table.
> Also, it seems that in "credit[q] = max(credit[q]-dtime[q].C.BWs[q],0)" the « . »  are in fact «  * ».
ok
>
> Section 3.1 "Motivations"
>
> ** s/Thank to the increase predictability/Thanks to the increased predictability/
>
>
> Section 6.2:
>
> ** Remove "<acceptedPaper>" but add a link to http://oatao.univ-toulouse.fr/18448/ open archive.
>
>
ok

Thanks again Vincent !

EL

-- 
Emmanuel LOCHIN
Professeur ISAE
ISAE SUPAERO - Institut Supérieur de l'Aéronautique et de l'Espace
10 avenue Edouard Belin - BP 54032 - 31055 TOULOUSE CEDEX 4 FRANCE -
http://www.isae-supaero.fr
Tel +33 5 61 33 84 85 - Fax (+33) 5 61 33 83 45
Web : http://personnel.isae.fr/emmanuel-lochin/