Re: [Stox] Review of: draft-ietf-stox-presence-06

[Peter Saint-Andre <[e-mail]>]

Hi Dave, sorry about the delayed processing of your feedback. Life has
gotten in the way here.

Further comments and proposed text inline.

On 12/13/2013 03:47 PM, Peter Saint-Andre wrote:
> Hi Dave, thank you for the review. Comments inline. I have not yet
> proposed text for the issues you raise, because that will take more time.
> 
> On 12/11/13 10:34 PM, Dave Crocker wrote:
>>
>> G'day.
>>
>> I have been selected as the Applications Area Review Team reviewer for
>> this draft (for background on apps-review, please see
>> http://www.apps.ietf.org/content/applications-area-review-team).
>> Please resolve these comments along with any other Last Call comments
>> you may receive. Please wait for direction from your document shepherd
>> or AD before posting a new version of the draft.
>>
>>
>>
>> Review of:    Interworking between the Session Initiation Protocol (SIP)
>> and the
>>               Extensible Messaging and Presence Protocol (XMPP): Presence
>> I-D:          draft-ietf-stox-presence-06
>>
>> Reviewer:     D. Crocker
>> Review date:  12 Dec 13
>>
>>
>>
>> Summary:
>>
>> XMPP and SIP both have deployed versions of instant messaging and
>> presence.  The current draft is part of a set of specifications that
>> define a gateway capability between the the two services.  In
>> particular, the current draft specifies the translation mechanism
>> between the presence mechanisms used by SIP and XMPP.
>>
>> The draft is well-structured and the text is mostly clear.  For an
>> implementer already familiar with the details of the two services and
>> the basics of the gatewaying specified here, the document probably is
>> sufficient -- although responses to some of the detailed comments,
>> below, might to turn out to show that a bit more work is needed. However
>> at the most, any technical improvements that might be needed will be
>> minor.  And I expect these to more in the nature of clarifying language
>> than of changing bits over the wire.
>>
>> However for a reader who is new to the topic, the document needs to be
>> clearer and sometimes more complete.
>>
>> Specific changes or concerns:
>>
>>    1.  When citing the earlier efforts, there should be something to
>> explain why the current work is expected to be more successful. 
> 
> Not "expected", but "is". :-) AFAIK there are no implementations of the
> approach taken by RFC 3922 and draft-ietf-simple-cpim-mapping (i.e.,
> mapping to the abstract semantics of RFC 3860), whereas there are
> multiple implementations of the approach described here (i.e., direct
> mapping between SIP and XMPP).
> 
>> That is,
>> what makes the current approach notably tractable for implementation and
>> deployment?
> 
> Do you think that explaining why would improve the document? IMHO this
> verges into developer psychology (I've got this SIP thing here and this
> XMPP thing there, let me see how I can make them work as easily as
> possible without venturing off into abstract semantic stuff).

Text adjusted to read:

   One approach to helping ensure interworking between these protocols
   is to map each protocol to the abstract semantics described in
   [RFC3860]; although that is the approach taken by both [RFC3922] and
   [I-D.ietf-simple-cpim-mapping], to the best of our knowledge that
   approach has never been implemented.  The approach taken in this
   document is to directly map semantics from one protocol to another
   (i.e., from SIP/SIMPLE to XMPP and vice-versa), since that is how
   existing systems solve the interworking problem.

See also http://tools.ietf.org/rfcdiff?url2=draft-ietf-stox-core-09.txt
for relevant changes to the "core" STOX document.

>>    2.  The document is not clear about the prerequisites for reading
>> this draft.  What must the reader already know in depth?  What must they
>> have at least superficial knowledge of?  I suspect that, in particular,
>> they need deep familiarity with stox-core.
> 
> At the very least. Also RFC 3261, RFC 3856, RFC 6120, and RFC 6121.
> (Note: the total page count of those RFCs is 621.) The specifications
> normatively referenced by RFC 3856 are also relevant.

Added the following section:

2.  Intended Audience

   The documents in this series are intended for use by software
   developers who have an existing system based on one of these
   technologies (e.g., SIP), and would like to enable communication from
   that existing system to systems based on the other technology (e.g.,
   XMPP).  We assume that readers are familiar with the core
   specifications for both SIP [RFC3261] and XMPP [RFC6120], with the
   base document for this series [I-D.ietf-stox-core], and with the
   following presence-related specifications:

   o  A Presence Event Package for the Session Initiation Protocol
      [RFC3856]

   o  Presence Information Data Format (PIDF) [RFC3863]

   o  Extensible Messaging and Presence Protocol: Instant Messaging and
      Presence [RFC6121]

   o  SIP-Specific Event Notification [RFC6665]

>>    3.  Saying that terms are taken from a substantial number of other
>> documents, and then merely citing the documents in their entirety, is
>> not helpful to the reader, unless the reader is required to be deeply
>> familiar with those documents. 
> 
> I do think it's required.

See text posted above.

>> If that's what this document requires,
>> say that.  If it's not, I suggest listing terms explicitly and
>> indicating what document they are drawn from.
>>
>>    4.  As the architecture diagram in Section 3 of stox-core shows, this
>> service has at least separate actors Actually I think it actually has at
>> least 6, which that diagram probably should show explicitly.  The six are:
>>
>>        a. SIP user
>>        b. SIP server
>>        c. SIP-XMPP gateway
>>        d. XMPP-SIP gateway
>>        e. XMPP server
>>        f. XMPP user
>>
>>        In any event, the specification needs to be very diligent to
>> carefully identify each actor involved in an activity and the
>> interaction between actors.  The current document uses language that
>> often left me wondering such things as who was the target of the action.
>>
>>        Much of this would be aided by some form of protocol sequence
>> diagrams, to show who does what and in what order.
> 
> I'll review the document again with these comments in mind. Your
> suggestion of sequence diagrams is a good one.

I've added sequence diagrams.

>>    5.  When showing protocol examples, usually only one side of the
>> sequence is shown.  For gatewaying that does interesting transforms,
>> such as this one, an example should show both the before and after
>> versions.  That is, the 'native' protocol unit that was created and then
>> the one that results from the translation.
> 
> I thought we'd already done that. For instance, Example 2 is the SIP
> transformation of the XMPP stanza shown in Example 1.

I've made the example descriptions a bit clearer in this regard.

>> Detailed Comments:
>>
>>
>>> Table of Contents
>>
>>
>> Nicely organized and concise TOC [ie, document structure).
>>
>>
>>
>>> 1.  Introduction
>> ...
>>>    One approach to helping ensure interworking between these protocols
>>>    is to map each protocol to the abstract semantics described in
>>>    [RFC3860]; that is the approach taken by both [RFC3922] and
>>>    [I-D.ietf-simple-cpim-mapping].  The approach taken in this document
>>>    is to directly map semantics from one protocol to another (i.e., from
>>>    SIP/SIMPLE to XMPP and vice-versa).
>>
>> This begs the obvious question:  Why?  What was wrong with the previous
>> approach?
> 
> No one implemented it. The running code all takes the direct gatewaying
> approach.
> 
>> The purpose of the question is not for criticizing prior work but to
>> understand the expected benefit of the approach taken in the current
>> work.  What are the function, or OA&M differences produced through this
>> new approach, that are expected to be helpful?
> 
> These documents describe what people do in the field, what works and
> what doesn't, etc. We're not saying that other approaches are bad or
> infeasible.

See previous note.

>>>    The architectural assumptions underlying such direct mappings are
>>>    provided in [I-D.ietf-stox-core], including mapping of addresses and
>>>    error conditions.  The mappings specified in this document cover
>>>    basic presence functionality.  Mapping of more advanced functionality
>>>    (e.g., so-called "rich presence") is out of scope for this document.
>>>
>>>    SIP and XMPP differ significantly in their presence subscription
>>>    models, since SIP subscriptions are short-lived (requiring relatively
>>>    frequent refreshes even during a presence session) whereas XMPP
>>>    subscriptions last across presence sessions until they are explicitly
>>>    cancelled.  This document provides suggestions for bridging the gap
>>>    between these very different models.
>>>
>>>
>>> 2.  Terminology
>>>
>>>    A number of terms used here are explained in [RFC3261], [RFC3856],
>>>    [RFC6120], and [RFC6121].
>>
>> This sentence probably implies that the current draft should not be read
>> in the absence of familiarity with all 4 of those RFCs.  I suggest some
>> sort of explicit statement about how much prior knowledge is needed for
>> understanding the current draft, and where to get that knowledge.
> 
> Agreed.

