[MMUSIC] Adam Roach's Discuss on draft-ietf-mmusic-trickle-ice-sip-14: (with DISCUSS and COMMENT)

[Adam Roach <[e-mail]>]

Adam Roach has entered the following ballot position for
draft-ietf-mmusic-trickle-ice-sip-14: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-mmusic-trickle-ice-sip/



----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Thanks to everyone who worked on this document. I have a couple of concerns that
need to be cleared up before the document can move forward.

---------------------------------------------------------------------------

§4.3.4 discusses the interaction of 3PCC with the trickle ICE mechanism.
Unfortunately, the diagrams used in this section do not show a 3PCC signaling
flow; they show a two-party call flow with an offerless INVITE. A 3PCC call flow
would necessarily involve a 3PCC controller sending an offerless INVITE to one
party, receiving an offer from that party (typically in a reliable provisional
response or in a 200 OK), and then sending an INVITE to the other party
containing that offer.

The text in this section matches the diagrams, and consequently does not appear
to be an analysis of 3PCC behavior. It is an analysis of two-party offerless
INVITE behavior.

If this section remains, it needs to be substantially re-worked: the diagrams
need to show three parties, with a 3PCC controller performing the controlling
role as described in RFC 3725. While I haven't stepped through the implications
for Trickle ICE when a controller is actually involved and is moving offers and
answers around between different message types, I suspect that the analysis in
here is substantially different once this starts happening.

I would personally be okay if the entire section were removed; however, I
have no desire to override working group consensus regarding the value of a
section dealing with 3PCC considerations.

---------------------------------------------------------------------------

The second issue doesn't necessarily pertain to this document per se as much
as it does the interaction among this document, draft-ietf-ice-trickle (Trickle
ICE), draft-ietf-mmusic-ice-sip-sdp (ICE SDP), and draft-ietf-rtcweb-jsep
(JSEP). The problem doesn't lie with any single document, but in the overall
result of how they're currently structured.

JSEP (in the RFC editor queue) refers to the "a=end-of-candidates" SDP attribute
as appearing in Trickle ICE, section 9.3, which was true at one point in time.
Somewhere along the line, this attribute's definition was moved from there
into this document.

There are several ways to fix this, each with their own drawbacks:

1. Move the SDP syntax for "a=end-of-candidates" back into the Trickle ICE
   draft. Drawback: Trickle ICE does not currently define any normative SDP
   behavior; and, in fact, could work in a system that doesn't use SDP at all.

2. Move the SDP syntax into the ICE SDP draft. This is pretty elegant from the
   perspective that ICE SDP defines SDP syntax for ICE in general (for both
   SIP and JSEP), and such a move aggregates all of the SDP syntax into a
   single document that is already necessary to reference from any document
   that uses SDP for Trickle ICE. Drawback: the document doesn't presently
   discuss Trickle ICE at all, and this would require a somewhat awkward
   section that basically says "If you use [Trickle ICE] with SDP, the syntax
   for the end-of-candidate marker is..."

3. Change JSEP to normatively depend on this document for the
   "a=end-of-candidates" syntax. Drawback: This document is extremely
   SIP-specific, while JSEP is based solely on RFC 4566 syntax and RFC 3264
   behavior, independent of any SIP semantics.  Forcing JSEP to normatively
   depend on a SIP specific document for a simple attribute syntax definition
   seems to be letting the tail wag the dog.

I believe that #2 is the least inelegant option, but I'm open to #1 and #3.
However, The *current* situation -- in which JSEP normatively points to a
document from which the text is cites has been removed out from under it -- is
clearly wrong.


----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

§4.1.4:

>  If the initial Answer included the attribute a=ice-options:trickle,
>  the Offerer MUST be prepared for receiving trickled candidates later
>  on.

I don't think this makes sense. If the initial *Offer* includes that option,
then the Offerer must be prepared to receive trickled candidates, even before
the Answer arrives. Figure 6 shows an example of this. I believe that the
paragraph I quote above needs to be entirely removed, as it may lead
implementors to incorrectly assume that they can't get trickled candidates prior
to an Answer.

---------------------------------------------------------------------------

§4.3:

>    o  The dialog at both peers MUST be in early or confirmed state.

This makes no sense. Dialogs exist in precisely two states, ever: early or
confirmed. Prior to "early," they don't exist. The only exit from "confirmed" is
destruction. (See RFC 3261 §12).

I think what this means to say is "A dialog MUST have been created between the
peers."

---------------------------------------------------------------------------

§4.3.2:

