Hi Andy, thanks for responding so promptly. A few comments in line:

On 2010-12-18 18:36, Andy Kessler (kessler) wrote:
> Hi, 
> 
> Thank you for the review. 
> My comments are below annotated with Andy>
> 
> Andy
> 
> 
> I am the assigned Gen-ART reviewer for this draft. For background on
> Gen-ART, please see the FAQ at
> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
> 
> Please resolve these comments along with any other Last Call comments
> you may receive.
> 
> Document: draft-ietf-pim-group-rp-mapping-07.txt
> Reviewer: Brian Carpenter
> Review Date: 2010-12-18
> IETF LC End Date: 2010-12-23
> IESG Telechat date: 
> 
> Summary: In good shape, a few issues.
> --------
> 
> Major issues:
> -------------
> 
> 3.  Existing algorithm
> ...
>    The algorithm defined in this document updates algorithm defined in
>    PIM-SM ( Section 4.7.1 in [RFC4601]).  The new algorithm is backward
>    compatible and will produce the same result only if the Group-to-RP
>    mappings are learned from a single mapping source.
> 
> A logical implication of the last sentence is that if some of the Group-to-RP
> mappings are learned from more than one mapping source, the new algorithm 
> might 
> not produce the same result as the existing algorithm. Is that indeed the 
> case,
> and if so, don't we have a problem until all PIM-SM routers have been updated?
> 
> Andy> Most vendors today do not officially support deployments with multiple
> Andy> dynamic routing protocols. They probably have inconsistent results now. 
> Andy> This algorithm will standardize the results for those vendors. It will
> Andy> be more consistent when all vendors do support the new algorithm. 

OK. Maybe you could add a sentence to make it clear that adding the new
algorithm doesn't fix the problem completely until it's universally deployed.

> 5.  Common use cases
> 
>    o  Default static Group-to-RP mappings with dynamically learned
>       entries
> 
>    Many network operators will have a dedicated infrastructure for the
>    standard multicast group range (224/4) and so might be using
>    statically configured Group-to-RP mappings for this range.  
> 
> Please indicate whether this issue also applies to IPv6.
> 
> Andy> There is no issue with IPv6. IPv4 has some older baggage because
> Andy> the 224/4 addressing scheme has been around since before there was
> Andy> SSM and BSR. IPv6 Multicast does not have that history. 

OK. For someone ignorant of the history, it might be worth adding
"This issue does not arise for IPv6."

> 
> 
> 
> 6.  Proposed algorithm
> ...
>    10.  From the remaining set of Group-to-RP Mappings we will select
>         the RP with the highest IP address.  This will serve as a final
>         tiebreaker.
> 
> Please define "highest". Does it mean "greatest when considered as an
> unsigned integer"?
> 
> Andy> This should be the same method that is used to pick 
> Andy> the highest IP address in many protocols. One I know well is
> Andy> the way that a designated router is selected in RFC 4601 in
> Andy> section 4.3.2. The main part of the function says: 
> Andy>      a.primary_ip_address > b.primary_ip_address
> Andy> To me that has always meant you compare the first byte,  
> Andy> then the 2nd, the 3rd and then the 4th. 

and the 5th, etc, for IPv6 ;-)

> Andy> I don't know where that would be defined. Maybe someone else knows. 
> Andy> It is not defined in RFC 4601. 

I have no idea either. Maybe it's a term of art in routing protocols,
so I won't insist; I was just looking at it from the viewpoint
of a naive implementor.

> 8.  Clarification for MIB Objects
> 
>    When a Group-to-RP mapping entry is created in the
>    pimGroupMappingTable in the PIM-STD MIB[RFC5060], it would be
>    acceptable to have an entry with an RP with address type 'unknown'
>    and a PimMode of Dense Mode or SSM.  These entries would represent
>    group ranges for Dense mode or SSM.
> 
>    Also all the entries which are already included in the SSM Range
>    table in the IP Mcast MIB would be copied over to
>    pimGroupMappingTable.  They would have a type of configSSM and an RP
>    with address type 'unknown' as described above.
> 
> Is this section intended to be a normative update of RFC 5060, or is
> it just commentary? The phrase "it would be acceptable" almost sounds
> like a normative MAY.  
> 
> Also, shouldn't it be 'unknown(0)' in the context of the MIB? (This
> applies in three places in the draft.)
> 
> Andy> At some point we received comments and requests to include all
> Andy> the group mappings in this table - even for Dense Mode or SSM
> Andy> which do not have an RP. We are merely stating that it OK to 
> Andy> do that and use the RP address and type (of the RP) of unknown. 
> 

Yes, it makes sense, but please make it very clear whether you
are writing normatively or not.

> 
> Nits:
> -----
> 
> 5.  Common use cases
> ...
>    o  More use cases
> 
>    By no means, the above list is complete.  Please drop a mail to
>    'authors' if you see any other use case for this.
> 
> This seems a bit strange to appear in the final RFC; at the least it needs 
> rewriting.
> 
> Andy> I agree. That was for the draft and should be removed. Should we
> Andy> just remove the whole section ? Or change the wording to somehow
> Andy> say there are more use cases that will benefit from this algorithm ? 

Either way is fine for me.

> 
> The checker says:
> 
> == There are 1 instance of lines with multicast IPv4 addresses in the
>      document.  If these are generic example addresses, they should be changed
>      to use the 233.252.0.x range defined in RFC 5771
> 
> Andy> We have discussed this before. We need to use the addresses that in the
> Andy> examples. 239/8 is the Administratively Scoped Block as defined in RFC 
> 5771.
> Andy> Its the same thing for 239.100.0.0/16 in section 5. We were told to add
> Andy> the comment that says its an administratively scoped multicast address
> Andy> range. Using GLOP will not make the same point we are trying to 
> explain. 

Fair enough. Don't be surprised if someone in the IESG asks the same question.

   Brian
_______________________________________________
Gen-art mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/gen-art

Reply via email to