Subject: Re: [pim] Rtgdir last call review of draft-ietf-pim-drlb-13

From: Ben Niven-Jenkins <[email-masked]>
Date: Tue, 7 Jan 2020 16:51:41 -0500

Hi Stig,

Closing the loop on this…your responses answer all my questions, I have no issues with the document now, which I see has been approved for publication :-)

Apologies for the delay in my response…holiday season and all that.

Regards
Ben

> On 11 Dec 2019, at 18:06, Stig Venaas <[email-masked]> wrote:
> 
> Hi
> 
> Thanks a lot for the good review with a lot of comments. I believe
> I've addressed them in the new version. Please see inline for further
> comments.
> 
> On Tue, Nov 5, 2019 at 7:59 AM Ben Niven-Jenkins via Datatracker
> <[email-masked] <mailto:[email-masked]>> wrote:
>> 
>> Reviewer: Ben Niven-Jenkins
>> Review result: Has Issues
>> 
>> Hello,
>> 
>> I have been selected as the Routing Directorate reviewer for this draft. The
>> Routing Directorate seeks to review all routing or routing-related drafts as
>> they pass through IETF last call and IESG review, and sometimes on special
>> request. The purpose of the review is to provide assistance to the Routing ADs.
>> For more information about the Routing Directorate, please see
>> http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
>> 
>> Although these comments are primarily for the use of the Routing ADs, it would
>> be helpful if you could consider them along with any other IETF Last Call
>> comments that you receive, and strive to resolve them through discussion or by
>> updating the draft.
>> 
>> Document: draft-ietf-pim-drlb-13.txt
>> Reviewer: Ben Niven-Jenkins
>> Review Date: 5th November 2019
>> IETF LC End Date: 7th November 2019
>> Intended Status: Standards Track
>> 
>> Summary: I have some minor concerns about this document that I think should be
>> resolved before publication.
>> 
>> Comments: The document is well written and easy to understand.
> 
> Thanks.
> 
>> Major Issues: No major issues found.
>> 
>> Minor Issues:
>> 
>> 1) Section 4.1 says:
>> 
>> “To become a GDR Candidate, a router must have the same DR priority and run the
>> same GDR election Hash Algorithm as the DR on the LAN.”
>> 
>> and
>> 
>> “Furthermore, assume router R1 wins the PIM DR election, R1 and R2 run the same
>> Hash Algorithm for GDR election, while R3 runs a different one.”
>> 
>> I think it would be clearer if you said “support”/“supports” (or maybe
>> “advertise”/“advertises”) rather than “run”/“runs”. As I think what you are
>> trying to say is if a router has the same DR priority as the DR it is only a
>> GDR candidate if it also supports the same hash algorithm as advertised by the
>> DR.
> 
> Fixed.
> 
>> 2) In section 5.3.1 the PIM DR Load Balancing Capability (DRLB-Cap) Hello
>> Option only contains a single value for Hash Algorithm. How is transition
>> between different hash algorithm expected to be achieved?
> 
> It is not clear that we will define other algorithms. If we do, I
> think it might be sufficient that the administrator manually
> configures the candidates with the preferred algorithm. We may come up
> with an election scheme later once we have additional algorithms
> though.
> 
>> Does a router that supports multiple hash algorithms include multiple DRLB-Cap
>> Hello Options each containing a different hash algorithm and the DR selects the
>> hash algorithm it prefers (or is configured to use)?
>> 
>> If so, then it might be worth explicitly mentioning that in the document.
> 
> In pim we only use each option at most once in the hello, but I've
> added a comment saying at most once.
> 
>> 3) Section 5.3 says “The DRLB-List Hello Option consists of three Hash Masks as
>> defined above and also a sorted list of GDR Candidate addresses on the LAN.”
>> 
>> Section 5.3.2 says “All addresses MUST be in the same address family as the PIM
>> Hello IP header.  It is RECOMMENDED that the addresses are sorted in descending
>> order.”
>> 
>> Section 5.3 implies (without explicitly stating it) that GDR Candidate
>> addresses MUST be sorted, but section 5.3.2 states that sorting is only
>> RECOMMENDED.
>> 
>> Maybe remove any possibility of ambiguity by rewording Section 5.3 to something
>> like “The DRLB-List Hello Option consists of three Hash Masks as defined above
>> and also a list of GDR Candidate addresses on the LAN. It is RECOMMENDED that
>> the GDR Candidate addresses are sorted in descending order.”
> 
> I've tried to clarify this now. I added that it is recommended that
> first time I mention a sorted list. Also changed RECOMMENDED to
> recommended, as things will work just fine either way. I also added
> text explaining why it is a good idea to sort them.
> 
>> Nits:
>> 
>> Section 3 says “The extension specified in this document applies to PIM-SM when
>> they act as last hop routers (there are directly connected receivers).”
>> 
>> Do you mean “applies to PIM-SM DRs”? Otherwise who is the “they” referring to
>> in that sentence?
> 
> Good catch.
> 
>> Section 3 says “This is because the source tree is built using the IP address
>> of the sender, not the IP address of the PIM DR that sends the registers
>> towards the RP.”
>> 
>> Do you mean “registration” instead of “registers”?
> 
> There are PIM register messages. It is a specific message type. I added "PIM".
> 
> Thanks,
> Stig