Subject: [P2PSIP] reload review from eric

From: Cullen Jennings <[email-masked]>
Date: Tue, 12 Oct 2010 09:11:44 -0600

Thanks once again for the review ... comments inline and also thanks to ekr for updating most of these in the doc. This is changed in next rev of the draft. 

> SUBSTANTIVE COMMENTS
> Section 5.3.2.1, Processing Configuration Sequence Numbers The
> configuration sequence number is a monotonically increasing, 16-bit
> integer. Age is determined modulo 65536. That is, 3 is typically
> considered newer than 65530. RELOAD uses the sequence number to
> determine if a received configuration is newer than the current
> configuration. However, the value 65535 is special, in that the
> recipient must accept the configuration. This is great, unless a
> peer is at configuration sequence 65534. Let us say the peer is
> disconnected, and missed the next two updates. Now the global
> sequence number is 1. During normal operations, 1 is newer than
> 65534. However, the peer will send out its configuration with the
> next number in the sequence, 65535. Unfortunately, this is the
> special, must accept sequence number, meaning the out-of-date peer's
> configuration will overwrite the new configuration.
>
> I would suggest adding language similar to the following at the end
> of the last paragraph in the section: "Since 65535 is a special
> value, peers sending a new configuration when the configuration
> sequence is currently 65534 MUST set the configuration sequence
> number to 0 when they send out a new configuration."

Done.

> Section 5.3.2.2, Destination and Via Lists
> How is it possible to enforce "the encoding MUST include a
> generation counter..."?  I didn't do that.  Nanny nanny boo boo. The
> protocol police in the black helicopters will get you if you don't!

Removed.

> Section 5.4.2.5, Probe
> Are there any security or privacy issues exposed by willy-nilly
>  probe requests, or is the nature of the p2p network to be trusting?
> The latter is OK; this is a peer network after all.

As far as I know, there are no issues. Do you think this needs to go into
sec cons.

> CLARIFYING COMMENTS
> Section 1.2, Architecture
> Someone unfamiliar with P2P technologies may wonder, "Why are we
> defining a message routing protocol? What is wrong with IP? What is
> the relation between RELOAD and, for example, MANET? Is all this
> routing for NAT traversal? Won't IPv6 make this obsolete?" To be
> honest, it was not until I reviewed Section 9 on Chord did message
> routing make some sense to me. A bit of clarifying text here or a
> forward reference to Section 9 might head off knee-jerk reactions of
> "we are not going to standardize IP over P2P over IP."

Added some text.

> Section 2, Terminology, Resource Name
> The text states the resource name is human readable. In this case,
> it must have some indication of language and character set.

I think "potentially" defuses this objection.

> Section 2, Terminology, Connection Table and Routing Table
> I read the words in the definitions for these terms, but they made
> absolutely no sense to me on the first reading. They make perfect
> sense on the second reading. I would almost offer we have some,
> "There are circular definitions here; if these definitions do not
> make sense to you now, they will after you finish reading this
> document in its entirety." That also has the benefit of encouraging
> people to actually read the document.

Actually, I wonder if we should just move them to the end and define
on first introduction. Thoughts?

> Section 5.1.3, Private ID
> On my first pass, I said to myself, "It looks like a Private ID
> could clash with a Node ID." It was not until I ready the pseudo
> code defining a Destination that I saw these IDs have their own name
> spaces. It would be helpful to say something in 5.1 or 5.1.3 similar
> to, "Private IDs have their own, distinct namespace with Node IDs
> and Resource IDs."

Done.

> Section 5.2.1, Request Origination, request timeout
> Why a 3 second timer? Satellite IP can have up to 2.5 seconds of
> one-way latency. Is this arbitrary? If so, I would think 3
> micro-fortnights would be a better timer (apologies to Dave Oran).
> On the serious side, should there not be a relationship between this
> timeout and the RTO of Section 5.6.3.1.1? For example, if a
> satellite RTO is five sections, an implementation should be able to
> adjust this time accordingly.

The relation with RTO really only has to do with a single hop in the overlay so I don't think it should be related to that. The RTO for one hop might be very fast if both nodes are on the same spaceship and very slow to the next node that is on a different planet. ABout the best I can see us being able to do is make this configurable. Should we do that?

> Section 5.2.1, Request Origination, write requests
> Assume X and Y have write permissions for a resource. X does a write
> that gets lost, then Y does a write, then X retries the write. Who
> wins?

Y wins. X gets an error. How can I make this clearer?

> Section 5.3.1, Presentation Language, style issue
> WAY too many adjectives and adverbs that do not add any value. I
> would offer it is easy and very familiar to write straightforward
> and clear language.

I think I addressed this.

