Subject: Re: [IPFIX] AD review of draft-ietf-ipfix-mib-05.txt

From: "Thomas Dietz" <[email-masked]>
Date: Mon, 9 Mar 2009 12:25:36 +0100

Dear Dan, all,

find my comments inline...

-- 
Thomas Dietz                 E-mail: [email-masked]
NEC Europe Ltd.              Phone:  +49 6221 4342-128
NEC Laboratories Europe      Fax:    +49 6221 4342-155
Network Research Division
Kurfuersten-Anlage 36
69115 Heidelberg, Germany    http://www.nw.neclab.eu

NEC Europe Limited           Registered in England 2832014
Registered Office: NEC House, 1 Victoria Road, London W3 6BL

> -----Original Message-----
> From: [email-masked] [mailto:[email-masked]] On Behalf
> Of Romascanu, Dan (Dan)
> Sent: Dienstag, 17. Februar 2009 17:27
> To: IETF IPFIX Working Group
> Cc: MIB Doctors (E-mail)
> Subject: [IPFIX] AD review of draft-ietf-ipfix-mib-05.txt
> 
> Please find below the AD (and MIB doctor) review of
> draft-ietf-ipfix-mib-05.txt. This I-D is not ready yet for IETF Last
> Call. Please address the issues below and issue a new I-D.
> 
> Part of the comments were contributed by Bert Wijnen.
> 
> I have divided the comments into Technical and Editorial.
> 
> Thanks and Regards,
> 
> Dan
> 
> 
> T1. INET-ADDRESS-MIB is IMPORT-ed from RFC 4001and not from RFC 3291

Fixed.

> 
> T2. There is no real justification for defining a new TC for
> IpfixFunctionAvailability. As per RFC 4181, use TruthValue instead.
> 

You are right. Fixed.

> T3. The top level structure of the MIB does not follow the
> recommendation in Appendix D of RFC 4181 concerning the OID layout:
> 
>   xxxMIB
>          |
>          +-- xxxNotifications(0)
>          +-- xxxObjects(1)
>          +-- xxxConformance(2)
>              |
>              +-- xxxCompliances(1)
>              +-- xxxGroups(2)
> 

Fixed as well.

> T4. The object iffixTransportSessionProtocol uses only 3 out of the 255
> possible values in the range (6, 17, 132). A better SYNTAX would be
> just
> INTEGER and then enumerate the three values in the MODULE-COMPLIANCE
> clause. This would allow future changes by just another
> MODULE-COMPLIANCE if there is a need to add a new protocol. If you
> believe that another transport will be never added the appropriate
> SYNTAX would be INTEGER (6|17|132). Also provide
> http://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml
> as REFERENCE.
> 

I have put that into the module compliance section.

> T5. There is only one InetAddressType object for both source and
> destination addresses. Does this mean that an IPFIX session is only
> IPv4
> to IPv4 or IPv6 to IPv6? This is a question, I apologize if it is a
> layman one. In any case, it would be good to write an explanation, as
> users of the RFC 4001 addresses TCs are used to always encounter pairs
> of type-address objects.

Just a mistake by myself. I added a object for source and destination
address.

> 
> T6. Why are the address type and addresses objects only valid for UDP
> and TCP, and not for SCTP? Are not STCP session also conducted between
> IP hosts that can be described by IP addresses?
> 

Because we use the SCTP Association Id defined in the SCTP MIB for that.
Thus the SCTP MIB holds IPs and ports for the association Id. Clarified that
in the description.

> T7. What does 'this object is only valid' mean for the address type and
> addresses objects? What is returned by the agent in this case on a Get
> operation on these objects?
> 

Also made this clear.

> T8. The InetPort TC IMPORT-ed from RFC 4001 looks like a more
> appropriate SYNTAX for the ipfixTransportSessionSourcePort and
> ipfixTransportSessionDestinationPort objects
> 

You are right again. Fixed.

> T9. I cannot figure out what is the role of all the DEFVAL clauses in
> this MIB module. All the MIB is read-only and there is no dynamic row
> creation. Why are they needed?
> 

