Re: [IPFIX] WGLC for draft-ietf-ipfix-text-adt-00.txt

Brian Trammell <[email protected]> Tue, 28 Jan 2014 17:27:16 +0100
Newsgroups	gmane.ietf.ipfix
Message-ID	<[email protected]>
hi Paul,

many thanks for the review! Will put these on the list for a post-wglc -01 revision; have snipped those I accept without comment from the following, with commentcomments inline...

On 28 Jan 2014, at 15:33, Paul Aitken <[email protected]> wrote:
>> 1.  Introduction
>> 
>>    The IPFIX Information Model, as defined by the IANA IPFIX Information
>>    Element Registry [iana-ipfix-assignments], provides a rich set of
>>    Information Elements for description of information about network
>>    entities and network traffic data, and abstract data types for these
>>    Information Elements.  The IPFIX Protocol Specification [RFC7011], in
> 
> Perhaps, though it's not limited just to networks ;-)

The protocol no, the information model is presently

> You're suggesting that the IANA registry is the reference for the ADTs, rather than 7012?
> 7012 isn't mentioned until section 4.

I am, and that’s incorrect. I’ll correct this:

The IPFIX Information Model [RFC 7012] provides a set of abstract data types for the IPFIX Information Element Registry [IANA-IPFIX], which in turn contains a rich set of...
>>    turn, defines a big-endian binary encoding for these abstract data
>>    types suitable for use with the IPFIX Protocol.
> 
> +xref for the protocol spec?

Hm? Already cited [RFC7011] in this sentence.

>>    However, present and future operations and management protocols and
>>    applications may use textual encodings, and generic framing and
>>    structure as in JSON or XML.  A definition of canonical textual
>>    encodings for the IPFIX abstract data types would allow this set of
>>    Information Elements to be used for such applications, and for these
>>    applications to interoperate with IPFIX applications at the
>>    Information Element definition level.
>> 
>>    Note that templating or other mechanisms for data description for
>>    such applications and protocols are application specific, and
>>    therefore out of scope for this document: only Information Element
>>    identification and data value representation are defined here.
>> 
>> 
>> 
>> 
>> Trammell                  Expires July 24, 2014                 [Page 2]
>> 
>> Internet-Draft              IPFIX Text Types                January 2014
>> 
>> 
>> 2.  Terminology
>> 
>>    Capitalized terms defined in the IPFIX Protocol Specification
>>    [RFC7011] and the IPFIX Information Model [RFC7012] are used in this
>>    document as defined in those documents.  In addition, this document
>>    defines the following terminology for its own use:
>> 
>>    Enclosing Context
>>       Textual representation of IPFIX data values is applied to use the
>>       IPFIX Information Model within some existing textual format (e.g.
>>       XML, JSON).  This outer format is referred to as the Enclosing
> 
> I can't parse that. Should it be, "A textual representation ...” ?

Yes it should. (I tried to write this such that the first sentence could be a definition rather than just exposition, but wasn’t able to come up with anything that was less awkward than this.)

<snip>

>> 
>>    Instead, applications using textual representations of Information
>>    Elements SHOULD use Information Element names to identify them; see
>>    Appendix A for examples illustrating this principle.
> 
> It's a SHOULD rather than a MUST. Would the IE identifier numbers be equally as valid?

Hm. Not equally valid, but not invalid (consider an application transcoding IPFIX to JSON that only had number information because some lazy exporter vendor didn’t stick 5610 data in the stream). And I didn’t want to MUST this here because I could also think of situations where a text representation handled Information Element values positionally. The point here is just to define the ADT representations in an interoperable way, not to cover every possible application therefor.

>> 4.  Data Type Encodings
>> 
>>    Each subsection of this section defines a textual encoding for the
>>    abstract data types defined in [RFC7012].  This section uses ABNF
>>    [RFC5234], including the Core Rules in Appendix B, to describe the
>>    format of textual representations of IPFIX abstract data types.
> 
> It's unclear whether that's Appendix B of this document or of 5234.

Of 5234, will fix.
>> 
>> 4.1.  octetArray
>> 
>>    If the Enclosing Context defines a representation for binary objects,
>>    that representation SHOULD be used.
>> 
>>    Otherwise, since the goal of textual representation of Information
>>    Elements is readability over compactness, the values of Information
> 
> This goal should be mentioned much earlier!

