Here is my review of draft-ietf-p2psip-base-06
that I agreed to provide at IETF76 WG meeting.
John
General
=======
1.4 Structure of the Document
Adding a new usage - it would be useful to have a section on this
Adding a new overlay algorithm - it would be useful to have a section on
this
3.2 Clients
How does a peer transition to a client?
3.3 Routing
"Client promotion" - is there client demotion?
4.1.1 Storage Permissions
This limits the storage load which another peer can impose,
but doesn't address loading due to frequency of
write/fetches another peer can impose.
6.3 Access Control Policies
How is access control managed for replicated values?
It might be helpful for implementers to have a table
in an appendix listing the various config parameters
and their default / max values, etc, such as:
holdown_time 30 sec
periodic Update requests 10 min
Ping interval 3000 sec
Partition detection
Max Message size
etc.
Detailed Comments
=================
1.1 "partly connected graph" -- connected graph subsumes partly connected
case
1.2.5 "a packet" => a message
3.3 "In all cases, the response to
a request needs ..." -- should this be written as a MUST?
4 Application Support Overview
"To form direct connections ..." -- why "direct"?
5.1.1 Responsible ID (2nd bullet)
"If the message is a response ... the state
is reinserted ... first" -- <before forwarding or
before sending to the upper layer?>
5.3.2 Forward Header
relo_token = 0xc2454c4f, i.e., 'RELO' with msb set in first byte.
Shouldn't this be 0xd2454c4f ? (this was commented on the
list already in July 2008)
transaction_id "unique" - globally unique in the overlay or unique
respect to transaction ids generated by this peer?
5.3.2.2
"This may be one of "peer", ..." -- should be node instead of peer
"A DestinationData can be one of three types ... peer
A Node-id" -- change "peer" to "node" for consistency with
the select statement
5.3.2.2 bottom p. 44 to top of p. 45
"the encoding MUST include a generation counter designed ..."
-- since this is opaque, I don't see how it is "MUST".
there could be other ways for nodes to prevent misrouting
due to dynamics of the connection table.
5.3.2.3
should the flags be defined as enums in the notation?
5.3.3
message_code 0x8000 .. 0xfffe seems to be omitted
5.3.3.1 Response Codes and Response Errors
There seem to be a number of cases where "peer" is used in the
text where it should be "node":
Error_Forbidden: The requesting _peer_ does not have permission to
make this request.
Error_Not_Found: The resource or _peer_ cannot be found ...
Error_Request_Timeout: ...The requesting _peer_ MAY resend the
request at a later time.
Also it is not clear whether clients can receive some requests,
such as update_req, stat_req, or ping. If so, some of the other
Response Error text may need to be changed to refer to "node" instead
of "peer".
Finally, these definitions appear here but are not in section 13.7:
ERror_Response_Too_Large, Error_Config_Too_Old, Error_Config_Too_New.
5.4.1 Topology Plugin Requirements
"The length of the Resource-IDs and Node-IDs". -- Doesn't RELOAD fix these
to 128 bits?
5.4.2.5.1
"The two currently defined..." -- strike "two".
Also should "peer" be "node" in this section, at least for uptime?
If so, this would effect usage of peer in sec. 5.4.2.5.2 as well.
5.5.3.1 Request Definition
should it be "requesting node" instead of "requesting peer"?
(also in 5.5.4.1)
5.5.5 Ping
what are the operational semantics of Ping to broadcast Node-ID?
5.5.6.2 Config_UpdateReq
This could potentially be used to attack the overlay by disabling nodes
with erroneous configurations.
5.6.2.1
if we replace N-M with N-M-1, then the bit numberings are 0..N-1
instead of 1..N in the mask, which is the usual convention.
5.6.2.2
"Another would be to implement the _AIMD_ scheme described in
Appendix B".
6 Data Storage Protocol
storage_time
what about a store request whose time equals the time of the
current value?
lifetime
can a usage define a max_lifetime for a kind?
6.3 Access Control Policies
what about control of fetch access?
6.4.1.1 Request Definition
resource: The _Resource-ID_ to store at.
6.4..4.2 Response Definition
"... it SHOULD return a 404 error" -- replace 404 with a RELOAD error
code
9.4 "joining party" -- "joining node" seems clearer and more consistent
terminology
A statement that the enrollment server assigns the Node-Id could be useful
here.
"numBitsInNodeId" - this is already fixed to 128, so should we drop the
variable use here?
9.6.3 Receiving Updates
"The UpdateReq defines peers that indicate a neighbor table further away
from
N than some of its neighbor table" -- could be made clearer, e.g.,
=> The UpdateReq contains peers that are further away from N than some of
the
peers in N's neighbor table.
11. Message Flow Example
PPP, PP, NP, NNP acronyms should be defined
Some other examples that could be useful:
client-peer interaction
bootstrap with enrollment server
interactions between the node internal architecture elements
12.6.5 Residual Attacks
enrollment server is a central point of failure
(as implied by 3.6.2)
13.6
Message codes remove_req and remove_ans seem to
be no longer used according to 6.4.1.3
14 Acknowledgements - "Michael Chen" is listed twice.
Spelling/Grammar
================
1.2
"To clarify the role of the various layer,..." => layers
4.1.3 "on the its" => "on its"
5.3 "introduce new potential" => introduces
5.3.1 "the ability to mechanically (compile)..."
-- is a verb missing here, such as generate or translate?
5.5.2.4 "be done be doing" => be done by doing
5.6.2.1 "that the sender to figure" => "that the sender can figure"
5.6.2.2 "no more aggressive then" => "no more aggressive than"
also add "(see Appendix C)" after [RFC5348] in this sentence.
9.1 "entreis" => entries
9.6.3 "These Update requests are what ends up" => end up
9.8 extentions => extensions (also several places in 10.1)
10.1 "boradcast" => broadcast
10.3 "[RFC5272])" => "[RFC5272]"
10.3 "HTTP stats codes" => HTTP status codes
12.3 "certificates are signed by _require_ a central enrollment...." -- ?
12.6.3 "Replay is typical prevented ..." => typically
Will be resolved by RFC Editor, or before WGLC?
====================================
1.2.1 "this draft"
2 "are marked with an asterisk"
5.3.1 "This presentation is to some extent a placeholder....
We expect this to be a question for the WG to decide."
5.6.1 Future Support for HIP
"We leave this work as a topic for another draft".
Is this to be addressed before WGLC?
_______________________________________________
P2PSIP mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/p2psip
