Document: draft-ietf-pim-join-attributes-05.txt
Reviewer: Elwyn Davies
Review Date: 1 October 2008
IESG Telechat date: (if known) -

Summary:
The document is now targeted at standards track instead of informational which looks correct to me. 
I guess it should be considered as updating RFC 4601.

This is now nearly ready for the IESG.  There are a couple of minor issues noted below.  
I found the document generally much easier to understand and the issues from the previous review 
are now sorted out.  There are some subtleties in the interaction of 'conflict resolution' and 
'change of attributes' that took me a while to convince myself did not result in problems.  
I have suggested a couple of textual changes that might save some head scratching for readers.

Comments:
s2 (and probably abstract): Make it clearer that it only applies to Sparse mode in the first instance.

s3.2: I am not sure why there is such emphasis on 'MUST process the first attribute of a given type'.  It would seem to me that the interaction between multiple instances of a particular attribute is something that would be specified in the definition of particular attributes.  Maybe something like:
'If multiple attributes are present in a type 1 Encoded-Source Address, the receiving PIM router MUST examine all the attributes in the order received. The specifications of individual attributes MUST specify the processing to be applied to the first and subsequent attributes of the type that are encountered. 
  This processing MAY be different for multiple instances (including ignoring second and subsequent instances or discarding a packet as malformed if there are multiple instances) and MAY affect the processing of instances of specific types of attribute found (later?) in the attribute sequence.'

s3.3.2, last para: s/would be forwarded/MUST be forwarded/

s3.3.3: Somewhere in s3 it should be explicitly stated that state needs to be held for each tree regarding the attributes applied AND the adjacency from which they were received.

s3.3.3, para after Figure 2: s/best attribute/adjacency that supplied the selected attribute/, s/second best attribute/the attribute from the adjacency that was next in order of preference/... 'best' seems a bit enthusiastic for selection according to IP address ordering!

s3.3.3, next to last para: s/would be used/MUST be used/, s/the described/the procedure described/

s3.3.3, last para: How can a router determine that transitive attributes conflict if it does not understand them?  Maybe it should be explicit both here and in the earlier default case that 'conflict' means any or all of different numbers of instances of the same attribute, inconsistent values (using octet for octet comparison) with the same number of instances or same value but in different order.

s3.3.4, next to last para: It might help to add to the comment about conflict resolution something like: 'If the router needs to apply conflict resolution, only changes resulting from Join messages on the highest priority adjacency will
  be actioned (see Section 3.3.3).'

s3.4.1: The method of encoding the 'Length' (assume unsigned binary integer) need to be specified.

s4: The reference for the 'Encoding Type' field registry needs to be this document with an additional pointer to RFC 4601, since the registry is actually defined here even if should have been defined in 4601.


Editorial:
s3.1, last para: s/apriori/a priori/