>  These retransmissions MUST
>  cease on receipt of an INFO request carrying a 'trickle-ice' Info
>  Package body or on transmission of the Answer in a 2xx response.

Why does this require that specific INFO to stop retransmission? The only
requirement is that the answerer knows that the offerer has received the
provisional response. The offerer cannot send post-INVITE requests to the
answerer until a dialog is established, and the only ways a dialog can be
established at the offerer is receipt of a provisional or final response. So, as
soon as you get *any* request from the offerer, you know that the provisional
response has arrived.

I think this passage means to say "These retransmissions MUST cease on receipt
of any in-dialog request from the offerer. The offerer cannot send in-dialog
requests until it receives a response, so the arrival of such a request proves
that the response has arrived."

---------------------------------------------------------------------------

§4.3.2:

>  The Offerer MUST send a Trickle ICE INFO request as soon as it
>  receives an SDP Answer in an unreliable provisional response.  This
>  INFO request MUST repeat the candidates that were already provided in
>  the Offer (as would be the case when Half Trickle is performed or
>  when new candidates have not been learned since then).

It is probably worth mentioning that it's possible that this INFO contains no
candidates, depending on how the offerer gathers its candidates and how long it
takes to do so.

---------------------------------------------------------------------------

§4.3.3:

>  These retransmissions MUST cease on receipt
>  of an INFO request or on transmission of the Answer in a 2xx
>  response.

Same comment as above for §4.3.2.

---------------------------------------------------------------------------

§4.3.4:

>  As specified in Section 4 this offerless INVITE
>  MUST include the SIP option-tag 'trickle-ice' in a SIP Supported:
>  header field in order to indicate support for Trickle-ICE to the
>  Offerer (at the User Agent Server (UAS)).

How does the 3PCC controller know to include this option tag? Is it simply
indicating that the 3PCC controller supports the Trickle mechanism? If so,
what happens if the second party (the one who receives the offer in an INVITE)
doesn't support Trickle ICE? This is one of the sharper corner cases that I
think arises when you go to fix the analysis I describe in my DISCUSS above.

---------------------------------------------------------------------------

§4.4:

>     a=candidate:2 1 UDP 1694498815 2001:db8:a0b:12f0::3 5000 typ srflx
>         raddr 2001:db8:a0b:12f0::1 rport 8998

Thanks for the IPv6 example; however, I have a *lot* of heartburn with the
selection of an example that demonstrates IPv6 NAT behavior. Since ICE's srflx
behavior is fundamentally tied to IPv4 NATs (and should not be an issue with
IPv6, as NATs are unnecessary), I think it's okay for the srvflx examples to go
ahead and show IPv4 addresses.

I *really* don't want to publish an RFC that demonstrates NATting of IPv6.

---------------------------------------------------------------------------

§4.4:

>  An "a=end-of-candidates" attribute, preceding any pseudo "m=" line,
>  indicates the end of all trickling from that agent, as opposed to end
>  of trickling for a specific "m=" line, which would be indicated by a
>  media level "a=end-of-candidates" attribute.

I finally did figure out what this meant, but it's confusing. The use of "any"
in the phrase "any pseudo 'm=' line" implies that the following sdpfrag would
qualify:

      a=ice-pwd:asd88fgpdd777uzjYhagZg
      a=ice-ufrag:8hhY
      m=audio 9 RTP/AVP 0
      a=mid:1
      a=candidate:1 1 UDP 2130706431 2001:db8:a0b:12f0::1 5000 typ host
      a=candidate:1 2 UDP 2130706431 2001:db8:a0b:12f0::1 5001 typ host
      a=end-of-candidates
      m=audio 9 RTP/AVP 0
      a=mid:2
      a=candidate:1 1 UDP 2130706431 2001:db8:a0b:12f0::1 6000 typ host
      a=candidate:1 2 UDP 2130706431 2001:db8:a0b:12f0::1 6001 typ host

The end-of-candidates *does* appear before the second pseudo "m=" line, which is
one of the lines (thereby satisfying the "any" criteria).

I think you mean "preceding the first pseudo 'm=' line..."

This is true later in the section also:

>  will not send any further candidates.  When included at session
>  level, i.e. before any pseudo "m=" line, this indication applies to

---------------------------------------------------------------------------

§4.4:

>  Note: At the time of writing this specification there were ongoing
>  discussions if a functionality for removing already exchanged
>  candidates would be useful.  Such a functionality is out of the scope
>  of this specification and most likely needs to be signaled by means
>  of a yet to be defined ICE extension, although it could in principle
>  be achieved quite easily, e.g. without anticipating any solution by
>  simply omitting a previously sent candidate from a subsequent INFO
>  request.  However, if an implementation according to this
>  specification receives such an INFO request with a missing candidate
>  it would have to treat that as an exceptional case.  Implementing
>  appropriate recovery procedures at the receiving side is advisable
>  for this situation.  Ignoring that a candidate was missing might be a
>  sensible strategy.

