Subject: Re: [P2PSIP] I-D Action:draft-ietf-p2psip-base-03.txt

From: Cullen Jennings <[email-masked]>
Date: Thu, 15 Oct 2009 19:04:54 -0600

Thanks for the careful read -  I fixed up this stuff in next version

On Jul 14, 2009, at 6:13 PM, Craig, Dave wrote:

> I had a few questions on my interpretation of the latest updates as  
> well a few typos.
>
>
> 1. I was trying to understand the change made in 3.2.1.  Could that  
> change also be read as follows?
>
> Note that clients that chose this option MUST process Update  
> messages from the peer.  Those updates can indicate that the peer no  
> longer owns the Client's Node-ID.  The client then forms a  
> connection to the appropriate peer.
>
>

Done - thanks for text

> 2. The change made at the end of 5.4.1.
>
> All overlay algorithms MUST specify maintenance procedures _that_  
> send Updates to all members of the Connection Table...
fixed

>
>
> 3. The change made at the beginning on 5.4.2.3 seems to conflict  
> with that at the end of 5.4.1.  If you MUST send Updates to all  
> members of the connection table, then why is it a SHOULD in 5.4.2.3?

fixed - I can't find any explanation of when you would not send the  
update in 5.4.2.3 so I changed that to a MUST.

>
>
> 4. Should the IceExtension in 5.5.1.1 be a TLV, so stacks not  
> comprehending the extensions can still Attach?  That preserves a  
> little more of the extensibility in the SDP string encoding.

If a node does not understand ICE, it needs to be using AttachLite or  
else the sending node will think it is doing ICE when it is not and it  
can become really a mess. The node using ICE may not be in a place  
where things will work without ICE but it has no way to indicate that  
if the node that does not understand ICE tries to use the Attach  
without ICE.

>
>
> 5. In 5.5.2.3, first paragraph, would this read a little easier as,  
> "... When a connection forms, the node MUST check _that_ the  
> certificate is for the node that _sent_ AttachLiteReqAns..."
>
fixed - thanks

>
> 6. In 5.5.3., first paragraph, "The AppAttach request and its  
> response contain _an_ application attribute..."
>
fixed

>
> 7. In 5.6.2.2., end of paragraph, it looks like a dangling sentence  
> was not finished being removed.
>
fixed

>
> 8. In 5.6.2.2.1. third paragraph, "...looking at the time the ACK  
> was received and the time when the message was sent."
>

fixed

>
> 9. In 5.6.2.2.1. fifth paragraph, was the following intended?  "If  
> all retransmissions for a message fail, _the sending node should  
> close the connection routing the message_."  Should there also be  
> additional hysteresis to make sure that a failing connection is not  
> quickly reestablished under a fast chord-reload-ping-frequency?
>

fixed but did address the hysteresis issue

>
> 10. In 5.6.3, second paragraph, "take into account space to allow  
> the via and _destination_ lists to grow."

Fixed -

>
>
> 11. In 10.1, your -frequency elements actually specify periods not  
> occurrences per unit of time.  Suggest renaming these to end in "- 
> period" or "-interval".  End users or developers may enter in the  
> wrong value here if they go by the label name alone.
>

good point - move to interval

>
> 12. In 10.1, I found descriptions for the namespace and encoding  
> requirements for the overlay configuration document.  Is it ok to  
> leave it implicit that these apply to the kind configuration  
> documents sent in 5.5.6.1?
>
fixed - nice catch

>
> 13. In 10.1, "root-cert   This element contains PEM encoded X.509v3  
> certificates that are a..."
>
fixed but check what I did as not exactly as you suggested

>
> 14. In 10.1, Could the following:
>
>   kinds-signature   This signature is computed across the prevision
>      kind from the beginning of the first < of the kind to the end of
>      the last > of the kind in the same was as the "signature element
>      described later in this section.
>
> be reworded as?
>
>   kind-signature   This signature is computed across the
>      kind from the beginning of the first < of the kind to the end of
>      the last > of the kind in the same way as the "signature element
>      described later in this section.
>
fixed

> Likewise, kinds-signer in the descriptions appears to be kind-signer  
> in the example.  Suggest updating example with kinds-signer.
>
fixed

>
> 15. In 10.5 there's a reference to a broadcast-peers element in the  
> configuration document that is not defined in 10.1.  Is this really  
> the bootstrap-peer element in 10.1?

This was the multicast stuff but I'm suggesting removing this.