Re: [MMUSIC] Some issues with draft-ietf-mmusic-trickle-ice-sip-02

[Paul Kyzivat <[e-mail]>]

Thomas - further comments inline...

On 7/27/15 11:10 AM, Thomas Stach wrote:
> Paul,
>
> Am 24.07.2015 um 13:27 schrieb Paul Kyzivat:
>> Thomas,
>>
>> Comments inline...
>>
>> On 7/24/15 6:46 AM, Thomas Stach wrote:
>>> Paul,
>>> thanks for looking into this and pointing out the issues
>>> I think they can be fixed quite easily.
>>> More below
>>>
>>> Am 23.07.2015 um 01:58 schrieb Paul Kyzivat:
>>>> This is not a complete review of the document - I'm focusing only on
>>>> the sdpfrag related stuff.
>>>>
>>>> Section 8.1 says:
>>>>
>>>>    A valid application/sdpfrag part could be obtained by starting with
>>>>    some valid SDP session description [I-D.ietf-mmusic-rfc4566bis] and
>>>>    deleting any number of lines.
>>>>
>>>> Then section 8.2 says:
>>>>
>>>>    The ABNF of an 'application/sdpfrag' body is based on
>>>>    [I-D.ietf-mmusic-rfc4566bis] with the following modification
>>>>
>>>>       ; SDPfrag Syntax
>>>>              sdp-frag = *1proto-version
>>>>                         *1origin-field
>>>>                         *1session-name-field
>>>>                         *1information-field
>>>>                         *1uri-field
>>>>                         *1email-fields
>>>>                         *1phone-fields
>>>>                         *1connection-field
>>>>                         *1bandwidth-fields
>>>>                         *1time-fields
>>>>                         *1key-field
>>>>                         *1attribute-fields
>>>>                         *1media-descriptions
>>>>
>>>> Those two sections are inconsistent. Some of the individual elements
>>>> above (media-descriptions and time-fields) expand into more than one
>>>> line. This leads to the possibility that you could take valid SDP body
>>>> and delete some lines, leading to something that doesn't match the
>>>> above syntax.
>>> I'll propose a more elaborate ABNF that will address this in the updated
>>> draft. I think it will just need a few tweaks.
>>
>> This whole issue is aggravated by the obtuse way that the ABNF syntax
>> of SDP is constructed. It makes it easy to miss seeing why there is a
>> problem here. (It was only recently that I realized the complex
>> relationship of t=,r=,z= and I've been studying this syntax for
>> years.) It also complicates writing an ABNF for sdpfrag using
>> references to rules in the SDP draft.
>> Since we have an SDPbis in progress, it might be worth discussing if
>> we should clean up the ABNF used to define the SDP syntax, in a way
>> that would make it clearer to a reader what is going on, and also
>> simplify what you are looking for.
> Agreed that the ABNF is ugly, but that's a good fit for the rest of SDP.
> Also agreed that it can make things easier when defining ABNF for
> sdpfrag, but I'm not conviced that we should go down that road
> considering the level of maturity that SDP has achieved in the last 17
> years.

I offered it up as a possibility. But I'm not going to insist.

>> For that matter, we might consider defining sdpfrag in sdpbis.
> I'd like to avoid making trickle-ice-sdp depending to 4566bis. This
> could delay things quite a bit.

Fair enough.