This seems like an interop nightmare. If you want a note about this possibly
being included in the future, please simply indicate that such an ability will
need to be explicitly signaled. Don't imply that implementations might get SDP
that violates the behavior defined in this document in some ill-defined way,
and just need to kind of gracefully deal with it, even if we don't really know
what it means at this point in time. There's no sensible way to code for that.

---------------------------------------------------------------------------

§4.4:

>     a=candidate:2 1 UDP 1694498815 2001:db8:a0b:12f0::3 5000 typ srflx
>        raddr 2001:db8:a0b:12f0::1 rport 8998
>     a=candidate:2 2 UDP 1694498815 2001:db8:a0b:12f0::3 5001 typ srflx
>        raddr 2001:db8:a0b:12f0::1 rport 8998
...
>     a=candidate:2 1 UDP 1694498815 2001:db8:a0b:12f0::3 6000 typ srflx
>        raddr 2001:db8:a0b:12f0::1 rport 9998
>     a=candidate:2 2 UDP 1694498815 2001:db8:a0b:12f0::3 6001 typ srflx
>        raddr 2001:db8:a0b:12f0::1 rport 9998

Same comment as above regarding IPv6 and NATs. Note that it would be Good and
Helpful to show SDP with a mixture of IPv4 and IPv6 anyway.

---------------------------------------------------------------------------

§5:

>  SIP User Agents (UAs) that support and intend to use trickle ICE are
>  required by [I-D.ietf-ice-trickle] to indicate that in their Offers
>  and Answers using the attribute "a=ice-options:trickle" and MUST
>  include the SIP option-tag "trickle-ice" in a SIP Supported: header
>  field.

Is this intended to imply that the option tag cannot be used in a Require header
field? I don't think that's a good idea; however, if that is the working group's
intention, please say so explicitly.

Suggest "...in a SIP 'Supported' or 'Require' header field."

---------------------------------------------------------------------------

§5.1:

This section discusses behavior in a walled garden. I'm generally fine with
this, as long as the failure modes when someone starts adding doors to the
walled garden are well-defined. I don't think this kind of provisioning is
okay unless it has a requirement that Offerers assuming Trickle ICE support
MUST include a "Require: trickle-ice" header field. That way, if the
provisioned assumption of Trickle ICE support ends up being incorrect, the
failure is (a) operationally easy to track down, and (b) recoverable by the
client (i.e.: they can re-send the request without the "Require" header field
and without the assumption of Trickle ICE support).

---------------------------------------------------------------------------

§6:

>  This INFO request indicates that the Answerer supports and uses RTP
>  and RTCP multiplexing as well.  It allows the Offerer to omit
>  gathering of RTCP candidates or releasing already gathered RTCP
>  candidates.

It's not clear how this "releasing already gathered RTCP candidates" interacts
with the requirement in §4.4:

>  In other words, the sequence
>  of a previously sent list of candidates MUST NOT change in subsequent
>  INFO requests and newly gathered candidates MUST be added at the end
>  of that list.

So, if I had already trickled candidates for the RTCP component of a connection
and *then* got an INFO from the Answerer indicating that it supports
multiplexing, does that mean I free the resources and exclude them from future
INFO messages? That seems to contravene the §4.4 requirement. Or do I free them
but continue to include them? I suppose that's okay (maybe?), but it's
extremely counterintuitive, and would need to be called out explicitly if you
expect implementors to get it right.

---------------------------------------------------------------------------

§7:

>  A Trickle Answerer SHOULD include an "a=group: BUNDLE" attribute
>  [I-D.ietf-mmusic-sdp-bundle-negotiation] in the application/trickle-
>  ice-sdpfrag body if it supports and uses bundling.

This needs to clearly indicate that this needs to be included at the session
level (rather than the media level).

---------------------------------------------------------------------------

§7:

>  The Offerer can use this information e.g. for stopping the gathering
>  of candidates for the remaining "m=" lines in a bundle and/or for
>  freeing corresponding resources.

Same comment as for §6, above (regarding interaction with the requirement from
§4.4)

---------------------------------------------------------------------------

§7:

