Subject: Re: [tsvwg] I-D Action: draft-ietf-tsvwg-sctp-ndata-05.txt

From: Marcelo Ricardo Leitner <[email-masked]>
Date: Thu, 19 May 2016 17:03:35 -0300

Em 21-03-2016 12:56, [email-masked] escreveu:
>
> A New Internet-Draft is available from the on-line Internet-Drafts directories.
> This draft is a work item of the Transport Area Working Group of the IETF.
>
>         Title           : Stream Schedulers and User Message Interleaving for the Stream Control Transmission Protocol
>         Authors         : Randall R. Stewart
>                           Michael Tuexen
>                           Salvatore Loreto
>                           Robin Seggelmann
> 	Filename        : draft-ietf-tsvwg-sctp-ndata-05.txt
> 	Pages           : 18
> 	Date            : 2016-03-21
>
...
> The IETF datatracker status page for this draft is:
> https://datatracker.ietf.org/doc/draft-ietf-tsvwg-sctp-ndata/
>
> There's also a htmlized version available at:
> https://tools.ietf.org/html/draft-ietf-tsvwg-sctp-ndata-05
>
> A diff from the previous version is available at:
> https://www.ietf.org/rfcdiff?url2=draft-ietf-tsvwg-sctp-ndata-05
...

Hi Michael, tsvwg,

Seems there is a typo in this section:

4.1.  SCTP_ASSOC_CHANGE Notification

    When an SCTP_ASSOC_CHANGE notification is delivered indicating a
    sac_state of SCTP_COMM_UP or SCTP_RESTART for an SCTP association
    where both peers support the I-DATA chunk,
    SCTP_ASSOC_SUPPORTS_INTERLEAVING should be listen in the sac_info
    field.

It should read "listed" instead of "listen", right?

Thanks,
Marcelo

---

From: Michael Tuexen <[email-masked]>
Date: Thu, 19 May 2016 22:32:44 +0200

> On 19 May 2016, at 22:03, Marcelo Ricardo Leitner <[email-masked]> wrote:
> 
Yes it should. I fixed that a couple of weeks ago:
https://github.com/sctplab/sctp-idata/commit/2609a4e7a9e9dfb77891b2e2dfe4e0ae4e261544
Thanks for reporting it!
Best regards
Michael

---

From: Marcelo Ricardo Leitner <[email-masked]>
Date: Fri, 20 May 2016 19:14:09 -0300

> https://github.com/sctplab/sctp-idata/commit/2609a4e7a9e9dfb77891b2e2dfe4e0ae4e261544

Ahh, cool.

On this other paragraph, on section 2.2.2:
"""
    The sender MUST NOT be fragmenting more than one ordered message in
    any one stream at any time.  The sender MUST NOT be fragmenting more
    than one un-ordered user message in any one stream at any time.  The
    sender MAY fragment one ordered and one unordered user message within
    a single stream.  At any time a sender MAY fragment an ordered and an
    unordered user message each off them on different streams.
"""
(seems still like that in the git repo above)

"each off" -> "each of"
"un-ordered" -> "unordered"

The wording "in any one stream" seems weird to me but it may be just me, 
I'm not a native speaker. "in any given stream" perhaps?

It's not clear to me in that paragraph if we can fragment more than one 
ordered message if they are on multiple streams? Same for unordered 
messages.

Thanks,
Marcelo

---

From: Marcelo Ricardo Leitner <[email-masked]>
Date: Fri, 20 May 2016 20:10:19 -0300

Em 20-05-2016 19:14, Marcelo Ricardo Leitner escreveu:
>>>>        Title           : Stream Schedulers and User Message
>>>> Interleaving for the Stream Control Transmission Protocol

Hi Michael,

Some other suggestions/doubts below.

On 2.1.  The I-DATA Chunk supporting User Message Interleaving:

"The sender uses two counters for
each outgoing streams, one for ordered messages, one for unordered
messages.  All counters are independent"
   ->
  s/streams/stream/
  s/All/Both/

