Subject: [pim] AD-review comments on draft-ietf-pim-sm-v2-new-11.txt

From: Alex Zinin <[email-masked]>
Date: Thu, 18 Aug 2005 23:24:59 -0700

Folks-

Let me first point out that I was pleased by the quality of this document,
clearly a lot of work went into it, and it's appreciated. Thanks to the
authors, WG chairs, and everyone who helped with reviews.

Please fine some comments below. I will start the IETF LC right away, so we
can address those comments (if any) together when it ends.

Meta: Please check "should", "must", "may" through the text and make
sure 2119 keywords are used where necessary.

Venu pointed out that there are two issues with Asserts that don't appear
to be fixed, namely:

  a) Assert looser still forwarding traffic on a LAN, see thread from
     23-Aug-2004, subject "FW: [pim] PIM-SM: Assert Issue resulting in
     assert-loser forwarding onto LAN".

  b) Assert cancel received on RPF-interface resulting in wrong upstream
     RPF-neighbor. See thread from 12-Jul-2004, subject "[pim] Hello HoldTime
     and Assert questions"

Please advise if those were dropped balls and should be fixed or an
informed decision was made to record and address these issues later.

> 1.  Introduction
...
> PIM-SM version 2 was originally specified in RFC 2117, and revised in
> RFC 2362.  This document is intended to obsolete RFC 2362, and to
> correct a number of deficiencies that have been identified with the way
> PIM-SM was previously specified.  As far as possible, this document
> specifies the same protocol as RFC 2362, and only diverges from the
> behavior intended by RFC 2362 when the previously specified behavior was
> clearly incorrect.  Routers implemented according to the specification
> in this document will be able to successfully interoperate with routers
> implemented according to RFC 2362.

The header should say "Obsoletes: RFC 2362".
A section "Changes since RFC 2362" would also be very useful.

> 4.3.1.  Sending Hello Messages
...

> DR_Priority Option SHOULD be included

what's the behavior if it's not there?

> Generation_Identifier (GenID) Option SHOULD be included

ditto

> To allow new or rebooting routers to learn of PIM neighbors quickly,
> when a Hello message is received from a new neighbor, or a Hello message
> with a new GenID is received from an existing neighbor, a new Hello
> message should be sent on this interface after a randomized delay
> between 0 and Triggered_Hello_Delay.

An example where 2119 "SHOULD" seems appropriate

> 4.3.2.  DR Election

> Neighbor state is deleted when the neighbor timeout expires.
> 
> The function for computing the DR on interface I is:

>   host
>   DR(I) {
>       dr = me
>       for each neighbor on interface I {
>           if ( dr_is_better( neighbor, dr, I ) == TRUE ) {
>               dr = neighbor
>           }
>       }
>       return dr
>   }

As a suggestion, would it be useful to add some words explaining that "me"
represents the calculating router and that fields in it are filled out
based on the local information rather than from a received packet?

4.3.4

> Within one Address List Hello option, all the addresses MUST be of the
> same address family.  It is not permitted to mix IPv4 and IPv6 addresses
> within the same message.  In addition, the address family of the fields
> in the message SHOULD be the same as the IP source and destination
> addresses of the packet header.

What if it's not?

> 4.4.2.  Receiving Register Messages at the RP
> 
> When an RP receives a Register message, the course of action is decided
> according to the following pseudocode:
> 
> packet_arrives_on_rp_tunnel( pkt ) {
>     if( outer.dst is not one of my addresses ) {
>         drop the packet silently.
>         # Note: this may be a spoofing attempt
>     }
>     if( I_am_RP(G) AND outer.dst == RP(G) ) {
>           sentRegisterStop = FALSE;
>           if ( register.borderbit == TRUE ) {
>                if ( PMBR(S,G) == unknown ) {
>                     PMBR(S,G) = outer.src
>                } else if ( outer.src != PMBR(S,G) ) {
>                     send Register-Stop(S,G) to outer.src
>                     drop the packet silently.

Do you really want to send Register-Stop for every Register Message,
instead of pacing it somehow?

IANA:

> 5.1.  PIM Address Family
> 
> The PIM Address Family field was chosen to be 8 bits as a tradeoff
> between packet format and use of the IANA assigned numbers.  Since when
> the PIM packet format was designed only 15 values were assigned for
> Address Families, and large numbers of new Address Family values were
> not envisioned, 8 bits seemed large enough.  However, the IANA assigns
> Address Families in a 16-bit field.  Therefore, the PIM Address Family
> is allocated as follows:
> 
>      Values 0 through 127 are designated to have the same meaning as
>      IANA-assigned Address Family Numbers [6].
> 
>      Values 128 through 250 are designated to be assigned for PIM by the
>      IANA based upon IESG Approval, as defined in [8].

I assume IANA is supposed to create and maintain a registry for the
PIM-specific AFIs, right? If so, would be good to spell this out.

>      Values 251 through 255 are designated for Private Use, as defined
>      in [8].
> 
> 5.2.  PIM Hello Options
>
Ditto here.

> Values 17 through 65000 are to be assigned by the IANA.  Since the space
> is large, they may be assigned as First Come First Served as defined in
> [8]. Such assignments are valid for one year, and may be renewed.
> Permanent assignments require a specification (see "Specification
> Required" in [8].)

Hmmmm... I can see a problem with the one-year timeout here. I don't think
we should expect IANA to keep track of the expiration of assignments. What
is the goal here?

Nit checker says:

idnits 1.74

tmp/draft-ietf-pim-sm-v2-new-11.txt:

  Checking nits according to http://www.ietf.org/ID-Checklist.html:
    Checking conformance with RFC 3978/3979 boilerplate...
  * The document seems to lack an RFC 3978 Section 5.1 IPR Disclosure
    Acknowledgement -- however, there's a paragraph with a matching
    beginning. Boilerplate error?
  * The document seems to lack an RFC 3978 Section 5.4 Copyright Line
    -- however, there's a paragraph with a matching beginning. Boilerplate
    error?
  * The document seems to lack an RFC 3978 Section 5.4 Reference to BCP
    78. 
    (The document uses RFC 3667 boilerplate or RFC 3978-like
    boilerplate instead of verbatim RFC 3978 boilerplate.  After 6 May 2005,
    submission of drafts without verbatim RFC 3978 boilerplate is not
    accepted.)
  * There are 580 instances of too long lines in the document, the
    longest one being 11 characters in excess of 72.

  Checking nits according to http://www.ietf.org/ietf/1id-guidelines.txt:
    Nothing found here (but these checks do not cover all of
    1id-guidelines.txt yet).

  Miscellaneous warnings:
    None.

Thank you.
--
Alex

_______________________________________________
pim mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/pim