Subject: FW: MIB Doctor review of: draft-ietf-mpls-ftn-mib-08.txt

From: "Wijnen, Bert (Bert)" <[email-masked]>
Date: Wed, 30 Jul 2003 23:36:28 +0200

This review was on a prerelease revision 8 that I did
send to the authors/editors.
However, I think most (if not all of it) equally applies
to rev 07. So the WG members should be able to evaluate
my comments as well.

Thanks,
Bert 

-----Original Message-----
From: Wijnen, Bert (Bert) 
Sent: woensdag 30 juli 2003 23:33
To: Wijnen, Bert (Bert); 'Joan Cucchiara x302';
[email-masked]'
Cc: [email-masked]'; [email-masked]'; [email-masked]';
[email-masked]'; [email-masked]'; [email-masked]';
[email-masked]'; [email-masked]';
[email-masked]'; [email-masked]';
[email-masked]'; [email-masked]'; [email-masked]';
[email-masked]'; [email-masked]'
Subject: RE: MIB Doctor review of: draft-ietf-mpls-ftn-mib-08.txt

Ok, here is a more complete review.
I am sorry to say, that this one is not in very good shape yet.
And a lot of the stuff I will be listing should (in my eyes)
have been found by the WG members themselves. Oh well.
Here we go:

1. When you use IP addresses in examples (you do so at several
   places), then you are supposed to use addresses that are
   set aside (reserved) for samples. See 
      https://www1.ietf.org/ID-nits.html
   Where it states:
    -  Addresses used in examples should prefer use of fully
       qualified domain names to literal IP addresses, and prefer
       use of example fqdn's such as foo.example.com to real-world
       fqdn's. See RFC 2606 for example domain names that can be
       used. There is also a range of IP addresses set aside for
       this purpose. These are 192.0.2.0/24 (see RFC 3330).
       Private addressess that would be used in the real world
       should be avoided in examples. 

2. In your list of prefixes in sect 5.1.1 you have a few dots too
   many in 192.168..254.0/24, 192.168..255.0/25
   But as I said under 1. above, these addresses may need to be
   changed anyway?

3. Section 6.1 I see:
   The use of address 1.4.0.1 as an example address. See point 1 above.

   Further I see:
     In mplsXCTable:
     {
        mplsXCIndex = "2",
        mplsInSegmentIndex = NULL,
        mplsOutSegmentIndex = "3",
        mplsXCLabelStackIndex = 0
     }
     Note that the mplsInSegmentIfIndex value used to index this entry is
     the NULL string as required for an originating LSP [LSRMIB].   
   The "Note that... talks about "mplsInSegmentIfIndex" while the real
   name is "mplsInSegmentIndex" (with the "If").
   Also, "NULL" and "NULL string" may be confusing cause according to
   the MPLS-LSR-STD_MIB, there needs to be one octet with a valu of
   hexadecimal 00. 

   I also see  use of addresses 1.3.0.0 and 1.5.0.0, see point 1 above.

4. In section 6.2 I see

     {
        mplsFTNIndex = 1,
        mplsFTNDescr = "Rule #1 for destination address 1.4.0.1",
        -- source address only
        mplsFTNMask = 0x80,
        mplsFTNAddrType = ipv4,
        mplsFTNDestAddrMin = 1.4.0.1,
        mplsFTNDestAddrMax = 1.4.0.1,
        mplsFTNActionType = redirectLsp(1),
        mplsFTNActionPointer = mplsXCLspId.1.2.0.1.3
     }

   So the Descr field says it is a destination address, while the
   Mask says that it is a source address (as does the comment line)
   Yet the DestAddrMin and DestAddrMax fields are set ???
   Section 6.1 also mentioned this as a source address by the way.
   That seems to be all pretty incosistent here!!

