Subject: [Gen-art] Gen-Art review of draft-ietf-sipcore-rfc3265bis-07.txt

From: Alexey Melnikov <[email-masked]>
Date: Tue, 10 Apr 2012 11:46:44 +0100

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-sipcore-rfc3265bis-07.txt
Reviewer: Alexey Melnikov
Review Date: 10-April-2012
IETF LC End Date: past
IESG Telechat date: 26-April-2012

Summary: This documents is ready for publication as a Proposed Standard 
RFC (with nits). But please see a couple of questions below.

3.2.1.  Identification of Reported Events, Event Classes, and Current
         State

    When present, the body of the NOTIFY request MUST be formatted into
    one of the body formats specified in the "Accept" header field of the
    corresponding SUBSCRIBE request.  This body will contain either the
    state of the subscribed resource or a pointer to such state in the
    form of a URI (see Section 5.4.13).

Nit: or the default according to the event package definition, if no Accept
header field was specified.

Also, it might be good to reference RFC 3986 for URIs here.

4.1.1.  Detecting Support for SIP Events

    The extension described in this document does not make use of the use
    of "Require" or "Proxy-Require" header fields; similarly, there is no

Nit: too many "use of".

    token defined for "Supported" header fields.  Potential subscribers
    may probe for the support of SIP Events using the OPTIONS request
    defined in [RFC3261].

4.1.3.  Receiving and Processing State Information

    To prevent spoofing of events, NOTIFY requests SHOULD be
    authenticated, using any defined SIP authentication mechanism.

Minor: How can this SHOULD be satisfied? Any reference which might be 
appropriate here?

4.2.1.3.  Authentication/Authorization of SUBSCRIBE Requests

    SIP authentication mechanisms are discussed in [RFC3261].  Note that,
    even if the notifier node typically acts as a proxy, authentication
    for SUBSCRIBE requests will always be performed via a "401" response,
    not a "407;" notifiers always act as a user agents when accepting

Nit: Is the ";" after "407" a typo?

    subscriptions and sending notifications.

4.4.4.  Allow-Events header field usage

    The "Allow-Events" header field does not include a list of the etvent

  typo: event

    template packages supported by an implementation.  If a subscriber
    wishes to determine which event template packages are supported by a
    notifier, it can probe for such support by attempting to subscribe to
    the event template packages it wishes to use.

Can you clarify how such request would look like? An example would be nice.

5.4.3.  SUBSCRIBE Request Bodies

    It is expected that most, but not all, event packages will define
    syntax and semantics for SUBSCRIBE request bodies; these bodies will
    typically modify, expand, filter, throttle, and/or set thresholds for
    the class of events being requested.  Designers of event packages are
    strongly encouraged to re-use existing MIME types for message bodies
    where practical.

Nit: MIME types are now called "media types" in more recent IETF RFCs.
I would recommend pointing to the Media Type Registration Procedure 
document [RFC 4288] here, which points to the IANA registry.

5.4.5.  NOTIFY Request Bodies

    Event packages also MUST define which MIME type is to be assumed if
    none are specified in the "Accept" header field of the SUBSCRIBE
    request.

The same nit as above.

7.2.  Reason Codes

    This document further defines "reason" codes for use in the
    "Subscription-State" header field (see Section 4.1.3).

    Following the policies outlined in "Guidelines for Writing an IANA
    Considerations Section in RFCs" [RFC5226], new reason codes require a
    Standards Action.

Minor: This would prevent registration of new Reason Codes in an 
Experimental RFC (for example). I would like to double check that that 
is intentional.

    Registrations with the IANA include the reason code being registered
    and a reference to a published document which describes the event
    package.  Insertion of such values takes place as part of the RFC
    publication process or as the result of inter-SDO liaison activity.

I don't think Standards Action allows for "inter-SDO liaison activity", 
unless such documents from other SDOs are published as Standard Track 
RFCs. So I find your text confusing: either your registration procedure 
should also allow for direct IESG approvals (to allow registrations from 
other SDOs with no RFCs), or you should remove "as the result of 
inter-SDO liaison activity".

    New reason codes must conform to the syntax of the ABNF "token"
    element defined in [RFC3261].

8.4.  Augmented BNF Definitions

    event-type        =  event-package *( "." event-template )

Minor: Does this mean that multiple template packages can be applied?
Is there any ordering for them? How would "foo.A.B" differ from "foo.B.A"?

Nit: id-nits complains:

   -- Duplicate reference: RFC4660, mentioned in 'RFC4660', was also 
mentioned
      in 'RFC 4660'.

"[RFC 4660]" reference is used in section 7.2.