On 2.2.3.  Receiver Side Considerations:
"Note that a non-
    fragmented messages is indicated by the fact that both the 'E' and
    'B' bits are set.  An ordered or unordered fragmented message is thus
    identified with any message not having both bits set.
"

Two things:
"Note that a non-fragmented messages" ->
    "Note that non-fragmented messages"

"is thus identified with any message not having both bits set" ->
   "is thus identified by any not having both bits set"

On 2.3.1.  SCTP Partial Reliability Extension
"When the SCTP extension defined in [RFC3758] is used, the the 
I-FORWARD-TSN ..."
"the" is there twice

On 3.1.  Stream Scheduler without User Message Interleaving Support
This title is confusing if compared to section 3. paragraph as the title 
says these schedules doesn't support user message interleaving, while 
section 3 paragraph says they do support but may be used without it.
Which one is correct? If the paragraph, then maybe something like 
"Stream Scheduler without User Message Interleaving Support requirement"
Same applies to 3.2 section header.

What would happen if I select like SCTP_SS_RR and the association does 
support User Message Interleaving, should it become SCTP_SS_RR_INTER 
automatically? I guess not but as it is worded it seems it could evolve 
if I-DATA support is detected.

On 3.1.3.  Round Robin Scheduler per Packet(SCTP_SS_RR_PKT)

"This is a round-robin scheduler but only bundles user messages"
  s/only//

On 3.1.4.  Priority Based Scheduler (SCTP_SS_PRIO)

What's the default scheduler mentioned in there? And what if the default 
if SCTP_SS_PRIO again?

On 4.2.3.  Get or Set the Stream Scheduler Parameter
"Some schedulers require additional information to be set for single 
streams as ..."
   s/single/individual/

Thanks!
Marcelo

---

From: Michael Tuexen <[email-masked]>
Date: Sun, 22 May 2016 20:36:43 +0200

> On 21 May 2016, at 01:10, Marcelo Ricardo Leitner <[email-masked]> wrote:
> 
Fixed for the next revision.
> s/All/Both/
Well. We have two counters for each stream. They all are independent, not
only the ones for the same stream. So I think "All" is the correct term.
>   "Note that non-fragmented messages"
Fixed for the next revision.
>  "is thus identified by any not having both bits set"
I think
"is thus identified by not having both bits set"
is even better.
> "When the SCTP extension defined in [RFC3758] is used, the the I-FORWARD-TSN ..."
> "the" is there twice
Fixed for the next revision.
> This title is confusing if compared to section 3. paragraph as the title says these schedules doesn't support user message interleaving, while section 3 paragraph says they do support but may be used without it.
> Which one is correct? If the paragraph, then maybe something like "Stream Scheduler without User Message Interleaving Support requirement"
I see. I changed the title of 3.1 to
"Stream Schedulers Not Requiring User Message Interleaving Support"
and the title of 3.2 to
"Stream Schedulers Requiring User Message Interleaving Support"
> 
> What would happen if I select like SCTP_SS_RR and the association does support User Message Interleaving, should it become SCTP_SS_RR_INTER automatically? I guess not but as it is worded it seems it could evolve if I-DATA support is detected.
At least I hadn't any automagically change in mind.
Once an association is established, you can always select schedulers not requiring
user message interleaving. You can only select scheduler requiring user message
interleaving, if that is supported by the association.
If you don't do anything, the default scheduler will be used.
Before the association has been established, but user message interleaving
has locally been enabled, you can select schedulers all schedulers, especially
ones requiring user message interleaving. So if the association comes up
and the user message interleaving has not been negotiated, going back to the
default scheduler is the best option I think. The same applies to the restart case.
What do you think? This is tracked in
https://github.com/sctplab/sctp-idata/issues/10
> "This is a round-robin scheduler but only bundles user messages"
I want to make clear that only user messages of the same stream are bundled
in a packet.
Changed to
This is a round-robin scheduler but bundles only user messages of the same
stream in one packet.
Is that clearer?
> What's the default scheduler mentioned in there? And what if the default if SCTP_SS_PRIO again?
I see your point. Changed to