You are right again. Since we first discussed if the MIB should be writable
or not this is a remainder of these times.

> T10. Many of the objects in the MIB modules - especially counters and
> gauges - would benefit from defining UNITS clauses. In some places they
> are mentioned in the DESCRIPTION clauses, but using explicit UNITS
> clauses wherever possible is better.
> 

Added where I thought it is applicable.

> T11. Some object names in table do not respect the naming conventions
> that recommends that table names and objects have an identical suffix -
> ipfixObservationDomainId,  ipfixObservationPointGroupReference,
> ipfixPhysicalEntity, ipfixPhysicalEntityDirection
> 

Fixed.

> T12. Some objects are completely missing REFERENCE clauses - especially
> in the first tables o the MIB. Some other clauses could benefit from
> more exact referencing, saying just [RFC5101] without mentioning the
> exact section is not very useful.
> 

Added where applicable. Improved if possible.

> T13. Why is the range of ipfixTemplateDefinitionLength 1..2147483547)?
> According to RFC 5101, section 7, page 31 it looks like it should be
> (0..65535)
> 

Fixed.

> T14. Give http://www.iana.org/assignments/enterprise-numbers/ as
> REFERENCE for ipfixTemplateDefinitionEnterprise
> 

Done.

> T15. Use hex notations for describing values in the DESCRIPTION clause
> of ipfixTemplateDefintionFlags - the decimal values do not correspond
> to
> the bits positions
> 

Done.

> T16. ipfixExportEntry leads to a warning in the smicng compilation
> 
> W: f(ipfix.mi2), (676,12) Row "ipfixExportEntry" does not have a
> consistent indexing scheme - index items from current table must come
> after index items from other tables
> 

Since I have only libsmi (which does not catch this) I did not realize it.
Fixed.

> T17. A couple of enumerated objects start to value 0 which is not
> recommended unless special cases. Are there special cases here for
> ipfixExportMemberType and ipfixPhysicalEntityDirection
> 

Well, those cases indicate an unknown value which I think is a special case
and the same as in the TC InetAddressType.

> T18. Unsigned32 seems to be a better SYNTAX than Integer32 (see RFC
> 4181) for the following objects: ipfixMeteringProcessId,
> ipfixObservationPointGroupReference, ipfixObservationPointIndex,
> ipfixTransportSessionRate
> 

Changed all types to Unsigned32 where possible.

> T19. I cannot understand what useful information carries
> ipfixmeteringProcessId - it is not an index and its value is
> implementation dependent.

We wanted to give a kind of ID to make it possible to identify the process
that creates the Flow Records. This may be a process id or something
similar. Maybe we should specify it as process id. This needs further
discussion.

> 
> T20. Does it make sense for the ipfixSelectorFunction to point to a
> function that is not available?
> 

Basically it doesn't. But we wanted to have the possibility to have a list
of Selector Functions that are basically available (and thus present in the
MIB tree) but may be disabled somehow.

> T21. I assume that selection functions will be added in the future. If
> I
> am wrong please delete all mention of extensibility and take out the
> ipfixSelectorFunctions group. If I am right, I suggest to put
> ipfixSelectorFunctions in a separate MIB module that will be IANA
> maintained, so that new functions can be added in the future without
> the
> need to revise the MIB module and the RFC. The separate MIB module will
> become the initial version of the IANA maintained module, and new
> selector functions will be added using for example Expert Review
> policy.
> 
> 

I personally think this makes sense to put it in a separate MIB module which
is maintained by IANA but this needs further discussion on the list which I
will start soon.

> T22. it looks like Gauge32 would be a better SYNTAX for the following
> objects: ipfixTransportSessionRate,
> ipfixMeteringProcessCacheActiveFlows, ipfixMetering
> ProcessCacheInactiveFlows
> 

Changed whereever applicable.

> T23. How is ipfixTransportSessionRate computed? Every second? Every T
> seconds? T=?
> 

I fixed this to one second intervals. Benoit, Atsushi any objections??

> T24. I am wondering whether more counters in this MIB module need to be
> Counter64 rather than Counter32 - especially the bytes and packets
> counters
> 

