Re: [pim] Secdir last call review of draft-ietf-pim-bfd-p2mp-use-case-07

[Greg Mirsky <[e-mail]>]

Hi Russ,
thank you for your thorough review, thoughtful and helpful suggestions.
Please find my notes in-lined below under the GIM>> tag. I've attached the
new working version and the diff.

Regards,
Greg

On Tue, Sep 21, 2021 at 9:17 AM Russ Housley via Datatracker <
[e-mail]> wrote:

> Reviewer: Russ Housley
> Review result: Has Issues
>
> I reviewed this document as part of the Security Directorate's ongoing
> effort to review all IETF documents being processed by the IESG.  These
> comments were written primarily for the benefit of the Security Area
> Directors.  Document authors, document editors, and WG chairs should
> treat these comments just like any other IETF Last Call comments.
>
> Document: draft-ietf-pim-bfd-p2mp-use-case-07
> Reviewer: Russ Housley
> Review Date: 2021-09-21
> IETF LC End Date: 2021-09-28
> IESG Telechat date: Unknown
>
>
> Summary: Has Issues
>
>
> Major Concerns:  None
>
>
> Minor Concerns:
>
> General: All of the field names in this document use camel case, except
> one.  I think the document would be easier to read if My Discriminator
> were to use the same convention.  Also, HeadDiscriminator would be
> more descriptive.
>
GIM>> Thank you for pointing this out to me. I agree with the proposed
update of the field name, The remaining in the text references to My
Discriminator use the convention of RFC 5880. I hope that is acceptable.

>
> Section 2.1 says:
>
>    The head MUST include the BFD Discriminator option in its Hello
>    messages.
>
> This MUST statement cold me much more complete:
>
>    The head MUST include the BFD Discriminator option in its Hello
>    messages, and it MUST include a 4-byte My Discriminator with a
>    value other than zero.
>
GIM>> Thank you, I agree with the proposed text with a minor modification
based on re-naming of the field to HeadDiscriminator. Below is the update:
OLD TEXT:
   The head MUST include the BFD Discriminator option in its Hello
   messages.
NEW TEXT:
    The head MUST include the BFD Discriminator option in its Hello
   messages, and it MUST include a 4-byte HeadDiscriminator with a value
   other than zero.


> Section 2.3: s/must set/MUST set/
>
GIM>> Thank you. Done.

>
>
> Nits:
>
> Section 1, para 1 could be more clear and more forceful.  I suggest:
>
>    Faster convergence in the control plane minimizes the periods of
>    traffic blackholing, transient routing loops, and other situations
>    that may negatively affect service data flow.  Faster convergence
>    in the control plane is beneficial to unicast and multicast routing
>    protocols.
>
GIM>> Thank you for the suggested text. Accepted.

>
> Section 1, para 2: s/DR is to act on behalf/DR acts on behalf/
>
GIM>> Thank you. Done.

>
> Section 1, para 3: The first sentence is very unclear.  I cannot offer
> an improvement because it is too hard to parse.
>
GIM>> Would the following update make it clearer:
OLD TEXT:
   Bidirectional Forwarding Detection (BFD) [RFC5880] had been
   originally defined to detect a failure of point-to-point (p2p) paths
   - single-hop [RFC5881], multihop [RFC5883].
 NEW TEXT:
   Bidirectional Forwarding Detection (BFD) [RFC5880] had been
   originally defined to detect a failure of a point-to-point (p2p)
   path, single-hop [RFC5881] or multihop [RFC5883].


> Section 1, para 3: s/networks precisely/networks, and it precisely/
>
GIM>> Thank you. Accepted.

>
> Section 1.1.1: s/familiarity/Familiarity/
>
GIM>> Done.