Subject: [MMUSIC] [Gen-art] Gen-ART Last Call review of draft-ietf-mmusic-dtls-sdp-22

From: Paul Kyzivat <[email-masked]>
Date: Mon, 27 Mar 2017 14:09:31 -0400

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-dtls-sdp-22
Reviewer: Paul Kyzivat
Review Date: 2017-03-27
IETF LC End Date: 2017-04-06
IESG Telechat date: TBD

Summary:

This draft is basically ready for publication, but has some minor issues 
and nits that should be fixed before publication.

(I reviewed this previously as part of LC and had no comments. After 
being asked to do a Gen-Art review I went through it more carefully. I 
surprised myself by finding a few thing, though nothing major. This 
confirms my feeling that I can *always* find *something* to improve in a 
draft.)

Issues:

Major: 0
Minor: 1
Nits:  3

(1) Nit:

Regarding the following in section 5.1:

    When an offerer or answerer indicates that it wants to establish a
    new DTLS association, it needs to make sure that media packets in the
    existing DTLS association and new DTLS association can be de-
    multiplexed.

This text presumes there is an existing association. To explicitly cover 
the case where there is not, I suggest the following:

    When an offerer or answerer indicates that it wants to establish a
    new DTLS association to replace an existing association, it needs to
    ensure that media packets in the existing DTLS association and new
    DTLS association can be de-multiplexed.

Later in the section there is a language error is the following:

    The certificate received during the DTLS handshake MUST match a
    certificate fingerprints received in SDP 'fingerprint' attributes
    according to the procedures defined in [I-D.ietf-mmusic-4572-update].

s/match a/match the/

OR

s/certificate fingerprints/certificate fingerprint/

(2) Nit:

In Section 5.4 there is again a presumption of an existing association 
in the following:

    If the answer does not establish a new DTLS association, the offerer
    will continue using the previously established DTLS association.

To fix, I suggest:

    If the offer indicated a desire to reuse an existing DTLS association
    and the answer does not request establishment of a new DTLS
    association, the offerer will continue using the previously
    established DTLS association.

(3) Minor:

I concur with the comments in the ops-dir review by Carlos Pignataro 
regarding the formatting of section 9. He didn't suggest a fix. Perhaps 
some special marker (e.g. "|" or "<" and ">") can be placed in every 
line to indicate it is test from or for another document - either at the 
beginning or end of every line.

(4) Nit:

In Section 9:

The following text is repeated multiple times:

    [RFC EDITOR NOTE: Please replace RFCXXXX with the RFC number
    of this document.]

It would be sufficient and less distracting to the user to simply state 
this once for the entire document.

---

From: Christer Holmberg <[email-masked]>
Date: Mon, 27 Mar 2017 23:42:05 +0000

Hi Paul,

Thanks for your review! Please see inline.

(1) Nit:

>This text presumes there is an existing association. To explicitly cover the case where there is not, I suggest the following:
>

I could do that. Or, I could use say "make sure that media packets in *any* existing DTLS association"

>s/certificate fingerprints/certificate fingerprint/

That was the intention, so I will fix it (s/certificate fingerprints/certificate fingerprint/).

(2) Nit:

>In Section 5.4 there is again a presumption of an existing association in the following:
>

I will fix as suggested.

(3) Minor:

>I concur with the comments in the ops-dir review by Carlos Pignataro regarding the formatting of 
>section 9. He didn't suggest a fix. Perhaps some special marker (e.g. "|" or "<" and ">") can be placed 
>in every line to indicate it is test from or for another document - either at the beginning or end of every line.

I have never seen that been used before - not in documents I have authored, or in documents written by others.

(4) Nit:

>It would be sufficient and less distracting to the user to simply state this once for the entire document.

I will fix as suggested.

Regards,

Christer

---

From: Roman Shpount <[email-masked]>
Date: Mon, 27 Mar 2017 19:57:59 -0400

Hi All,

> *any* existing DTLS association"

If we are nitpicking we need to define what "existing" DTLS association
means here. In this particular case this means any DTLS association for
which it still possible to receive packets. This includes currently
established DTLS associations as well as any DTLS association which were
closed but can still receive packets due to network transmission delays.

