[MMUSIC] Review of draft-ietf-mmusic-ice-sip-sdp

[Thomas Stach <[e-mail]>]

All,

I did a review of draft-ietf-mmusic-ice-sip-sdp.
Apart from some editorial stuff (at the end), my two main points are
- there is some text throughout the document, that in my opinion does 
not cover SDP/SIP issues,
   but rather some details of the ICE algorithm.
   Therefore it might be better to have that text in 
draft-ietf-ice-rfc5245bis.
   Details below
- There has been a lot of discussion about the updated offer.
   Is it the rough(?) consensus of the WG that it is still required?
   Note that for BUNDLE, which has similar issues, this requirement was 
removed.


Technical

Sections 4.1.1.1 .

  ... an updated offer/answer will be
    required after ICE processing completes in order to "fix up" the SDP ...

    and 4.1.5.1.1

  If any of those
    candidate pairs differ from the default candidate pairs in the most
    recent offer/answer exchange, the controlling agent MUST generate an
    updated offer as described in Section 4.2.

These are the sections that cover the updated offer.
they might need some changes dependent on what the WG's consensus on the 
issue.


Section 4.1.2.4
This text has nothing to do with SDP and should go to the end of section 
5.1.2. of draft-ietf-ice-rfc5245bis.
The same applies for Annex C, that gives the motivation.
BTW: The reference to 6.1.2.3. of ICE-BIS is wrong it should be 6.1.3.1

Section 4.1.4
Again, this text has nothing to do with SDP and should go to 
draft-ietf-ice-rfc5245bis (somewhere in section 6?).

Section 4.1.5
Except for the bit about forking and the updated offer I again think 
that this section should go to draft-ietf-ice-rfc5245bis as this section 
mostly describes ICE algorithm details and not SDP specifics.

Section 4.2.4
Once more this section describes details of the ICE algorithm and might 
be better suited for draft-ietf-ice-rfc5245bis.
However, the paragraph assumes that media streams are only added/removed 
via offer/answer, possibly together with an ICE restart. Since ICEbis 
should be usable by other protocols than SIP, moving the text from the 
above sections would also require a slightly more generic term than 
"offer/answer".

Section 5
I haven't checked up to 5.4 this since I assume it was a pure cut&paste 
from 5245.
Section 5.5 refers to section 11 of draft-ietf-ice-rfc5245bis, shouldn't 
that be section 12?

BTW: Section 11 of draft-ietf-ice-rfc5245bis talks about Extensibility 
Considerations. Shouldn't some text from there go into this document, 
e.g. the text about the a=ice-options attribute?

Section 10
Again I think this text belongs to draft-ietf-ice-rfc5245bis.




Editorial

Global

There is some variety how SDP attributes are written in the text.
I found
"There will be a candidate attribute for each candidate... "
"... it MUST include an "a=ice-lite" session-level attribute ..."
"... the "a=ice-options:ice2" SDP line ..."
You might want to consider doing some harmonization.

Similar with "offer", "answer", "SDP offer", "SDP answer"


Section 1
consider adding articles in first sentence (ditto in the Abstract)

old
.. is used with Session Description Protocol (SDP) offer/answer
    [RFC3264] and Session Initiation Protocol (SIP).
new
  ... is used with the Session Description Protocol (SDP) offer/answer
    [RFC3264] and the Session Initiation Protocol (SIP).

consider clarifying the following
old
     Note that ICE is not intended for NAT traversal for SIP, which is
     assumed to be provided via another mechanism [RFC5626].

new
     Note that ICE is only intended for NAT traversal of UDP-based 
multimedia
     and not intended for NAT traversal of SIP messages, which is
     assumed to be provided via another mechanism [RFC5626].


Section 2
The following reads a bit awkward.
old
     For the RTCP component, it is in the rtcp
       attribute when present, and when not present, the IP address is in
       the "c=" line and 1 plus the port is in the "m=" line.
new
For the RTCP component, it is in the rtcp
       attribute when present, and when not present, it is the same 
address and
    the next higher port number of the RTP candidate


Section 3

missing comma in last sentence, unnecessary article

old
     ... from the [ICE-BIS] respectively.
new
     ... from [ICE-BIS], respectively.

Section 4.1.1

could be misleading
old
        The offerer shall follow the procedures defined in section 4 of
    [ICE-BIS] to gather, prioritize and eliminate the redundant
    candidates.

new
        The offerer shall follow the procedures defined in section 4 of
    [ICE-BIS] to gather and prioritize candidates and to eliminate the 
redundant
    candidates.

section 4.1.1.1, 4.1.5

Why all caps for " DEFAULT DESTINATION" ?
Section 2 defines "Default Destination"


Section 4.1.1.2, 1st para
old
     The process of encoding the SDP is identical between full and lite
    implementations.
new
     Section 4.1.1.2
     The process of encoding the SDP is identical between full and lite
    implementations, except for including the "a=ice-lite" attribute.

    Alternatively, you could also consider removing that sentence since 
the 4th para gives explicit instructions.

Section 4.1.1.2, 5th para
s/compliancy/compliance/
s/to the [RFC5245]/to [RFC5245]

Section 4.1.1.2, 7th para
s/if RTCP candidate is present/if a RTCP candidate is present/


Section 4.1.1.2, 1st para
s/The answerer shall then follow/The answerer SHALL then follow/

Section 4.1.2.3, 1st bullet item
Old
The agent MUST follow the rules of section 8 of [ICE-BIS] ...
new
The agent MUST follow the rules of section 9 of [ICE-BIS] ...

You might want to consider referring to section 11.2.2 that gives the 
motivation for these procedures.
The same reference is already in the corresponding section 4.1.2.1 for 
the answerer.

Section 4.1.3 3rd para
    If the offerer had included the "ice2" ICE Option in the offer and
    the SDP answer also includes a similar session level ICE option, then
    the peers are [ICE-BIS] complaint implementations.

    s/a similar/the same/
    s/complaint/compliant/

Section 5
I haven't checked up to 5.4 this since I assume it was a pure cut&paste 
from 5245.
Section 5.5 refers to section 11 of ICEbis, shouldn't that be section 12?

BTW: Section 11 of ICEbis talks about Extensibility Considerations. 
Shouldn't some text from there go into this document, e.g. the text 
about the a=ice-options attribute?

Section 8.2
[RFC5768]  misses a hyperlink.
Section 8.4
Mentioning the RFC number twice like in "... RFC 3312 [RFC3312] ..." 
looks awkward,
found also in other places of the document.

Section 9
Is this still necessary since already RFC5245 deprecated ANAT?
If still necessary this history should be somehow reflected by 
rephrasing the content accordingly.

Section 12
    Contact Name:  Jonathan Rosenberg, [e-mail].
Is this still appropriate?

Section 12.2
draft-ietf-mmusic-rfc4566bis-17 does the IANA registering in a more 
formal way.
There are drafts that already adhere to this, e.g. 
draft-ietf-mmusic-trickle-ice-sip and 
draft-ietf-mmusic-sdp-bundle-negotiation. Shouldn't this document do the 
same for the ice2 option?




Regards

Thomas