[Last-Call] Re: Intdir last call review of draft-ietf-opsawg-tsvwg-udp-ipfix-08

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Joe,

 

Thank you for the follow-up.

 

A candidate version can be seen at: Diff: draft-ietf-opsawg-tsvwg-udp-ipfix.txt - draft-ietf-opsawg-tsvwg-udp-ipfix.txt

 

Please see inline.

 

Cheers,

Med

 

De : touch@xxxxxxxxxxxxxx <touch@xxxxxxxxxxxxxx>
Envoyé : mercredi 15 mai 2024 04:13
À : BOUCADAIR Mohamed INNOV/NET <mohamed.boucadair@xxxxxxxxxx>
Cc : int-dir@xxxxxxxx; draft-ietf-opsawg-tsvwg-udp-ipfix.all@xxxxxxxx; last-call@xxxxxxxx; opsawg@xxxxxxxx
Objet : Re: [Last-Call] Intdir last call review of draft-ietf-opsawg-tsvwg-udp-ipfix-08

 

Hi, Med,

 

I still have issue with the same two key parts:

 

1. This doc refers to unsigned192. 

 

[Med] Before defining the unsigned192, I first considered reusing unsigned256 + mandate that 64bits to be always set to zero. These leading zeros will thus be dropped per the reduced size encoding.

 

             - That is not a native data type of any known computer. It needs to be defined.

[Med] Unsigned192 structure is used in many contexts (e.g., U192 in crypto_bigint - Rust (bsx.fi)).

             - Reduced-size encoding per RFC7011 does not apply, unless you are restricting them to 64, 32, 16, and 8.

[Med] There is no such restriction because of this part in the base spec:

 

   This behavior is indicated by the Exporter by specifying a size in
   the Template with a smaller length than that associated with the
   assigned type of the Information Element.

 

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).

[Med] Added a note about this.

 

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

 

[Med] This is not an option. All bitfield IEs in IPFIX are defined as unsigned data types.

OR

             - define safe as three unsigned64s, each potentially using reduced-size encoding

[Med] We already considered this in the past https://datatracker.ietf.org/doc/draft-boucadair-opsawg-tsvwg-udp-ipfix/03/, but changed the design because Benoît Claise rightfully pointed to a an issue with reordering in IPFIX.

 

==

   *  Description: Observed UDP options of a Flow.  The information is

      encoded in a set of bit fields.  Multiple instances of the

      udpOptions IE are included to cover the 0-255 range (Figure 2);

      each 64 values are mapped to an IE with th order preserved.

      Options are mapped to bits according to their option numbers.

      Option number X is mapped to bit X[64] of the IE instance

      determined by the order "1+1[X/64]".  A bit is set to 1 (or 0) if

      the corresponding UDP option is observed (or not).  A udpOptions

      IE instance MAY be ommited if there is no ambiguity to determine

      the position of an observed UDP option.  For example, (1) if only

      option kinds =<63 are observed, then only one udpOptions IE

      instance is included, (2) if only option kinds =<127 are observed,

      then two udpOptions IEs instances are included, (3) if some option

      kinds =<63 while others are >=192 are observed, then four

      udpOptions IE instances are included with the second and third IE

      instances are both set to 0, etc.

 

   *  Abstract Data Type: unsigned64

==

 

See additional notes below.

 

Joe

 

Dr. Joe Touch, temporal epistemologist



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] ACK.




==
[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.

[Med] OK.



* 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.

[Med] Added this note :

 

NEW :

Given the Kind structure of safe and unsafe UDP options, distinct IEs are defined to report safe and unsafe UDP options rather using one single IE that would multiplex both types of option. The design in the document is compatible with the use of the reduced-size encoding.


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.

[Med] I added this text to the new data type:

 

NEW:

The reduction in size can be to any number of octets smaller than the unsigned192 type if the data value still fits, i.e., so that only leading zeroes are dropped.

 

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.

[Med] Actually, no. What you are describing is more a basicList type (RFC6313).  

 

It’s insufficient to refer to them as “16-bit fields”; that’s not an existing Information Element type registered with IANA.

[Med] We used 16-bit as a way to describe the “interpretation of the octets” in the octetArray data type.

 

   The octetArray data type has no encoding rules; it represents a raw

   array of zero or more octets, with the interpretation of the octets

    defined in the Information Element definition.

 

But what I’m hearing from your review is that readers are likely to have the same confusion as you. Updated the definition to use RFC6313 basicList type.



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.

 

[Med] Added titles to better highlight that. Thanks.

--

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.
-- 
last-call mailing list -- last-call@xxxxxxxx
To unsubscribe send an email to last-call-leave@xxxxxxxx

[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux