Subject: Re: [tsvwg] Comments on draft-ietf-tsvwg-sctp-ndata-08

From: Michael Tuexen <[email-masked]>
Date: Sun, 12 Mar 2017 23:15:39 +0100

> On 14 Dec 2016, at 14:08, Michael Welzl <[email-masked]> wrote:
> 
> Hi all,
> 
> I also read the document now. 90% editorial / nits below.
> I do think this is a very valuable document!  (hence also my comment: why does the text say that this is “off by default” ?)
Hi Michael,

thank you very much for your review. See my comments in-line.

Best regards
Michael
> 
> 
> Cheers,
> Michael
> 
> 
> ---
> 
> Shouldn't it say "arbitrarily" in:
> 
> "The Stream Control Transmission Protocol (SCTP) is a message oriented
>  transport protocol supporting arbitrary large user messages."
Fixed. 
> 
> 
> 1.1: I think this misses ", this can" after the reference to RFC4960:
> "When such large messages are sent over
>  SCTP as specified in [RFC4960] can result in a form..."
It was already fixed by using:

The sending of such large messages over SCTP as specified in
<xref target='RFC4960'/> can result in a form
> 
> 
> It should be "Figure 1" or "The following figure", I think, in:
> " The following Figure 1 illustrates the behaviour of a round robin
>  stream scheduler using DATA chunks."
Fixed by using "Figure 1".
> 
> Fig.1 and fig.2: I'm guessing that SID is Stream Identifier? I don't think it's defined / explained before the figure, it wouldn't hurt to remind the reader what that means. I expected it to be defined in RFC4960 at least, but the abbreviation list there doesn't contain "SID" either.
Added a clarification:
<t><xref target='RRwithoutMI'/> illustrates the behaviour of a round robin
stream scheduler using DATA chunks when three streams with the Stream
Identifiers (SIDs) 0, 1, and 2 are used.
> 
> Suggest to remove comma after "should be noted" in:
> "It should be
>  noted, that an SCTP implementation needs to support the coexistence
>  of associations using DATA chunks and associations using I-DATA
>  chunks."
Fixed.
> 
> section 2.2.2:
> "Please note" strikes me as a bit odd for an RFC, here:
> "Please note that this is the only form of interleaving support."
Hmm. We have used it also in other RFCs. For example in the second paragraph of
https://tools.ietf.org/html/rfc6525#section-6

Will keep it for now.
> 
> This sentence seems a bit broken:
> " Upon reception of an SCTP packet containing an I-DATA chunk if the
>  message needs to be reassembled, then the receiver MUST use the FSN
>  for reassembly of the message and not the TSN.  "
> I'd fix this to: "If, upon reception of ...., the message needs..."
The issue is already fixed by using:
<t>Upon reception of an SCTP packet containing an I-DATA chunk whose
user message needs to be reassembled, the receiver MUST use the FSN
for reassembly of the message and not the TSN.
> 
> 2.3.1: missing "to" after "refers" in:
> "The U bit specifies if the Message Identifier of this entry refers
>     unordered messages"
Fixed.
> 
> 
> sec 3: "The set
>   of schedulers being implemented is implementation dependent.”
> this seems a little odd to me. Maybe this is the place for clearer, 2119-style language? To say that the following is a set of possible schedulers, any of them MAY be implemented?

> 
> sec 3.3: this description isn’t clear enough, I think - the use of “bundles” here confuses me… not sure how this should be implemented?
You basically switch only to a new stream when you start a new packet. This ensures
that only one stream is affected by a packet loss. Any suggestion on how to improve the text?
> 
> 3.4: again, the use of “implementation specific” here seems strange - I’d rather say: “, … any form of scheduling between them MAY be used.”
OK. Changed to
An implementation MAY implement any subset of them.
> 
> Much here is a bit vague… e.g. in 3.6:
> "schedules them in a certain way to use the bandwidth according to the given weights.  The details are implementation specific.”
> 
> I understand that you don’t want to prescribe details on how to implement the WFQ scheduler, but “in a certain way to us the bandwidth according to the given weights” doesn’t even precisely prescribe the resulting behavior. If you leave both the implementation and the resulting behavior open, this section may be worthless?
> To be clearer about my concern: the priorities could, for example, be linear or exponential. E.g., priority 3 could mean 3 times as much bandwidth as priority 1. But, for example, https://tools.ietf.org/html/draft-ietf-rtcweb-transports-17#section-4.1 specifies this differently: “... SHOULD cause data to be emitted in such a way that each stream at each level of priority is being given approximately twice the transmission capacity (measured in payload bytes) of the level below.”   So, IMO, the precise meaning of priorities must be clear from the text in this draft.
Right. I added:
If the weight of stream S1 is n times the weight of stream S2, the
scheduler should assign to stream S1 n times the bandwidth it assigns to
stream S2.
> 
> Summarizing for section 3, it would be good to add an example illustrating the expected behavior (a bit like Fig.1) for each of the schedulers.
I think this is hard to do and would over-specify the scheduler. Therefore I'm not adding them.
> 
> Also: as shown by figure 1, these stream schedulers can operate without interleaving too. This doesn’t get very clear … the introduction should explain that this document also describes stream schedulers, how they are exposed in the sockets API, and how they would operate with and without interleaving.
Yes. I changed the text in the introduction to read:
<t>This document also defines several stream schedulers for general SCTP
associations. They can be used with and without user message interleaving
being negotiated and possibly behave differently.</t>
> 
> Section 4.3.1 says: "   User message interleaving is disabled per default.”
> Isn’t this a bad idea? This just seems to me to make things better, what’s the reason not to always use it?
> Come to think of it - if there indeed is a trade-off here, a good reason to sometimes *not* use this mechanism, this should be explained, probably in the introduction.
> (I could only think of slightly more header overhead - is this such a big deal?)
I added the following text to explain why it is off by default. Since this is a API argument
it belongs here. If you have an implementation with a different API, you might enable it by
default.

I moved the text specifying the default down after the discussion of the fragment interleave level and added there:

Since the default fragment interleave level is not 2, user message interleaving
is disabled per default.</t>
> 
> 4.3.2: is this possible with or without interleaving? I’d guess it’s possible without, but it would be nice to have this explicit, as somehow this whole document reads as if it’s really about interleaving.
Sure. There is no restriction. Only the schedulers behave differently when user message interleaving has
been negotiated as indicated in the text describing each scheduler.

---

From: Michael Welzl <[email-masked]>
Date: Mon, 13 Mar 2017 09:24:15 +0100

Hi,

> On 12 Mar 2017, at 23:15, Michael Tuexen <[email-masked]> wrote:
> 

I carefully read how you addressed my comments and I'm happy with all of these fixes.

> that only one stream is affected by a packet loss. Any suggestion on how to improve the text?

I would suggest:

OLD:
***
This is a round-robin scheduler but bundles only DATA or I-DATA
chunks referring to the same stream in a packet.  This minimizes
head-of-line blocking when a packet is lost because only a single
stream is affected.
***

NEW:
***
This is a round-robin scheduler which only switches streams when
starting to fill a new packet. It bundles only DATA or I-DATA
chunks referring to the same stream in a packet.  This scheduler
minimizes head-of-line blocking when a packet is lost because only
a single stream is affected.
***

Cheers,
Michael

---

From: Michael Tuexen <[email-masked]>
Date: Mon, 13 Mar 2017 10:42:48 +0100

> On 13 Mar 2017, at 09:24, Michael Welzl <[email-masked]> wrote:
> 
> Hi,
> 
OK. Text taken.

Thank you very much.

Best regards
Michael