5. In section 6.3 on page 10 I see:
     {
        mplsFTNIndex = 3,
        mplsFTNDescr = "Rule #3 for destination prefix 1.4.0.0/16",
        -- destination address only
        mplsFTNMask = 0x40,
        mplsFTNAddrType = ipv4,
        -- address range equivalent to CIDR prefix 1.4.0.0/16
        mplsFTNDestAddrMin = 1.4.0.0,
        mplsFTNDestAddrMax = 1.4.255.255,
        mplsFTNActionType = redirectTunnel,
        mplsFTNActionPointer = mplsTunnelName.3.0.3.3.3.3.4.4.4.4
     }
   Now, I understand that you may want to express LSRIDs as
   3.3.3.3 and 4.4.4.4 (although there is no DISPLAY-HINT
   in the textual convention MplsExtendedTunnelId, which is the
   syntax for the index objects in the mplsTunnelTable, so the
   default is to display as an unsigned number I think), 
   but the underlying type is a Unsigned32,
   and so when you represent them as a rowpointer as follows:
        mplsFTNActionPointer = mplsTunnelName.3.0.3.3.3.3.4.4.4.4
   then that is completely bogus, cause I think that the proper
   pointer (assuming the 3.3.3.3 (0x03030303), and 4.4.4.4 (0x04040404)
   as mplsTunnelIngressLSRId and mplsTunnelEgressLSRId, and assuming
   3 and 0 for mplsTunnelIndex and mplsTunnelInstance) would be:
        mplsFTNActionPointer = mplsTunnelName.3.0.50529027.67372036
   I understand tha this makes reading the example not easier, but 
   the way you present it, I can see people get confused when they
   compare it with actual implementations (unless such implementations
   are broken and compose pointers according to your example).
   
