Subject: [rtcweb] Comments on draft-ietf-rtcweb-transports-08

From: Magnus Westerlund <[email-masked]>
Date: Tue, 17 Mar 2015 15:33:26 +0100

Hi,

I have reviewed draft-ietf-rtcweb-transports-08 and have some comments.

1. Section 1:

   This document describes requirements that apply to all WebRTC
   devices.  When there are requirements that apply only to WebRTC
   browsers, this is called out by using the word "browser".

Shouldn't this be a reference to overview and rather grab the relevant
terms from there?

2. Section 3.1:
   o  UDP.  This is the protocol assumed by most protocol elements
      described.

   o  TCP.  This is used for HTTP/WebSockets, as well as for TURN/SSL
      and ICE-TCP.

References to UDP and TCP?

3. Section 3.3:

   When a client gathers all IPv6 addresses on a host, and both
   temporary addresses and permanent addresses of the same scope are
   present, the client SHOULD discard the permanent addresses before
   forming pairs.

I think the last two words are wrong in this statement. Isn't the point
to actually discard the local candidates with permanent addresses where
a temporary address candidate has been created for the same interface?

4. Section 3.5:

WebRTC implementations MUST support multiplexing of DTLS and RTP over
   the same port pair, as described in the DTLS_SRTP specification
   [RFC5764], section 5.1.2.

DTLS_SRTP should be DTLS-SRTP

5. Section 4.1:
(Question: Does there need
   to be an API knob to turn off DSCP markings?)

So the thinking here would be that one like to apply the other
prioritization, but like to avoid any fear that any DSCP marking other
than Best Effort can cause issues on the network path.

I am personally quite uncertain if it is needed, but if it is to exist,
I think the know should be such that one force the DSCP setting to best
effort for all transport flows.

But, I think it is time this open issue is resolved.

6.  Section 4.1:

All packets arrying data from the SCTP association supporting the
   data channels MUST use a single DSCP code point.

s/arrying/carrying

7. Section 3.5:

   The setup protocol for WebRTC data channels is described in
   [I-D.ietf-rtcweb-data-protocol].

I am a bit surprised over the none RFC2119 requirement here and that the
data-protocol is an informative reference. Looking in the data-channel
document, it is also not normatively requiring the data-protocol
documents implementation. Overview is requiring that implementation, but
I don't think the formulation here correspond to the requirement level
in overview. Can we please align?

Cheers

Magnus Westerlund

----------------------------------------------------------------------
Services, Media and Network features, Ericsson Research EAB/TXM
----------------------------------------------------------------------
Ericsson AB                 | Phone  +46 10 7148287
Färögatan 6                 | Mobile +46 73 0949079
SE-164 80 Stockholm, Sweden | mailto: [email-masked]
----------------------------------------------------------------------

---

From: Harald Alvestrand <[email-masked]>
Date: Wed, 18 Mar 2015 14:02:55 +0100

Thanks for the review!
Skipping nits....

> Shouldn't this be a reference to overview and rather grab the relevant

Good point.

> References to UDP and TCP?

What would you suggest? RFC 791 is not the best TCP reference any more...

> a temporary address candidate has been created for the same interface?

"forming pairs"?

I don't understand how your comment relates to those 2 words, so I'm not 
sure what you want me to change.

The point is that if we get the same connectivity with a temporary 
address, we shouldn't expose a permanent address to the JS layer at all.

In the case of multiple interfaces, some of which have temporary 
addresses and some of which have only permanent addresses, I'm not sure 
what the right thing is. Commentary?

> DTLS_SRTP should be DTLS-SRTP

Yup.
> But, I think it is time this open issue is resolved.

That makes two of us being uncertain :-) Anyone else?
> in overview. Can we please align?

Suggested fix: "The setup protocol for WebRTC data channels described in 
[I-D....] MUST be supported".
And data-protocol moved to normative references.

Any objections to this?

> _______________________________________________
> rtcweb mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/rtcweb

---

From: Magnus Westerlund <[email-masked]>
Date: Wed, 18 Mar 2015 16:31:11 +0100

Hi,

Please see inline.

> What would you suggest? RFC 791 is not the best TCP reference any more...

I assume you mean RFC 793.

A possible alternative is RFC 7414.

The TSV directorate had a discussion about this, and for a general TCP
reference it is quite fine to continue use RFC 793.

> sure what you want me to change.

Well, if the purpose is to not use the permanent address, then I don't
see how one can create candidates even with these addressed.

My reaction is that forming pairs is done later in the process, when the
gathering is already completed. Thus, pruning should happen during the
gathering phase.

> address, we shouldn't expose a permanent address to the JS layer at all.

Fully agree with this purpose.

> what the right thing is. Commentary?

I think one would have to expose the permanent address if that is the
only thing one can get from that interface.

The one alternative I could think of, is that the gathering of any
permanent address would depend on the privacy setting of the WebRTC
endpoint.

> And data-protocol moved to normative references.

Sounds good to me.

Cheers

Magnus Westerlund

---

From: Harald Alvestrand <[email-masked]>
Date: Wed, 18 Mar 2015 16:42:25 +0100

> gathering is already completed. Thus, pruning should happen during the

<lightbulb switching on>

aha - in the case where you're doing a createOffer, the addresses must 
be discarded before you do the createOffer, which is long before the 
time at which you form pairs (after setRemoteDescription) - so the text 
"before forming pairs" is misleading.

I'll replace the text with "before exposing addresses to the application".

> permanent address would depend on the privacy setting of the WebRTC

Saying that would require introducing the term "privacy setting", which 
we so far have not done.

(snipping stuff that's OK with me)

---

From: Harald Alvestrand <[email-masked]>
Date: Thu, 19 Mar 2015 12:10:38 +0100

A suggested PR for addressing Magnus' comments is here:

https://github.com/rtcweb-wg/rtcweb-transport/pull/3

For nits, please make comments on the PR. For substantive issues, please 
send to the list.