>      a=group:BUNDLE foo bar
>      a=ice-pwd:asd88fgpdd777uzjYhagZg
>      a=ice-ufrag:8hhY
>      m=audio 9 RTP/AVP 0
>      a=mid:foo
>      a=rtcp-mux
>      a=candidate:1 1 UDP 1658497328 2001:db8:a0b:12f0::3 5000 typ host
>      m=audio 9 RTP/AVP 0
>      a=mid:bar

The presence of the second m= line here is confusing, given the following text
from §4.4:

>  Note, that there is no requirement that the Info request body
>  contains as many pseudo m= lines as the Offer/Answer contains m=
>  lines, nor that the pseudo m= lines be in the same order as the
>  m=lines that they pertain to.  The correspondence can be made via the
>  "a=mid:" attributes.

The §4.4 text strongly implies that it is nonsensical to include an m= section
without candidates. Is the example in §7 intended to imply that the section
needs to be included because its mid is mentioned in the "a=group:BUNDLE" line?
If so, please say so. If not, please remove the second m= section.

---------------------------------------------------------------------------

§9.2:

The syntax for "session-level-fields", "pseudo-media-descriptions", and
"trickle-ice-attribute-fields" include extremely strict rules around ordering of
fields (e.g., including ice-ufrag before ice-pwd would be syntactically
invalid). That level of strictness seems unlikely to lead to interoperable
implementations.

If the intention is to be rigid in this fashion, please add prominent prose
that warns implementors that fields MUST appear in the order specified, and
that all other orders are invalid and MUST be rejected.

If that's *not* your intention (and I suspect it isn't), then please fix the
syntax definition to allow for arbitrary ordering of attributes in the same way
as SDP does. For example:

    session-level-fields = *(session-level-field CRLF)

    session-level-field = bundle-group-attribute /
                          ice-lite-attribute /
                          ice-pwd-attribute /
                          ice-ufrag-attribute /
                          ice-options-attribute /
                          ice-pacing-attribute /
                          end-of-candidates-attribute /
                          extension-attribute-fields
                                 ; for future extensions


---------------------------------------------------------------------------

§9.2:

The syntax for "session-level-fields" implies that "ice-pwd" and "ice-ufrag" are
mandatory at the session level. This seems to conflict with the text in §4.4:

>  The "a=ice-pwd:" and "a=ice-ufrag:" attributes MUST appear at the
>  same level as the ones in the Offer/Answer exchange.  In other words,
>  if they were present as session-level attributes, they will also
>  appear at the beginning of all INFO request payloads, i.e. preceding
>  all pseudo "m=" lines.  If they were originally exchanged as media
>  level attributes, potentially overriding session-level values, then
>  they will also be included in INFO request payloads following the
>  corresponding pseudo "m=" lines.

The fix I propose above removes this implication that such attributes are
mandatory at the session level; however, if you choose to fix it some other way,
please ensure that the ABNF does not conflict with the normative language I
quote above.

---------------------------------------------------------------------------

§10.6:

Same comment as for §5 above.

---------------------------------------------------------------------------

§10.9:

>  If the INFO requests are sent on top of TCP, which is probably the
>  standard way, this is not an issue for the network anymore, but it
>  can remain one for SIP proxies and other intermediaries forwarding
>  the SIP INFO messages.  Also, an endpoint may not be able to tell
>  that it has congestion controlled transport all the way.

This seems kind of blithe about congestion control, as it concedes that the INFO
requests might end up on UDP at some point. Minimally, I would think that you
want to require some minimum quarantine period between INFO requests (probably
something in the range of 20 ms), during which any new candidates that are
gathered are aggregated into the next INFO message.

===========================================================================

Nits:

Expand "ICE" in the title.

---------------------------------------------------------------------------

draft-ietf-mmusic-ice-sip-sdp has undergone a major restructuring since version
16; as a consequence, all notes of the form:

>  [RFC EDITOR NOTE: The section 4.2.1.2.1 in above sentence is correct
>  for version 16 of said I-D.  Authors need to cross-check during
>  Auth48 since it could have have changed in the meantime.]

...are already out of date. It's probably worthwhile to fix these now, as they
are (in my opinion) quite unlikely to change again; and the current misdirection
makes it somewhat difficult to use the draft in its current form.

---------------------------------------------------------------------------

§4.1.3:

>  If the Answerer includes candidates in its initial Answerer, it MUST

"...initial Answer..."

---------------------------------------------------------------------------

§7:

>  A Trickle Answerer SHOULD include an "a=group: BUNDLE" attribute

Remove the space before "BUNDLE"

---------------------------------------------------------------------------

§11:

>  Trickle ICE uses two mechanism for exchange of candidate information.

"mechanisms"