6. In sect 6.4 I also think there are confusing things.

   For example, mplsFTNMapEntry.1.0.1 is confusing, because that OID
   will (or should) never exist. In fact a valid OID would be,
   mplsFTNMapIndex.1.0.1, although this one is not-accessible. One
   that is accessible would be mplsFTNMapRowStatus.1.0.1 
   Now I understand what you are trying to express here... but I cannot
   say that it is helpfull for people who try to understand the
   exact OIDs and indexing.
   Maybe better would be a notation aka:
            |
            |  mplsFTNMapTable:
            |   mplsFTNMapEntry (1.0.1): <--------------------+
            +<---(mplsFTNMapIndex = 1,                        |
            |     mplsFTNMapPrevIndex = 0, ---> (NULL)        |
            |     mplsFTNMapCurrIndex = 1) ------------+      |
            |                                          |      |
   By the way, the same is true for IfEntry.1 (it does not exist,
   it would be IfIndex.1 = 1.

7. Note that all of this may become moot after you (we) find
   a proper solution for the linked-list issue that I have 
   described as a SERIOUS  problem in my other email (attached
   at the bottom).

8. If you specify a DEFVAL of {ipv4} for the mplsFTNAddrType,
   then it seems to me that you MUST also specify a DEFVAL with
   an IPv4 Address for all the mplsFTNXxxxAddrXxx objects.
   Maybe a defval of { '00000000'h } -- 0.0.0.0
   If you do not do that, then potentially people could create
   rows that have AddrType and Address not consistent.

9. I wonder if good DEFVAL values for xxxMinPort and xxxMaxPort
   would be 0 and 65535 respectively ??

10. I wonder if a good DEFVAL for mplsFTNActionPointer would be 0.0

11. I see in the DESCRIPTION clause of mplsFTNActionPointer
      DESCRIPTION
       "If mplsFTNActionType is redirectLsp(2), then this
        object MUST contain zeroDotZero or point to a instance
        of mplsXCEntry indicating the LSP to redirect matching
        packets to.

        If mplsFTNActionType is redirectTunnel(3), then this
        object MUST contain zeroDotZero or point to a instance
        of mplsTunnelEntry indicating the MPLS TE tunnel to
        redirect matching packets to.
    But the mplsFTNActionType enumerates the two types as
       SYNTAX    INTEGER {
               redirectLsp(1),   -- redirect into LSP
               redirectTunnel(2) -- redirect into tunnel
    So that seems inconsistent

12. I wonder why 
       mplsFTNProtocol OBJECT-TYPE
         SYNTAX             Integer32 (0..65535)
    has such a large range, The protocol field is only 1 octet
    long, and in all other places I have seen it, it has:
         SYNTAX             Integer32 (0..255)

13. In the various DESCRIPTION clauses in mplsFTNMaptable I
    see you talk about "FNT entry". Probably it would be better
    to speak of "mplsFTNEntry"

That is it. Oh well...

Thanks,
Bert 

> -----Original Message-----
> From: Wijnen, Bert (Bert) 
> Sent: woensdag 30 juli 2003 19:54
> To: 'Joan Cucchiara x302'; [email-masked]'
> Cc: [email-masked]'; [email-masked]'; [email-masked]';
> [email-masked]'; [email-masked]'; [email-masked]';
> [email-masked]'; [email-masked]';
> [email-masked]'; [email-masked]';
> [email-masked]'; [email-masked]'; 
> [email-masked]';
> [email-masked]'; [email-masked]'
> Subject: MIB Doctor review of: draft-ietf-mpls-ftn-mib-08.txt
> 
> 
> I am not done with review of this doc yet.
> But I see one SERIOUS issue that I rather point out
> now than later.
> 
> Back with revision 6 or so I wrote:
> 
> > > The mplsFTNMapEntry DESCRIPTION clause says:
> > > 
> > >         entry. This linked-list indexing style structure allows
> > >         FTN entries to be inserted at arbitrary positions in
> > >         the list. 
> > > Maybe it is me, but I still fail to see how this can be true.
> > > Can you explain that for me?
> > 
> And Cheenu (I believe) answered:
> 
> > I have included all relevant explanation about this table's indexing
> > (essentially material from section 5.2 but making the MIB 
> module self
> > contained) in the DESCRIPTION clause of mplsFTNMapTable. Please take
> > a look at that and tell me if this is clear now.
> > 
> 
> Well, I look at the new MIB and the new descriptions in section 5.
> And all I can conclude is that according to the SNMP protocol
> this is completely unacceptable.
> If I understand it correctly, then to insert a row, you issue
> (in your example in sect 6.3) a SNMP SET command as follows:
> (I know this is not the exact format, but you get the idea)
> 
>    SNMP SET ( mplsFTNMapIndex = 1,
>               mplsFTNPrevIndex = 1,
>               mplsFTNMapCurrIndex = 3
>               -- it also has a rowStatus and a StorageType
>              ) -- so you create mplsFTNMapEntry  1.1.3
> 
> Before you do so, the table had these entries (the number are the
> indices for the entries)
> 
>    1.0.1
>    1.1.2
> 
> The normal SNMP SET result is these 3 entries:
> 
>    1.0.1
>    1.1.2
>    1.1.3
> 
> But you seem to describe that the side effects of you creating row
> 1.1.3, the other entry 1.1.2 gets changed, and that the resulting
> table will have these entries:
> 
>   1.0.1
>   1.1.3
>   1.3.2
> 
> That to me seems a TOTALLY UNACCEPTABLE behaviour for the SNMP
> SET operation that was received.
> 
> Similar, once you have that set of 3 tables entries that you describe,
> you suggest that a SNMP SET to destroy entry 1.1.3
> 
> The normal SNMP SET result would be a table of these 2 entries,
> 
>   1.0.1
>   1.3.2
> 
> but you describe that the SNMP agent should have some side effect
> which results in a table with these entries:
> 
>    1.0.1
>    1.1.2
> 
> First, did I understand the descriptions correctly?
> If not pls tell me where I am wrong.
> 
> If I did understand it correctly, how do you think that a normal
> SNMP agent or manager can deal with the side-effects you are
> describing. It seems totally against the normal behaviour of
> SNMP SET operation and SNMP agent and manager behaviour.
> 
> Once you tell me that my reading of section 5.2 and the examples
> in section 6 is correct, I will check with the rest of the MIB
> doctors, but I suspect that they will unanimously agree with me.
> 
> Thanks,
> Bert 
>