Made all Counters Counter64.

> T25. There is no definition of the discontinuity behavior of all the
> counter objects or definition of discontinuity objects.
> 
> T26. It looks like Counter32 (or Counter64) with the appropriate
> discontinuity objects are better SYNTAX for
> ipfixSelectorStatsPacketsObserved and ipfixSelectorStatspacketsDropped.
> 

Added discontinuity objects and fixed the two issues above.

> T27. Some of the DESCRIPTION clauses of the OBJECT-GROUPs contain
> statements about the mandatory or optional characteristics of the
> objects. This is inappropriate, such statements must be made in
> MODULE-COMPLIANCE definitions, not in OBJECT-GROUP.
> 

Fixed.

> T.28. Security Consideration sections - The description of the
> vulnerability of objects in the tables should be more precise than
> 'contains configuration data that might be sensitive'.
> 

Changed.

I also fixed the text issues below. Dan, please review version 06 if all
your issues are fixed except those which I marked under discussion above.

Thanks,

Thomas

> 
> E1. Section 5.1 s/is defined in the MIB/is defined in the MIB module/
> 
> E2. The language in the sub-section of section 5 may be polished - for
> example in section 5.3 and 5.4 s/may want to export/is configured to
> export/
> 
> E3. Section 5.6 s/consists of a set of function/consists of a set of
> functions/
> 
> E4. Section 5.6 - 'the Metering Process table (ipfixMetering
> ProcessTable) is grouped by maintained by ...' - this phrase needs to
> be
> fixed.
> 
> E5. Section 6.1 - s/they should also implement the ENTITY MIB/they
> SHOULD also implement the ENTITY MIB/
> 
> E6. Section 6.1 s/all entries in the Observation Point Table contain an
> ipfixPhysicalEntity of zero(0)/all values of the ipfixPhysicalEntity
> columns in the ipfixObservationPointTable are 0 and the values of the
> ipfixPhysicalEntityDirection columns are none(0).
> 
> E7. The DESCRIPTION clause and the name of the object
> ipfixTransportSessionMode seem mis-leading. I think that this object
> describes the device role in a session, not the mode of the session in
> other words it's an attribute of the device that runs the agent and not
> of the session.
> 
> E8. The DESCRIPTION clause of the ipfixTemplateAccessTime object is
> confusing. The second and third paragraph seem to duplicate the content
> of the first paragraph. The last phrase of the clause seems to belong
> rather in the DESCRIPTION clause of the table than here.
> 
> E9. DESCRIPTION clause of ipfixObservationPointIndex - is this
> 'management system' or rather 'management agent'?
> 
> E10. DESCRIPTION clause of ipfixPhysicalEntity s/the object contains
> 0/the value of the object is 0/
> 
> E11. What does 'a direction is not applicable' mean in the DESCRIPTION
> clause of ipfixPhysicalEntityDirection?
> 
> 
> _______________________________________________
> IPFIX mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/ipfix

---

From: Atsushi Kobayashi <[email-masked]>
Date: Mon, 09 Mar 2009 21:38:09 +0900

Dear Thomas and all,

Thomas, thank you for your effort.

> is maintained by IANA but this needs further discussion on the list which I

I agree selection function MIB separated from main MIB module.
It would come more flexible.

> I fixed this to one second intervals. Benoit, Atsushi any objections??

No. I don't have strong opinion.

---

From: "Romascanu, Dan (Dan)" <[email-masked]>
Date: Wed, 18 Mar 2009 14:57:16 +0100

Thank you for addressing my comments. Most of the issues were
acknowledged and fixed accordingly. T19 and T21 still need discussion,
while T23 needs to be acknowledged by the other editors. 

My question to the WG chairs is whether we should send the document to
IETF Last Call, considering the open issues as Last Call issues, to be
answered and resolved together with other Last Call comments we receive.

Dan

