Re: [MMUSIC] Review of draft-ietf-mmusic-dtls-sdp-22

"Carlos Pignataro (cpignata)" <cpignata@cisco.com> Thu, 30 March 2017 16:13 UTC

Return-Path: <cpignata@cisco.com>
X-Original-To: mmusic@ietfa.amsl.com
Delivered-To: mmusic@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id B61F9129631; Thu, 30 Mar 2017 09:13:56 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.523
X-Spam-Level:
X-Spam-Status: No, score=-14.523 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_HI=-5, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RP_MATCHES_RCVD=-0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=cisco.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id szimti7CGMRy; Thu, 30 Mar 2017 09:13:54 -0700 (PDT)
Received: from alln-iport-7.cisco.com (alln-iport-7.cisco.com [173.37.142.94]) (using TLSv1.2 with cipher DHE-RSA-SEED-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id C829612997E; Thu, 30 Mar 2017 09:13:52 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=5288; q=dns/txt; s=iport; t=1490890432; x=1492100032; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=rJB4pQa5I/S7VqwMNoWxZOViQg5tF/Mp+EXm5eOD+lc=; b=avY70Sh7Df6t3ZfbNUyW6a65ZIGjHx0cL+eT/7yZcbgGkJ0mEomeQcLU TCe/cWRp5FKUC884stiGS2pUEHpFP0G2tlW1N7YBKs8Pi/pZM5OnkEHJG phibfTa22MGZ5PnNXglmooaE3Y+SAqxv2CCzASGsV4JDPc+6H7MqZxVge Q=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: A0BaAgBoLt1Y/4cNJK1dGQEBAQEBAQEBAQEBBwEBAQEBg1aBbAeDW4oRkTMflVCCDoYiAhqDHD8YAQIBAQEBAQEBayiFFQEBAQECASMEDUUFCwIBCBgCAiYCAgIwFRABAQQOBYoDCK4KgWw6ilUBAQEBAQEBAQEBAQEBAQEBAQEBAQEdgQuFQ4IFCIFZgQmHWi6CMQEEiSKMdoZSAZJPgXyFKooRiFmLEwEfOIEFWxVSAYRHHYFjdYgHgQ0BAQE
X-IronPort-AV: E=Sophos;i="5.36,247,1486425600"; d="scan'208";a="405245993"
Received: from alln-core-2.cisco.com ([173.36.13.135]) by alln-iport-7.cisco.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 30 Mar 2017 16:13:36 +0000
Received: from XCH-RTP-017.cisco.com (xch-rtp-017.cisco.com [64.101.220.157]) by alln-core-2.cisco.com (8.14.5/8.14.5) with ESMTP id v2UGDa6k022073 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Thu, 30 Mar 2017 16:13:36 GMT
Received: from xch-rtp-020.cisco.com (64.101.220.160) by XCH-RTP-017.cisco.com (64.101.220.157) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Thu, 30 Mar 2017 12:13:35 -0400
Received: from xch-rtp-020.cisco.com ([64.101.220.160]) by XCH-RTP-020.cisco.com ([64.101.220.160]) with mapi id 15.00.1210.000; Thu, 30 Mar 2017 12:13:35 -0400
From: "Carlos Pignataro (cpignata)" <cpignata@cisco.com>
To: Christer Holmberg <christer.holmberg@ericsson.com>
CC: "draft-ietf-mmusic-dtls-sdp.all@ietf.org" <draft-ietf-mmusic-dtls-sdp.all@ietf.org>, "ietf@ietf.org" <ietf@ietf.org>, "mmusic@ietf.org" <mmusic@ietf.org>
Thread-Topic: Review of draft-ietf-mmusic-dtls-sdp-22
Thread-Index: AdKn2P/3kP9b7RTPScO/+C/SvNdcJgBuRuYA
Date: Thu, 30 Mar 2017 16:13:35 +0000
Message-ID: <8F8DC522-9FF6-4414-9E02-95211155A87C@cisco.com>
References: <7594FB04B1934943A5C02806D1A2204B4CB3298B@ESESSMB109.ericsson.se>
In-Reply-To: <7594FB04B1934943A5C02806D1A2204B4CB3298B@ESESSMB109.ericsson.se>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.82.169.56]
Content-Type: text/plain; charset="utf-8"
Content-ID: <92CD995A2BABA04794D28942B85EE1CC@emea.cisco.com>
Content-Transfer-Encoding: base64
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/mmusic/3OqtznpymXEHKNFkSw8mdAY3Tug>
Subject: Re: [MMUSIC] Review of draft-ietf-mmusic-dtls-sdp-22
X-BeenThere: mmusic@ietf.org
X-Mailman-Version: 2.1.22
Precedence: list
List-Id: Multiparty Multimedia Session Control Working Group <mmusic.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/mmusic>, <mailto:mmusic-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/mmusic/>
List-Post: <mailto:mmusic@ietf.org>
List-Help: <mailto:mmusic-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/mmusic>, <mailto:mmusic-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 30 Mar 2017 16:13:57 -0000