Indeed, will move to introduction.
>>    Elements of the octetArray data type are represented as a string of
>>    pairs of hexadecimal digits, one pair per byte, in the order the
>>    bytes would appear on the wire were the octetArray encoded directly
>>    in IPFIX per [RFC7011].  Whitespace may occur between any pair of
>>    digits to assist in human readability of the string, but is not
>>    necessary, and must be disregarded by any process reading the string.
>>    In ABNF:
>> 
>> 
>> 
>> Trammell                  Expires July 24, 2014                 [Page 3]
>> 
>> Internet-Draft              IPFIX Text Types                January 2014
>> 
>> 
>>    hex-octet = 2HEXDIGIT
>> 
>>    octetarray = 1* (hex-octet [WSP])
> 
> There's an implicit assumption of 8-bit bytes here.

Also in the name of the abstract data type: “octet” means group of eight bits, so I’m happy sticking with this assumption.

> An alternative encoding of "0b [10]*" (eg, 0b10101100) might sometimes be useful, eg for bitflags

Good point. Flags are usually implemented as unsigned, so we’d want to add binary there too as well.

<snap>
>> 4.3.  signed8, signed16, signed32, and signed64
>> 
>>    If the Enclosing Context defines a representation for signed
>>    integers, that representation SHOULD be used.
>> 
>> 
>> 
>> 
>> Trammell                  Expires July 24, 2014                 [Page 4]
>> 
>> Internet-Draft              IPFIX Text Types                January 2014
>> 
>> 
>>    Otherwise, the values of Information Elements of signed integer types
>>    should be represented as optionally-prefixed base-10 (decimal)
>>    strings.  In ABNF:
>> 
>>    sign = "+" / "-"
>> 
>>    signed = [sign] 1*DIGIT
>> 
>>    If the sign is omitted, it is assumed to be positive.  Leading zeroes
>>    are allowed, and do not signify base-8 (octal) encoding.
>> 
>>    The encoded value must be in range for the corresponding abstract
>>    data type or Information Element.  Out of range values should be
>>    interpreted as clipped to the implicit range for the Information
>>    Element as defined by the abstract data type, or to the explicit
>>    range of the Information Element if defined.  Minimum and maximum
>>    values for abstract data types are shown in Table 2 below.
>> 
>>         +----------+----------------------+----------------------+
>>         |     type |              minimum |              maximum |
>>         +----------+----------------------+----------------------+
>>         |  signed8 |                 -128 |                 +127 |
>>         | signed16 |               -32768 |               +32767 |
>>         | signed32 |          -2147483648 |          +2147483647 |
>>         | signed64 | -9223372036854775808 | +9223372036854775807 |
>>         +----------+----------------------+----------------------+
>> 
>>               Table 2: Ranges for signed abstract data types
> 
> Are +0, 0, and -0 all valid?

As long as they’re treated to be equal, yes.

>> 
>> 4.4.  float32 and float64
>> 
>>    If the Enclosing Context defines a representation for floating point
>>    numbers, that representation SHOULD be used.
>> 
>>    Otherwise, the values of Information Elements of float32 or float64
>>    types are represented as an optionally sign-prefixed, optionally
>>    base-10 exponent-suffixed, floating point decimal number.  In ABNF:
>> 
>>    sign = "+" / "-"
>> 
>>    exponent = 'e' 1*3DIGIT
>> 
>>    right-decimal = '.' 0*DIGIT
>> 
>>    mantissa = 1*DIGIT [right-decimal]
>> 
>>    float = [sign] mantissa [exponent]

There’s an error here: exponent should be ‘e’ [sign] 1*3DIGIT (see below)
>> 
>>                +---------+----------------+----------------+
>>                |    type | minimum abs(x) | maximum abs(x) |
>>                +---------+----------------+----------------+
>>                | float32 |      5.877e-39 |       3.403e38 |
>>                | float64 |    1.1125e-308 |     +1.798e308 |
>>                +---------+----------------+----------------+
>> 
>>           Table 3: Ranges for floating-point abstract data types
> 
> It's not clear how you got these values. The minimum is surely zero, and the encoding doesn't allow negative exponents. I agree with the maximums.

minimum nonzero abs(x) is what’s intended here. These came from IEEE 754 (via Wikipedia, I think); this needs a cite.
>> 4.5.  boolean
>> 
>>    If the Enclosing Context defines a representation for boolean values,
>>    that representation SHOULD be used.
>> 
>>    Otherwise, a true boolean value should be represented with the
>>    literal string 1, and a false boolean value with the literal string
>>    0.  In ABNF:
>> 
>>    boolean-yes = "1"
>> 
>>    boolean-no = "0"
>> 
>>    boolean = boolean-yes / boolean-no
> 
> Why 1/0 rather than true/false or yes/no ?

Internationalization. See my other message for my rant “why not 1/2”. :)

