Re: [MMUSIC] AD Evaluation of draft-ietf-mmusic-sdp-mux-attributes-12

Hello Ben

 Sorry for the delay in responding. Many thanks for the detailed review.

 Please find my responses inline (marked by [Suhas]).

Cheers
Suhas

On Fri, Feb 19, 2016 at 2:28 PM, Ben Campbell <ben@nostrum.com> wrote:

> Hi,
>
> Here is my AD evaluation of draft-ietf-mmusic-sdp-mux-attributes-12. I
> would like at least the substantive
> section to be discussed before going to IETF last call.
>
> Thanks!
>
> Ben.
>
> ----------
>
> # Substantive #
>
> - This is very long and detailed. Are the authors and chairs comfortable
> that all sections have had sufficient review?
>
> -4.0: It would be helpful to add a paragraph describing how these semantic
> groupings map to the categories.
>

[Suhas] The semantic groupings were added as a informal reference and they
don't really interact with the categories as such.

Does the following rephrasing work

*"Attributes in an SDP session description can be defined at the
**session-level
or  media-level or  source-level. Informally, there are various semantic
grouping for these attributes. One such grouping could be as noted below:*

*   o  Attributes related to media content such as media type, encoding
     schemes, payload types.   o  Attributes specifying media transport
characteristics like RTP/RTP      Control Protocol (RTCP) port numbers,
network addresses, QOS.   o  Metadata description attributes capturing
session timing and      origin information.   o  Attributes establishing
relationships between media descriptions      such as grouping framework
[RFC5888 <https://tools.ietf.org/html/rfc5888>]The proposed framework
defines categories for the SDP attributes under multiplexing and assigns
each SDP attribute to an appropriate multiplexing category.*

*Since the multiplexing categories defined in this specification are
independent of any informal semantic grouping of the SDP attributes, the
categorizations assigned against each attribute  MUST be considered as
normative assignments under multiplexing. "*

> -4.2: I recommend choosing a category name that is not a 2119 keyword.
>   Maybe something like "not mux-able" or "unsafe"?
>

[Suhas]

Initially that category was called "BAD" as in

https://datatracker.ietf.org/doc/draft-nandakumar-mmusic-sdp-mux-attributes/01/

However we ended upon NOT RECOMMENDED since “BAD” was a bad choice :-)

Either of the following options seems fine to me

   1.

   NOT-RECOMMENDED [ we use hyphen separated multi word format in other
   category names]
   2. UNSAFE

>
> -4.3: Please don't use a 2119 keyword as part of the definition
> of a category. This creates sort of a circular dependency; that is
> attributes are in this category because they MUST be identical; but they
> MUST be identical because they are in this category. I suggest describing
> the category descriptively, and adding a separate sentence to say
> "Attributes
> in the IDENTICAL category MUST be identical across all media descriptions."
>

[Suhas]. I have removed the usage of word identical from the definition as
below. Please let me know if this works.

Category: IDENTICAL

Attributes  and their associated values (if present) in the IDENTICAL
category MUST be repeated across all the media descriptions under
multiplexing.

>
> -4.6:
>  >"This in turn MAY place constraints on what constitutes a valid
>    configuration from
>    a multiplexing point of view, e.g. because some attributes MUST be
>    IDENTICAL (see Section 14 for further details)."
>
>    It seems like it would make sense to state that last sentence
>    more strongly. Does it make sense to say something along the lines
>    of "inherited attributes MUST be coherent."?
>

[Suhas]. Works as suggested.

>
> -4.9: "For the purposes of implementations it is
>    advised to consider "NOT RECOMMENDED" as the category when
>    multiplexing these attributes."
>
>    So why not have them in the NOT RECOMMENDED category, with
>    an explanation that they have not been sufficiently analyzed?
>    Or if you want to keep a separate category, you can say something
>    like :"Attributes in the TBD category SHOULD NOT be multiplexed."
>
>
[Suhas] We would want a separate category to differentiate between the

actual NOT-RECOMMENDED ones and the ones which has not yet been

Analysed.

I am happy to consider the suggestion "Attributes in the TBD category
SHOULD NOT be multiplexed."

> -5.8 and 5.9:
>     The last paragraph of 5.8 seems redundant with section 5.9.
>
> -5.8, last paragraph:
>
>     "Hence multiplexing MUST NOT be performed even in this
>     alternate scenario." seems to contradict the category selection of
>     "NOT RECOMMENDED" (but see previous comment about not using 2119
>     keywords in a category title.)
>
>
[Suhas] The idea here is to not perform multiplexing and hence the category
of NOT RECOMMENDED assigned. I can update this once we finalize on the
right naming for "NOT RECOMMENDED"

> -5.14
>   Do you really expect to see such an "early mechanism" be
>   multiplexed?
>
>
[Suhas]

In the context of WebRTC, implementations today do use use both a=rtcp
attribute and BUNDLE framework for multiplexing media descriptions.

> -5.24, last paragraph: "it is recommended to consult the
>   document defining the attribute."
>
>   I assume one should _always_ refer to the specification of
>   an SDP attribute before trying to mux it. But that doc is
>   not going to explain muxing issues, is it?
>
>
[Suhas]

