Subject: Re: [pim] AD review draft-ietf-pim-group-rp-mapping-05.txt

From: Bharat Joshi <[email-masked]>
Date: Tue, 19 Oct 2010 08:58:50 +0530

Hi Adrian,

      Thanks for these very thoughtful suggestions. There are couple of them which we need to discuss before addressing them. Please see my response in line for each of the issue.

Regards,
Bharat

> Hi,
>
>  I have performed an AD review of your draft.
>
>  Don't panic!
>
>  I review all drafts that I am responsible for before putting them forward
> for IETF last call. The main objective is to catch nits and minor issues
> that would show up during the last call or in IESG review. The intention is
> to help polish your document and make sure it is clean and shiny so that
> other reviewers will stick to the technical details.
>
> Most of my comments are pretty trivial, but a couple have more meat on them
> and I'd like to see a quick respin of the document before I issue the IETF
> last call. As soon as I see a new revision posted, I'll set the ball in
> motion.
>
> Of course, all of my issues are up for discussion.
>
> Thanks for the work,
> Adrian
>
> ---
>
> The RFC Editor is a little sensitive around the boilerplate for RFC 2119
> terms.
>
> In Section 2 you have:
>
>    In this document, the key words "MUST", "MUST NOT", "REQUIRED",
>    "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY",
>    and "OPTIONAL" are to be interpreted as described in RFC 2119
>    [RFC2119].
>
> idnits points out that the standard boilerplate is:
>
>    The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
>    "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in
>    this document are to be interpreted as described in RFC 2119
>    [RFC2119].
>
> Can you make the change?
>

Yes. We will fix this.

> ---
>
> Can you confirm that there is no requirement to include an IETF Trust
> Legal Provisions of 28-dec-2009, Section 6.c(iii) paragraph? In other
> words, can you confirm that no disclaimer for pre-RFC5378 work is
> required?
>
> You may be OK with this, but it is good to check.
>

No. We do not need any disclaimer.

> ---
>
> Acronyms
>
> It is easy to get so wrapped up in our own technology that we forget that
> some of our readers may be less familiar with the acronyms.
>
> The RFC Editor requires that all acronyms except for the ones marked with an
> asterisk at their web page
> http://www.rfc-editor.org/rfc-style-guide/abbrev.expansion.txt are expanded:
> - on first use in the Abstract
> - on first use in the rest of the document
>
> Could you take care of this, please.
>

Ok. We will expand the first use of each acronym.

> ---
>
> Section 1
>
>    It is critical that each router select the same 'RP' for a specific
>    multicast group address. This RP address may correspond to a
>    different physical router but it is one logical RP address and must
>    be consistent across the PIM domain.  This is usually achieved
>    by using the same algorithm to select the RP in all the PIM routers
>    in a domain.
>
> In this document you are defining a new algorithm. Unless you are proposing
> a simultaneous upgrade of all nodes in the network, this is going to result
> in two different algorithms being in use at the same time in the same
> domain.
>
> You need to address this point at the very least by explaining that your new
> algorithm will produce the same results as the old algorithm so there is no
> problem with them both running in the same domain.
>

Yes. They both will produce the same result only if the Group-to-RP mappings are learned from only one source.

> But wait! If the new algorithm produces the same result as the old
> algorithm, why is it needed? :-)
>

This new algorithm is an extension to take care of case where group-to-rp mappings are learned from multiple sources.

> So, I'm a bit confused :-(
>
> - Is this document updating 4601 by replacing the algorithm?
> - Is this document updating 4601 by providing a second algorithm?
> - Is this document simply providing an optional second algorithm?
> - How does interworking work? I.e., what are the backward
>   commpatibility issues and solutions?
>
> The way to address this probably by adding backward compatibility in your
> migration situations in "5. Common use cases".
>
> It looks like the point of the draft is that vendors have different
> algorithms in choosing an RP today, so for multivendor environments it would
> be good to have a consistent approach in choosing an RP. This improves the
> 4601 algorithm but backward compatibility needs to be discussed.

Yes. The above is the right interpretation.

We can do following:

1. Add a use-case for backward compatibility
2. Add some text as suggested above.

Do you think this is good enough to clear this confusion?

> ---
>
> Router implementations and MIB modules
>
> In section 1 you have this text:
>
>    PIM-STD-MIB [RFC5060] has defined an algorithm that allows
>    administrators to override Group-to-RP mappings with static
>    configuration.  But this algorithm is not completely deterministic,
>    because it includes an implementation-specific 'precedence' value.
>
> While the RFC does describe an algorithm, I don't think it is an algorithm
> that a PIM router would use to select the RP, rather, it is an algorithm
> that a management station can use to work out which node will have been
> selected as an RP.
>
> But my confusion about the purpose of this document deepens! In section 6
> you list in step 2 you have:
>
>  2. If the Multicast Group Address being looked up is in the SSM
>     range or is configured for Dense mode, no Group-to-RP mapping is
>     selected, and this algorithm terminates.  Alternatively, a RP
>     with address type 'unknown' can be selected.  Please look at
>    section #8 for more details on this.
>
> Now, section 8 is about MIB objects. So it describes how the fact that no
> Group-to-RP mapping has been selected can be indicated to the management
> station in the MIB module.
>
> So I think that maybe step 2 of the algorithm should read...
>
>   2. If the Multicast Group Address being looked up is in the SSM
>      range or is configured for Dense mode, no Group-to-RP mapping is
>      selected, and this algorithm terminates.  The fact that no
>     Group-to-RP mapping has been selected can be represented in the
>     PIM-STD-MIB module by setting the address type of the RP to
>    'unknown' as described in Section 8.
>