>>
>> If we did this, we would have to be careful that the new abnf is
>> equivalent to the old one, for backward compatibility. That should be
>> quite straightforward to do.
>>
>> There is a possibility that such a change could invalidate some other
>> existing rfc or draft that references one of the affected rules. That
>> would require some research.
> Another source for additional delay.
>>
>>>> For example, media-descriptions allows a variety of other lines
>>>> following an m= line. If you take a valid SDP and delete the m= lines
>>>> you can get something that has a c= following an a=.
>>>>
>>>> Similarly, time-fields only allows an r= line following a t= line.
>>>> According to 8.1 you could take an SDP containing t=,r= and delete the
>>>> t=, leaving a "naked" r= line. But the sdp-frag syntax in 8.2 can't
>>>> generate that.
>>> Will be fixed.
>>>>
>>>> Beyond that, both of those introduce context-sensitivity. E.g., an a=
>>>> following an m= has a different significance than one preceding the
>>>> first m=. If you removed all the m= lines from an SDP the result would
>>>> be incomprehensible. So the notion of stripping arbitrary lines from
>>>> and SDP and making sense of the result is nonsense.
>>> I agree that you can generate nonsense by removing arbitray lines from
>>> SDP. However, I don't intent to prevent that in general.
>>> In the next update I can put some warning text around this .
>>
>> That would be helpful.
>>
>>> The draft already says "The exact content of an 'application/sdpfrag'
>>> body MUST be specified the using protocol."
>>> Section 9.6 defines the necessary contraints for trickle-ice. Other
>>> future application will need to do something similar.
>>
>> My concern here is that this really means nobody needs/uses the syntax
>> of sdpfrag.
> That's a valid concern. The sdpfrag syntax  would of course be the
> basis/superset for other more constrained app-specifiic ABNFs.

But it really isn't the basis. Each needs to define its own syntax.
That is possible whether sdpfrag is defined or not.

The only good use I can think of for sdpfrag is as supporting 
information in reporting errors, or for something like the way sipfrag 
is used with the refer event package.

>> Everybody must define a new syntax, such that every body that matches
>> the protocol-specific syntax also matches the sdpfrag syntax.
> Exactly. Isn't this what we wanted? It constrains what you can put into
> the app-specific syntax and avoids that app-specifc syntax is totally
> arbitrary.
>> This is in addition to the rule that every body that matches the sdp
>> syntax must also match the sdpfrag syntax.
> It should follow naturally that SDP syntax is also a valid sdpfrag and I
> don't consider it a bad thing.

My point is that the requirement that app-specific subsets of sdp *also* 
match the syntax of sdpfrag isn't helpful.

AFAICT, the only real reason is so that a single generic content-type 
(application/sdpfrag) can be used rather than defining an app-specific 
content-type.

>> IIUC, the "value" of sdpfrag is that a single content-type can be
>> defined and used for all these different protocols. But then the
>> content-type isn't really precise, since not all bodies that conform
>> to its syntax are value for any particular usage of it.
> What do mean by 'precise' in this context?
> A valid body for a particular usage is always a valid sdpfrag, but no
> vice versa.
> I seem to remember that we didn't want that a trickle-ice agent needs to
> make sense out of arbitrary sdpfrags.
> Thus, a trickle-ice INFO package body is certainly a valid sdpfrag, but
> it adheres to a specifc structure and does not allow e.g. c=, b=, t=,...
> lines.

And that constraint would be *enhanced* if the content-type were 
application/trickle-ice-sdpfrag, rather than just application/sdpfrag.

>> ISTM that having a content-type that is specific to the allowed
>> content would be preferable. If that is done, then there ceases to be
>> any real need for sdpfrag.
>>
>>>> It does turn out that the definition of trickle-ice-sdpfrag is
>>>> consistent with the syntax of sdp-frag. So there isn't a big problem.
>>>> If you remove/replace the textual definition of sipfrag in 8.1 then
>>>> things are consistent and make sense.
>>>>
>>>> But is there any need for application/sdpfrag? Wouldn't it be more
>>>> straightforward to define application/trickle-ice-sdpfrag?
>>> My understanding from IETF90 was that there was consensus that the
>>> principle of taking vaild SDP and remove arbitrary lines should be kept,
>>> but there need to be application-specific contraints that define the
>>> semantics of a specific sdpfrag body.
>>> Since your issues can be fixed quite easily there isn't need to deviate
>>> from that concept.
>>
>> I agree that it *can* be fixed as you say. The question is whether
>> that is the best way forward.
> Maybe not the best way, but certainly a pragmatic way. Engineers often
> don't need it perfect, it's ok if it works just good enough.
> As next step I propose to correct the ABNF based on what we have now and
> continue from there.

Certainly it will work if the syntax for sdpfrag is "fixed".

An alternative is:

- drop all mention of application/sdpfrag

- instead, define content-type application/trickle-ice-sdpfrag, using
   the syntax in section 9.6.

	Thanks,
	Paul