Last Call Review of draft-ietf-opsawg-tsvwg-udp-ipfix-08
review-ietf-opsawg-tsvwg-udp-ipfix-08-intdir-lc-touch-2024-05-07-00

Note that I previously performed an INTAREA early review on Jan 12, 2024.

This document sufficiently addresses the issues previously raised, with the
exception below. It introduces no new concerns.

The sole remaining issue is the use of "unsigned256" as a data type. This is
necessary to represent the potential bitfield to support all 256 possible UDP
option Kind values. This document cites draft-ietf-opsawg-ipfix-tcpo-v6eh as
defining this type (in Sec 8.3). That (tcpo-v6eh) document defines the range of
this type correctly, but then refers back to RFC7011 Sec 6.1.1 to define the
encoding. Unfortunately, RFC7011 defines encodings for unsigned integers only
up to 64 bits.

This (udp-ipfix) document cites Section 6.2 of RFC7011 to define reduced size
encodings of unsigned256, but RFC7011 defines those encodings only for 64, 32,
and 16 bit unsigned integers. Additionally, those reductions apply only when
the high-order byte(s) are all zero; for UDP options, the partitioning of
options into SAFE and UNSAFE groups and the assignment of experiment codepoints
to the highest values in both ranges suggests that these reduced size encodings
may be of limited relevance.

At least one document needs to define unsigned256 normatively - this doc,
tcpo-v6eh, or some other. That document needs to explain the byte order
representation and reduced size encoding.

This (udp-fix) document also uses the octetArray type, but then defines it as
being interpreted as "16-bit fields" (Sec 4.2, 4.3). This should probably refer
to unsigned16 values, but it's not clear that this is a valid use of
octetArrays. Being able to read such an array as unsigned16 values may require
half-word (16bit) or word (32bit) alignment, such that reading an unsigned16
from a misaligned offset may either result in an error or a misinterpreted
(byte-swapped) value. Please provide an example of use of an octetArray as a
list of multi-octet values in a published RFC or provide an alternate
representation (or define one in detail).

Some additional minor suggestions:

Note that the "ex" in ExID already means "experimental option". It thus may be
useful to consider shortening the name of the related two element IDs, e.g.,
reducing udpUnsafeExperimentalOptionExID to udpUnsafeExID and reducing
udpSafeExperimentalOptionExID to udpSafeExID.

Fig 2 shows the full unsigned256 field, but it is not clear why this is useful
if the reduced length variant is sufficient. It might be useful to show just
the appropriate byte.