Subject: grow: Review of draft-ietf-grow-mrt-03.txt

From: Geoff Huston <[email-masked]>
Date: Sun, 02 Jul 2006 14:53:34 +1000

Larry has reminded me that this document is nearing completion for a 
Working Group Last Call as a Proposed Standard.

With my working group chair hat OFF, I've reviewed the document, and I have 
noted the following comments. Given the intended stats of Proposed Standard 
I've attempted to be careful in identifying potential ambiguities here, so 
that the document that we pass to the IESG is as clear and as precise as we 
can get it.

I'd like to propose that the document should have one further iteration 
before a WG Last Call, so now would be a good time for any other WG members 
with an interest in this MRT spec to review the document and send in their 
comments to the WG and to Larry.

regards,

    Geoff

-----------------------------------------
Review comment of draft-ietf-grow-mrt-03.txt

1. Introduction

It may be useful to point out that this is the specification of the MRT 
format, and does not specify any protocol for the exchange of MRT-formatted 
messages, nor any transport protocol to carry MRT format messages, nor any 
storage format for MRT messages

It also may be useful to point out that this specification provides a 
standard mechanism to pass routing protocol log messages between a routing 
system and a logging system, as well as providing a common storage format 
for such messages.

Given that this is intended to be a Proposed Standard I expected to see the 
standard boilerplate:

    The keywords "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
    "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
    document are to be interpreted as described in RFC 2119.

in the intro, as I also expected to see the various uses of these terms in 
the text to be capitalized

2. Basic MRT Format

Isn't this "Common MRT Header Format" as distinct from a "Basic" MRT Format

in this section the text says "Types 1 through 5 are used for MRT control 
information", but in section 3, MRT Control Types, there is a control types 
0 through 4. I suspect that the text should read "Types 0 through 4 are 
used ..."

The Length section does not define the units of length. Is it the length of 
the message field in octets? You also have not define "the header" Perhaps 
a subtly different wording that would remove these potential ambiguities 
would be:

"A 4-octet message length field.  The length field contains the length (in 
octets) of the Message field."

Also in the Message field description you have:

"The contents of this field are context dependent on the Type and Subtype 
fields."

I'm trying to understand why to include the word "context"

Surely what you are saying is

"The contents of this field are dependent on the Type and Subtype fields."

Section 3:

"These control messages are unidirectional in nature and there is no form 
of an acknowledgment or response from the receiver to the sender. "

I'm confused here - as there is nowhere else that I see that any other 
message should be acknowledged, and I also wonder why this piece of 
protocol information should be in a message format specification. I'd 
suggest dropping this sentence completely

3.1 "A sender may wish to send these for synchronization or keep-alive 
purposes."

I'd suggest

"This message type MAY be used for synchronization or keep-alive purposes."

3.2.  START Type

    The START Type indicates a sender is about to begin sending MRT
    messages

I'd suggest something like

"This message SHOULD be used to commence a MRT message stream"

(i.e. rephrase this to talk about the semantics of the message type rather 
than its intended role in some form of MRT-based message exchange protocol)

3.3.  DIE Type

    A DIE Type signals that the receiver should shut down.

similarly:

"This message closes a MRT message stream."

3.4.  I_AM_DEAD Type

    A I_AM_DEAD indicates that the sender is shutting down.

I'm not completely sure, but it looks like this message means that the MRT 
generator has encountered a condition that has caused the message generator 
to cease. i.e. DIE is a 'normal' stream close, and "I_AM_DEAD" is an 
abnormal close - is this right?

3.5.  PEER_DOWN Type

    A PEER_DOWN is sent when the sender's peer is down.  In practice, a
    sender will likely have multiple peers.  It is recommended that the
    sender use the Message field to convey the IP address of the peer
    represented in US-ASCII.

"represented in US-ASCII"???

how about :

"using conventional dotted quad IPv4 notation or standard IPv6 address 
representation."

Section 4

Also I'm a little confused about what has been deprecated and what has not.

Maybe you might consider the following

"The following types have been defined in this specification. Types marked 
as "deprecated" SHOULD NOT be used in new implementations of this format. 
These deprecated types are defined here as an aid to their interpretation 
in applications that support legacy compatibility in interpreting old 
MRT-format logs.

        5    BGP    deprecated
        6    RIP
        7    IDRP   deprecated
        8    RIPNG
        9    BGP4PLUS  deprecated
        10   BGP4PLUS_01  deprecated
        11   OSPF
        12   TABLE_DUMP
        16   BGP4MP
        17   BGP4MP_ET
        32   ISIS
        33   ISIS_ET
        64   OSPF_ET

