Subject: [MMUSIC] Review of draft-ietf-mmusic-sdp-bundle-negotiation-32 (Part I)

From: Eric Rescorla <[email-masked]>
Date: Sat, 27 Aug 2016 09:58:15 -0700

I have reviewed the first half of this document (up through the examples).
The following comments apply only to that.

OVERALL
The overall framing of this document as being about attaching multiple
m= sections to a single "shared address" is extremely problematic,
for two reasons:

- With ICE there isn't actually any notion of a shared address except
  in some vague sense. I suppose you could argue that because with
  ICE you are required to specify a port on the m= line then you
  still have a concept of an address, but that's not true in
  trickle ICE, where the port is 9, so all m= lines share the same
  address/port. So, actually when you use trickle a whole bunch of
  the statements in this document are wrong.

- A lot more is being overloaded here than addresses. In fact, a whole
  pile of semantics are being overloaded, so the way to think about
  this is actually as gluing a whole bunch of m= lines together at
  the transport level.

I think the right concept here is not "shared address" but "bundle group"
and the document should be written in that way, namely that there are
a  bunch of m= lines in a bundle group and the one with the bundle tag
is the one that sets all the shared parameters.

The document is badly in need of some overall framing that explains
what we are trying to accomplish and the rationale for why any
particular m= section property needs to be the same for all members of
a bundle group and why other ones do not. I'm not saying it's
necessarily bad to be explicit in (for instance Sections 11 and 12)
but it would be better if it were clear where these rules come
from. One way to deal with this would be to simply defer this to the
mux-attributes draft.

I'll just note again that I find the overloading of "associated" makes this
document very hard to read. At minimum it would be best to talk about
an m= line as "belonging" to a BUNDLE group. If others agree I would
be happy to provide a PR that cleans this up.

MAJOR TECHNICAL
Maybe I missed this, but I don't see any requirement that m= lines
appear in the a=group:BUNDLE line in the order they appear in the
SDP. That would make life a lot easier.

DETAILED COMMENTS (MIXED EDITORIAL/TECHNICAL)
S 1.
This really needs some explanation of why you would want this feature.

S 2; Initial offer.
"The first offer, within" . This comma is superfluous.

S 5.
The first three grafs of this section seem to duplicate the intro
and the abstract.

Also, this claim about sharing a 5-tuple is not quite right with ICE
because ICE can switch 5-tuples during aggressive nomination.

S 6.
   The usage of the 'bundle-only' attribute is only defined for a
   bundled "m=" line with a zero port value, within an offer.  Other
   usage is unspecified.

It would probably be better to actually forbid this, because it seems
like an interop problem if you somehow were to introduce it later.

S 8.
Nit:
List the forward references in section order.

S 8.1.
This seems like a key concept: there are a number of different categories
of attributes and some are shared with all members of a bundle group
and some are not. To what extent is this draft just duplicative of those
listed in the mux-attributes draft?

S 8.2 and following
Really would be useful to have some examples here to make it less
abstract.

S 8.3.3.
It seems like the implication here is that an answerer cannot debundle
something that was bundled in a previous o/a transaction. Can you
make that point clearly?

S 8.5.
How does this interact with ICE Restart?

S 8.5.2
When you say "extend" do you mean "add to the end"?

S 10.1
In this first bullet is "if" "iff"?

S 10.3.1.1.
   When an offerer generates an initial offer, the offerer MUST
   associate either an SDP 'rtcp-mux' attribute [RFC5761] or an SDP
   'rtcp-mux-only' attribute [I-D.ietf-mmusic-mux-exclusive] with each
   bundled RTP-based "m=" line in the offer.  The offerer MUST associate
   an SDP 'rtcp-mux-only' attribute with each bundle-only "m=" line.  If
   the offerer associates a 'rtcp-mux-only' attribute with an "m=" line,
   the offerer may also associate a 'rtcp-mux' attribute with the same
   "m=" line, as described in [I-D.ietf-mmusic-mux-exclusive].

This is a particularly egregious case of associate disease that could
be easily solved by using "add" when referring to the attributes
you put in the m= section.

S 11.1
   When an offerer associates a unique address with a bundled "m=" line
   (excluding any bundle-only "m=" line), the offerer MUST associate SDP
   'candidate' attributes (and other applicable ICE-related media-level
   SDP attributes), containing unique ICE properties (candidates etc),
   with the "m=" line, according to the procedures in
   [I-D.ietf-mmusic-ice-sip-sdp].

Here is an example of where the "address" framing doesn't work, if you
are using trickle.

---

From: Christer Holmberg <[email-masked]>
Date: Mon, 29 Aug 2016 19:54:27 +0000

Hi Ekr,

Thanks for the comments! Please see inline.

OVERALL

> is the one that sets all the shared parameters.

It is true that the actual port value in the m- line, and the IP address in the c- line address, represent the address that is actually being used.

But, even with ICE there is a shared address, isn't there? Yes, that shared address may change, but at any given time there will be one address that is shared among the media streams within the BUNDLE group. Or?

> from. One way to deal with this would be to simply defer this to the

Ok, I'll look into that. People are also welcome to provide text suggestions.

> be happy to provide a PR that cleans this up.

The intention is to say "m= line within a BUNDLE group". If there are instances of "associated" I am happy to fix that.

MAJOR TECHNICAL

> SDP. That would make life a lot easier.

There is no such requirement.

AFAIR, it was discussed at some point, but it was agreed to not mandate m- lines to be listed in the order of preference (as far as selecting the BUNDLE address is concerned).

Also, if you mid-session adds a new m- line, and wants that to represent the new BUNDLE address you have to place the new m- line after the existing m- lines, but you have to place it first in the a=group:BUNDLE attribute list.

>This really needs some explanation of why you would want this feature.

You represent the community that wants the feature, so feel free to suggest some text explaining why you want it :)

>"The first offer, within" . This comma is superfluous.

Will be fixed.

>because ICE can switch 5-tuples during aggressive nomination.

Yes. But, at any given time there will only be one 5-tuple, won't it?

> like an interop problem if you somehow were to introduce it later.

Ok, I'll look into that.

>List the forward references in section order.

Will be fixed.

>listed in the mux-attributes draft?

Not sure I understand the question.

>Really would be useful to have some examples here to make it less

You don't think the examples in section 17 are enough? You want examples in the normative sections describing how to create offers, answers etc?

>something that was bundled in a previous o/a transaction. Can you

Ok, I'll look into that.

>From a pure ICE Restart perspective, I don't see any conflict. ICE Restart is triggered by changing the user name and password, and the text does not forbid that.

If you also want to change the BUNDLE address, you follow the rules in section 8.5.1.

>When you say "extend" do you mean "add to the end"?

Yes. I can fix that.

>In this first bullet is "if" "iff"?

"iff"?

> you put in the m= section.

People have previously said that you don't "add" attributes to m- lines: you add port values etc, but not attributes.

This is an example of where I've had to re-write big parts of the documents a number of times already, because every time someone else reads the document he/she is not happy with the terminology. I just don't want to do it again.

> Here is an example of where the "address" framing doesn't work, if you

See top of e-mail.

Regards,

Christer