Subject: [MMUSIC] AD Evaluation of draft-ietf-mmusic-sdp-bundle-negotiation-47

From: Ben Campbell <[email-masked]>
Date: Fri, 19 Jan 2018 18:00:27 -0600

Hi,

This is my AD Evaluation of  draft-ietf-mmusic-sdp-bundle-negotiation-47.

First, thank you for all the work on this. I know this has been a difficult process. I do have a few substantive comments and a number of editorial comments. I would like to resolve the substantive comments and make at least a pass over the editorial comments prior to IETF Last Call.

Thanks!

Ben.

——————————

*** Substantive Comments ***

- boilerplate: I assume the pre-2008 boilerplate is present due to the extensive quoted text from RFC 3264. I note that it is not usually necessary to quote large amounts of text from an RFC in order to update it. I don’t expect a change in the update approach this late in the process, but I suggest that for future drafts people try to avoid creating the need for the pre-2008 boilerplate.

- section 8, first paragraph after bullet list: I’m confused as to whether this talks about the initial offer or a subsequent offer. the comment about the “previously negotiated SDP parameters” seems to suggest the latter. I suspect this is intended to cover all offers, in which case it needs clarification.

-8.3.3: IIUC, the criteria mean that the only m= sections that the answerer can move out of the bundle are those that have unique addresses but are not listed first in the group attribute. If that is correct, a sentence or two to say that would be helpful.

-8.5, first paragraph. IIUC, it’s not syntactically possible to assign more than one m= section the bundle address, since only one can be first in the group attribute. If that is correct, the MUST here is a statement of fact. Or am I missing something?

-9: There are several assertions that in order to do something you MUST have a spec saying how to do it. Formally, I think the requirement is really that you MUST NOT do that thing unless such a spec exists. (2119/8174 keywords are normally about implementation behavior.)

-10.2,
 — paragraph 7: What implementations are we talking about that may not follow the requirements to include the tag in RTP/RTCP? Early implementations of this draft? Something else? As it is, it seems like we are talking about implementations that do not follow this spec. That makes the statement “  … In this situation, each "m=" section MUST use unique payload type values…” problematic, as it seems to be putting requirements on implementations that do not follow this draft in the first place.

— '  Therefore, when processing a compound packet, any contained SDES packet MUST be handled first.
      Note that this is a change from [RFC3550] Section 6.1 which
      states that "Each individual RTCP packet in the compound packet
      may be processed independently with no requirements upon the order
      or combination of packets”.’

Doesn’t this require an update of 3550? It’s not merely a tightening of requirements, it’s a reversal of guidance from that document when used in this context.

-10.3.1.3, last paragraph: What is the expected behavior when that "protocol error" occurs?

-15.2, last paragraph: Is it worth recommending against patterns which could be used as persistent identifiers, even if they don’t expressly identify a person or application?

-16.3: Please use the IESG and “[email-masked]” as the contact for the IANA registrations.

-17, 2nd paragraph:
— Again, is it worth mentioning the risk of persistent identifiers?
—  “… SHOULD be 3 bytes or less, to allow them to efficiently fit into the MID RTP header extension.”
It seems like that requirement belongs in 15.2

-17, last paragraph: Doesn’t that require an update to 7941

-18: Would it make sense to have one of the examples use media-level c= lines?

*** Editorial Comments and nits ***

- Terminology issues: I think some of the terminology here will confuse readers who are new to the concepts. In particular, IIUC, “bundle address” means address + port. I think many readers will fall back to assuming that address means IP address. (Especially when we talk about a “unique address” assigned to an m= section, when much of the time that may just mean a unique port.)

I also find defining the term “Bundle Tag” to mean the first tag in the list to be confusing. It may be easier to just refer to the “first tag in the group list”.

— Examples: Any chance of some of the examples showing IPv6? I suspect it’s a trivial change for at least some examples, since they use domain names.

-abstract: The abstract seems longer and more detailed than strictly necessary. The first two sentences along with a brief mention of the update to 3264 and the SDES item/RTP header extension would be sufficient.

