Subject: Comments on draft-ietf-rsvp-refresh-reduct-02.txt

From: Adrian Farrel <[email-masked]>
Date: Fri, 4 Feb 2000 15:14:57 -0000

Lou et al,
Good job with the latest version of the draft.
I have a few very minor questions and editorial points, and a bunch of typos
(sorry, I can't read a draft w/o marking up typos!)
Hope this is of use.
Regards,
Adrian

1) Discovery that neighbor supports bundling.
   Section 3.3 suggests using configuration to discover that 
   the neighbor supports bundling. Is this of any use given
   that the neighbor may change its support at any time and 
   indicate the fact using the RRC bit?
   Are you just covering a possible implementation where it
   is well known what the behavior of the neighbor is so that
   it is never necessary to examine the RRC bit?  If so, this
   is in contradiction with section 2 that says that the node
   MUST examine the RRO on every received message.
  
2) Refresh period/ Refresh delay
   Section 3 suggests that refresh messages may be delayed up
   to their refresh interval. This isn't clear to me.
   Are you saying that a refresh message that is generated 
   _before_ the refresh timer pops can be delayed up until the
   time that the timer pops? 
   Or are you saying that a refresh message that is generated 
   when a refresh timer pops can be delayed up to at most 
   another time period equal to the value of the refresh timer?

3) Bundle message Send_TTL
   Section 3.4 says that the Send_TTL is used to calculate the
   number of non-IP hops, and that this is used later in 
   processing sub-messages.
   The same section says that each sub-message is processed as
   if it was received individually. Since each sub-message also
   contains a Send_TTL, what is the use of the Bundle Send_TTL?

4) Message Acknowledgement
   The last paragraph of section 4 says...
      Each message containing an MESSAGE_ID object may be 
      acknowledged via a MESSAGE_ID_ACK object.
   Is "may be" too generous? An Ack may only be sent if the
   MESSAGE_ID object requests an Ack.
   
5) Epoch
   I think the definition in 4.2 would benefit from some of the 
   text in 4.5.  In particular:
   - include "the RSVP agent restarts" with the "node reboots"
   - the value SHOULD be different from the last time the node 
     cycled

6) Message_Identifier
   Section 4 para 2 makes this unique within a sender/receiver
   pairing. The definition in 4.2 says that it is unique for
   a sender.
   Although this is unlikely to make a difference to the 
   implementation at a receiver, it does make a difference to
   sender and line trace implementations.

7) MESSAGE_ID_NACK object
   See section 4.3
   Is there scope for a reason code? There may be no use at the 
   moment, but this looks good for future extensions.
   
8) Epoch value
   Section 4.5 para 3...
        On startup, a node SHOULD randomly select
        a value to be used in the Epoch field. The
        node SHOULD ensure that the selected value
        is not the same as was used when the node 
        was last operational.
   - Why is it important that the assignment is random? The 
     important bit is to try to keep the value different from 
     previous use. Perhaps you should use RECOMMEND?
   - Is the previous use the only value to avoid? Might it be
     desirable to avoid several previous values, especially
     if the RSVP agent is cycled several times in quick 
     succession?
     
9) Message_Identifier value
   See section 4.5, para 4...
       When a node is sending a trigger message, the 
       Message_Identifier value MUST have a value that
       is greater than any other previously used value.
   This is within the scope of the current value of Epoch, right?
   Perhaps the text should say so.

10) Message_Identifier presence
    Section 5.1
    Is it worth stating that at least one Message_Identifier must
    be present in the MESSAGE_ID LIST object?
    Ditto MESSAGE_ID SRC_LIST and MCAST_LIST.