Ok. We can use this text.

> ---
>
> Section 3
>
> OLD
>    Existing algorithm defined in PIM-SM (Section 4.7.1 in [RFC4601])
>    does not consider following constraints:
> NEW
>    The existing algorithm defined in PIM-SM (Section 4.7.1 in
>    [RFC4601]) does not consider the following constraints:
>

Ok. We can use this text.

> ---
>
> Section 4
>
> There seems to be some contradiction between the second and third bullets.
> The second says that dynamic mappings override static ones except under
> specific circumstances. The third says that dynamically learned mappings are
> always preferred over static configuration.
>
> I think this needs clarification, perhaps through moving the third bullet to
> the top of the list and indicating that these are the "general order of
> preference".
>

Ok. Yes, this will avoid the confusion.

> ---
>
> Section 6 - Title
>
> Don't be so timid!
> You are defining an algorithm, not simply proposing one.
>
> ---
>
> Section 6 - nit
>
>   3.   From the set of all Group-to-RP mapping entries, the subset
>         whose group prefix contains the multicast group that is being
>         looked up, are selected.
>
> s/are selected/is selected/
>

Will fix.

> ---
>
> Section 6
>
>   4.   If there are no entries available, then the Group-to-RP mapping
>         is undefined.
>
> Does the algorithm also terminate at this point, and does the same rule
> about setting the address type in the MIB module to 'unknown' apply?
>

Yes. The algorithm terminates here.

No, the rule with address-type as unknown does not apply.

> ---
>
> Section 6.
>
>  7.   From the remaining set of Group-to-RP Mappings we select the
>        subset of the entries based on the origin.  Group-to-RP mappings
>        learned dynamically are preferred over static mappings.  If the
>        remaining dynamic Group-to-RP mappings are from BSR and
>        Auto-RP then the mappings from BSR SHOULD be preferred.
>
> Why this sudden use of "SHOULD". You haven't used 2119 language anywhere
> else in the algorithm.
>
> I suggest you change this s/SHOULD be/is/  and add "unless..." to give an
> explanation of how some other choice might be made.
>

Ok. Will fix.

> ---
>
>  Section 6.
>
>  9.   If the remaining Group-to-RP mappings were learned through BSR
>       and the PIM Mode of the Group is 'PIM-SM' then the hash function
>       will be used to choose the RP.  The RP with the highest
>       resulting hash value will be selected.
>
> Could you add a forward pointer to section 10 next to the mention of the
> "hash function".
>

Yes. Will do that.

> ---
>
> Section 7
>
>    Group-to-RP mapping algorithm defined in PIM-STD-MIB [RFC5060] does
>    not specify the usage of 'pimGroupMappingPrecedence' and
>    'pimStaticRPPrecedence' objects in 'pimGroupMappingTable' table
>    clearly.  With the newly proposed algorithm in this document, these
>    MIB objects would not be required.  So we propose to deprecate these
>    MIB objects from PIM-STD-MIB.  Also the newly proposed algorithm in
>    this document MUST be preferred over Group-to-RP mapping algorithm
>    defined in either PIM-SM[RFC4601] or in PIM-STD-MIB[RFC5060].
>
>  Wow! There is a lot covered in this little section.
>
>  a. If you want to deprecate some MIB objects, I think you need more
>     than a sentence hidden in this draft. You would need a full
>     revision of RFC 5060. A tidier way to handle this would be an
>    erratum on RFC 5060 that points out that the objects are not used
>    and updates the Description clauses, but doesn't actually deprecate
>    the objects.

Ok. We can create an errata for RFC 5060 to address this.

>
> b. It seems to me that step 5 of the algorithm in the Description
>    clause of pimGroupMappingTable *does* mention how the
>    pimGroupMappingPrecedence object is used. Similarly, the
>    description clause of pimStaticRPPrecedence seems relatively
>    detailed. So perhaps what you want is to clarify the Description
>    clauses rather than deprecate the objects?

Ok.

> c. What does it mean to "propose to deprecate" in an RFC?
>

Will fix.

> d. s/the newly proposed algorithm/the algorithm defined/
>

Will fix.

> e. See my previous comments about requiring that this new algorithm
>    replaces the one in 4601. While you can require that for new
>    implementations you would need to:
>    - be clear that this document updates 4601
>    - carefully consider how you interwork with legacy implementations
>

Ok. Let me see how we can improve the text to address this.

