Subject: [pim] Minor errors and typos in draft-ietf-pim-sm-v2-new-08.txt

From: James Lingard <[email-masked]>
Date: Wed, 4 Feb 2004 16:27:56 -0000

Hi,

I have several comments on the current PIM-SM draft
(draft-ietf-pim-sm-v2-new-08.txt), which I've listed below.  Most of these
are very minor errors or typos.

I apologise for my lateness in sending these comments, but I have only
fairly recently begun looking at PIM in detail.  Hopefully, as they are
mostly very minor, there will still be time to make the necessary changes
before the next revision of the draft.

Please feel free to ask me for clarification if anything is unclear.

Thanks,
James.

==========================================================================

Note: all page numbers refer to the "plain text" version of the draft.

==========================================================================
*   Throughout.

Inconsistent use of AND, OR vs. &&, || in pseudo-code.

==========================================================================
*   Section 4.1.4.

At a DR, a router also needs to store the Register state-machine state and
the Register-Stop timer per (S,G).

==========================================================================
*   Section 4.1.6 (page 21).

>>    The inherited_olist(S,G,rpt) is [...] a strict subset of
>>    immediate_olist(*,G).

It might not be a strict subset if joins(*,*,RP(G)) is not NULL.

==========================================================================
*   Section 4.1.6 (page 24).

>>    return NBR( RPF_interface(S), MRIB.next_hop( S ) ) )
                                                         ^
Typo: unmatched close-bracket.

==========================================================================
*   Section 4.2 (page 26).