> From: Thomas Dietz [mailto:[email-masked]] 
> Sent: Monday, March 09, 2009 1:26 PM
> To: Romascanu, Dan (Dan); IETF IPFIX Working Group
> Cc: MIB Doctors (E-mail)
> Subject: RE: [IPFIX] AD review of draft-ietf-ipfix-mib-05.txt
> 
> Dear Dan, all,
> 
> > From: [email-masked] 
> [mailto:[email-masked]] On Behalf 
> > Of Romascanu, Dan (Dan)
> > T4. The object iffixTransportSessionProtocol uses only 3 out of the 
> > 255 possible values in the range (6, 17, 132). A better 
> SYNTAX would 
> > be just INTEGER and then enumerate the three values in the 
> > MODULE-COMPLIANCE clause. This would allow future changes by just 
> > another MODULE-COMPLIANCE if there is a need to add a new 
> protocol. If 
> > you believe that another transport will be never added the 
> appropriate 
> Just a mistake by myself. I added a object for source and 
> destination address.
> 
> 
> > 
> Thus the SCTP MIB holds IPs and ports for the association Id. 
> Clarified that in the description.
> 
> > T7. What does 'this object is only valid' mean for the address type 
> > and addresses objects? What is returned by the agent in 
> this case on a 
> > Get operation on these objects?
> > 
> 
> Also made this clear.
> 
> You are right again. Since we first discussed if the MIB 
> should be writable or not this is a remainder of these times.
> 
> > gauges - would benefit from defining UNITS clauses. In some places 
> > they are mentioned in the DESCRIPTION clauses, but using explicit 
> > UNITS clauses wherever possible is better.
> > 
> 
> > that recommends that table names and objects have an 
> identical suffix 
> > - ipfixObservationDomainId,  ipfixObservationPointGroupReference,
> > T12. Some objects are completely missing REFERENCE clauses - 
> > especially in the first tables o the MIB. Some other clauses could 
> > benefit from more exact referencing, saying just [RFC5101] without 
> > mentioning the exact section is not very useful.
> > 
> 
> > to the bits positions
> Well, those cases indicate an unknown value which I think is 
> a special case and the same as in the TC InetAddressType.
> 
> We wanted to give a kind of ID to make it possible to 
> identify the process that creates the Flow Records. This may 
> be a process id or something similar. Maybe we should specify 
> it as process id. This needs further discussion.
> 
> > 
> Basically it doesn't. But we wanted to have the possibility 
> to have a list of Selector Functions that are basically 
> available (and thus present in the MIB tree) but may be 
> disabled somehow.
> 
> > I am wrong please delete all mention of extensibility and 
> take out the 
> > the need to revise the MIB module and the RFC. The separate 
> MIB module 
> > will become the initial version of the IANA maintained 
> module, and new 
> I personally think this makes sense to put it in a separate 
> MIB module which is maintained by IANA but this needs further 
> discussion on the list which I will start soon.
> 
> > T24. I am wondering whether more counters in this MIB 
> module need to 
> > be
> I also fixed the text issues below. Dan, please review 
> version 06 if all your issues are fixed except those which I 
> marked under discussion above.
> 
> Thanks,
> 
> Thomas
> 
> > 
> > be fixed.
> > E6. Section 6.1 s/all entries in the Observation Point 
> Table contain 
> > an ipfixPhysicalEntity of zero(0)/all values of the 
> > ipfixPhysicalEntity columns in the ipfixObservationPointTable are 0 
> > and the values of the ipfixPhysicalEntityDirection columns 
> are none(0).
> > 
> > other words it's an attribute of the device that runs the agent and 
> > not of the session.
> > 
> > confusing. The second and third paragraph seem to duplicate the 
> > content of the first paragraph. The last phrase of the 
> clause seems to 
> > belong rather in the DESCRIPTION clause of the table than here.
> >

---

From: "Juergen Quittek" <[email-masked]>
Date: Fri, 20 Mar 2009 02:12:06 +0100

Dear Dan,

I would support sending the document to IETF last call.
The open issues are not critical for this review and we
would speed up the process of completing this document.

Thanks,

    Juergen

---

From: Benoit Claise <[email-masked]>
Date: Sat, 21 Mar 2009 16:28:04 -0700

Thomas,
> No. I don't have strong opinion.
Every second is fine with me.

Regards, Benoit.