Subject: [Gen-art] Gen-art review of draft-ietf-pim-join-attributes-03.txt

From: Elwyn Davies <[email-masked]>
Date: Wed, 01 Oct 2008 19:45:38 +0100

I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
_http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html_).

This is a re-review of the document since it has been considerably
adapted since the review of -03 back in March 2008.

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/
_______________________________________________
Gen-art mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/gen-art

---

From: Eric Rosen <[email-masked]>
Date: Mon, 06 Oct 2008 13:37:45 -0400

Elwyn,

I  have just posted  draft-ietf-pim-join-attributes-06.txt, which  I believe
addresses your remaining concerns.

Well, with one exception ;-) With regard to:

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).'

I think it is  better just to leave the current text  which just refers back
to the  conflict resolution  procedures, rather than  to try to  restate the
outcome of those procedures.

Thank you again  for your review, and if you are  satisified with the latest
set of changes, don't forget to tell Russ to lift his discuss!

Eric