Hi,
We had another closer look at especially sections 5 and 9 of the draft;
comments follow.
Page 59, Section "5.5.1.1. Request Definition"
Is there a need to send foundations if there is only single stream and
component ID?
Page 62, Section "5.5.1.5. Gathering Candidates"
"An agent MUST implement ICE-tcp [I-D.ietf-mmusic-ice], and MUST gather
at least one UDP and one TCP host candidate for RELOAD and for SIP."
(1) Why ICE-tcp is a MUST? Wouldn't UDP be enough for many cases and
ICE-tcp could be a "SHOULD"?
(2) Why base draft defines that one must gather candidates for SIP?
Should this be in the SIP usage draft instead and this section could
talk about candidates for applications/usages in general? There seem to
be quite a few references to SIP usage elsewhere in the draft too.
Page 62, Section "5.5.1.6. Encoding the Attach Message"
"Instead of actually encoding an SDP, the candidate information (IP
address and port and transport protocol, priority, foundation, component
ID, type and related address) is carried within the attributes of the
Attach request or its response"
Component ID is not in the attributes, and probably foundation shouldn't
be either.
Page 62, Section "5.5.1.8. Role Determination"
"The connectivity checks MUST still contain the ICE-CONTROLLED and
ICE-CONTROLLING attributes, however, even though the role reversal
capability for which they are defined will never be needed with RELOAD.
This is to allow for a common codebase between ICE for RELOAD and ICE
for SDP."
Should you rather just generate the value for the ICE-CONTROLL{ED,ING}
attribute locally instead of sending it in the messages? This would
allow for using common code base and still make the message simpler/save
bandwidth.
Page 64, Section "5.5.2. AttachLite"
As the ICE-TCP draft says, "Unlike UDP, there [is] no lite
implementation defined for TCP", so this section may need some
additional text about TCP lite candidates.
Page 66, Section "5.5.3. AppAttach"
"The AppAttach request and its response contain an application
attribute, with a value of SIP or RELOAD, which indicates what protocol
is to be run over the connection."
(1) Isn't normal Attach used for RELOAD instead of AppAttach? And you
seem to have SIP usage specific text also here.
(2) The text talks about AppAttachReq and AppAttachAns whereas the
struct is called AppAttachReqAns. There are similar issues with the
other req/ans structures too.
Page 67, Section "5.5.3.1. Request Definition"
"application A 16-bit port number. This port number represents the
IANA registered port of the protocol that is going to be sent on this
connection."
What about applications without an IANA registered port number?
Page 68, Section "5.5.5.2 Response Definition"
"response_id: A randomly generated 64-bit response ID. This is used to
distinguish Ping responses."
Why aren't transaction ID and sender peer-ID enough for distinguishing
responses?
Page 71, Section "5.6.2. Reliability for Unreliable Links"
This whole section, especially with retransmission, flow control and
fragmentation support, adds quite a bit complexity to the link layer
implementation. This, and all the NAT traversal complexity, could be
avoided by running RELOAD over TCP over NAT traversing connection (such
as one created by Teredo or HIP).
Page 86, Section "6.4.1.2. Response Definition"
"If any type of request tries to access a data kind that the node does
not know about, an Error_Unknown_Kind MUST be generated."
While the previous section, page 82 says:
"Implementations SHOULD reject requests corresponding to unknown kinds
unless specifically configured otherwise."
These two seem to conflict. I would keep the ability to configure an
overlay to accept unknown Kinds. Later (e.g., page 89) this seems to be
the wording that is used.
Page 99, Section "9.4 Joining"
"In order to populate its neighbor table, JP sends a Ping via the
bootstrap node directed at Resource-ID n+1 (directly after its own
Resource-ID). This allows it to discover its own successor. Call that
node p0. It then sends a ping to p0+1 to discover its successor (p1).
This process can be repeated to discover as many successors as desired."
Could the AP send a copy of its neighbor table (e.g., in an Update
message) to the JP? The JP could then initialize its neighbor table
using the received entries and there would be no need to send Ping requests.
Page 100, Section "9.5. Routing Attaches"
"If a peer is unable to successfully Attach with a peer that should be
in its neighborhood, it MUST locate either a TURN server or another peer
in the overlay, but not in its neighborhood, through which it can
exchange messages with its neighbor peer."
(1) The Attach, if ICE was used, should have already included TURN
candidates (if any were available), so using TURN here again is likely
to fail. What if a peer is still unable to exchange messages with it's
should-be-neighbor?
(2) What is the method for using "another peer" (but not TURN server)
for exchanging messages with the neighbor peer?
Page 103, Section "9.6.3. Receiving Updates"
"The UpdateReq defines peers that indicate a neighbor table further away
from N than some of its neighbor table. Note that merely receiving
peers further away does not demonstrate this, since the update could be
from a node far away from N. Rather, the peers would need to bracket N."
What is meant by "need to bracket N"? Also, the other sentences of this
bullet point seem a bit confusing.
Page 106, Section "9.7. Route Query"
The struct uses "next_id" and the following text "next_peer".
Finally, one more nit:
The word "extension" seems to be misspelled as "extention" couple of
times in the draft.
Cheers,
Ari Keranen
Jouni Maenpaa
_______________________________________________
P2PSIP mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/p2psip