(I'm also confused whether TABLE_DUMP has been deprecated)

I'd then bunch the 4 deprecated types in their own subsection

i.e.  use the following logical structure of the type descriptions

4.1 RIP
4.2 RIPNG
4.3 OSPF
4.4 OSPF_ET
4.5 BGP4MP
4.6 BGP4_ET
4.7 ISIS
4.8 ISIS_ET
4.9 TABLE_DUMP
4.10 Deprecated Types
4.10.1 BGP
4.10.2 BGP4PLUS
4.10.3 BGP4PLOS_01
4.10.4 IDRP

4.1.2  perhaps you should explain the semantics to the source AS and IP 
address abd destination AS and IP address - i.e. that this message reflects 
a captured BGP update message being passed between two BGP speakers, and 
that the MRT header fields identify the source and destination BGP speakers.

4.1.4 when you refer to the BGP FSM do you mean to refer RFC1771, or should 
you be referring to the current BGP FSM spec. namely section 8 of RFC4271?

4.7  TABLE_DUMP - the peer AS is 16 bits long - what's the upgrade path to 
32 bit peer AS numbers for this packet format? Are the update attributes 
stored in binary format using the BGP binary encoding?

4.8.1. should reference section 8 of  RFC4271

I don't understand how 32 bit ASNs can be used within this subtype/. Or is 
this a deprecated subtype, and SHOULD subtype
  4    BGP4MP_MESSAGE_32BIT_AS be used by all new implementations of this 
format?

4.8.2 same comment about 32 bit AS number values applies here

What does "interface index" refer to?

4.8.3 Does this deprecate the TABLE_DUMP type? If not, which type SHOULD be 
used for dumps?

Why has the PEER IP and AS fields been dropped from this subtype.

You might also want to define these fields a bit further - in particular, 
what is:
"Status", and  "Time last change"
Also, this format appears not to record the peer associated with this entry 
- is this deliberate?

if the subtype BGP4MP_MESSAGE_32BIT_AS deprecates the 16 bit asn bgp 
message type, then what message type has 32 bit support for STATE_CHANGE 
messages?

4.9 (and a general comment about the _ET types)

you should explain the "32-bit microsecond timestamp" field. what are its 
units? what it its epoch?

5.1 The document notes that  "Type codes 0 - 64 have been allocated" - this 
document defines type codes 0-4, 5-12, 16, 17, 32-33 and 64. Are the other 
type codes _really_ allocated? Or are they "reserved". I'm confused here.

references - I'd use RFC4271 in preference to the now obsoleted 1771. I'd 
also include a ref to the ISO IDRP standard

_________________________________________________________________
web user interface: http://darkwing.uoregon.edu/~llynch/grow.html
web archive:        http://darkwing.uoregon.edu/~llynch/grow/

---

From: Larry Blunk <[email-masked]>
Date: Tue, 18 Jul 2006 13:00:17 -0400

Geoff,
     Thanks for the comments.    Responses in line.

 -Larry

Geoff Huston wrote:
> With my working group chair hat OFF, I've reviewed the document, and I 
> have noted the following comments. Given the intended stats of 
> Proposed Standard I've attempted to be careful in identifying 
> potential ambiguities here, so that the document that we pass to the 
> IESG is as clear and as precise as we can get it.
>
> I'd like to propose that the document should have one further 
> iteration before a WG Last Call, so now would be a good time for any 
> other WG members with an interest in this MRT spec to review the 
> document and send in their comments to the WG and to Larry.
>
> regards,
>
>    Geoff
>
> It may be useful to point out that this is the specification of the 
> MRT format, and does not specify any protocol for the exchange of 
> MRT-formatted messages, nor any transport protocol to carry MRT format 
> messages, nor any storage format for MRT messages
>
> standard mechanism to pass routing protocol log messages between a 
> routing system and a logging system, as well as providing a common 
> storage format for such messages.
>
> Given that this is intended to be a Proposed Standard I expected to 
> see the standard boilerplate:
>
> in the intro, as I also expected to see the various uses of these 
> terms in the text to be capitalized

  Okay, I will incorporate these suggestions.

> in this section the text says "Types 1 through 5 are used for MRT 
> control information", but in section 3, MRT Control Types, there is a 
> control types 0 through 4. I suspect that the text should read "Types 
> 0 through 4 are used ..."
>
> The Length section does not define the units of length. Is it the 
> length of the message field in octets? You also have not define "the 
> header" Perhaps a subtly different wording that would remove these 
> potential ambiguities would be:
>
> "A 4-octet message length field.  The length field contains the length 
> (in octets) of the Message field."
>
>
> "The contents of this field are context dependent on the Type and 
> Subtype fields."
>

    Yes, this sounds better.
>
> Section 3:
>
> "These control messages are unidirectional in nature and there is no 
> form of an acknowledgment or response from the receiver to the sender. "
>

  Okay.

> (i.e. rephrase this to talk about the semantics of the message type 
> rather than its intended role in some form of MRT-based message 
> exchange protocol)
  Okay will do.
> I'm not completely sure, but it looks like this message means that the 
> MRT generator has encountered a condition that has caused the message 
> generator to cease. i.e. DIE is a 'normal' stream close, and 
> "I_AM_DEAD" is an abnormal close - is this right?
>
>
     There have been no implementations of these types that I'm aware 
of.    I just
took the definition of these type from comments in the enum 
definitions.   In practice,
MRT implementations have logged the MRT messages to a local file system
and do not implement the concept of a "sender" and "receiver".    I 
could try to
flesh these out somewhat more, or we could simply declare them to be
deprecated.

> "using conventional dotted quad IPv4 notation or standard IPv6 address 

 okay.
> "The following types have been defined in this specification. Types 
> marked as "deprecated" SHOULD NOT be used in new implementations of 
> this format. These deprecated types are defined here as an aid to 
> their interpretation in applications that support legacy compatibility 
> in interpreting old MRT-format logs.
>
>
>        5    BGP    deprecated
>        6    RIP
>        7    IDRP   deprecated
>        8    RIPNG
>        9    BGP4PLUS  deprecated

  Good suggestion.
> (I'm also confused whether TABLE_DUMP has been deprecated)

  The TABLE_DUMP type is still used by Zebra/Quagga even though Zebra
defined a new type (BGP4MP_ENTRY) which provides the same functionality.
I was hesistant to declare the TABLE_DUMP deprecated given that it is still
actively in use.

> i.e.  use the following logical structure of the type descriptions

 sounds good.
>
> 4.1.2  perhaps you should explain the semantics to the source AS and 
> IP address abd destination AS and IP address - i.e. that this message 
> reflects a captured BGP update message being passed between two BGP 
> speakers, and that the MRT header fields identify the source and 
> destination BGP speakers.
  okay.
>
> 4.1.4 when you refer to the BGP FSM do you mean to refer RFC1771, or 
> should you be referring to the current BGP FSM spec. namely section 8 
> of RFC4271?

   It refers to RFC1771.     I would suggest deprecating the state 
change types
as they have seen limited implementation and are not all that useful.
>
>
> 4.7  TABLE_DUMP - the peer AS is 16 bits long - what's the upgrade 
> path to 32 bit peer AS numbers for this packet format? Are the update 
> attributes stored in binary format using the BGP binary encoding?

   My assumption is that implementations requiring 32 bit AS support would
migrate to the BGP4MP_ENTRY type for table dumps as it does encode the
peer AS within the MRT header.   Since TABLE_DUMP is still actively used
and BGP4MP_ENTRY seems to have some issues (see below).  I'll add
a type for 32 bit peer AS support.

> 4.8.1. should reference section 8 of  RFC4271

    As mentioned above this was defined with the RFC1771 in mind.
>
> I don't understand how 32 bit ASNs can be used within this subtype/. 
> Or is this a deprecated subtype, and SHOULD subtype
>  4    BGP4MP_MESSAGE_32BIT_AS be used by all new implementations of 
> this format?

    I would suggest deprecating.
> 4.8.2 same comment about 32 bit AS number values applies here
   BGP4MP_MESSAGE_32BIT_AS is the updated version of BGP4MP_MESSAGE with
32 bit AS support.
> What does "interface index" refer to?

      I'll add some text to this.   It's considered an optional field 
which can be used
to indicate the a numeric interface on which a peering session is 
established.
>
> 4.8.3 Does this deprecate the TABLE_DUMP type? If not, which type 
> SHOULD be used for dumps?

     As mentioned about, TABLE_DUMP is still used by Zebra/Quagga, so I 
wasn't
comfortable declaring it as deprecated.
> Why has the PEER IP and AS fields been dropped from this subtype.
  Not sure why these were dropped.   This information would seem to
be useful.    Maybe this is why Zebra/Quagga still use TABLE_DUMP.
>
> You might also want to define these fields a bit further - in 
> particular, what is:
> Also, this format appears not to record the peer associated with this 
> entry - is this deliberate?
    I'll add some text to define these.  Not sure why peer info was 
dropped.  I could
note this as a limitation in this type and suggest using TABLE_DUMP instead.
> message type, then what message type has 32 bit support for 
> STATE_CHANGE messages?

     As mentioned above, suggest deprecating state change types.
> you should explain the "32-bit microsecond timestamp" field. what are 
> its units? what it its epoch?

   okay.

>
>
> 5.1 The document notes that  "Type codes 0 - 64 have been allocated" - 
> this document defines type codes 0-4, 5-12, 16, 17, 32-33 and 64. Are 
> the other type codes _really_ allocated? Or are they "reserved". I'm 
> confused here.

  should be reserved.
>
>
> references - I'd use RFC4271 in preference to the now obsoleted 1771. 
> I'd also include a ref to the ISO IDRP standard
>
>

   okay, I'll add these.