<snip>
>> 
>> Appendix A.  Example
>> 
>>    In this section, we examine an IPFIX Template and a Data Record
>>    defined by that Template, and show how that Data Record would be
>>    represented in JSON according to the specification in this document.
>>    Note that this is specifically NOT a recommendation for a particular
>>    representation, merely an illustration of the encodings in this
>>    document.
>> 
>>    Figure 1 shows a Template in IESpec format as defined in section 10.1
>>    of [RFC7013].  A Message containing this Template and a Data Record
>> 
>> 
>> 
>> Trammell                  Expires July 24, 2014                 [Page 9]
>> 
>> Internet-Draft              IPFIX Text Types                January 2014
>> 
>> 
>>    is shown in Figure 2, and a corresponding JSON Object using the text
>>    format defined in this document is shown in Figure 3.
>> 
>>          flowStartMilliseconds(152)<dateTimeMilliseconds>[8]
>>          flowEndMilliseconds(153)<dateTimeMilliseconds>[8]
>>          octetDeltaCount(1)<unsigned64>[4]
>>          packetDeltaCount(2)<unsigned64>[4]
>>          sourceIPv6Address(27)<ipv4Address>[4]{key}
>>          destinationIPv6Address(28)<ipv4Address>[4]{key}
>>          sourceTransportPort(7)<unsigned16>[2]{key}
>>          destinationTransportPort(11)<unsigned16>[2]{key}
>>          protocolIdentifier(4)<unsigned8>[1]{key}
>>          tcpControlBits(6)<unsigned8>[1]
>>          flowEndReason(136)<unsigned8>[1]
>> 
>>                   Figure 1: Sample flow template (IPFIX)
> 
> Perhaps, "Sample IPFIX flow template in IESpec format"? Consider dropping the "IPFIX", since it's almost meaningless here.

Yep.

>> 
>>              1         2         3         4         5         6
>>    0 2 4 6 8 0 2 4 6 8 0 2 4 6 8 0 2 4 6 8 0 2 4 6 8 0 2 4 6 8 0 2
>>   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>>   | 0x000a        | length 135    | export time 1352140263        | msg
>>   | sequence 0                    | domain 1                      | hdr
>>   | SetID 2       | length 52     | tid 256       | fields 11     | tmpl
>>   | IE 152        | length 8      | IE 153        | length 8      | set
>>   | IE 1          | length 4      | IE 2          | length 4      |
>>   | IE 27         | length 16     | IE 28         | length 16     |
>>   | IE 7          | length 2      | IE 11         | length 2      |
>>   | IE 4          | length 1      | IE 6          | length 1      |
>>   | IE 136        | length 1      | SetID 256     | length 83     | data
>>   | start time                                     1352140261135  | set
>>   | end time                                       1352140262880  |
>>   | octets                195383  | packets                   88  |
>>   | sip6                                                          |
>>   |                       2001:0db8:000c:1337:0000:0000:0000:0002 |
>>   | dip6                                                          |
>>   |                       2001:0db8:000c:1337:0000:0000:0000:0003 |
>>   | sp        80  | dp     32991  | prt 6 | tcp 19| fe 3  |
>>   +-------------------------------------------------------+
> 
> The figure is 63 chars wide rather than 64; each column is 15 chars wide.

This is intentional, to get us back to 72-wide with some annotation space on the left.

See RFC 6235.

> It might be clearer to draw the usual 1-bit-per-column, 32-bit-wide figure.

Which won’t fit on a page and takes up ridiculous amounts of space for the v6 addresses.

>>               Figure 2: IPFIX message containing sample flow
> 
> Is it still an IPFIX message?
> Should message be capitalised?

Yes, indeed.
>> 
>> Trammell                  Expires July 24, 2014                [Page 10]
>> 
>> Internet-Draft              IPFIX Text Types                January 2014
>> 
>> 
>>            {
>>                "flowStartMilliseconds": "2012-11-05T18:31:01.135",
>>                "flowEndMilliseconds": "2012-11-05T18:31:02.880",
>>                "octetDeltaCount": 195383,
>>                "packetDeltaCount": 88,
>>                "sourceIPv6Address": "2001:db8:c:1337::2",
>>                "destinationIPv6Address": "2001:db8:c:1337::3",
>>                "sourceTransportPort": 80,
>>                "destinationTransportPort": 32991,
>>                "protocolIdentifier": "tcp",
>>                "tcpControlBits": 19,
>>                "flowEndReason": 3
>>            }
>> 
>>                Figure 3: JSON object containing sample flow
> 
> Note that the quoting, and colon/comma format are JSON specific.

Will do (above, in the intro text).

Thanks again! Cheers,

Brian

_______________________________________________
IPFIX mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/ipfix
signature.asc (application/pgp-signature, 496 B)