Hi, Christer,

> On Mar 28, 2017, at 12:00 PM, Christer Holmberg <christer.holmberg@ericsson.com> wrote:
> 
> Hi Carlos,
> 
> Thanks for your review! Please see inline.

Anytime — thanks for the follow-up!

> 
>> Reviewer: Carlos Pignataro
>> Review result: Has Nits
>> 
>> This document is very comprehensive. Operational Considerations are
>> adequately covered.
>> 
>> In reviewing this document, I did find two adjacent issues that I
>> thought useful to comment on:
>> 
>> 1. Clarity and Readability of Section 9
>> 
>> I appreciate the explicit OLD/NEW details and specifics on what is
>> changed on the updated RFCs. I wish more documents would do this!
>> 
>> However, the way in which this is done is very confusing and not
>> really optimizing clarity and readability. It is an operational issue
>> an implementor not understanding the spec :-)
>> 
>> The issue, in my view, is with the labels and markers. Subsections of
>> Section 9.2 do not follow the semantic structure of the document.
>> Instead they are included as follows:
>> "
>> Update to section 5:
>> --------------------
>> "
>> Which are then followed by OLD/NEW chunks. However, these chunks:
>> * include Section numbers and titles, 
>> * do not have extra indentation, and
>> * include only BEGIN marker but not END marker.
>> 
>> Like:
>> 
>> 9.2.  Update to RFC 5763
>> Update to section 5:
>> --------------------
>> OLD TEXT:
>> 5.  Establishing a Secure Channel
>> 
>> [... and then, two pages later ...]
>> 
>> NEW TEXT:
>> 5.  Establishing a Secure Channel
>> 
>> I'd suggest:
>> a. Using Section 9.2.1, 9.2.2, etc. for each change.
> 
> I can put each section change in a separate section.
> 
> 9.2.1 Update to section 5
> 9.2.2 Update to section 6.6
> ...
> 
> Or, do you want to have the old and new text in different sections too? Personally I would like to keep the old and new text in the same section.

I agree. Having a section for each change chunk (OLD/NEW) seems most clean and clear.

> 
>> b. Use more explicit chunk demarkators
> 
> It was been suggested to use "|". 
> 
>> c. Use beginning and ending markers.
> 
> [BEGIN]
> Blah blah blah...
> [END]
> 
> ----------

Thank you — any format that you feel will add clarity. Right now it was a bit challenging to parse.

> 
>> 2. The second issue, and likely this was discussed, relates to the use
>> of RFC 4572. A reference to RFC 4572 is Normative, and it is cited
>> within "NEW" text (not only "OLD" text). However RFC 4572 has been
>> Obsoleted by RFC 8122!
>> 
>> This is because draft-ietf-mmusic-4572-update published as RFC 8122,
>> which should be updated. 
> 
> Correct. I will do that in the next version.

Ack. Thanks.

> 
>> But for example, why does NEW text here still points to RFC 4572?
> 
> The reason is probably that, initially draft-4572-update did not obsolete RFC 4572 - it simply updated it. But, later it was agreed that draft-4572-update will obsolete RFC 4572, but that was not reflected in draft-dtls-sdp.
> 
> So, you are correct, the new text shall point to RFC 8122. I will fix that in the next version.
> 

Perfect.

> ---------
> 
>> --->8---
>> NEW TEXT:
>> 
>> 5.  Establishing a Secure Channel
>> 
>>  The two endpoints in the exchange present their identities as part
>>  of the DTLS handshake procedure using certificates. This document
>>  uses certificates in the same style as described in "Connection-Oriented
>>  Media Transport over the Transport Layer Security (TLS) Protocol
>>  in the Session Description Protocol (SDP)" [RFC4572].
>> --->8---
>> 
>> And why RFC 4572 is Normatively referenced?
> 
> I will make the reference Informative.
> 

Looks good — thanks again,

— Carlos.

> Thanks!
> 
> Regards,
> 
> Christer
>