Since cparmin/cparmax can define numerical values/ranges

for any of b= or a= attributes (identified via cpar), there is no easy way
to assign a multiplexing category unless one knows what cpar is being

used and the assignned numerical values to each of cparmin/cparmax mean.
Thus the category SPECIAL has been assigned as a recommendation for the
implementor to consult the spec that defines a particular usage of cpar,
cparmin and cparmax combination to really see what’s going on.

This is very similar to extmap or RTCP Application feedback message type

How about following rephrasing

*"The attributes (a=cparmin and a=cparmax) define minimum and maximum
numerical values associated with the attributes described in a=cpar. Since
a cpar attribute can either define*

*“b=” attribute or any “a=” attribute,  the multiplexing category *

*depends on actual attribute being encapsulated and the implications of the
numerical values assigned. Hence it is recommended to consult the
specification defining the the attributes (cparmin/cparmax) to further
analyze their behavior under multiplexing."*

> -5.26:
>   -"Support of this extension is OPTIONAL."
>   Please don't use 2119 keywords to describe existing requirements
>   from other docs, unless in direct quotes.
>
>

[Suhas] This sentence is taken directly from RFC6714. (
https://tools.ietf.org/html/rfc6714)

>   -Note: As stated, this is untrue. MSRP has the built in ability
>   to share multiple sessions across the same transport connection.
>   But I think your point is that there is no specification about
>   how to demux MSRP from among _other_ protocols.
>
>   The MUST in the last sentence does not seem appropriate. You can't
>   really create a 2119 level requirement for people to do work in
>   the future. I suggest s/"MUST be revisited"/"could be revisited"
>
> -5.28: Same comments as for 5.26.
>
>
[Suhas] Sure, will change as suggested for both 5.26 & 5.28 .

> - 5.39:
>   I’m not a zrtp expert, but it seems like it might
>   be more complicated than this. Section 8 of 6189 says
>   “The a=zrtp-hash attribute  can only be included in the
>   SDP at the media level since Hello messages sent in
>   different media streams will have unique hashes.”
>   I wonder if this shouldn't be “identical”?
>
>
[Suhas] The category TRANSPORT is the appropriate assignment. This spec
assigns category for these attributes over the multiplexed transport.

So, if we have 3 m= lines each with one zrtp-hash and all bundled.

The zrtp-hash selected must correspond to the m=line that is selected for
BUNDLE once the O/A negotiation completes. Thus the category TRANSPORT.

> -5.40:
>   I'm surprised that the comedia attributes are not "transport"
>
>
[Suhas] . Agree. Will change as suggested.

> -5.42:
>   See previous comments about MUST level requirements for
>   doing future work. (I think this recurs for some or all TBD
>   sections.)
>

[Suhas] . Agree. Will change as suggested.

>
> -5.45:
>   6193 is an ISE stream RFC. Does anyone actually use
>   in a context where muxing matters?
>

[Suhas] . I doubt it. Do you recommend as NOT-RECOMMENDED instead.

> - 5.57:
>   See comments from 5.26
>

[Suhas] Sure , will update accordingly.

>
> -15.1:
>   - Is there no need for a spec for any newly registered categories?
>   - 2nd to last paragraph: "This seems vague for a MUST. Do you expect
>     the registration of a new category to change the categories
>     already assigned to existing SDP parameters? Is it possible
>     that a new category only applies to some new SDP parameters?"
>

[Suhas] The following 2 paragraphs are supposed to mean that new
multiplexing categories require new registrations via a spec and
applicability of the new category to new SDP attributes and possibly
existing SDP attributes ( if the new category makes more sense). What would
be a better framing to capture the intent ?

"*Further entries can be registered on a first-come first-serve basis. Each
registration needs to indicate the multiplexing category value to be added
to the "Multiplexing Categories" subregistry as defined in this section.*

*Such a registration MUST also indicate the applicability of the newly
defined multiplexing category value to various subregistries defined at
"Session Description Protocol (SDP) Parameters".*

>   - Last paragraph: 4566 procedures apply to what? The text just said
>     new categories are first-come-first server. I don’t think
>     4566 set policies for how to change mux categories.
>

[Suhas] Agreed. I prefer removing this line altogether. Please let me know

>
> -15.2.1:
>   How do you expect the table to look after adding this RFC to the
>   references? Will this RFC be added to the entries in the MUX
>   column?
>

[Suhas] The plan was to add the reference to this RFC in the "Reference
Column"

>
> -16:
>   - Do the assumptions for things like “fingerprint” and the zrtp
>     hash not have security considerations since they force multiple
>     media streams to share the same security associations?
>     (Maybe that is covered by bundle?)
>

 [Suhas]  This should be covered in bundle spec.

  - 2nd paragraph: There are more than one srtp keying mechanism—is
>     this assumed true for all of them?
>
> [Suhas]  SDES, DTLS doesn't violate RFC3711 and thus no two-time pad.
These are the currently defined mechanisms and thus i believe the statement
 holds true.

>
>
> # Editorial #
>
> - xml2rfc reports some outdated references. Are those intentional?
>
>
[Suhas] not intentional. will update them

> - The abstract is more detailed than needed. The point is to describe the
> purpose of the draft. The second paragraph pretty much accomplishes that.
>
>
[Suhas] How do this rephrase look

“The purpose of this specification is to provide a framework for analyzing
the multiplexing characteristics of SDP attributes when SDP is used to
negotiate the usage of single 5-tuple for sending and receiving media
associated with multiple media descriptions,. This specification also
categorizes the existing SDP attributes based on the framework described
herein.”

> - Please include table numbers.
>

[Suhas] Sure,  will do.

>
> - 1:
>   - First paragraph:
>   s/"This would for e.g."/This would, for example,"
>

[Suhas] Will change

>   s/"has made necessary to understand"/"has made it necessary to
> understand"
>

[Suhas] Will change

>   - 2nd paragraph:
>   "Generic future-proof framework" - This seems excessive--can we just call
>   it a framework?
>

[Suhas] Sure

>
> -3:
>     - 1st paragraph, first sentence: Convoluted sentence. Can it be
>     simplified?
>
> -4:
>     - first and second bullet example lists are each missing "and".
>
> -4.2, first paragraph: Sentence fragment. (same for several category
>   descriptions.)
>
> -4.4, first sentence: Spurious comma.
>

[Suhas] . Will fix these as suggested

>
> -4.6, last paragraph: "In the above example, the category IDENTICAL is
>    inherited for the cpar encapsulated rtcp-mux attribute."
>
>    "For" doesn't seem like the right proposition. Is the category
>    inherited "from" the cpar? "by" the cpar?
>

[Suhas] True . I will change it to "by" the cpar.

>
> -5 and subsections: Is there any method to the ordering? They aren't
>    in RFC order. It might make sense to group by the nature of the
>    attributes, but if that is the case here it's not explained.
>
>
[Suhas] These are not ordered as of today. I can order the 5.X subsections
by RFC order, if needed.

> -5.4, note: "... due to inability in meeting
>    the right resource reservations requested."
>
>    I can't parse that clause. Does it mean "inability to meet the
>    resource reservation request"? "Inability to determine the correct
>    resource reservation request"?
>

[Suhas] It's the former. Will correct it.

>
> -5.8, last paragraph:
>   - "None of this is standardized yet and it wouldn’t work
>   as explained. "
>
>   Does this mean that it has been explained why it wouldn't work?

  Or do you mean the way it is explained wouldn't work?
>

[Suhas] . It means that it doesn't work for the reasons explained in this
paragraph.

>
> -5.9:
>   -s/"[RFC6773] document specifies"/"[RFC6773] specifies"
>   - last paragraph: s/"can or cannot"/"may or may not"
>

[Suhas] . Sure will update

>
> -5.13, table:
>   "Specific RTP extension document MUST be referred"
>
>
[Suhas] The idea was to mean that , one should refer to the specification
that defines a specific RTP extension in order to understand the
multiplexing behavior.

Would the following rephrasing help instead

"Refer to the document defining specific RTP Extension"

>   I don’t understand what that means. If you mean to
>    say “refer to specific document”, that shouldn’t use MUST.
>
> -5.22, table: "document needs to be referred" (several repetitions)
>   I'm not sure what this means. Should it say "Refer to the
>   specific document"?
>
>
Similarly as above, would this reprhasing help

"Refer to the document defining specific FEC Scheme

> -5.27, table: If the "MUST be globally unique" requirement refers to
>   and existing requirment from 4583, please avoid 2119 language.
>

[Suhas] Sure , will update

>
> -5.55: "... to be referred ..."
>   I suggest "Refer to...."
>

[Suhas] Will do so

>
> -6.3: "not clearly defined offer/answer usage"
>   I'm not sure what is meant here. (Also, why is this not TBD or NOT
>   RECOMMENDED?)
>

