| Hi, Med,
I still have issue with the same two key parts:
1. This doc refers to unsigned192. - That is not a native data type of any known computer. It needs to be defined. - Reduced-size encoding per RFC7011 does not apply, unless you are restricting them to 64, 32, 16, and 8.
2. Neither unsigned64 nor unsigned192 are compatible with octetArray as used in this document, AFAICT - octetArray would be for a sequence of unsigned64s, for example, but not for bytes that might be interpreted in different ways depending on length
It would be useful to explain why the safe and unsafe options are split (i.e., to support reduced-size encoding when both are in use).
The only approach that I think is compatible with RFC7011 is:
- use an octetArray and explain its length and mapping (e.g., of unsigned8 values), representing the values 0…256 as consecutive groups of 8, truncated when the remaining bytes are all zeros
OR - define safe as three unsigned64s, each potentially using reduced-size encoding
See additional notes below.
Joe
— Dr. Joe Touch, temporal epistemologist www.strayalpha.com
On May 13, 2024, at 8:46 AM, mohamed.boucadair@xxxxxxxxxx wrote:
Hi Joe, Thank you for the excellent review. The changes to address the review can be seen here: https://author-tools.ietf.org/api/iddiff?url_1=https://boucadair.github.io/udp-ipfix/draft-ietf-opsawg-tsvwg-udp-ipfix.txt&url_2=https://boucadair.github.io/udp-ipfix/Joe-Review/draft-ietf-opsawg-tsvwg-udp-ipfix.txt Please see inline for more context. Cheers, Med -----Message d'origine-----
De : Joseph Touch via Datatracker <noreply@xxxxxxxx>
Envoyé : mercredi 8 mai 2024 06:06
À : int-dir@xxxxxxxx
Cc : draft-ietf-opsawg-tsvwg-udp-ipfix.all@xxxxxxxx; last-
call@xxxxxxxx; opsawg@xxxxxxxx
Objet : Intdir last call review of draft-ietf-opsawg-tsvwg-udp-
ipfix-08
Reviewer: Joseph Touch
Review result: Ready with Issues
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.
Med] Good point. As you can see in https://mailarchive.ietf.org/arch/msg/opsawg/JJAD7ooBNe_gFHRUAiHmK24HBNk/, we already made changes so that reduced-size encoding can be used even in the presence of shared options; see specifically this part:
I believe you are referring to experimental options, and yes, I see how that is now fixed, with the text noted below.
==
[Med] These considerations are not specific to this document and fall under the generic considerations in base spec 7011 (Section 10.3.3). After think about this, I added a new section called (ops considerations) with the following content:
* moved the text about reduced-size encoding to that section
* encourage the use of reduced-size encoding in the presence of EXP/UEXP (that is the *ExID IEs) takes precedence and thus their flags can be ignored
MUST be ignored. * add a pointer to transport (including MTU) IPFIX cons.
==
We can go one step further to export SAFE and UNSAFE options in separate IEs.
Yes, and it would be useful to add a note explaining why these are separated.
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.
[Med] Makes sense. Removed the pointer to 6.1.1 of 7011 and copy/paste the encoding in the tcp-eh I-D.
That does not resolve the use reduced encoding in a way that 7011’s text does not define.
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).
[Med] Please see https://mailarchive.ietf.org/arch/msg/opsawg/XUeSAPc22OkAPKGJwO54BN2H5wM/.
This mail appears to validate my observation above (that octetArray is of sequences of an *existing* Information Element (e.g., unsigned16 or unsigned8). The Information Element component is required in the element definition.
It’s insufficient to refer to them as “16-bit fields”; that’s not an existing Information Element type registered with IANA. 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.
[Med] I prefer to keep the current names as we don't know if other IEs will be defined in the future to cover other aspects of these options.
The UDP options document refers to them as UDP ExIDs; that ought to be sufficient to ensure that future options cannot refer to that term for something else.
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.
[Med] The intent is to further insist on the reduced encoding.
I don’t understand “insist”. If this is an example of non-reduced-length encoding, then it should be labeled as such. -- |
