Folks,

the P2PSIP WG chairs are in the process of requesting the publication of
the following draft:

https://datatracker.ietf.org/doc/draft-ietf-p2psip-base/

At this point, they are working with the secretary to fix the state of
the document in the tracker (it should be publication requested) and to
upload the document's PROTO write up.

While that gets fixed, I have done my AD review (see below) in order to
gain some time. As soon as these comments are addressed, I will start
the IETF LC on this draft.

Cheers,

Gonzalo


Review of draft-ietf-p2psip-base-15:

I have also included a number nits in this review because I spotted them
while reviewing the document. While some of them could have been caught
by the RFC editor at a later point, I chose to report them now anyway.

The last paragraph of page 8 says: This specification also defines how
RELOAD is used with the Chord DHT algorithm, which is mandatory to
implement.

However, the draft says later: This specification defines a DHT based on
Chord.

The draft should always talk about a Chord-based DHT or something along
those lines, then.

The draft references a few drafts that have expired such as the
following two:

http://tools.ietf.org/html/draft-ietf-p2psip-sip-05
http://tools.ietf.org/html/draft-ietf-p2psip-concepts-03

We need to have non-expired versions of those drafts in the repository
during the IETF LC and the IESG review. Chairs, please make sure that
happens.

On page 12, in the "Overlay Link Layer" bullet there is a normative MAY.
Given that Section 1 consists  on an introduction, the statement is not
about the protocol, and the RFC 2119 terms have not been introduced yet
(they are introduced in Section 2), I would s/MAY/may/.

Last two words of page 13: s/An usage/A usage/

Page 18 in the Kind bullet: s/story/store/

First paragraph of page 19: when talking about unstructured and
structured P2P networks, reference the IAB RFC that discusses them: RFC
5694.

Section 3 is an overview section. However, Section 3.2.1 contains a few
normative statements. They should probably be replaced by non-normative
statements.

Section 3.3 starts with:  This section will discuss the requirements
RELOAD's routing capabilities must meet...

Now that RELOAD is (virtually) finished, the sentences should talk about
the requirements as those that were used when designing RELOAD and that
RELOAD meets instead.

Page 24 says: Symmetric recursive routing requires that a message follow
a path through the overlay to the destination without returning to the
originating node:  each peer forwards the message closer to its
destination.  The return path of the response is then the same path
followed in reverse.

The fact that recursive routing is defined as a path that does not
"return" and then the following sentence talks about the return path can
be confusing. The authors may want to slightly rephrase that sentence to
avoid that.

The following two references:

o [I-D.maenpaa-p2psip-service-discovery]
o [I-D.maenpaa-p2psip-self-tuning]

should be replaced with the following ones:

o draft-ietf-p2psip-service-discovery
o draft-ietf-p2psip-self-tuning

The structure of Section 5.1 is confusing. Section 5.1 presents three
possible cases in three bullets. Those bullets seems to be expanded in
the three (sub)sections that follow (Sections 5.1.1, 5.1.2, and 5.1.3).
If that is the intent, Section 5.1 needs to be clearer. Right now, the
link between the bullets and those three sections is confusing. For
example, the first bullet uses the term peer while the first sentence in
Section 5.1.1 uses the term node. The third bullet refers to Section
5.3.2.2 but does not mention Section 5.1.3. Section 5.1.2 talks about
"the other three cases" but it is not clear what other three cases it
refers to.

Section 5.2.1 defines a set of timers by value. Why doesn't it define
the timers by name and then provides default values instead?

First paragraph of Section 5.3.1: "define TLS. [RFC5246]". The period
goes after the reference.

The last paragraph of Section 5.3.1 says: For instance, "uint16
array<0..2^8-2>;" represents up to 254 bytes but only up to 127 values
of two bytes (16 bits) each.

Doing s/but only up to/which corresponds to up to/ (or something
similar) would make the sentence clearer.

Section 5.3.2 says: (the string 'RELO' with the high bit of the first
byte set.).

The first period should be removed.

Section 5.5.1.3 says:

   o  In this case, the "stream" refers not to RTP or other types of
      media, but rather to a connection for RELOAD itself or for SIP
      signaling.

The connection could also be for other application-layer protocols other
than SIP.

Section 5.5.1.5 says: Therefore it is RECOMMENDED that full ICE be used
even for a node that has a public, unfiltered IP address, to take
advantage of STUN connectivity checks, etc.

This sentence seems to indicate that each node defines whether or not to
use full ICE. However, the previous paragraph talks about the use of ICE
being an overlay-wide setting that affects every node in a given overlay.

Last paragraph of page 66: include a reference to ICE TCP when talking
about new uses of ICE.

The three bullets at the beginning of page 67 prioritize different
transport protocols. The second bullet talks about "stream-oriented
protocols". However, that is not the main feature that makes TCP better
than the protocols in the third bullet. The main feature is that it
offers "well-understood congestion and flow control", as the protocols
in the first bullet do. The bullets should be clearer that bullet two is
better than bullet three because of its congestion avoidance properties
and worse than bullet 1 because of its lack of message orientation and
HOLB avoidance.

The same bullets and other sections in the draft talk about DCCP. Does
the WG think DCCP could be an appropriate transport for RELOAD?

Section 5.5.1.10 says: When neither side has provided an No-ICE
candidate, connectivity checks and nominations are used as in regular ICE.

The same question as before. Isn't the use of ICE an overlay-wide property?

First sentence of Section 5.5.1.13: s/(RELOAD)/(e.g., RELOAD)/

Section 5.6 explicitly talks about "three Overlay Link protocols" but
Section 5.6.1 says: The only currently defined overlay link protocols
are TLS and DTLS.

That sentence should include both DTLS with and without ICE for consistency.

Section 5.6.1.4: note that Baset's proposal for TCP over UDP
encapsulation was just one of the existing the proposals for TCP over
UDP encapsulation.

Section 5.6.3.1 says: "simple, inefficient, scheme". Remove the second
comma.

Section 6.4.1.2 talks about the Stat method for the same time. Add a
forward reference to Section 6.4.3, which describes it). For example,
adding "(see Section 6.4.3) would be enough.

Second paragraph of Section 14: for consistency with the other
paragraphs, instead of writing the initials, the given names are Jouni,
Gonzalo, and Jani respectively.

ID nits complains about the following references:

  ** Downref: Normative reference to an Informational RFC: RFC 2818
  ** Obsolete normative reference: RFC 2988 (Obsoleted by RFC 6298)
  ** Downref: Normative reference to an Informational RFC: RFC 3174
  ** Downref: Normative reference to an Informational RFC: RFC 3447
  ** Downref: Normative reference to an Informational RFC: RFC 6091
  ** Downref: Normative reference to an Informational RFC: RFC 6234

Can the authors confirm that all these downrefs need to be normative
references?

Also, should the reference to RFC 2988 be updated to refer to RFC 6298?

I am also working on getting an XML reviewer to have a look at Sections
10.1 and 10.1.1 of the draft. If you are interested, let me know.










_______________________________________________
P2PSIP mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/p2psip

Reply via email to