> f. If you are also updating the MIB module, you will need to indicate
>    that this document updates RFC 5060, but I am not sure that this is
>    a very good way to go about things. We would need to consult with a
>    MIB expert because I find it a bit confusing if we "hide" the
>    update to a MIB Description clause in this document.

Ok, We are also not sure now whether we need an Errata or an update RFC for fixing these Object's description.

>    But now I am also worried about what 5060 is trying to do with that
>    description clause. If the PIM implementation is capable of working
>    out the Group-to-RP mapping, why isn't the RP simply made available
>    in a MIB object? Why is the management agent required to read the
>    whole table and work it out for itself?

This is because the router will have a table of group-to-rp mappings. Using MIB, we can fetch the table but can not find the RP that will be used for a given group. This algorithm needs to be run on management station. Is this clear?

> ---
>
> Section 8.
>
> Previous comments about this section, but is this an update to a Description
> clause in 5060? If so, again, are you updating 5060? Is this the best way to
> do it?
>

Not sure about this. I think we assumed that we can update RFC 5060 this way. Do we need to come up with another draft to update this or an Errata will do?

> ---
>
>  Section 9
>
>    In practice, it is not usually necessary to run several dynamic
>    Group-to-RP mapping mechanisms in one administrative domain.
>    Specifically, interoperation of BSR and Auto-RP is OPTIONAL and not
>    recommended by this document.
>
>  a. Delete "In practice".
>
Ok.

>  b. Are *you* defining that interoperation is OPTIONAL, or is that
>    definition elsewhere? If defined elsewhere, please include a
>    reference.
>
Yes, we mean interoperation is OPTIONAL.

> c. According to 2119, "NOT RECOMMENDED" == "SHOULD NOT"
>    "SHOULD NOT" allows the exceptional "MAY"
>     "OPTIONAL" == "MAY"
>    So I am not sure that "and not recommended by thus document" adds
>    value. Furthermore it is slightly disconcerting since it suggests
>    that maybe someone else is maybe recommending it.
>

Ok. Will reword it.

>  ---
>
>  Section 9
>
>    The algorithm in this document MUST be used.
>
> This is definitely a statement that you are updating RFC 4601
>
>  Then in the following paragraph...
>
>    An implementation of PIM that supports only one mechanism for
>    learning Group-to-RP mappings SHOULD also use this algorithm.  The
>    algorithm has been chosen so that existing standard implementations
>    are already compliant.
>
>  The "SHOULD" appears to contradict the previous "MUST".
>
>  If existing implementations are already compliant with this algorithm then
> you surely don't need this new specification. I think this is trying to
> answer all of my backward compatibility issues, but it needs some work to
> explain the how and why.
>

It should be using MUST only as we are updating RFC 4601.

> ---
>
>  Section 10
>
>    It is RECOMMENDED that
>    network operators configure only one PIM-Bidir RP for each RP
>    Priority.
>
>  Is this updating RFC 5015?

No. It is not updating RFC 5015.

> ---
>
> Section 11
>
>    An implementation of PIM SHOULD support configuration to block
>    specific dynamic mechanism for a valid group prefix range.
>
> This looks like an update to 4601. Is it?
>

No. Its not an update.

> ---
>
> Section 12
>
> I'd suggest that this section will not (should not?) get through review by
> the Security Directorate. You are proposing a change to the way in which a
> PIM node operates. At the very least you should consider how the algorithm
> could be gamed/disrupted and why the protocol is protected (by the standard
> security mechanisms in 4601). It will turn out that the bottom line is the
> same (nothing new needs to be done), but your current text is just too terse
> to prove the point.
>

Ok. Will expand it.

---

From: Adrian Farrel <[email-masked]>
Date: Tue, 19 Oct 2010 17:19:19 +0100

Hi Bharat,

Thanks for engaging...

[snipped to issues for discussion]

> Do you think this is good enough to clear this confusion?

Yes, that should be good enough. Thanks.

>> Section 7

[snip]

> This is because the router will have a table of group-to-rp mappings.
> Using MIB, we can fetch the table but can not find the RP that will be
> used for a given group. This algorithm needs to be run on management
> station. Is this clear?

Yes. That's clear.

> Not sure about this. I think we assumed that we can update RFC 5060 this
> way. Do we need to come up with another draft to update this or an Errata
> will do?

I suggest you update the I-D with all of the other changes and then I will 
take it to the MIB doctors and ask their opinion on updating 5060.

>> This looks like an update to 4601. Is it?

I assume you mean that *this* is not an update to 4601, but that the draft 
in general is (as you agreed in the comment about Section 9).

Many thanks,
Adrian

---

From: Bharat Joshi <[email-masked]>
Date: Wed, 20 Oct 2010 08:49:33 +0530

Hi Adrian,

     Thanks for responding quickly. I will post a new revision as quickly as I can after addressing all the comments. Please see in line.

Regards,
Bharat

Ok.

Ok.

Ok.
 
> in general is (as you agreed in the comment about Section 9).

Yes. I agree that it updates RFC 4601.