-Section 1,
— first paragraph: This sort of jumps into things without much context. (e.g. that we are talking about media sessions signaled using SDP offer/answer.”
s/consume/consumes
— paragraphs 5-9: Most of the instances of the word “also” are unneeded.
— paragraphs 7 and 9: These paragraphs both talk about updates to 3264; it would be better to keep them together.
— 2nd to last paragraph, “The procedures in this specification apply independently to a given BUNDLE group.” : I don’t understand what that sentence is trying to convey.

- Section 2: The terminology list would be more readable as a bullet list or hanging indent list.

-3: There are at least one lower case “must” and a number of lower case “shoulds”. Please consider using the boilerplate from RFC 8174, or verify that the lower case versions are not intended to be keywords.

-5:
— First paragraph: inconsistent tense. Please consider s/ “ … section will use …” / “… section uses…”
— 2nd paragraph: The citation to RFC5888 looks like a reference for the “BUNDLE” semantic value, rather than for the grouping framework. Please consider something to the effect of the following:
OLD:
     The BUNDLE extension is indicated using an SDP ’group’ attribute with a "BUNDLE" semantics value [RFC5888].
NEW:
     The BUNDLE extension is indicated using an SDP ’group’ attribute with a semantics value [RFC5888] of "BUNDLE”.

- Section 8.1:
— Consider moving this section to later in the offer/answer details. I think it would be easier for a reader to understand this section after they have some more general context.
— Title: Spell out “Multiplex” in the title.
— First Paragraph: The wording could be taken to mean that all attributes with a mux category of IDENTICAL or TRANSPORT must be included in the first place. I think the point is that, if they are present, they must be included in each m= section. I suggest something like the following:
OLD:
   … IDENTICAL and TRANSPORT mux category SDP attributes MUST explicitly be included in each bundled "m=“ section ...
NEW:
   … Any IDENTICAL and TRANSPORT mux category SDP attributes included in the BUNDLE group MUST explicitly be included in each bundled "m=“ section …

-8.2.1, first paragraph:
— The phrase “represented by the … BUNDLE-tag” occurs repeatedly throughout the document. The use of “represented” there seems awkward. I think “indicated” would be a better word choice.
— should “offerer BUNDLE address” be “offerer suggested BUNDLE address”?

-8.5.2: Should “add a bundled m= section” be “add a _new_ bundled m= section”?

-9:
— first paragraph: “ … different protocols on top of the transport-layer protocol …”
Upper layer protocols?
— Last paragraph: This paragraph seems mostly (or entirely) redundant to section 9.1.

-10.2:

— " RTCP packets for which no appropriate "m=" section can be identified
   MUST be processed as usual by the RTP layer, updating the metadata
   associated with the corresponding RTP streams, but are not passed to
   any "m=" section.”

I don’t think things gets passed to sections of SDP. What does this really talk about?

— “ Rules for additional processing of the various types of RTCP packets
   are explained below.”

Should that say “Additional rules for processing…”?

-11, 2nd bullet: " previously selected [Section 8.3.1 ] or new suggested [ Section 8.5.1]”
s/new/newly

— last paragraph: It may be worth clarifying that this doesn't change any requirement specified by the application or signaling protocol. For example, WebRTC still requires ICE even though bundle is used.

11.1:
 — 2nd paragraph: “ previously selected or new suggested “
s/new/newly

— 3rd paragraph: Construct of the form of “MUST only” can be ambiguous. Please consider restating in the form of a MUST NOT construction. (For example, “MUST only do foo when….” can be interpreted as “MUST NOT do foo unless…” or “MUST NOT do anything but foo when…”   [Note: This construct repeats several times in the next few sections. ]

-19, 3rd paragraph from end: It’s seems a bit odd to thank a co-author (Cullen) for text contributions :-)

---

From: Magnus Westerlund <[email-masked]>
Date: Thu, 25 Jan 2018 15:04:51 +0100

Hi,

Christer invoked me on the RTP related questions. Here are my views on 
these three issues.

Den 2018-01-24 kl. 12:46, skrev Christer Holmberg:
>
>
> TEXT (Section 10.2):
>
> However, some implementations may not include this identification-tag
>     in their RTP and RTCP traffic when using the BUNDLE mechanism, and
>     instead use a payload type based mechanism to associate RTP streams
>     with SDP "m=" sections.  In this situation, each "m=" section MUST
>     use unique payload type values, in order for the payload type to be a
>     reliable indicator of the relevant "m=" section for the RTP stream.
>     Note that when using the payload type to associate RTP streams with
>     "m=" sections an RTP stream, identified by its SSRC, will be mapped
>     to an "m=" section when the first packet of that RTP stream is
>     received, and the mapping will not be changed even if the payload
>     type used by that RTP stream changes.  In other words, the SSRC
>     cannot "move" to a different "m=" section simply by changing the
>     payload type.
>
>
>
>> -10.2,
>> ‹ paragraph 7: What implementations are we talking about that may not
>> follow the requirements to include the tag in RTP/RTCP? Early
>> implementations of this draft? Something else? As it is, it seems 
>> like we
>> are talking about implementations that do not follow this spec. That
>> makes the statement ³  Š In this situation, each "m=" section MUST use
>> unique payload type valuesŠ² problematic, as it seems to be putting
>> requirements on implementations that do not follow this draft in the
>> first place.

Yes, this is a form of legacy adaptations. It applies towards certain 
existing systems that already follows this rule. This include 3GPP MTSI 
clients. It also applies to clients where the signalling system or the 
signalling part of the client can be updated to follow this requirement. 
In some cases also middleboxes or gateways can have functionality for PT 
rewriting, but lack capability of inserting the id-tag.

So, a MUST where you either follow this requirement, or it will simply 
not work at all. It can likely be rewritten either without a MUST, or 
with an explanation that this is just stating a hard requirement for 
making it work.
> ----
>
> TEXT (Section 10.2):
>
> Note that if an SDES packet is received as part of a compound RTCP
>        packet, the SSRC to "m=" section mapping may not exist until the
>        SDES packet is handled (e.g., in the case where RTCP for a source
>        is received before any RTP packets).  Therefore, when processing a
>        compound packet, any contained SDES packet MUST be handled first.
>        Note that this is a change from [RFC3550] Section 6.1, which
>        or combination of packets".
>
>
>
>> ‹ '  Therefore, when processing a compound packet, any contained SDES
>> packet MUST be handled first.
>>       or combination of packets².¹
>>
>> Doesn¹t this require an update of 3550? It¹s not merely a tightening of
>> requirements, it¹s a reversal of guidance from that document when 
>> used in
>> this context.

Looking at this now, I think this formulation may be a bit unnecessarily 
strict. If one fails to process the SDES item with the ID-tag in a 
timely fashion, i.e. prior to needing to route any other information 
element to the consumer, the only implication is that either the element 
is delayed or discarded. In the case discussed here, also the ID-tag 
will likely to have been processed before the arrival of the RTP packets.

I would suggest that that the MUST is changed to an implementation note 
that simply recommends implementations to prioritize processing of the 
SDES item, or otherwise consider methods for holding the routing for the 
other elements for a new non-associated SSRC until the SDES item(s) have 
been processed.

>
> ----
>
> TEXT (Section 17):
>
> The security considerations of "RTP Header Extension for the RTP
>     Control Protocol (RTCP) Source Description Items" [RFC7941] requires
>     that when RTCP is confidentiality protected, and that any SDES RTP
>     header extension carrying an SDES item, such as the MID RTP header
>     extension, is also protected using commensurate strength algorithms.
>     However, assuming the above requirements and recommendations are
>     followed, there are no known significant security risks with leaving
>     the MID RTP header extension without confidentiality protection.
>     Thus, the requirements in RFC 7941 MAY be ignored for the MID RTP
>     header extension.  Security mechanisms for RTP/RTCP are discussed in
>     Options for Securing RTP Sessions [RFC7201], for example SRTP
>     [RFC3711] can provide the necessary security functions of ensuring
>     the integrity and source authenticity.
>
>
>> -17, last paragraph: Doesn¹t that require an update to 7941

Maybe a question of style in how one handles exceptions. I would not 
update 7941, as the general rule still holds. But in this particular 
case the exception is motivated for this particular SDES item, and that 
is documented and agreed on (when RFC is published).

Cheers

Magnus Westerlund

----------------------------------------------------------------------
Media Technologies, Ericsson Research
----------------------------------------------------------------------
Ericsson AB                 | Phone  +46 10 7148287
Torshamnsgatan 23           | Mobile +46 73 0949079
SE-164 80 Stockholm, Sweden | mailto: [email-masked]
----------------------------------------------------------------------

---

From: Ben Campbell <[email-masked]>
Date: Thu, 25 Jan 2018 16:35:40 -0600

> On Jan 25, 2018, at 8:04 AM, Magnus Westerlund <[email-masked]> wrote:
> 
> Hi,
> 
> Christer invoked me on the RTP related questions. Here are my views on these three issues.
> 
> 
>>> implementations of this draft? Something else? As it is, it seems like we
> Yes, this is a form of legacy adaptations. It applies towards certain existing systems that already follows this rule. This include 3GPP MTSI clients. It also applies to clients where the signalling system or the signalling part of the client can be updated to follow this requirement. In some cases also middleboxes or gateways can have functionality for PT rewriting, but lack capability of inserting the id-tag.
> 
> So, a MUST where you either follow this requirement, or it will simply not work at all. It can likely be rewritten either without a MUST, or with an explanation that this is just stating a hard requirement for making it work.

I think that could work for me, but would like to see it in text before commiting :-)