> Section 5.3.2.1, Forwarding Options [Real Issue]
> If RESPONSE_COPY is set, I understand copying the option from the
> request to the response.  However, the text goes on to say, "and
> clear the ... flags."  The expected behavior is not clear to me.

Not sure what's unclear. I tried to fix a bit.

> Section 5.3.3, Message Contents Format
> Is there a reason for the alternating request / response codes? If
> it is to deal with overlapping responses, then you cannot have
> multiple requests with the same point code outstanding.  If one can
> have multiple requests with the same point codes, then you will need
> sequence numbers.  If you have sequence numbers, then you do not
> need to correlate requests with responses via the request / response
> code.  The concept of odd / even request / response codes seems a
> bit unnecessarily complex.  I do have this comment in the CLARIFYING
> section as I am sure there is an engineering reason to do odd / even
> codes.  If there is no reason to do it, than I would put this
> comment in the SUBSTANTIVE comment section.

IIRC it's the last  remaining feature from P2PP. Do you feel strongly
that it's bad?

> Section 5.3.3, Message Contents Format
> If critical = False, the recipient SHOULD process the message,
> *UNLESS*????  Why would the peer chose not to process the message?
> Or, is this really a MAY?

Changed to MAY.

> Section 5.3.3.1, Response Codes and Response Errors, Error_Request_Timeout
> The text does not make it clear *who* received what, when?  Which end times out?  Which end sends the message?

I tend to think that this is a stupid error and should just be removed.

> Section 5.3.3.1, discussion of Error_Config_Too_New and Error_Unknown_Kind
> It took me three readings to figure out the text says what it means,
> but we need better text.  The text talks about responses to
> messages, which is a message that gets sent.  These two response
> codes tell the implementor to send this message, then send a
> Config_Update message.  That is just what we want to do.  However,
> it took me two readings to understand that Config_Update was not an
> error message.  The problem is there are way too many "messages"
> floating around.  In SIP terms, we would say something like send a
> Config_Update method.  In the context of this document, could we say
> something like "A node which generates this response MUST send a
> Config_Update request containing the appropriate kind definition
> after sending the response."?

I fear you have this backward.

ConfigTooNew and UnknownKind are sent by a responder and reacted to by
the requester (who has the new info). ConfigTooOld is sent by a responder
and followed by an update. I've tried to clarify.

> Section 5.5.1.2
> Under what circumstances would a peer not process an Attach request?
> If you cannot think if it, then a peer MUST process an Attach
> request.  The peer can always reject the request.  If you can think
> of it, like if the peer is under a DoS attack and is purposely
> dropping Attach requests, say so.  Otherwise, peers MAY process
> Attach requests.

Fixed.

>  Also, this paragraph looks like it has a lot of
> editing shrapnel.  For example, it says the peer needs to start ICE
> checks, and then start ICE checks.  ICE is so slow as it is, so we
> do not need to do it twice.

Can you give this another read. The second start refers to the requester,
hence the phrase "receives an Attach response".

> Section 5.5.1.4, last paragraph
> The text does not describe under what circumstances would a peer not
>   select a new stun server from the same group?  Are you really saying
>   that a peer MUST select a new STUN server from the same group,
>   unless there are no STUN servers remaining?  Are you really saying
>   that a peer can pick a STUN server from anywhere, but it is
>   RECOMMENDED to pick a server from the same group?

fixed

> Section 5.5.1.6, last bullet
> "For example, the DTLS/UDP with SR overlay link protocol." is not a sentence.  Where is the verb?

Fixed.

> Section 5.5.2.2 - but really a general comment
> Most RFC's have the structure Clients do This; Servers do That.  I
>  get this is a peer-to-peer protocol.  However, even in a peer
>  protocol, someone sends a request, and the other end sends a
>  response.  This section, which is where a responder would go, has
>  lots of directions for the requestor as well.  This is begging for
>  implementor confusion.  A constructive comment would be to either
>  break this out and have the normative language for the requestor to
>  be in section 5.5.2.1 or have a comment up front warning
>  implementors to basically look everywhere for the normative protocol
>  definition.

Can you suggest some text and a location for it.

> Section 5.5.3.1, Ping Request Definition
> Since a Ping can be sent to a ResourceID, should that not also be in
> the PingReq?  Or will the header take care of ResourceID versus
> NodeID addressing?

The latter.

> Section 6.4.4.1, page 87, first full paragraph after the structure definition, last sentence.
> Should the last line read "and, if not, the peer MUST reject the request"?

Done.

> Section 6.4.1.2
> The peer returns the StoreAns, and the text says the response
> includes a list of peers where the data will be replicated.  This is
> where passive voice does not work (by the way, passive voice never
> works!): WHO replicates the data?

