Re: [ipfix] AD review for: draft-ietf-ipfix-info-11.txt
Juergen Quittek <quittek <at> netlab.nec.de>
2006-05-07 22:31:13 GMT
Benoit,
Thank you for solving these issues. Please find
text suggestions inline.
--On 27.04.2006 23:44 Uhr +0200 Benoit Claise wrote:
> Hi Juergen,
>> Hi Bert,
>>
>> Many thanks for the detailed comments.
>> Please find replies inline.
>>
>> Dear all,
>>
>> Please check my replies and please speak up quickly
>> if you have problems with what I suggested.
>>
>> Most issues should be closed by the replies.
>>
>> The following ones definitely need further work.
>> I will address them in a follow-up message:
>>
>> - INFO-AD#1: Do reported statistics include or exclude the
>> reporting IPFIX message? Affected IEs are
>> # 40 exportedOctetTotalCount
>> # 41 exportedMessageTotalCount
>> # 42 exportedFlowTotalCount
> All of the counters exclude the reporting IPFIX Message
Suggestion:
append to description of #40 exportedOctetTotalCount
"The reported number excludes octets in the message that
carries the counter value."
append to description of #41 exportedMessageTotalCount
"The reported number excludes the message that carries the
counter value."
append to description of #40 exportedFlowTotalCount
"The reported number excludes flow records in the message
that carries the counter value."
>>
>> - INFO-AD#2: Does IE #42 exportedFlowTotalCount
>> report the number of flows or the number of flow records?
>>
>> - INFO-AD#3: Is it OK to change dataty of IEs
>> # 16 bgpSourceAsNumber
>> # 17 bgpDestinationAsNumber
>> from unsiogned16 to unsigned32?
> RFC 3954 specifies already 32 bits
>
> Source BGP autonomous
> SRC_AS 16 N system number where N could
> be 2 or 4. By default N is
> 2
>
> Destination BGP autonomous
> DST_AS 17 N system number where N could
> be 2 or 4. By default N is
> 2
>
> Anyway, we've got the reduced size encoding, so unsigned32 makes sense.
Good.
>>
>> - INFO-AD#4: How to add new label types to the definition of
>> IE #46 mplsTopLabelType?
> The references from RFC 3954 have been assigned by the NetFlow development team
>
> MPLS_TOP_LABEL_TYPE 46 1 MPLS Top Label Type:
> 0x00 UNKNOWN
> 0x01 TE-MIDPT
> 0x02 ATOM
> 0x03 VPN
> 0x04 BGP
> 0x05 LDP
>
> I could not find any IANA registry for that.
> So I guess we have only one choice, i.e. assign a new registry for it in IANA.
Would this be worth the effort?
Thanks,
Juergen
> Regards, Benoit.
>>
>> - INFO-AD#5: Explain in the capabilities and limitations of the
>> different dateTimeXX data types.
>>
>> - INFO-AD#6:
>> Review last paragraph of section 7.
>>
>> - INFO-AD#7:
>> Elaborate security considerations
>>
>> Thanks,
>>
>> Juergen
>>
>>
>>> - sect 5.5.x
>>> Would it be useful to add a line of text to explain how long
>>> (how many minutes, months, years) each granularity allows
>>> based on the underlying datatype?
>>
>> I will work on a text suggestion.
>>
>>
>> --On 17.03.2006 17:42 Uhr +0100 Wijnen, Bert (Bert) wrote:
>>
>>> Sorry that it took so long (if I say it 4 times, i.e.
>>> for each doc I have reviewed, will you please forgive me).
>>>
>>> Seems that a new rev might be in order ??
>>>
>>> Bert
>>>
>>> - bottom of page 8. enterpriseId -
>>> it speaks about "Information Element Identifier described above"
>>> I do not see where "above" it is described. In fact, I think the
>>> Identifier is described in sect 4, no?
>>
>> Oooops, the Element Id is a mandatory element and not listed above.
>> This was a problem of the code generating this section from the schema.
>> Now, the list of mandatory IE properties contains in addition between
>> 'name' and 'description' the following property:
>>
>> "elementId - A numeric identifier of the Information Element. If this
>> identifier is used without an enterprise identifier (see below),
>> then it is globally unique and the list of allowed values is
>> administered by IANA. It is used for compact identification of an
>> Information Element when encoding templates in the protocol."
>>
>>> - In the 3 paragraphs of sect 3 and 3.1, I would insert the word
>>> "abstract"
>>> in from of each occurrence of "data type(s)".
>>> Also the title of sect 3.1 probably reads better as "Abstract Data
>>> Types".
>>
>> done.
>>
>>> - sect 3.1.9
>>> "it is expected that strings will be encoded in UTF-8"
>>> That does not make it interoperable, does it?
>>> Would it not be better to say "strings MUST be encoded..."
>>> And I would add a citation and reference to RFC3629.
>>
>> done.
>> added RFC3629 as normative reference.
>>
>>> - sect 3.2
>>> s/future protocol extensions/future information model extensions/ ??
>>
>> fixed, and also in section 3.1.
>>
>>> - page 16
>>> What are ID 211 and 212 ?? blank ?? reserved?? something else?
>>> I see the explanation on page 17. I'd suggest to make them RESERVED
>>> or OBSOLLETE or DEPRECATEd or give them some name with the note
>>> that they are not part of the standard.
>>
>> I used RESERVED. Maybe we can solve the issues of these two elements
>> before submitting the next version.
>>
>>> - section 5 1st sentence
>>> s/Flow attributes/Information Elements/ ??
>>
>> fixed.
>>
>>> - sections 5.1.3, 5.1.4., 5.1.5 and 5.1.6
>>> I worry about referential integrety when the ifIndex gets stored
>>> in offline/archive storage. On a reboot, many devices renumber the
>>> ifIndex for various interfaces.
>>> Is this not a problem? I'd think it is at least something to
>>> mention/discuss/warn for.
>>
>> added
>>
>> "Please note that ifIndex
>> values are not assigned statically to an interface.
>> Interfaces may be renumbered every time the device is
>> rebooted."
>>
>> to the description of 5.1.3 and 5.1.4.
>>
>> added
>>
>> "Please note that
>> process identifiers are typically assigned dynamically.
>> After a reboot, a system failure, a crash of the Metering
>> Process, etc. the Metering Process may be re-started
>> with a different ID.
>>
>> to the description of 5.1.5 and 5.1.6. For 5.1.6 "Metering"
>> was replaced with "Exporting".
>>
>>> - sect 5.2.3
>>> Is the message that contains this Information Element included in the
>>> count?? May want to make that clear for better interoperability.
>>> Same for some of the other exportXXXCounters
>>
>> INFO-AD#1:
>> These IEs are specified to be compatible with NetFlow v9.
>> I'm checking with my co-authors from Cisco which alternative
>> was chosen for NF v9.
>>
>> When I have this information, I will remove the ambiguity
>> from the descriptions of IEs
>> # 40 exportedOctetTotalCount
>> # 41 exportedMessageTotalCount
>> # 42 exportedFlowTotalCount
>>
>>> - sect 5.2.5
>>> Text says: number of flow records
>>> But in Units it says: flows
>>> So what is it?
>>> Same for sect 5.2.9
>>
>> INFO-AD#2:
>> This is another NF v9 compatible IE. As above, I will check
>> and make sure that description and units are consistent.
>>
>>> - sect 5.2.8
>>> Does the count include header octets?
>>
>> added clarification:
>>
>> OLD
>> Description:
>> The total number of octets in observed IP packets that the
>> Metering Process did not process since the (re-)initialization of
>> the Metering Process.
>> NEW
>> Description:
>> The total number of octets in observed IP packets (including the
>> IP header) that the Metering Process did not process since the
>> (re-)initialization of the Metering Process.
>>
>>> - sect 5.2.12
>>> It is not clear/sopecific as to which bit is bit zero, bit one etc.
>>
>> I am not sure what is the problem here, but I tried to clarify
>> the description:
>>
>> OLD
>> Description:
>> This set of bit fields is used for marking the Information
>> Elements of a Data Record that serve as Flow Key. Each bit
>> represents an Information Element in the Data Record with the n-th
>> bit representing the n-th Information Element. A set bit with
>> value 1 indicates that the corresponding Information element is a
>> Flow Key of the reported Flow. A value of 0 indicates that this
>> is not the case. ...
>> NEW
>> Description:
>> This set of bit fields is used for marking the Information
>> Elements of a Data Record that serve as Flow Key. Each bit
>> represents an Information Element in the Data Record with the n-th
>> bit representing the n-th Information Element. A bit set to value
>> 1 indicates that the corresponding Information element is a Flow
>> Key of the reported Flow. A bit set to value 0 indicates that
>> this is not the case.
>>
>>> - Sect 5.2.3 and 5.2.5
>>> Is it best to speak about Mask?
>>> Or would speaking of (and naming it) PrefixLength be better?
>>
>> renamed sourceIPv4Mask to sourceIPv4PrefixLength
>> and sourceIPv6Mask to sourceIPv6PrefixLength
>>
>>> - Sect 5.6.3
>>> I worry about the fact that there is discussion already about AS
>>> numbers
>>> of 32-bit length. So are we future proof here?
>>> In the MIB/SMI world we have made it a 32bit unsigned, see
>>> InetAutonomousSystemNumber in RFC4001.
>>
>> INFO-AD#3:
>> change data type of
>> # 16 bgpSourceAsNumber
>> # 17 bgpDestinationAsNumber
>> # 128 bgpNextAdjacentAsNumber
>> # 129 bgpPrevAdjacentAsNumber
>> from unsigned16 to unsigned32.
>>
>>> - sect 5.6.9
>>> How are new values of Labeltypes be added in the future?
>>> last line in this section, remove "and IP addresses" ??
>>
>> INFO-AD#4:
>> I have no good answer on this comment. Does anyone else have?
>>
>>> - sect 5.8
>>> Just for my understanding, why do you need all 4 levels
>>> of granularity here (i.e. sec, milisec, microsec and nanosec)??
>>
>> For efficient reporting from probes with different precisions and for
>> applications with different precision requirements.
>>
>>> - sect 5.5.x
>>> Would it be useful to add a line of text to explain how long
>>> (how many minutes, months, years) each granularity allows
>>> based on the underlying datatype?
>>
>> INFO-AD#5:
>> I will work on a text suggestion. We avoid duplication if we put
>> this text to the data type descriptions in section 3.1.
>>
>>> - sect 5.9.11 to 5.9.14
>>> I wondered if it makes sense to say some thing more about
>>> "packet treatment". Like what sort of treatment? Any
>>> reference to an RFC?
>>
>> We discussed this issue several times in the past, but did not
>> find a good solution for it.
>>
>>> - sect 5.10.2 speaks about flowInactiveTimeout while sect 5.10.3
>>> speaks about idle timeout. Should you have flowIdleTimeout
>>> instead of flowInactievTimeout for consistency?
>>
>> renamed flowInactiveTimeout to flowIdleTimeout.
>>
>>> - sect 5.10.11
>>> Make it clear that the value is hex 00 (0 could be read
>>> as decimal zero)
>>
>> OLD
>> 5.11.1. paddingOctets
>>
>> Description:
>> The value of this Information Element is always 0.
>> NEW
>> 5.11.1. paddingOctets
>>
>> Description:
>> The value of this Information Element is always a sequence of 0x00
>> values.
>>
>>> - sect 6, page 69
>>> I think I would make it MUST instead of SHOULD in 2nd
>>> and 3rd para.
>>
>> These paragraphs are
>>
>> "Names of new Information Elements SHOULD be chosen according to the
>> naming conventions given in section 2.3.
>>
>> For extensions, the type space defined in section 3 can be used. If
>> required, new data types can be added. New data types SHOULD be
>> defined in IETF standards track documents."
>>
>> For IE naming I think that a SHOULD is OK, because fully consistent IE
>> naming appears to be very difficult. There might be situations where
>> deviations from a fixed scheme improve the readability and intuitive
>> understanding of IE semantics. For example we inconsistently use "IPv4"
>> instead of "IpV4" in IE names, because several people did not like
>> "IpV4".
>>
>> For new data types I agree. Changed SHOULD to MUST.
>>
>>> - sect 7
>>> first para, pls add a ptr to the list of Information
>>> Element Identifiers that need to be recorded as the
>>> initially assigned values for this registry.
>>> I guess you need to point them to sect 4.
>>> I doubt it is clear to IANA though what exactly to record
>>> from that section. You can check directly with iana to
>>> ask if things are clear or not and if not to work out
>>> text with them that they understand.
>>
>> I tried to address this comment:
>> OLD
>> 7. IANA Considerations
>>
>> This documents defines an initial set of IPFIX Information Elements.
>> For extending them in the future, IANA needs to create a new registry
>> for IPFIX Information Element identifiers.
>>
>> New assignments for IPFIX Information Elements will be administered
>> by IANA, on a First Come First Served basis [RFC2434], subject to
>> Expert Review [RFC2434], i.e. review by one of a group of experts
>> designated by an IETF Operations and Management Area Director. The
>> group of experts must double check the Information Elements
>> definitions with already defined Information Elements for
>> completeness, accuracy, redundancy, and correct naming following the
>> naming conventions in section 2.3. Those experts will initially be
>> drawn from the Working Group Chairs and document editors of the IPFIX
>> and PSAMP Working Groups.
>>
>> Appendix B defines an XML schema which may be used to create
>> consistent machine readable extensions to the IPFIX information
>> model. This schema introduces a new namespace, which will be
>> assigned by IANA according to RFC 3688. Currently the name space for
>> this schema is identified as http://www.ietf.org/ipfix.
>> NEW
>> 7. IANA Considerations
>>
>> This document specifies an initial set of IPFIX Information Elements.
>> The list of these Information Elements with their identifiers is
>> given in section 4. IANA needs to create a new registry for IPFIX
>> Information Element identifiers and fill it with the initial list in
>> section 4.
>>
>> New assignments for IPFIX Information Elements will be administered
>> by IANA, on a First Come First Served basis [RFC2434], subject to
>> Expert Review [RFC2434], i.e. review by one of a group of experts
>> designated by an IETF Operations and Management Area Director. The
>> group of experts must double check the Information Elements
>> definitions with already defined Information Elements for
>> completeness, accuracy, redundancy, and correct naming following the
>> naming conventions in section 2.3. The specification of new IPFIX
>> Information Elements MUST use the template specified in section 2.1
>> and MUST be published using a well established and persistent
>> publication medium. The experts will initially be drawn from the
>> Working Group Chairs and document editors of the IPFIX and PSAMP
>> Working Groups.
>>
>> Appendix B defines an XML schema which may be used to create
>> consistent machine readable extensions to the IPFIX information
>>
>>> I wonder if it would not be much better to have new
>>> Information Elements require a Standards Track action.
>>> I think that ensure much better review and evaluation
>>> and certainly ensure well-documented registrations.
>>
>> We discussed the issue several times and concluded that this
>> procedure would be too time consuming and too slow.
>>
>>> In any event, if you do do FCFS with expert review,
>>> then I would
>>> - require proper documentation to be publicly available
>>> - maybe setup some sort of template that MUST be filled
>>> out and approved to request registration.
>>
>> I added these two requirements to the new version of section 7
>> that you find above.
>>
>>> You speak about an Appendix B as having a schema/example
>>> on how to extend. It is however (I think) the Schema
>>> for the currently assigned values.
>>
>> INFO-AD#6:
>> Yes, but it should also be used for extensions. However, I
>> still need to investigate the XML schema issues that you posted
>> in a different email. I will come back to this issue, when
>> the XML schema issue is closed.
>>
>>> - Sect 8.
>>> I am pretty sure that the Security ADs will want to see
>>> more detail here. They probably want to understand which
>>> Information Elements contain sensitive data (and why it
>>> is sensitive) or privacy sensitive data (and why so).
>>> Probably similar to why they want to see that a MIB
>>> document lists the objects that are sensitive and
>>> why so and what the risks are if the data gets
>>> intercepted.
>>
>> INFO-AD#7:
>> Let's come back to this after the security AD review.
>>
>>> - Appendix A and B
>>> I am a bit confused when I read as title
>>> "Formal Specification..." and then in the first para
>>> I read that it is informational and not normative.
>>>
>>> If it is a "formal machine readable spec.." is it then not
>>> intended as input to tools, code-generators, data-structure
>>> generators and such? In that case I would be very worried
>>> if no this schema but the text earlier in the document
>>> is normative and authoritative.
>>
>> It is not a formal specification. But it is machine readable
>> and intended as input to tools. In one of the many passed
>> IPFIX sessions and on the mailing list we discussed the issue
>> and concluded that the text in section 5, which is generated
>> from the appendix with a tool would be normative, because this
>> is human readable.
>>
>>> Anyway, I asked an APPS AD if he could check the "formal"
>>> machine readable schema, but he does not recognize it as
>>> a schema that can be checked by any of his tools. So
>>> what is it? How can we (or anyone) check it for correctness?
>>
>> We got this review in a separate email.
>> Issues raised will be addressed in a reply to it.
>>
>>> administrative/bureaucracy/nits/spelling:
>>>
>>> - sect 5.10.4
>>>
>>> "bewtween in time" in first sentence ??
>>
>> fixed.
>>
>>> same in sect 5.10.5
>>
>> fixed.
>>
>>> - 3rd para in sect 1. I suspect that the ptrs to the various sections
>>> do not completely match with the actual content claimed to be
>>> in those sections. Specifically, the ptrs to 4 and 5 should probably
>>> be 5 and 6 ??
>>
>> fixed.
>>
>>> - I see that you use MUST (i.e. rfc2119 type) language and
>>> so there MUST be a normative citation/reference to RFC2119.
>>
>> added.
>>
>>> - problems with references/citations.
>>> Note that my tool may give false warnings, so just check
>>> them.
>>
>> In most of the cases below we cited "see RFC XXXX" in the
>> reference section. I replaced all these references with
>> "see RFC XXXX [RFCXXXX]".
>>
>>> !! Missing citation for Informative reference:
>>> P071 L006: [IEEE.802-11.1999]
>>>
>>> !! Missing citation for Informative reference:
>>> P071 L015: [IEEE.802-3.2002]
>>>
>>> !! Missing citation for Informative reference:
>>> P071 L023: [IEEE.P802-1Q.2003]
>>>
>>> !! Missing citation for Informative reference:
>>> P072 L026: [RFC2460] Deering, S. and R. Hinden, "Internet
>>> Protocol, Version 6
>>>
>>> !! Missing citation for Informative reference:
>>> P072 L029: [RFC2463] Conta, A. and S. Deering, "Internet
>>> Control Message
>>>
>>> !! Missing citation for Informative reference:
>>> P072 L033: [RFC2547] Rosen, E. and Y. Rekhter, "BGP/MPLS VPNs",
>>> RFC 2547,
>>>
>>> !! Missing citation for Informative reference:
>>> P072 L036: [RFC2629] Rose, M., "Writing I-Ds and RFCs using
>>> XML", RFC 2629,
>>>
>>> !! Missing citation for Informative reference:
>>> P072 L039: [RFC2863] McCloghrie, K. and F. Kastenholz, "The
>>> Interfaces Group
>>>
>>> !! Missing citation for Informative reference:
>>> P072 L042: [RFC2960] Stewart, R., Xie, Q., Morneault, K.,
>>> Sharp, C.,
>>>
>>> !! Missing citation for Informative reference:
>>> P072 L047: [RFC3031] Rosen, E., Viswanathan, A., and R. Callon,
>>> "Multiprotocol
>>>
>>> !! Missing citation for Informative reference:
>>> P072 L050: [RFC3032] Rosen, E., Tappan, D., Fedorkow, G.,
>>> Rekhter, Y.,
>>>
>>> !! Missing citation for Informative reference:
>>> P073 L006: [RFC3036] Andersson, L., Doolan, P., Feldman, N.,
>>> Fredette, A., and
>>>
>>> !! Missing citation for Informative reference:
>>> P073 L012: [RFC3260] Grossman, D., "New Terminology and
>>> Clarifications for
>>
>> The three references below are not cited. removed them.
>>
>>> !! Missing citation for Informative reference:
>>> P073 L015: [RFC3667] Bradner, S., "IETF Rights in
>>> Contributions", RFC 3667,
>>>
>>> !! Missing citation for Informative reference:
>>> P073 L018: [RFC3668] Bradner, S., "Intellectual Property Rights
>>> in IETF
>>>
>>> !! Missing citation for Informative reference:
>>> P073 L021: [RFC3917] Quittek, J., Zseby, T., Claise, B., and S.
>>> Zander,
>>>
>>> --
>>> Help mailto:majordomo <at> net.doit.wisc.edu and say "help" in
>>> message body
>>> Unsubscribe mailto:majordomo <at> net.doit.wisc.edu and say
>>> "unsubscribe ipfix" in message body
>>> Archive http://ipfix.doit.wisc.edu/archive/
>>>
>>
>>
>>
>> --
>> Help mailto:majordomo <at> net.doit.wisc.edu and say "help" in
>> message body
>> Unsubscribe mailto:majordomo <at> net.doit.wisc.edu and say
>> "unsubscribe ipfix" in message body
>> Archive http://ipfix.doit.wisc.edu/archive/
>
>
--
Help mailto:majordomo <at> net.doit.wisc.edu and say "help" in message body
Unsubscribe mailto:majordomo <at> net.doit.wisc.edu and say
"unsubscribe ipfix" in message body
Archive http://ipfix.doit.wisc.edu/archive/
RSS Feed