Re: [MMUSIC] WGLC: draft-ietf-mmusic-ice-15.txt

[Magnus Westerlund <[e-mail]>]

Hi,

I have reviewed the draft and have some comments. I will start with all 
the real issues. Then I a also have some nits. But let me say that I am 
impressed how good this document has become.

Issues:

I1. Section 7.2.1.1: Why isn't this section applicable for lite 
implementation. If there exist possibilities that the two peer in case 
of full implementations are confused about their roles, why can't a full 
peer be similar confused and think it is talking to a controlling full 
when it in reality talks to a lite. I have a hard time determining if 
that can happen at all or not when you think two full peers can be 
confused about their roles. It seems a simple fix to tell a lite server 
to always do tiebreaking as the loosing side.

I2. Section 7.2.1.1, first bullet:
   "o  If neither ICE-CONTROLLING or ICE-CONTROLLED are present in the
       request, there is no conflict."

When does this happen? To my understanding all ICE peers sending 
connectivity checks will indicate if they are controlling or controlled.

I3. Section 7.2.1.4:

       *  If the state of that pair is Waiting or Frozen, its state is
          changed to In-Progress and a check for that pair is performed
          immediately.  This is called a triggered check.

I am a bit concerned with the immediately part here. I do understand 
that the check should happen as soon as possible. However by stepping 
away from the pacing in these replies you further increases the 
burstiness of the connectivity checks. To be slightly more conservative. 
Why not simply bump the triggered checks to the top of the queue. The 
alternative I see if you like to keep the quick response (and potential 
bunching together of response and the triggered check request) is to 
delay the next scheduled check an extra Ta interval. That way the 
triggered check does not increase the consumed bit-rate.

I4. Section 10.
"Examples of formats which readily meet this
    goal are RTP No-Op [31] and RTP comfort noise [25]."

I would argue that RTP comfort noise does not readily meet it. It works 
in some cases, like for an audio stream where the peers both support it. 
Two solutions, a) remove comfort noise as an example. b) clarify the 
main limitations.

I5. Section 15.1:

There are a number of cases where one uses 1* in the ABNF. Can these 
please be changed to a upper limit on the number of characters. With the 
current definition I could use a 5000 digit long priority number from 
parsing perspective. This is likely to lead to overflow. In the case of 
priority as this is limited to 2^32-1 as largest number then using 
1*10DIGIT seems to be without risk. I guess that also component-ID and 
foundation can be limited to a reasonable upper limit. Making it easier 
to separate the cases of potentially valid numbers and plain errors. 
This comment also applies to section 15.4 and the password and ufrag 
definitions.

I6. Pacing of connectivity checks. I need to think a bit more about this 
and how ICE relates to the UDP usage guidelines 
(http://www.ietf.org/internet-drafts/draft-eggert-tsvwg-udp-guidelines-02.txt) 
we are working on getting accepted in the transport area. Ice with the 
current parameters are rather aggressive in its bandwidth usage in worst 
case. I understand in most cases it would not consume close to as much.

The biggest problem with this bloody protocol is its purpose resulting 
in that basically all STUN request packets could be dropped by NAT 
filtering rather than congestion. Or it could be the other way around. 
We have no way of knowing. This also causes the connectivity checks 
themselves some problem. As all checks easily can congest themselves 
thus causing worse performance then the correct rate limited sending 
would have. I think we need to at least consider putting some further 
limitation or adaptation of the pacing timers in some cases. At least 
when you have knowledge about link speeds.

I know we have had quite some discussion on this topic earlier and the 
algorithm has clearly been made to try as few checks as possible thanks 
to the freezing of some checklist etc. But in my current role I need to 
take this even more serious than before.

NITS:

N1. Section 2.6, third paragraph: Last sentence starts with what appears 
with an stray "A".

N2. Figure 4 and 5. Lack of directional arrows for the requests from L.

N3. section 5.7.1, page 30, first paragraph:
"As another example, the
    offerer can multiplex RTP and RTCP on the same port and signals it
    can do that in the SDP through some new attribute."

I think an informative reference to the RTP and RTCP MUXing draft 
(draft-ietf-avt-rtp-and-rtcp-mux-04) is needed in this sentence to make 
it clear what is being talked about.

N4. Section 5.7.2, second paragraph: "Where O-P>A-P?1:0 is an expression 
whose value is 1 if O-P is greater than A-P, and 0 otherwise." As the 
(O-P) part isn't used in the above formula, maybe using the actual 
(O>A?1:0) is better in this context.

N5. Section 7.1: Isn't "STUN Client Procedures" a better section title 
than "Client Procedures"? Similar for 7.2.

N6. Section 8.1.1:
"This check will succeed (since the previous did), causing
    the nominated flag of that and only that pair to be set."

I think this sentence is more correct if you change "will" to "should" 
or "is expected to". There are no guarantees that something else causes 
a failure of the path.

N7. Section 12.1.1:
   "With this optimization, provisional responses containing an SDP
    answer that begins ICE processing for one or more media streams can
    be sent reliably without RFC 3264."

Isn't the reference to RFC 3264 wrong? Shouldn't it be to RFC 3262?

N8. Section 16. The line breaks in the SDP example are bad and looks 
strange.

N9. The following references needs update:

   == Outdated reference: A later version (-03) exists of
      draft-ietf-behave-turn-02

   == Outdated reference: A later version (-01) exists of
      draft-ietf-sip-ice-option-tag-00

   == Outdated reference: A later version (-01) exists of
      draft-ietf-avt-rtp-no-op-00

   == Outdated reference: A later version (-08) exists of
      draft-ietf-sip-outbound-07

N10. Section B.2
Use of 192.168.1.1 and 10.0.1.1 as end-host addresses. As these 
addresses usually will be assigned the gateway/NAT in respective network 
isn't using them for end-host a bad choice.

N11. Section B.8:
"That section
    requires that the type preference for peer reflexive candidates
    always be lower than server reflexive. "

I think "lower" is supposed to be "higher".


Cheers

Magnus Westerlund

IETF Transport Area Director & TSVWG Chair
----------------------------------------------------------------------
Multimedia Technologies, Ericsson Research EAB/TVM/M
----------------------------------------------------------------------
Ericsson AB                | Phone [phone]
Torshamsgatan 23           | Fax   [phone]
S-164 80 Stockholm, Sweden | mailto: [e-mail]
----------------------------------------------------------------------

_______________________________________________
mmusic mailing list
[e-mail]
https://www1.ietf.org/mailman/listinfo/mmusic