Streams having a higher priority will be scheduled first and when multiple streams
have the same priority, the scheduling between them is implementation specific.

I think this covers it.
> "Some schedulers require additional information to be set for single streams as ..."
>  s/single/individual/
Fixed for the next revision.

Thanks a lot for the feedback. Any plans to implement it in Linux?

Best regards
Michael

---

From: Michael Tuexen <[email-masked]>
Date: Sun, 22 May 2016 23:39:15 +0200

> On 21 May 2016, at 00:14, Marcelo Ricardo Leitner <[email-masked]> wrote:
> 
I committed the above fixes.
> 
> The wording "in any one stream" seems weird to me but it may be just me, I'm not a native speaker. "in any given stream" perhaps?
OK.
> 
> It's not clear to me in that paragraph if we can fragment more than one ordered message if they are on multiple streams? Same for unordered messages.
Yes, you can do that for messages on different streams. There was also a discussion via
e-mail if it should be possible to interleave an ordered and an unordered message
on the same stream. The result was that this is not necessary and it is not supported
by the current interleaving level at the receiver side. So the text will change to
reflect this.
To be clear: It is allowed to interleave messages from different streams.

Best regards
Michael

---

From: Marcelo Ricardo Leitner <[email-masked]>
Date: Mon, 23 May 2016 14:22:19 -0300

> only the ones for the same stream. So I think "All" is the correct term.

Ah yes, considering the other streams too, okay.

> "is thus identified by not having both bits set"

Indeed.

> "Stream Schedulers Requiring User Message Interleaving Support"

Seems better.

> https://github.com/sctplab/sctp-idata/issues/10

Would it make sense to factor out the 'does User Message Interleaving' 
and make it an option aside of the Stream Scheduler itself? Or as a 
parameter of it. As the algorithms are pretty much the same ones and it 
just changes the selection method (as in based on I-DATA chunks instead 
of chunks).

Then it would just flip that bit off if necessary and keep using the 
selected scheduler, instead of falling back to a default value which may 
be very different from the intended one.

That's the way I'm planning on implementing it, btw. (internal to the 
implementation, while respecting the interfaces described in here)

> This is a round-robin scheduler but bundles only user messages of the same

Yup!

> have the same priority, the scheduling between them is implementation specific.

Okay.

> Thanks a lot for the feedback. Any plans to implement it in Linux?

Yes, starting it now. I expect to have something testable in a month or 
so. Thanks Michael.

Best regards,
Marcelo

---

From: Michael Tuexen <[email-masked]>
Date: Tue, 24 May 2016 09:27:08 +0200

> On 23 May 2016, at 19:22, Marcelo Ricardo Leitner <[email-masked]> wrote:
> 
> Would it make sense to factor out the 'does User Message Interleaving' and make it an option aside of the Stream Scheduler itself? Or as a parameter of it. As the algorithms are pretty much the same ones and it just changes the selection method (as in based on I-DATA chunks instead of chunks).
> 
> Then it would just flip that bit off if necessary and keep using the selected scheduler, instead of falling back to a default value which may be very different from the intended one.
This came also up in the discussion of issue 10 with Felix and I think this is the way to
go. We will specify how each scheduler behaves
* when used with user message interleaving enabled
* when used with user message interleaving disabled
It is clear to the app what happens and makes the scheduler selection simpler. I'll
put that in the next revision. We can refine it, it the wording needs to be improved.
> 
> That's the way I'm planning on implementing it, btw. (internal to the implementation, while respecting the interfaces described in here)
> 
>>> 
>>> 
> Yes, starting it now. I expect to have something testable in a month or so. Thanks Michael.
Great. Bring up any question you run into. We can also do some interop testing over the
Internet, if you want.

---

From: Marcelo Ricardo Leitner <[email-masked]>
Date: Tue, 24 May 2016 13:27:49 -0300

> put that in the next revision. We can refine it, it the wording needs to be improved.

Nice, ok.

> Great. Bring up any question you run into. We can also do some interop testing over the

Will certainly do :-)

Thanks,
Marcelo