Regards,
_____________
Roman Shpount

---

From: Paul Kyzivat <[email-masked]>
Date: Mon, 27 Mar 2017 23:55:21 -0400

Hi Christer,

[trimming]

> I could do that. Or, I could use say "make sure that media packets in *any* existing DTLS association"

That would also be fine. I have no preference.

> I have never seen that been used before - not in documents I have authored, or in documents written by others.

Yes, I know. I was just trying to be constructive by suggesting 
*something*. My first thought was to indent. But that would require 
reflowing all the text to avoid exceeding line length. That seemed like 
a bad idea.

I'm not attached to this solution. I've complained about the same 
problem in other drafts, but nobody came up with a solution so they 
didn't get resolved. Here I'm not the only one who sees a problem, so 
maybe it is worth trying to find a solution.

	Thanks,
	Paul

---

From: Christer Holmberg <[email-masked]>
Date: Tue, 28 Mar 2017 14:40:42 +0000

Hi,

...

(3) Minor:

>>> regarding the formatting of section 9. He didn't suggest a fix. 
>>> Perhaps some special marker (e.g. "|" or "<" and ">") can be placed in every line to indicate it 
>>> is test from or for another document - either at the beginning or end of every line.
>>
> Yes, I know. I was just trying to be constructive by suggesting *something*. My first thought was to indent. 
>
> But that would require reflowing all the text to avoid exceeding line length. That seemed like a bad idea.
>
> I'm not attached to this solution. I've complained about the same problem in other drafts, but nobody came
> up with a solution so they didn't get resolved. Here I'm not the only one who sees a problem, so maybe it is 
> worth trying to find a solution.

I don't have a problem to add e.g. "|", if that's what people want to do, but my preference would be to not do anything in this draft, at this point of time.

Regards,

Christer

---

From: Christer Holmberg <[email-masked]>
Date: Tue, 28 Mar 2017 14:44:57 +0000

Hi Roman,

What about:

“…media packets in the any previously established DTLS association and…”

That covers both the currently established and already closed associations.

Regards,

Christer

From: Roman Shpount [mailto:[email-masked]]
Sent: 28 March 2017 02:58
To: Christer Holmberg <[email-masked]>
Cc: Paul Kyzivat <[email-masked]>; [email-masked]; General Area Review Team <[email-masked]>; IETF MMUSIC WG <[email-masked]>
Subject: Re: [MMUSIC] [Gen-art] Gen-ART Last Call review of draft-ietf-mmusic-dtls-sdp-22

Hi All,

If we are nitpicking we need to define what "existing" DTLS association means here. In this particular case this means any DTLS association for which it still possible to receive packets. This includes currently established DTLS associations as well as any DTLS association which were closed but can still receive packets due to network transmission delays.

Regards,
_____________
Roman Shpount

---

From: Paul Kyzivat <[email-masked]>
Date: Tue, 28 Mar 2017 11:26:37 -0400

> I don't have a problem to add e.g. "|", if that's what people want to do, but my preference would be to not do anything in this draft, at this point of time.

Since Carlos Pignataro also commented on this in a different review, it 
would be good to see what he thinks. (He isn't on *this* thread.)

Also, what do others in the WG think?

	Thanks,
	Paul

---

From: Roman Shpount <[email-masked]>
Date: Tue, 28 Mar 2017 13:00:28 -0400

Hi Christer,

This sounds good to me.

Regards,

_____________
Roman Shpount

On Tue, Mar 28, 2017 at 10:44 AM, Christer Holmberg <
[email-masked]> wrote:

> *From:* Roman Shpount [mailto:[email-masked]]
> *Sent:* 28 March 2017 02:58
> *To:* Christer Holmberg <[email-masked]>
> *Cc:* Paul Kyzivat <[email-masked]>; draft-ietf-mmusic-dtls-sdp.
> [email-masked]; General Area Review Team <[email-masked]>; IETF MMUSIC WG
> <[email-masked]>
> *Subject:* Re: [MMUSIC] [Gen-art] Gen-ART Last Call review of
> draft-ietf-mmusic-dtls-sdp-22
>
>
>
> Hi All,
>
>
>
>