Re: [ppsp] peer protocol in WG last call

[Riccardo Bernardini <[e-mail]>]

Hi Arno,
please see inline.

Arno Bakker <[e-mail]> ha scritto:

> Hi Riccardo
>
> thanks for the in-depth review! Please see inline.
>
> On 19/11/2012 11:45, Riccardo Bernardini wrote:
>>
>> First, the easy stuff.  Some typos:
>>
>
> Will be corrected.
>
>
>> Now to the "consistent" stuff:
>>
>> == Page 9, Section 2.3 ==
>>     "If A crashes ungracefully, peers should remove A from their peer
>> list when they detect it no longer send messages."
>>
>> How much is "too much"?  Should we leave the peer free
>> to decide or should we give at least some guidelines?
>>
>
> Your proposal makes sense, I'm just not sure what is customary in RFCs.
> Should this go in this draft or in the planned usage guide?
> Chairs?
>
>
>> Also, there is the problem that when A crashes it remains registered
>> with the tracker, although maybe this is not a problem directly
>> related with PPSPP.
>>
>
> This is addressed in the current base tracker proposal. A peer needs  
> to periodically refresh its registration to stay tracked.
>
>
>> == Page 10, Section 3 ==
>>
>>    It is said that "absence of any response indicates and error" and
>> that "Invalid messages are discarded, and further communication with
>> the peer SHOULD be stopped."
>>
>> Isn't this criterion a bit too strong?  If a request is lost (e.g.,
>> because of congestion), the sender will interpret the absence of
>> response as an error or a bad peer.  Also, how long should a peer wait
>> for declaring absence of response?  Should we give guidelines?
>>
>
> We could give guidelines, but again, I'm not sure that is customary.  
> I assume implementers apply common sense and, for example, not  
> declare a peer dead after 1 packet loss over an unreliable  
> transport. Invalid messages mean that there is really something  
> wrong and that sender and receiver don't understand eachother, so  
> then the receiver really should stop the interaction.
>
>
>> == Page 12, Section 3.9. ==
>>
>> It is said "The CHOKE and UNCHOKE messages are informational as a peer
>> is not required to respond to REQUESTs."  However, if I understand
>> correctly, if a peer does not respond to REQUESTs, the peer will be
>> banned, according to the criterion in Section 3.  Am I missing
>> something?
>>
>
> No, I don't think you are missing something. The idea is that a peer  
> P requesting chunks from peer Q will look for another peer R when Q  
> doesn't send chunks, and Q didn't send a CHOKE to announce the fact  
> it wouldn't be responding. P should take into account packet loss  
> and swarm size (if there is just 1 other peer in the swarm, don't  
> disconnect), of course. If P is not getting chunks from Q it moves on.
>
>
>> == Page 13, Section 3.10.2 ==
>>
>> I have a couple of doubts about this
>>
>>    * This supposes that A can send messages to B.
>>
>>      so this should not be a problem. Would it be worth adding a
>> sentence to emphasize this point? (e.g. "Note that A can send a
>> message to B since by 3.10.1 A must have exchanged messages with C..."
>
> Sure.
>
>
>>   * With this model of hole punching, the peer can only connect
>> initially to peers with public IP; in particular, the addresses
>> provided by the tracker must be public IPs.  Am I right?
>>
>
> Yes.
>
>
>> == Page 16, last paragraph of Section 4.3.1 ==
>>
>>     Implementation note:
>>         To record which chunks ... discussed in detail in [BINMAP]
>>
>
> You are right.
>
>
>> == Page 16, Section 4.3.2 ==
>>
>> Another stylistic detail.  It is said that "...an implementation MAY
>> choose to use ACK messages..." than later "When a receiving peer has
>> successfully checked ... it MUST send an ACK message...".
>
> Will be corrected.
>
>
>> == Top of Page 17, Section 4.4 ==
>>
>>    "When a range translates to multiple bins, the byte-range peer the
>> byte-range peer should send multiple e.g. HAVE message."
>>
>> This sentence looks a bit garbled, at the very least "the byte-range
>> peer" is repeated twice.
>>
>
> Will be corrected.
>
>
>> == Page 20, Section 5.3 ==
>>
>>    "The hashes necessary to verify a chunk are in principle its
>> sibling's hash and all its uncle hashes ... can be optimized "
>>
>> If I understand correctly, this optimization requires that a peer MUST
>> save and preserve all the received hashes.
>
> Yes. The requirement that a sender must be able to provide these  
> hashes implies that you should record them. I'll explicitly add the  
> conclusion that this means you MUST record them, *if* you plan to  
> announce you have them to others.
>
> In other words, limited devices can circument this requirement by  
> not sending HAVEs to others after they received a chunk. Hence they  
> are not announcing the availability and are not required to provide  
> them. Not sending HAVEs is allowed by the protocol. Note, however,  
> that this does require that the implementation uses a reciprocity  
> policy that allows this behaviour, as the limited devices are  
> actually freeriding.
>
>
>> == Page 29, Section 8.2. "Version option" ==
>>
>> Maybe this is thinking a bit in advance, but what should a peer set as
>> version value when the peer is able to "speak" more than one protocol
>> version?
>
> Do you have pointers to protocols where this mechanism is used? This  
> could save a few roundtrips of HANDSHAKE messages, so good for  
> time-till-playback if there are indeed several incompatible  
> versions, which I hope there will not be ;o)