[Suhas] Since RFC3890 b=TIAS is not defined keeping O/A in mind, either the
BUNDLE spec or this spec cannot define its mux behavior. This is due to
applicability of these specs under SD{ O/A usages alone.

I can rephrase the above as
"not defined under SDP Offer/Answer usage"

>
> -14.2.1, last paragraph:
>   Do I understand this correctly to mean that the answerer can
>   always fall back to a “common-to-all” config even if the offerer
>   offered additional independent configs?
>

[Suhas]. That's a possibility if the answerer isn't able to use any of the
additional configurations offered

>
> -14.3:
>   s/"...extends capability..."/"...extends the capability..."
>

[Suhas] Will update

>
> -14.3.1: "which MUST be followed here as well."
>   If bundle-negotiation already requires identical pt values,
>   isn’t this MUST redundant to that one?
>
>
[Suhas] . agreed and will reword

> 14.3.2:
>   I suspect that first MAY is a statement of fact, not a new
>   requirement.
>

[Suhas] . agreed and will change to may instead of MAY

>
> -15, paragraph starting with "The IANA is requested to...":
>   This sentence seems to belong with the next paragraph.
>

[Suhas] . Sure will move it

>
> _______________________________________________
> mmusic mailing list
> mmusic@ietf.org
> https://www.ietf.org/mailman/listinfo/mmusic
>