Subject: [P2PSIP] draft-ietf-p2psip-base-11

From: Marc Petit-Huguenin <[email-masked]>
Date: Mon, 01 Nov 2010 10:53:14 -0700

A first batch of questions, comments and nits:

- - A.1. Section 4.1.2

This section lists what need to be provided to describe a RELOAD storage usage.
 OTOH section 4.3 states that "[e]ach usage specifies which type of connections
can be initiated using AppAttach."

Would not it be better to have to add the sentence from 4.3 in the list of
things that an usage must provide, and move the whole 4.1.2 section outside 4.1
(e.g. as 4.4 section)?

- - A.2. Section 5.2.1, 4th paragraph

Is this retransmission needed when sending messages that are clearly hop-by-hop,
like Leave, Update and RouteQuery?

- - A.3. Section 5.3.2, "The message is easy to demultiplex from STUN messages by
looking at the first bit."

I think that this sentence is misleading and should be removed.  As Message are
always encapsulated in FramedMessage, relo_token cannot be the first word on the
boundary and the STUN messages are not encapsulated in FramedMessage either.
They are not even encrypted by DTLS (5.5.1.13: "Consequently, once ICE
processing completes, the agent will begin TLS or DTLS procedures to establish a
secure connection.").  My understanding is that even keepalives (section
5.5.1.10.3) are not encrypted, probably using the FINGERPRINT attribute to
differentiate them from encrypted packets. That would fit with section 5.5.1.4
that says that "...every peers is a STUN server." (but see next entry).

- - A.4. Section 5.5.1.4 "Because all RELOAD peers implement ICE and use STUN
keepalives every peer is a STUN server [...]"

Something implementing ICE connectivity checks is not also a STUN server, at
least not in the sense of RFC 3489 section 13 (compare with RFC 5245 section 10).

- - A.5. Section 5.5.1.13 "[...] the media takes the form of application layer
protocols (RELOAD or SIP for example)[...]"

Attach only supports RELOAD messages.  SIP messages are carried by AppAttach.  I
think that this whole section must be copied under 5.5.2, and both sections cleaned.

- - A.6. Section 5.5.2.2

What error code should be used if the peer does not support the application-ID
in the AppAttachAns message?

- - A.7. Section 5.5.4.1  "config_data<2^24-1>"

