Subject: [Gen-art] Review of draft-ietf-mmusic-sctp-sdp-22

From: Brian Carpenter <[email-masked]>
Date: Sat, 04 Feb 2017 19:23:11 -0800

Reviewer: Brian Carpenter
Review result: Ready with Nits

Gen-ART Last Call review of draft-ietf-mmusic-sctp-sdp-22

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Document: draft-ietf-mmusic-sctp-sdp-22.txt
Reviewer: Brian Carpenter
Review Date: 2017-02-XX
IETF LC End Date: 2017-02-09
IESG Telechat date:  

Summary: Ready with nits
--------

Comments:
---------

Two points I noted in the writeup:
"There are existing implementations of earlier versions of the
document..."

Excellent, but I wonder why we don't see Implementation Status
sections
under RFC 6982 in more Last Call drafts.

"IPv6 address examples are not necessary since IP version differences
are immaterial to the purpose of the specification."

It's just as easy to give an IPv6 example though, and more future
proof.

Minor issue: (almost a nit)
------------

> 1.  Introduction
...
>   NOTE: Due to the characteristics of TCP, usage of 'TCP/DTLS/SCTP'
>   will always force ordered and reliable delivery of the SCTP
packets,
>   which limits the usage of the SCTP options.  Therefore, it is
>   RECOMMENDED that TCP is only used in situations where UDP traffic
is
>   blocked.

Why would one choose 'TCP/DTLS/SCTP' rather than just 'TCP/TLS'? I
don't
object to it being specified, but since you don't support multihoming
or multiple associations, what is the use case, in a few words?

Nits:
-----

> 4.4.2.  SDP Media Description values
>
>      m= line parameter        parameter value(s)
>     
------------------------------------------------------------------
>      <media>:                 "application"
>      <proto>:                 "UDP/DTLS/SCTP" or "TCP/DTLS/SCTP"
>      <port>:                  UDP port number (for "UDP/DTLS/SCTP")
>                               TCP port number (for
""UDP/DTLS/SCTP")

I think the last line should be: TCP port number (for
"TCP/DTLS/SCTP")

There is some inconsistency in the use of quotation marks:
"UDP/DTLS/SCTP"
or 'UDP/DTLS/SCTP'

---

From: Christer Holmberg <[email-masked]>
Date: Sun, 5 Feb 2017 14:14:05 +0000

Hi Brian,

Thanks for your review! Please see inline.

...

Comments:
---------

>"There are existing implementations of earlier versions of the document..."
>
>Excellent, but I wonder why we don't see Implementation Status sections under RFC 6982 in more Last Call drafts.

Perhaps a topic for an IETF chairs lunch presentation?

I have to admit that I did not know about 6982 (now obsoleted by 7942). My suggestion would to not collect that information for this draft at this point of time, as it could take some time.

>"IPv6 address examples are not necessary since IP version differences are immaterial to the purpose of the specification."
>
>It's just as easy to give an IPv6 example though, and more future proof.

I can modify the address in the example, from IPv4 to v6.

1.  Introduction
...
>>   packets, which limits the usage of the SCTP options.  Therefore, it is
>>   is blocked.
> Why would one choose 'TCP/DTLS/SCTP' rather than just 'TCP/TLS'? I don't object to it being specified, but since you 
> don't support multihoming or multiple associations, what is the use case, in a few words?

SCTP supports multiple SCTP streams over a single association, and you can still use that with 'TCP/DTLS/SCTP'. E.g, the WebRTC Data Channel protocol uses two streams to realize a data channel.

I could modify the first sentence as following:

        "NOTE: Due to the characteristics of TCP, while multiple SCTP streams
         can still be used, usage of 'TCP/DTLS/SCTP' will always force...."

Nits:
-----

>>                                     TCP port number (for ""UDP/DTLS/SCTP")
>
> I think the last line should be: TCP port number (for "TCP/DTLS/SCTP")

Correct. Will be fixed.

>There is some inconsistency in the use of quotation marks: "UDP/DTLS/SCTP" or 'UDP/DTLS/SCTP'

I'll fix that. I will use single quotes.

Regards,

Christer

---

From: Brian E Carpenter <[email-masked]>
Date: Mon, 6 Feb 2017 08:05:40 +1300

Hi Christer,
> Perhaps a topic for an IETF chairs lunch presentation?

Or at least a short reminder!

> I have to admit that I did not know about 6982 (now obsoleted by 7942). My suggestion would to not collect that information for this draft at this point of time, as it could take some time.

Agreed.

> I can modify the address in the example, from IPv4 to v6.

As you wish.

> Minor issue: (almost a nit)

OK.

(I now remember that I raised essentially the same question when reviewing
draft-ietf-clue-datachannel. Actually I'm glad to see SCTP proving useful.)

>          can still be used, usage of 'TCP/DTLS/SCTP' will always force...."

OK

> I'll fix that. I will use single quotes.

Thanks,
    Brian

---

From: Christer Holmberg <[email-masked]>
Date: Mon, 6 Feb 2017 09:15:50 +0000

Hi,

I¹ve created a pull request with the changes based on Brian¹s comments.

https://github.com/cdh4u/draft-sctp-sdp/pull/10

Regards,

Christer

On 05/02/17 21:05, "Brian E Carpenter" <[email-masked]> wrote:

>>>sections under RFC 6982 in more Last Call drafts.
>> 
>> I have to admit that I did not know about 6982 (now obsoleted by 7942).
>>My suggestion would to not collect that information for this draft at
>>this point of time, as it could take some time.
>
>Agreed.
>
>>>don't object to it being specified, but since you
>> SCTP supports multiple SCTP streams over a single association, and you
>>can still use that with 'TCP/DTLS/SCTP'. E.g, the WebRTC Data Channel
>>protocol uses two streams to realize a data channel.
>
>OK.
>
>>>"UDP/DTLS/SCTP" or 'UDP/DTLS/SCTP'
>>