11) Srefresh NACK
    I think it would help to add clarification that the
    MESSAGE_ID_NACK refers to the Message_Identifier in the (e.g.)
    MESSAGE_ID LIST object of the Srefresh, *not* to the 
    Message_Identifier in the MESSAGE_ID object of the Srefresh.

    Perhaps it would also be useful to add a section on 
    "Acknowledgement of Srefresh" to clarify what is meant by an 
    Ack of an Srefresh (which may be requested through the flags 
    in the MESSAGE_ID object in the Srefresh. In particular, an 
    Ack of an Srefresh may still be followed by some 
    MESSAGE_ID_NACKs relating to Message_Identifiers carried by 
    the Srefresh.

_Typos_

section 2. Flags. First line. Delete "to" to read "when set, indicates that
this..."

section 3.3. 2nd para. penultimate line.  For "MUST not" read "MUST NOT"

section 3.4. 1st line. For "an Bundle" read "a Bundle"

section 4.  final para. For "an MESSAGE_ID" read "a MESSAGE_ID"

section 4.6, final para. final sentence should be clause of previous
sentence. Read "...and longer time, although additional..."

section 5.3, p27 last para. For "an MESSAGE_ID LIST" read "a MESSAGE_ID
LIST"

section 5.4 first line. For "indicated" read "indicate"
--
Adrian Farrel  mailto:[email-masked]
ATM, MPLS and Telecoms Development Group
Data Connection Ltd., Chester, UK
http://www.datcon.co.uk/
Tel: +44 (0) 1244 313440  Fax: +44 (0) 1244 312422

---

From: Lou Berger <[email-masked]>
Date: Wed, 09 Feb 2000 07:25:30 -0500

>(sorry, I can't read a draft w/o marking up typos!)

Don't worry, we really appreciate the careful review!  We'll integrate your 
comments into the post last call version.

>Hope this is of use.

absolutely.

>    MUST examine the RRO on every received message.

This is primarily aimed at covering the initialization case.  When so 
configured a node need not wait to receive a message from a neighbor before 
sending a bundle message.   It may also be used when non-RSVP nodes exist 
in the network and all RSVP nodes are known to support the extensions.  In 
my opinion, it would be a configuration error for the a node to be 
configured to expect that a neighbor supported the extensions when it in 
fact does not.

>    to their refresh interval. This isn't clear to me.

oh well, I thought I was clear...

>    time that the timer pops?

yes.  Implicit in this is implementations can take advantage of this but 
aren't required to.

>    another time period equal to the value of the refresh timer?

definitely not.

>    contains a Send_TTL, what is the use of the Bundle Send_TTL?

You are correct.  The the results of the test on Bundle message Send_TTL 
should be used in Path sub-message processing to detect a non-RSVP 
hop.  All other sub-messages are processed without modification.

>    MESSAGE_ID object requests an Ack.

I don't see a real problem with this given where it is in the document.  I 
also don't mind making it more clear by adding some words.

>    - the value SHOULD be different from the last time the node

a reasonable comment.

>    sender and line trace implementations.

Another good comment.  Consistency here would be useful.

>    Is there scope for a reason code?

Scope?  Do you mean room or value?

>There may be no use at the

I'm not in favor of adding anything to the object that's not needed at this 
point.  We've been working to minimize features and options.  We can always 
define a "reason code" object or new c-type if and when the need arises.

>      previous use. Perhaps you should use RECOMMEND?

SHOULD=RECOMMEND see rfc 2119.

>      if the RSVP agent is cycled several times in quick

You're getting into probabilities of when values get reused.  Randomness 
helps here.

>    Perhaps the text should say so.

I think it's adequately covered by text in the preceding paragraph:
    The [Epoch] value MUST NOT be changed unless the node or the
    RSVP agent is restarted.

>     Ditto MESSAGE_ID SRC_LIST and MCAST_LIST.

sure.  You like things even more explicit than I do...

>     MESSAGE_ID_NACKs relating to Message_Identifiers carried by

I though the text was fairly explicit in this area.  I'll review it with 
this comment in mind and see if we can be clearer.

>section 5.4 first line. For "indicated" read "indicate"

most appreciated!

Thank you for the detailed review and very useful comments.

Lou