If I use the explanations in 5.3.1.1 ("The number of bytes to encode the length
on the wire is derived by range; i.e., it is the minimum number of bytes which
can encode the largest range value."), does this means that the length must be
encoded with 3 bytes?

- - A.8. Section 5.7 "Upon receipt of a fragmented message by the intended
peer[...]"

It is not clear if the reassembly is done hop-by-hop or end-to-end.  The
previous paragraph states that "[...] a message may be refragmented multiple
times as it traverse the overlay." which would indicate hop-by-hop reassembly,
but the next paragraph states that "[...] this time was derived from looking at
the end to end transmission time and saving fragments long enough for the full
end to end retransmissions to take place." which would imply end-to-end reassembly.

Nits
====

- - Section 1.2.1, 2nd paragraph

s/A usage/ An usage/

- - Section 3.3, "Destination Lists:"

s/NodeID/Node-ID/

- - Section 5.1, third bullet

s/NodeIDs/Node-IDs/
s/Resource IDs/Resource-IDs/

- - Section 5.4.2.4 title and 1rst paragraph

s/Route_Query/RouteQuery/

- - Section 5.5.1.1 "foundation<0..255>"

s/255/2^8-1/

Also name<2^16-1> should be name<0..2^16-1>, here and at various other places.

- - Section 5.6.3.1.1 1st paragraph

s/A peer/A node/
s/The peer MUST/The node MUST/

- - Section 5.6.6 title

Superfluous spaces.

- - Section 6.1. Input formula

Perhaps replacing the "+" with "|"?

- - Section 6.4.1.1 "KindId   kind;"

KindId is never defined.

- - Section 6.4.3.2. "hash_value<0..255>"

s/255/2^8-1/

- - Section 6.4.4 "[...] to walk the Overlay Instance by interactively fetching
[...]"

I am not a native English speaker, but "interactively" looks strange to me in
this context.

- - Section 6.4.4.2. "[...] it SHOULD return a 404 RELOAD error code."

"404 RELAOD" is not an error code.

- - Section 8, 2nd paragraph

s/Peer-ID/Node-ID/

- - Section 8, last paragraph, "[..] for the appropriate server type with that
Resource-ID."

s/server type/KindID/

- - Section 9, 2nd paragraph

s/peer id/Node-ID/

- - Section 9.6.1 3rd paragraph

s/ATTACH/Attach/

- - Section 9.6.3 last paragraph

s/UPDATE/Update/

- - Section 9.7 title

s/Route Query/Route query/

- - Section 13.5 table

RELOAD is not carried by AppAttach.

- - Section 13.8 table

The remove_req and remove_ans messages no longer exist.
The config_update_req and config_update_ans messages are missing.

- -- 
Marc Petit-Huguenin
Personal email: [email-masked]
Professional email: [email-masked]
Blog: http://blog.marc.petit-huguenin.org

---

From: Marc Petit-Huguenin <[email-masked]>
Date: Sat, 06 Nov 2010 08:01:26 -0700

Any chance to have an answer to the questions below?

Thanks.

On 11/01/2010 10:53 AM, Marc Petit-Huguenin wrote:
> - A.1. Section 4.1.2
> - A.2. Section 5.2.1, 4th paragraph
> 
> - A.3. Section 5.3.2, "The message is easy to demultiplex from STUN messages by
> - A.4. Section 5.5.1.4 "Because all RELOAD peers implement ICE and use STUN
> - A.5. Section 5.5.1.13 "[...] the media takes the form of application layer
> - A.6. Section 5.5.2.2
> - A.7. Section 5.5.4.1  "config_data<2^24-1>"
> 
> - A.8. Section 5.7 "Upon receipt of a fragmented message by the intended
> peer[...]"
> 
> - Section 1.2.1, 2nd paragraph
> 
> s/A usage/ An usage/
> 
> - Section 3.3, "Destination Lists:"
> 
> s/NodeID/Node-ID/
> 
> - Section 5.1, third bullet
> 
> s/NodeIDs/Node-IDs/
> - Section 5.4.2.4 title and 1rst paragraph
> 
> - Section 5.5.1.1 "foundation<0..255>"
> 
> s/255/2^8-1/
> 
> - Section 5.6.3.1.1 1st paragraph
> 
> s/A peer/A node/
> - Section 5.6.6 title
> - Section 6.1. Input formula
> 
> - Section 6.4.1.1 "KindId   kind;"
> 
> KindId is never defined.
> 
> - Section 6.4.3.2. "hash_value<0..255>"
> 
> s/255/2^8-1/
> 
> - Section 6.4.4 "[...] to walk the Overlay Instance by interactively fetching
> [...]"
> 
> - Section 6.4.4.2. "[...] it SHOULD return a 404 RELOAD error code."
> 
> - Section 8, 2nd paragraph
> 
> s/Peer-ID/Node-ID/
> 
> - Section 8, last paragraph, "[..] for the appropriate server type with that
> Resource-ID."
> 
> s/server type/KindID/
> 
> - Section 9, 2nd paragraph
> 
> s/peer id/Node-ID/
> 
> - Section 9.6.1 3rd paragraph
> 
> s/ATTACH/Attach/
> 
> - Section 9.6.3 last paragraph
> 
> s/UPDATE/Update/
> 
> - Section 9.7 title
> 
> - Section 13.5 table
> - Section 13.8 table
_______________________________________________
P2PSIP mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/p2psip

- -- 
Marc Petit-Huguenin

---

From: Cullen Jennings <[email-masked]>
Date: Wed, 10 Nov 2010 17:21:12 -0700

On Nov 1, 2010, at 11:53 AM, Marc Petit-Huguenin wrote:

> least not in the sense of RFC 3489 section 13 (compare with RFC 5245 section 10

Well 5389  softened that somewhat to allow for things like REALOD but I tried to update the text in RELOAD to not imply that the the nodes were full STUN servers.

---

From: Eric Rescorla <[email-masked]>
Date: Wed, 10 Nov 2010 16:27:03 -0800

Thanks very much for your comments.

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>

This is actually a stupid sentence and I removed it.
But I did hoist the section up a level.

> like Leave, Update and RouteQuery?

I think it's simpler to leave as-is than try to special case it.

> that says that "...every peers is a STUN server." (but see next entry).

Removed.

> least not in the sense of RFC 3489 section 13 (compare with RFC 5245
section 10).

I'm going to let Cullen handle this one.

> think that this whole section must be copied under 5.5.2, and both
sections cleaned.

I did edit this sentence but opted not to copy this Sxn.

> in the AppAttachAns message?

Fixed.

> can encode the largest range value."), does this means that the length
must be

Correct. I also cleaned up these defns to use the <0..2^24-1>  syntax.

> end to end retransmissions to take place." which would imply end-to-end
reassembly.

It's end-to-end. I clarified.

> - - Section 1.2.1, 2nd paragraph

Already fixed.

> - - Section 3.3, "Destination Lists:"

Fixed.

> s/Resource IDs/Resource-IDs/

Fixed.

> s/Route_Query/RouteQuery/

Fixed.

> - - Section 5.5.1.1 "foundation<0..255>"

This is OK.

> Also name<2^16-1> should be name<0..2^16-1>, here and at various other
places.

Fixed.

> s/The peer MUST/The node MUST/

Fixed.

> Superfluous spaces.

This is an XML2RFC glitch. I'm going to let RFC-Ed handle it.

> Perhaps replacing the "+" with "|"?

Used ||

> - - Section 6.4.1.1 "KindId   kind;"

Ouch. Fixed.

> - - Section 6.4.3.2. "hash_value<0..255>"

This is OK.

> I am not a native English speaker, but "interactively" looks strange to me
in

Changed to iteratively.

> "404 RELAOD" is not an error code.

Fixed.

> - - Section 9, 2nd paragraph

Fixed.

> - - Section 9.6.3 last paragraph

Fixed.

> s/Route Query/Route query/

Fixed.

> RELOAD is not carried by AppAttach.

Fixed.

> The config_update_req and config_update_ans messages are missing.

Fixed.

On Sat, Nov 6, 2010 at 8:01 AM, Marc Petit-Huguenin <[email-masked]>wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAkzVbcQACgkQ9RoMZyVa61eBSwCeM1Cm39S9NyBK17k+Pl62+hqm
> a2QAn0Rn5LUzkFtMeQjOuderg057WWJj
> =rrqv
> -----END PGP SIGNATURE-----