[Riccardo]
No specific protocol comes to my mind.  I checked the RFC 6709 "Design  
Considerations for Protocol Extensions"  
(http://tools.ietf.org/html/rfc6709) for hints.  In Section 4.1 there  
is a list of possible solutions for protocol versioning.  Should I  
propose a solution here "on the spot," I would suggest, inspired by  
the RFC, to make the payload of the version option a string of bytes,  
one for every supported version.
Suppose node A initiates the connection toward node B. Node A puts in  
the option all the version it supports, B picks a supported version  
(maybe the largest one?[supposing that later versions are better]) and  
sends back the HANDSHAKE with a single value in the option field.
An alternative solution, where the option payload has a fixed length,  
could be that A puts the numbers of the oldest and the newest  
supported version and B picks one version in that interval.

>
>
>> == Page 34, Section 9.3. Channels ==
>>
>> It is said that
>>
>>    "... PPSPP-over-UDP uses a multiplexing scheme, called "channels"..."
>>
>> Later it is said that
>>
>>    "When channels are used..."
>>
>> My doubt is: is the use of channels mandatory with UDP?  If yes, the
>> hypothesis "when channels are used..." is redundant;
>
> Yes it is mandatory. Pardon me if I'm wrong, but your point may just  
> be grammatical, "When X" in English implies that X will be the case  
> at some point. So you say "When I die" not "If I die", but I'm not a  
> native speaker myself. I'll make it clearer.

[Riccardo]
I am not a native speaker, either. Maybe some native English speaker  
could help us here.

>
>
>> == Page 42, Section 13.2.2. ==
>>
>> This section is about using the tracker as certification authority.
>> It is said
>>
>>    "Upon receipt, the tracker creates a membership certificate from
>> the request with swarm ID S, a timestamp T and the external IP and
>> port it received the message from..."
>>
>> Am I wrong, or this scheme could fail if the peer is behind a
>> symmetric NAT?  Would it be worth to add at least a note about this?
>>
>
> You are right, but that problem applies for any tracker registration.
> I'll add a note.
>
> CU,
>     Arno

Riccardo

> _______________________________________________
> ppsp mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/ppsp
>



-- 
Riccardo Bernardini
DIEGM -- University of Udine
via delle Scienze 208
33100 Udine
Tel: [phone]
Fax: [phone]

----------------------------------------------------------------------
SEMEL (SErvizio di Messaging ELettronico) - AINF, Universita' di Udine