Skip to main content

Last Call Review of draft-ietf-p2psip-base-24
review-ietf-p2psip-base-24-genart-lc-barnes-2013-02-20-00

Request Review of draft-ietf-p2psip-base
Requested revision No specific revision (document currently at 26)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2013-02-19
Requested 2013-02-07
Authors Cullen Fluffy Jennings , Bruce Lowekamp , Eric Rescorla , Salman Baset , Henning Schulzrinne
I-D last updated 2013-02-20
Completed reviews Genart Last Call review of -24 by Mary Barnes (diff)
Assignment Reviewer Mary Barnes
State Completed Snapshot
Review review-ietf-p2psip-base-24-genart-lc-barnes-2013-02-20
Reviewed revision 24 (document currently at 26)
Result Almost Ready
Completed 2013-02-20
review-ietf-p2psip-base-24-genart-lc-barnes-2013-02-20-00
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>.


Document: draft-ietf-p2psip-base-24
Reviewer:  Mary Barnes
Review Date: 19 February 2013
Previous Review Date (-23): 14 December 2012
Original Review Date (-17): 6 August 2011
IETF LC End Date: 22 July 2011
IETF 2nd LC End Date: 19 February 2013

Summary: Almost Ready.  This version is in significantly better shape
than the previous versions.

Comments:
=========
I reviewed against my review of the -23 up through section 6.  I
reviewed beyond section 6 of this version (section 5 of -17, section 6
of -23) against my comments on the -17, since I had not re-reviewed
those against the -23.


General:
--------

I still *strongly* recommend that you ensure Henning has reviewed this
document *before* it gets into the RFC editor's queue.  The last RFC I
had published with Henning as a co-author had much more extensive
changes suggested during AUTH 48 than I found acceptable. If all the
co-authors have not reviewed and approved the draft before it goes
into the RFC editor's queue, then the document should not go into the
RFC editor's queue. He has fairly strict (and quite accurate) views on
grammar and structure but it really isn't good to have so many changes
go in at AUTH48 as there is a risk of introducing true technical bugs
or changing something that was carefully crafted to achieve WG
consensus:


http://www.cs.columbia.edu/~hgs/etc/writing-bugs.html


Note, that there some are cases of incorrect grammar that I have not
identified because I think the RFC editor can fix, however, Henning
may have different views on this.


Major:
------
- [-17, section 10.5] Section 11.5, 3rd para:  text uses the phrase
"it can note the Node-ID in the response and use this Node-ID to start
sending requests".  It's not clear whether the use of the Node-ID is a
MAY or a MUST.    [Note: Marc's response to this was that it's an open
issue, but this should be clarified prior to publication].

Minor:
------
- idnits identifies 5 errors (downrefs).  I will note that in the
PROTO write-up it was noted that those should likely be moved to
Informative.

- [-17] Section 1.2.1, 2nd paragraph: I don't understand the example
as to why a single application requires multiple usages - i.e, why
voicemail?  Isn't the intent to say that an application might need to
use both SIP and XMPP - i.e., you wouldn't define a "usage" for an
application, would you?
[While Cullen responded to this comment with an explanation, there was
no change to clarify the text and Marc's response didn't help clarify
my concern]

- Section 3.3, 2nd paragraph after the capability bullet list, next to
last sentence.  There is at least an article missing from this
sentence and it reads rather awkwardly. Perhaps changing to something
like:
OLD:
   If there is a failure on
   the reverse path caused by topology change since the request was
   sent, this will be handled by the end-to-end retransmission of the
   response as described in Section 6.2.1.
NEW:
   Note that a failure on
   the reverse path caused by a topology change after the request was
   sent, will be handled by the end-to-end retransmission of the
   response as described in Section 6.2.1.

- [-17] Section 3.3, last paragraph.  Add a reference to 5.4.2.4
after "RouteQuery method"

- Section 3.4, last paragraph, 3rd sentence: "that the specified by
the algorithm" should be something like "than specified by the
algorithm".

- [-23] Section 6.6:  All my previous concerns were addressed, except,
the Note to implementors paragraph still seems out of context - it
should be deleted or this section should be restructured so it is in
context.

- [-17, section 11] Section 12, Second paragraph, 3rd sentence
says that "It gets routed to the admitting peer (AP), yet the flow
shows that the message first gets routed to the PP and then onto AP.
It would be helpful if that were clarified.   [Note: Marc's response
indicated that he thought this was fixed in the -23, however, the diff
shows no changes to that specific text between the -17 and the -24 ]


Nits:
-----

- Section 1.2.5, 2nd para, last sentence: this sentence is a bit tough
to interpret on a first read.  I would suggest rewording something
like the following:
OLD:
  This layer is to the Message Transport Layer as link-
   level congestion control and retransmission in modern wireless
   networks is to Internet transport protocols.
NEW:
   The relation of this layer to the Message Transport Layer
   "is similar to"|"can be likened to" the relation of the link-
   level congestion control and retransmission in modern wireless
   networks to Internet transport protocols.

- Section 3.4, last paragraph, 4th sentence: "in accord" -> "in accordance"

- Section 10.1, 2nd paragraph, 5th sentence: "can be thought of a
doubly-linked list" -> "can be thought of as a doubly-linked list"

- Section 15, last paragraph: "help resolve" -> "helped resolve"