Fixed.

> Section 6.1.4.2, KindId definition
> The definition is "KindId unknown_kinds<2^8-1>".  What does this
> mean?  Is unknown_kinds an array of fixed length of 127? Is it a
> variable length array of zero to 127 entries, "KindId
> unknown_kinds<0..2^8-1>"?  If the latter, this is a typo.  If the
> former, the syntax definition does not talk about scalar vectors.

Fixed.

> Section 6.4.4
> Is the statement "interactively fetching R_n+1=nearest(1 + R_n)" meaningful to the community?

Community ??? Comments ??? Anyone? Brueller, Brueller, ...

> Section 9, third from last bullet
> Is 2^128 nodes really a limit? Is that not something like making
> every quark in the universe a potential peer?  Or am I exaggerating
> and this is really just the number of atoms in the solar system?

Fixed.

> Section 10.1, definition of turn-density
> Just checking, but if the default value is 1, my read of the
> document is that every node is a TURN server. 1/1 = 1 = 100%. Along
> those lines, what if you wanted, for example in a closed LAN or
> enterprise, to have no TURN servers. How would you represent that?
>  A very large number so 1/large-number is almost zero?

Fixed to use 0 to mean infinity 

> Section 10.1, definition of bad-node
> My read of this section is there can be only one bad-node listed at
> a time.  If I read that wrong, it needs to be more clear that more
> than one bad-node can get listed.  However, I have another question.
> Since NodeID's get allocated or are generated, what if a node gets
> NodeID X, the network later says X is a bad-node, and much later a
> new node gets assigned NodeID X?  Is X forever taken out of the
> NodeID name space?  If so, that could make for an interesting
> denial-of-service attack, that of removing NodeIDs.  Conversely, if
> bad-node has a life time, the text should talk about it.  Given the
> large NodeID name space, and that they are not human readable, one
> could have a compromise of black listing for a "long, but finite"
> time.

Fixed.

> Section 10.3, Credentials
> The text says the Node-IDs need to be "unpredictable to the
> requesting user."  Is this a redundant requirement to the Node-ID
> being cryptographically random?  Since the reason for it being
> unpredictable is given in the security section, I would either
> clarify with text like "The enrollment server must chose Node-IDs
> such that they are unpredictable to the requesting user. Thus, the
> Node-IDs MUST be cryptographically random [RFC4086]."  If I am
> wrong, and cryptographically random is not sufficient as to what
> "unpredictable to the requesting user" means, then the text must
> describe what "unpredictable" means.

This isn't redundant. Addressed.

> Section 12.3
> Something I am clueless on.  The user name looks like a FQDN, like
  [email-masked].  If it looks like a FQDN, can one resolve
  the address from outside the peer network?  If one can, how?  If one
  cannot, why do we have a string of characters that looks like a
  FQDN?  This is a clarifying question, in that I can get it, but my
  brain hurts.

Hmm - not sure what to do. The FQDN has to do with how the namespace is allocated not if it is resolvable for or not. Even with email, [email-masked] is not going to be resolvable outside the split DNS. I think the important things is .net allocated example to someone that allocated dht to someone that allocated alice to someone to form a unique name. There e is also the question about what resolution means related to a separate conversation on if the reload URI should have a // in it or not.  The question comes down to does resolve mean resolve using DNS or resolve using some combination of protocols including DNS and RELOAD.  Anyway - I have no idea what we should do here. If you think some text change is needed ... let me know.

---

From: Brian Rosen <[email-masked]>
Date: Tue, 12 Oct 2010 11:40:01 -0400

> Hmm - not sure what to do. The FQDN has to do with how the namespace is allocated not if it is resolvable for or not. Even with email, [email-masked] is not going to be resolvable outside the split…
<as individual>
What we want is a unique userid.  We don't need an FQDN.  Would a URN be more appropriate?
We could even create a namespace for a URN that allowed a "username" and a "domain name" as components:
urn:reload:userid:dht.example.net:alice

Brian

---

From: Ted Hardie <[email-masked]>
Date: Tue, 12 Oct 2010 12:11:34 -0700

Some comments inline.

> urn:reload:userid:dht.example.net:alice

I don't think you really want URN, which requires a single namespace
authority for the
whole namespace (the UUID URN being the one counter example permitted so far).
A URI scheme for it would be easier.  That would require adjusting the
format to include
the URI scheme as a prepend(it could re-use existing schemes that were used as
an identifier or mint a new one); I think the same should be done with
the node-ids, frankly,
as they otherwise are fairly opaque values.  The node-id mechanism in
the now expired
p2p-overlay draft described one way to do that, but there are others.

Just my two cents,

regards,

Ted