>>> requirements, it¹s a reversal of guidance from that document when used in
>>> this context.
> 
> Looking at this now, I think this formulation may be a bit unnecessarily strict. If one fails to process the SDES item with the ID-tag in a timely fashion, i.e. prior to needing to route any other information element to the consumer, the only implication is that either the element is delayed or discarded. In the case discussed here, also the ID-tag will likely to have been processed before the arrival of the RTP packets.
> 
> I would suggest that that the MUST is changed to an implementation note that simply recommends implementations to prioritize processing of the SDES item, or otherwise consider methods for holding the routing for the other elements for a new non-associated SSRC until the SDES item(s) have been processed.

That seems reasonable.

> Maybe a question of style in how one handles exceptions. I would not update 7941, as the general rule still holds. But in this particular case the exception is motivated for this particular SDES item, and that is documented and agreed on (when RFC is published).

I’m not sure I agree. This adds an exception to a security related MUST from 7941. While I recognize that it may be safe to do that, I think it still requires an update. That doesn’t even have to be a generic update; something along the line of “This document updates $FC 7941 to add an exception to that requirement for the MID extension”.

Thanks!

Ben.

---

From: Magnus Westerlund <[email-masked]>
Date: Fri, 26 Jan 2018 16:25:49 +0100

Hi,

