Hi Mahesh, Thank you for the review.
The candidate version can be seen at: Diff: draft-ietf-opsawg-tsvwg-udp-ipfix.txt - draft-ietf-opsawg-tsvwg-udp-ipfix.txt<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/boucadair-patch-2/draft-ietf-opsawg-tsvwg-udp-ipfix.txt> Please see inline. Cheers, Med De : Mahesh Jethanandani <mjethanand...@gmail.com> Envoyé : mardi 16 avril 2024 18:48 À : draft-ietf-opsawg-tsvwg-udp-ip...@ietf.org Cc : opsawg@ietf.org Objet : AD review of draft-ietf-opsawg-tsvwg-udp-ipfix Hi Authors, Thank you for working on this document. And thanks to Tommy Pauly, and Joe Touch for providing their reviews. Here is my review that is divided between COMMENTs and NITs. I expect the COMMENTs to be resolved before the document is sent for IETF Last Call. ------------------------------------------------------------------------------- COMMENT ------------------------------------------------------------------------------- Section 1, paragraph 1 > IP Flow Information Export (IPFIX) [RFC7011] is a protocol that is > widely deployed in operators networks for traffic management > purposes Would it be fair to say, that IPFIX is used for traffic monitoring and not directly traffic management? [Med] It is actually given that it covers many aspects of the so-called FCAPS management function (see for example rfc5472#section-2). I added "({{Section 2 of !RFC6632}}) where it is cited as part of "Core Network Management Protocols"". Section 2, paragraph 2 > This document uses the IPFIX-specific terminology (e.g., Flow) > defined in Section 2 of [RFC7011]. As in [RFC7011], these IPFIX- > specific terms have the first letter of a word capitalized. > > Also, this document uses the terms defined in Section 3 of > [I-D.ietf-tsvwg-udp-options]. It would help to identify which terms are being used from each of the documents in this draft. [Med] cited the main terms. Also, is the rule that the IPFIX specific terms have the first letter captilized? [Med] This is a convention for terms such as Flow, Template, etc. What about udpOptions? [Med] For IEs, the convention in rfc7012#section-2.3 apply, that is: o Names of Information Elements MUST start with lowercase letters. o Composed names MUST use capital letters for the first letter of each component (except for the first one). All other letters are lowercase, even for acronyms. Exceptions are made for acronyms containing a mixture of lowercase and capital letters, such as 'IPv4' and 'IPv6'. Examples are "sourceMacAddress" and "destinationIPv4Address". [Med] Added this NEW: "The document adheres to the naming conventions for Information Elements per {{Section 2.3 of !RFC7012}}." Section 5, paragraph 3 > Figure 2: An Example of udpOptions IE Can the description be expaned to say "An Example of udpOptions with EOL and APC options? [Med] Sure. Fixed. Section 5, paragraph 10 > udpSafeExperimentalOptionExID IE: > > MSB LSB > 1 2 3 > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | 0x9858 | 0xE2D4 | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > udpUnsafeExperimentalOptionExID IE: > > 1 2 3 > 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > | 0xC3D9 | 0x9658 | > +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > Figure 3: Example of UDP Experimental option IEs This example is not clear or confusing. The explanation above talked about SAFE and UNSAFE Experimental options. How is the range of 0-191 for SAFE or the range of 192-255 encoded in these values? [Med] The shared option in the safe range is (EXP, Kind=127) while it is (UEXP, Kind=254) for the unsafe. When a shared SAFE option is observed, the ExIDs it carries are exported using udpSafeExperimentalOptionExID IE. The same happens for UEXP but with udpUnsafeExperimentalOptionExID IE. The presence of udpSafeExperimentalOptionExID or udpUnsafeExperimentalOptionExID is explicit about whether EXP or UEXP were observed. Both can be observed in the same Flow as exemplified in this Section. Or if they are not, because they come part of the UDP options, then how about showing what the UDP options field looks like? In other words, can Example 2 and Example 3 be combined to show a more complete example? [Med] I tweaked the example to also describe how the udpOptions IE looks like. Please let me know if any further change is needed. Thanks. No reference entries found for these items, which were mentioned in the text: [URL_IANA_UDP_OPTIONS] and [URL_IANA_UDP_ExIDs]. [Med] ACK. We have a note about that. Note, the following comment is boilerplate output from the review tool I use. However, it is true that draft-ietf-tsvwg-udp-options currently is in WG LC and does not have the intended RFC status set, and thus the message. Also note that this document will result in MISREF state unless the other document moves forward and gets approved as an RFC. DOWNREF [I-D.ietf-tsvwg-udp-options] from this Proposed Standard to draft-ietf-tsvwg-udp-options of unknown standards level. (For IESG discussion. It seems this DOWNREF was not mentioned in the Last Call and also seems to not appear in the DOWNREF registry.) ------------------------------------------------------------------------------- NIT ------------------------------------------------------------------------------- All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. Section 5, paragraph 1 > Given UDP kind allocation in Section 10 of > [I-D.ietf-tsvwg-udp-options] and the option mapping defined in > Section 4.1 of this document, fewer octets are likely to be used for > Flows with mandatory UDP options. Inconsistent use of the term Kind. Sometimes it is used with a captial K but other times it is used with a small k. [Med] Changed to « Kind » to be consistent with the upd-options spec. Document references draft-ietf-tsvwg-udp-options-28, but -32 is the latest available revision. Document references draft-ietf-opsawg-ipfix-tcpo-v6eh-08, but -10 is the latest available revision. Mahesh Jethanandani mjethanand...@gmail.com<mailto:mjethanand...@gmail.com> ____________________________________________________________________________________________________________ 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.
_______________________________________________ OPSAWG mailing list OPSAWG@ietf.org https://www.ietf.org/mailman/listinfo/opsawg