See note above about the new Section 2.

>> In purely mechanical terms, the problem with the above sentence is that
>> it means that when the reader sees a term they don't understand, they
>> have to run around to four different documents looking for definitions.
>>  This mostly ensures reader frustration, for everyone but experts.
>>
>> The cleanest fix for this is to list terms and where they are defined.
>> The other, usual fix is to indeed say that the reader must be familiar
>> with those other documents before reading this one.  (sigh.)
> 
> I think these documents are for experts. In all likelihood, members of
> the target audience already have a SIP implementation and need to
> connect it to the XMPP network, or vice-versa. I don't think someone who
> is not familiar with either SIP or XMPP (or both) is going to hack up a
> gateway. There are much more rewarding tasks one might choose.
> 
> To your point, we need to say that.
> 
>>> 3.1.  Overview
>> ...
>>>    As described in [RFC6121], XMPP presence subscriptions are managed
>>>    using XMPP presence stanzas of type "subscribe", "subscribed",
>>>    "unsubscribe", and "unsubscribed".  The main subscription states are
>>>    "none" (neither the user nor the contact is subscribed to the other's
>>>    presence information), "from" (the user has a subscription from the
>>>    contact), "to" (the user has a subscription to the contact's presence
>>>    information), and "both" (both user and contact are subscribed to
>>>    each other's presence information).
>>
>> Nit:  for technical documents, lists like the above should be shown in
>> list format.  It really does help for quick comprehension.
> 
> Will fix.

Done.

>>>    As described in [RFC3856], SIP presence subscriptions are managed
>>>    through the use of SIP SUBSCRIBE events sent from a SIP user agent to
>>>    an intended recipient who is most generally referenced by a Presence
>>>    URI of the form <pres:user@domain> but who might be referenced by a
>>>    SIP or SIPS URI of the form <sip:user@domain> or <sips:user@domain>.
>>>
>>>    The subscription models underlying XMPP and SIP are quite different.
>>>    For instance, XMPP presence subscriptions are long-lived (indeed
>>>    permanent if not explicitly cancelled), whereas SIP presence
>>>    subscriptions are short-lived (the default time-to-live of a SIP
>>>    presence subscription is 3600 seconds, as specified in Section 6.4 of
>>>    [RFC3856]).  These differences are addressed below.
>>
>> The text that follows this 'addresses' the differences in terms of
>> specifying how to handle specific differences.
>>
>> What's missing -- but I think would aid for the framework of this
>> document's effort -- is for the above "For instance" to instead be
>> expanded into a detailed statement of what the differences are, separate
>> from the later text that says how to deal with the differences.
> 
> That "for instance" is the main thing, so you're right that it needs to
> be described in more detail.

Changed to:

   The subscription models underlying XMPP and SIP differ mainly in the
   fact that XMPP presence subscriptions are long-lived (indeed
   permanent if not explicitly cancelled, so that a subscription need
   never be refreshed during any given presence "session"), whereas SIP
   presence subscriptions are short-lived (the default time-to-live of a
   SIP presence subscription is 3600 seconds, as specified in
   Section 6.4 of [RFC3856], so that a subscription needs to be
   explicitly refreshed if it will have the appearance of being
   permanent or even of lasting as for the duration of a presence
   "session").  This disparity has implications for the handling of
   subscription cancellations in either direction and, from the SIP
   side, subscription refreshes.

[much text snipped where we have areas of agreement]

Thanks again for the review! I'll post a revised I-D soon.

Peter