>>    On receipt of data from S to G on interface iif:
>>
>>     if( DirectlyConnected(S) == TRUE ) {

I believe it would make more sense if this test were replaced with
'(DirectlyConnected(S) == TRUE) AND (iif == RPF_interface(S))'.

==========================================================================
*   Section 4.4.1 (page 38).

In the register state-machine, in the two transitions from the
Join-Pending or Prune states to the NoInfo state, the action 'remove reg
tunnel' should be removed as the register tunnel can only exist in the
Join state.

==========================================================================
*   Section 4.4.1 (page 40, last paragraph).

>>    A Register-Stop(*,G) should be treated as a Register-Stop(S,G) for
>>    all existing (S,G) Register state machines.

What is really meant is '[...] all (S,G) Register state machines that are
not in NoInfo state.'  We should avoid making the implementation-specific
assumption that state-machines in NoInfo state 'don't exist'.

==========================================================================
*   Section 4.4.2 (page 41).

>>    if( outer.dst is not one of my addresses ) {
>>        drop the packet silently.
>>        # note that this should not happen if the lower layer is working
>>    }

I suggest removing this clause.  Should we not be able to assume that the
lower layer _is_ working?

==========================================================================
*   Section 4.4.2 (page 41).

>>    decapsulate and pass the inner packet to the normal
>>    forwarding path for forwarding on the (*,G) tree.

Does this phrase mean 'decapsulate and forward the inner packet to all
interfaces in inherited_olist(S,G,rpt)'?  If so, it should say this
explicitly.  If not, what does it mean?

==========================================================================
*   Section 4.5 (page 42).

There are two references to the 'PIM Destination field' in a Join/Prune
message, but there isn't a field of this name.  I believe this should be
replaced with 'Upstream Neighbor Address'.

==========================================================================
*   Sections 4.5.1, 4.5.2 and 4.5.3.

In PP state, if the PP timer expires, the action must include cancelling
the Expiry timer.

In PP state, if the Expiry timer expires, the action must include
cancelling the PP timer.

==========================================================================
*   Section 4.5.2 (page 48).

>>    Figure 3: Downstream per-interfacce (*,G) state-machine in tabular
                               ^^^^^^^^^^
Typo: should be 'interface'.

==========================================================================
*   Section 4.5.4 (page 55).

The difference between 'n/a', '-' and 'error' in the FSM table should be
explained more clearly.

==========================================================================
*   Section 4.5.4.

In P state, when the ET expires, delete the text that says 'ET and PPT are
cancelled' (since neither can be running).

In P' state, on reaching EOM, the action must not include cancelling the
PPT (as it cannot be running).

In PP' state, on reaching EOM, the action must include cancelling the ET
and PPT.

==========================================================================
*   Section 4.5.6 (page 63).

The description of the Not Joined state is missing the phrase 'and local
membership information' (from the corresponding text in Section 4.5.7).

==========================================================================
*   Section 4.5.6 (page 66) and Section 4.5.7 (page 71).

>>    JoinDesired(*,G) becomes True
>>        The downstream state for (*,G) has changed so that at least
>>        one interface is in immediate_olist(*,G), making
>>        JoinDesired(*,G) become True.

Not strictly correct.  See the definition of JoinDesired(*,G) - it can be
TRUE even if immediate_olist(*,G) is NULL.  Same comment applies to
"JoinDesired(*,G) becomes False" further down the page, and to the
equivalent paragraphs in Section 4.5.7.

==========================================================================
*   Section 4.5.7 (page 68).

>>    If a (S,G) Assert occurs on the upstream interface, and this changes
>>    the this router's idea of the upstream neighbor, it should be [...]
      ^^^

Typo: delete superfluous "the".

==========================================================================
*   Section 4.5.8 (page 73).

>>    This section describes the rules for including an Prune(S,G,rpt)
                                                     ^^
Typo: should be "a".

==========================================================================
*   Section 4.5.9 (page 74).

>>    RPTNotJoined(G)
>>         neither (*,G) nor (*,*,RP(G)) has not been joined.
                                             ^^^
Typo: delete superfluous "not".

==========================================================================
*   Section 4.5.9 (page 75).

In the table describing transitions from the NotPruned(S,G,rpt) state, the
actions taken when OT expires should not include "Cancel OT".

==========================================================================
*   Section 4.5.9 (page 75).

In the table describing transitions from the NotPruned(S,G,rpt) state, the
notation 'OT = min(OT, t_override)' is inconsistent with the notation for
the same thing in Sections 4.5.5 - 4.5.7 (e.g. 'Decrease Join Timer to
t_override').  Is there supposed to be a difference?

==========================================================================
*   Section 4.6.1 (page 81 and page 84).

When in NoInfo state, there is an event 'Receive Preferred Assert with
RPTbit clear and AssTrDes(S,G,I)'.  But you can't have a 'Preferred
Assert' in NoInfo state since there is no current Assert winner.  I
believe this should be 'Acceptable Assert'.

==========================================================================
*   Section 4.6.1.

Should clarify in each case whether a received assert may be a (*,G)
Assert or whether it must be an (S,G) Assert.

==========================================================================
*   Section 4.6.1 (page 84).

>>    Receive Preferred Assert with RPT bit clear AND
>>        AssertTrackingDesired(S,G,I)==TRUE
>>        [...]
>>        The received assert that has a better metric than our own [...]
                              ^^^^
Typo: delete superfluous "that".

==========================================================================
*   Section 4.6.2 (page 88).

>>    [...] unless the (S,G) assert state machine for the relevant S and G
>>    is in the "NoInfo" state [...]

What if there is no 'relevant S', i.e. if the Source-Address field is
INADDR_ANY?

==========================================================================
*   Section 4.6.2 (page 90).

In the definition of AssertTrackingDesired(*,G,I), 'CouldAssert(*,G)'
should be 'CouldAssert(*,G,I)'.

==========================================================================
*   Section 4.6.3 (page 94).

Should clarify the ordering to be used for IP addresses.

==========================================================================
*   Section 4.6.4 (page 96).

For an AssertCancel(*,G), why should the source be set to RP(G) and not
INADDR_ANY?

==========================================================================
*   Section 4.6.5 (page 97).

>>    AssertWinner(S,G,I) is the IP source address of the Assert(S,G)
>>    packet that won an Assert.

Couldn't it actually be an Assert(*,G) packet that has won the (S,G)
Assert?

==========================================================================
*   Section 4.7 (page 99).

>>   >From the point of view of PIM-SM, a PMBR has two tasks:
     ^
Typo: delete ">".

==========================================================================
*   Section 4.7.1 (page 100).

>>    were the DR for this source (see Section 4.4.1 with the following
                                                    ^
Typo: missing close bracket.

==========================================================================
*   Section 4.7.2 (page 100).

>>    In the former case, the PMBR will need to issue send a Join(*,*,RP)
                                                ^^^^^
Typo: delete superfluous "issue".

==========================================================================
*   Section 4.7.2 (page 101).

>>    o If the PMBR has joined on the (*,*,RP) tree, then it should set
>>      DownstreamJPState(*,G,I) to TRUE on the discard interface for all
                                    ^^^^
Typo: should be "Join".

==========================================================================
*   Section 4.8.1 (page 104).

>>    However of the implementations we are are of, [...]
                                            ^^^
Typo: should be "aware".

==========================================================================
*   Section 4.10.5.1 (page 123).

In the table of Join/Prune rules, the cell 'Join(S,G,rpt) x Prune(S,G)'
should be '?' and the cell 'Prune(S,G,rpt) x Prune(S,G)' should be 'yes'.

==========================================================================
*   Section 4.10.5.2 (page 124).

>>    [...] may result in packets that are larger in the maximum IP [...]
                                                  ^^
Typo: should be "than".

---
James Lingard
Network Protocols Group
Data Connection Ltd (DCL)
Tel: +44 20 8366 1177
Fax: +44 20 8363 1039
Email: [email-masked]
Web: http://www.dataconnection.com/

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