---

From: Adam Roach <[email-masked]>
Date: Wed, 11 Apr 2012 17:03:04 -0500

> header field was specified.

I've added text to point this case out.

> Also, it might be good to reference RFC 3986 for URIs here.

Given that SIP uses URIs everywhere, I'm not sure what benefit is 
derived by referencing the URI syntax draft here.

>    of "Require" or "Proxy-Require" header fields; similarly, there is no

Fixed.

> Minor: How can this SHOULD be satisfied? Any reference which might be 

Added: "...such as those described in sections 22.2 and 23 of [RFC3261]."

> Nit: Is the ";" after "407" a typo?

Thanks -- that should have been outside the quotation marks (and 
probably should be a full stop, which I've changed it to).

>    The "Allow-Events" header field does not include a list of the etvent

I don't find "etvent" in the document:

http://tools.ietf.org/html/draft-ietf-sipcore-rfc3265bis-07#section-4.4.4

Is it possible you accidentally edited a local copy of the file prior to 
your review?

I also don't find this typo in the source (which would surprise me in 
any case, as I made sure to run the -07 version through a spell checker).
> Can you clarify how such request would look like? An example would be 

I'm not sure what you're asking for here. It would be a SUBSCRIBE 
message, with an "Event" header field set to name the template you want 
to use. Basically it just says "we don't negotiate templates -- just try 
it and see if it fails."

> Nit: MIME types are now called "media types" in more recent IETF RFCs.

Thanks. Fixed in three places.

> document [RFC 4288] here, which points to the IANA registry.

Done.

>    none are specified in the "Accept" header field of the SUBSCRIBE

Fixed.

> Experimental RFC (for example). I would like to double check that that 

It never came up explicitly during working group discussions, to my memory.

> I don't think Standards Action allows for "inter-SDO liaison 
> activity", unless such documents from other SDOs are published as 
> Standard Track RFCs. So I find your text confusing: either your 
> registration procedure should also allow for direct IESG approvals (to 
> allow registrations from other SDOs with no RFCs), or you should 
> remove "as the result of inter-SDO liaison activity".

You're right. I suspect we really intended to make this registrable by 
external SDOs, meaning we probably really wanted Specification Required. 
I'm leaving this as-is for now, and will need to coordinate with our AD 
to iron out where to go with this.

> Is there any ordering for them?
Yes, and yes.
> How would "foo.A.B" differ from "foo.B.A"?

Right now, we have only one template defined -- but imagine that we did 
define a new "list" template event package (we almost did this some 
years ago) which is used to aggregate several resources into a single 
subscription.

"Event: presence.winfo.list" would subscribe to the aggregation of 
several watcher-info documents into a single list.

"Event: presence.list.winfo" would subscribe to the watcher-info state 
of the indicated list.

> "[RFC 4660]" reference is used in section 7.2.

id-nits is wrong. It incorrectly thinks that the paragraph starting with 
[RFC4660] on page 40 is trying to define a reference.

/a

---

From: Alexey Melnikov <[email-masked]>
Date: Tue, 24 Apr 2012 16:06:27 +0100

Hi Adam,
Sorry I've missed your reply earlier.

> On 4/10/12 5:46 AM, Alexey Melnikov wrote:
  [...]
> derived by referencing the URI syntax draft here.
Ok.

> Is it possible you accidentally edited a local copy of the file prior 
> to your review?
It is possible. Not a big deal either way.
> message, with an "Event" header field set to name the template you 
> want to use. Basically it just says "we don't negotiate templates -- 
> just try it and see if it fails." 
So my understanding is that it is not possible to just request a 
template event (it needs to be combined with something else)? I think 
some explanation and/or examples of this would be good, I don't think 
this is very clear in the document.
  [...]

>> Experimental RFC (for example). I would like to double check that 
>> that is intentional.
>
>> registration procedure should also allow for direct IESG approvals 
>> (to allow registrations from other SDOs with no RFCs), or you should 
> external SDOs, meaning we probably really wanted Specification 
> Required. I'm leaving this as-is for now, and will need to coordinate 
> with our AD to iron out where to go with this.
>
Ok, I will tell Russ that these 2 issues are still pending resolution.
> Right now, we have only one template defined -- but imagine that we 
> did define a new "list" template event package (we almost did this 
> some years ago) which is used to aggregate several resources into a 
> single subscription.
>
Ok, so the ordering is important. Is this documented anywhere?
> id-nits is wrong. It incorrectly thinks that the paragraph starting 
> with [RFC4660] on page 40 is trying to define a reference.
I thought it was a reference, but this is not a big deal.