Re: [tcpm] [IPFIX] WG LC: IPFIX documents

https://datatracker.ietf.org/doc/draft-ietf-opsawg-ipfix-tcpo-v6eh

Essentially the middle of this document is missing: a summary of issues is given and new IEs are proposed as a solution. But the issues are not developed or explained.

    1.1.  Issues with ipv6ExtensionHeaders Information Element

   *  Specify a means to export the order and the number of occurences
      of a given extension header.

Typo, "occurrences"

    The specification of ipv6ExtensionHeaders IPFIX IE does not:

Consider including the IE number for clarity, eg "The specification of ipv6ExtensionHeaders IPFIX IE (#64) does not:"

Same for 1.2 (tcpOptions, #209).

   *  Describe how any observed TCP option in a Flow can be exported
      using IPFIX.  Only TCP options having a kind =< 63 can be exported
      in a tcpOptions IPFIX IE.

Conventionally "<=" is used.

3.  Information Elements for IPv6 Extension Headers

   The definition of the ipv6ExtensionHeaders IE is updated in
   Section 4.1 of [I-D.ietf-opsawg-ipfix-fixes] to address some of the
   issues listed in Section 1.1.
For clarity say, "Section 1.1 above", else it seems to refer to 1.1 of ietf-opsawg-ipfix-fixes.

   The definition of the ipv6ExtensionHeaders IE is updated in
   Section 4.1 of [I-D.ietf-opsawg-ipfix-fixes] to address some of the
   issues listed in Section 1.1.  Because some of these limitations
   cannot be addressed by simple updates to ipv6ExtensionHeaders, this
   section specifies a set of new IEs to address all the
   ipv6ExtensionHeaders IE limitations.

Please expand on this and explain "some of": which issues are addressed and which are not? Why does the document identify issues without proposing solutions to them all? How and when will those other issues be fixed?

    3.1 - 3.4

These definitions should be in the IANA section.

Section 3 should explain the problems and how they are solved, rather than jumping straight to the IEs as a fait-accompli.

3.1.  ipv6ExtensionHeadersFull Information Element
Please include some discussion and justification for creating this new element rather than updating the existing ipv6ExtensionHeaders IE (#64). If the existing IE cannot be updated then explain backwards compatibility between the proposed and existing IEs and deprecate the existing IE.

The information is encoded in a set of bit fields.
It sounds like a singular bit field rather than a set of bit fields.

      In doing so,
      few octets will be needed to encode common IPv6 extension headers
      when observed in a Flow.
This justification should be part of the discussion I mentioned above, and not part of the definition.

        The "No Next Header" (59) value is used

Add an xref for the 59.

      This Information Element SHOULD NOT be exported if
      ipv6ExtensionHeaderCount Information Element is also present.

Explain why not. This explanation should be in the text rather than in the definition.
This should possibly be in section 5.1 where the 3rd paragraph discusses similar limitations.

   Abstract Data Type:  unsigned256

No, it's a bitfield. unsigned256 represents an integer, which this is not.

3.2.  ipv6ExtensionHeaderCount Information Element

   Description:  As per Section 4.1 of [RFC8200], IPv6 nodes must accept
      and attempt to process extension headers in occurring any number
Typo, "in".

 MSB                                                                 LSB
  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 ...
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
 |  EH Type#1    |   Count       |...|  EH Type#n      |   Count       |
 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

   Abstract Data Type:  unsigned64

This is neither a simple IE, nor an unsigned64. It's a structured data type, ie a variable-length array of { type, count } tuples.

    3.3.  ipv6ExtensionHeadersLimit Information Element

What is to be understood when this IE is not included in the IPFIX data? Is it the same as "true"; the same as "false"; or something else?

    3.4.  ipv6ExtensionHeadersChainLength Information Element

      See [RFC9098] for an overview of operational implications of IPv6
      packets with extension headerss.

Typo, "headers"

    4.  Information Elements for TCP Options

Again this jumps directly from the introduction to the solution without any discussion or explanation.

   The definition of the tcpOptions IE is updated in
   [I-D.ietf-opsawg-ipfix-fixes] to address some of the issues listed in
   Section 1.2.  Because some of these limitations cannot be addressed
   by simple updates to tcpOptions, this section specifies a set of new
   IEs to address all the tcpOptions IE limitations.

Please expand on this and explain "some of": which issues are addressed and which are not? Why does the document identify issues without proposing solutions to them all? How and when will those other issues be fixed?

    4.1.  tcpOptionsFull Information Element

Please include some discussion and justification for creating this new element rather than updating the existing tcpOptions IE (#209). If the existing IE cannot be updated then explain backwards compatibility between the proposed and existing IEs and deprecate the existing IE.

    The information is encoded in a set of bit fields.

Again, this sounds like a singular bit field rather than a set of bit fields.

   Abstract Data Type:  unsigned256

No, it's a bitfield.

    4.2.  tcpSharedOptionExID16 Information Element

   Description:  Any observed 2-byte Experiments IDs (ExIDs) in a shared
      TCP option (Kind=253 or 254) in a Flow.  The information is
      encoded in a set of 16-bit fields.  Each 16-bit field carries an
      observed 2-byte ExID in a shared option.

I did not understand what this measures, nor how to encode it, until I reviewed section 6.2.

As this is an array of 16-bit values, the "octetArray" type is somewhat misleading as it's really a hextetArray.

    4.3.  tcpSharedOptionExID32 Information Element

   Description:  Any observed 4-byte Experiments IDs (ExIDs) in a shared
      TCP option (Kind=253 or 254) in a Flow.  The information is
      encoded in a set of 32-bit fields.  Each 32-bit field carries an
      observed 4-byte ExID in a shared option.

I do not understand what this measures, nor how to encode it, until later.

Again, the "octetArray" type is somewhat misleading as it's really a 32tetArray.

    5.1.  IPv6 Extension Headers

   If an implementation determines that it includes an extension header
   that it does not support,

Consider "that the observed packet stream includes".

    5.2.  TCP Options

   If a TCP Flow contains packets with a mix of 2-byte and 4-byte
   Experiment IDs, the same Template Record is used with both
   tcpSharedOptionExID16 and tcpSharedOptionExID32 IEs.

Consider:

   If a TCP Flow contains packets with a mix of 2-byte and 4-byte
   Experiment IDs, then a single Template Record SHOULD be used with both
   tcpSharedOptionExID16 and tcpSharedOptionExID32 IEs.

    6.  Examples

   This section provides few examples to illustrate the use of some IEs
   defined in the document.

Typos: "provides a few"; "in this document".

    6.1.  IPv6 Extension Headers

   Concretely, the ipv6ExtensionHeadersFull IE will be set to 35.

It would be more intuitive to say "0x23" as that matches the bit pattern. Ideally all 8 LS bits would be shown in the figure.

    6.2.  TCP Options

   Given TCP kind allocation practices and the option mapping defined in
   Section 4.1, fewer octers are likely to be used for Flows with common
   TCP options.

Typo, "octets"

    Concretely, the tcpOptionsFull IE will be set to 13.

It would be more intuitive to say "0x0D" as that matches the bit pattern. Ideally all 8 LS bits would be shown in the figure.

   1.  The tcpSharedOptionExID16 IE set to 55067982 (i.e., 0x348454E) to
       report observed 2-byte ExIDs: HOST_ID and TCP-ENO ExIDs.

   2.  The tcpSharedOptionExID32 IE set to 3805594585 (i.e., 0xE2D4C3D9)
       to report the only observed 4-byte ExID.

Remove the base 10 numbers. They're irrelevant and confusing.

    7.  Security Considerations

The ipv6ExtensionHeadersChainLength could be used to determine hardware capabilities and limitations, and possibly even to identify the hardware through which the traffic is flowing.

It is to be hoped that anyone capable of enabling IPFIX export on a device would already know this information.

I can't think of any other IEs whose specific purpose is to identify hardware limitations, so this should be called out in this section.

    8.2.  New IPFIX Information Element Data Type

   The type "unsigned256" represents a non-negative integer value in the
   range of '0' to '2^256 - 1'.

The IEs which use this new type are not exporting integers, but flags - so it would make more sense to create a new bitfield type.

P.

On 18/12/23 19:22, Joe Clarke (jclarke) wrote:
We’d like to kick off a [rather extended] WG LC on the three IPFIX-related “fixes” documents we have in the hopper.  We’ve already requested some directorate reviews for these, and the authors feel they have stabilized well.

Please review:

  *   https://datatracker.ietf.org/doc/draft-ietf-opsawg-ipfix-tcpo-v6eh/ [datatracker.ietf.org]<https://urldefense.com/v3/__https://datatracker.ietf.org/doc/draft-ietf-opsawg-ipfix-tcpo-v6eh/__;!!OSsGDw!I6YR3K0-150A1C0ElDbVuvpjkK-Ylkh69dILZCWJl3lPScov6t5X2FzyrjqruiynmXjCnHZBzRn1Gy8IthVfCMEo_jvU$>
  *   https://datatracker.ietf.org/doc/draft-ietf-opsawg-tsvwg-udp-ipfix/ [datatracker.ietf.org]<https://urldefense.com/v3/__https://datatracker.ietf.org/doc/draft-ietf-opsawg-tsvwg-udp-ipfix/__;!!OSsGDw!I6YR3K0-150A1C0ElDbVuvpjkK-Ylkh69dILZCWJl3lPScov6t5X2FzyrjqruiynmXjCnHZBzRn1Gy8IthVfCG_d35-j$>
  *   https://datatracker.ietf.org/doc/draft-ietf-opsawg-ipfix-fixes/ [datatracker.ietf.org]<https://urldefense.com/v3/__https://datatracker.ietf.org/doc/draft-ietf-opsawg-ipfix-fixes/__;!!OSsGDw!I6YR3K0-150A1C0ElDbVuvpjkK-Ylkh69dILZCWJl3lPScov6t5X2FzyrjqruiynmXjCnHZBzRn1Gy8IthVfCAzlAEMT$>

And comment as to whether or not you feel they are in the right shape to progress to the IESG.  We will run this LC until Jan 8 given that the holidays means some people will not be around.  Also note that an IPR poll was conducted prior to adoption and no known IPR has been disclosed.

Thanks.

Joe

_______________________________________________
IPFIX mailing list
IPFIX@ietf.org<mailto:IPFIX@ietf.org>
https://urldefense.com/v3/__https://www.ietf.org/mailman/listinfo/ipfix__;!!OSsGDw!I6YR3K0-150A1C0ElDbVuvpjkK-Ylkh69dILZCWJl3lPScov6t5X2FzyrjqruiynmXjCnHZBzRn1Gy8IthVfCO14NBgv$ [ietf[.]org]