See inline for response and text proposals.

> I think that could work for me, but would like to see it in text before commiting :-)

So my proposed text is:

    However, some legacy implementations may not include this 
identification-tag
    in their RTP and RTCP traffic when using the BUNDLE mechanism, and
    instead use a payload type based mechanism to associate RTP streams
    with SDP "m=" sections.  In this situation, each "m=" section needs to
    use unique payload type values, in order for the payload type to be a
    reliable indicator of the relevant "m=" section for the RTP stream. 
If an
    implementation fails to ensure unique payload type values it will be
    impossible to associate the RTP stream using that payload type value
    to a particular "m=" section.
    Note that when using the payload type to associate RTP streams with
    "m=" sections an RTP stream, identified by its SSRC, will be mapped
    to an "m=" section when the first packet of that RTP stream is
    received, and the mapping will not be changed even if the payload
    type used by that RTP stream changes.  In other words, the SSRC
    cannot "move" to a different "m=" section simply by changing the
    payload type.

>> I would suggest that that the MUST is changed to an implementation note that simply recommends implementations to prioritize processing of the SDES item, or otherwise consider methods for holding…

So text proposal for this item:

OLD:

NEW:
       Note that if an SDES packet is received as part of a compound RTCP
       packet, the SSRC to "m=" section mapping might not exist until the
       SDES packet is handled (e.g., in the case where RTCP for a source
       is received before any RTP packets). Therefore, it can be beneficial
       for an implementation to delay RTCP packet routing, such that it
       either prioritizes processing of the SDES item to generate or update
       the mapping, or buffers the RTCP information that needs to be
       routed until the SDES item(s) has been processed. If the
       implementation is unable to follow this recommendation, the
       consequence could be that some RTCP information from this
       particular RTCP compound packet is not provided to higher layers.
       The impact from this is likely minor, when this information relates
       to a future incoming RTP stream.

> I’m not sure I agree. This adds an exception to a security related MUST from 7941. While I recognize that it may be safe to do that, I think it still requires an update. That doesn’t even have to b…

I have no real issue with an Updates, as long as it is clear that this 
is just to note that there was an exception.

Has IESG clarified in statement or RFC the use of updates, especially in 
these cases when it comes to overriding RFC2119 language of earlier 
RFCs? From my perspective this has as long as I been active in IETF a 
bit unclear.

Cheers

Magnus Westerlund