Hi Med, Thanks for addressing most of the comments. One comment and one clarification.
This might be the iddiff tool, but in the diff file you shared I see the following under the title of the draft: draft-ietf-opsawg-ipfix-fixes-latest I was expecting to see a number, not the word latest. For the clarification, see below with [mj]. > On Apr 14, 2024, at 11:07 PM, mohamed.boucad...@orange.com wrote: > > Hi Mahesh, > > Thank you for the review. > > The candidate version to address your review can be seen at: Diff: > draft-ietf-opsawg-ipfix-fixes.txt - draft-ietf-opsawg-ipfix-fixes.txt > <https://author-tools.ietf.org/api/iddiff?url_1=https://boucadair.github.io/simple-ipfix-fixes/draft-ietf-opsawg-ipfix-fixes.txt&url_2=https://boucadair.github.io/simple-ipfix-fixes/AD-Review/draft-ietf-opsawg-ipfix-fixes.txt>. > > Please see inline for more context. > > Cheers, > Med > > De : Mahesh Jethanandani <mjethanand...@gmail.com > <mailto:mjethanand...@gmail.com>> > Envoyé : dimanche 14 avril 2024 05:21 > À : draft-ietf-opsawg-ipfix-fi...@ietf.org > <mailto:draft-ietf-opsawg-ipfix-fi...@ietf.org> > Cc : opsawg@ietf.org <mailto:opsawg@ietf.org> > Objet : AD review of draft-ietf-opsawg-ipfix-fixes > > > > Hi Authors, > > Thank you for working on this document, This is my review of > draft-ietf-opsawg-ipfix-fixes-06 draft. They are roughly divided between > COMMENT and NIT. The COMMENTs should be resolved before this document is sent > for IETF LC for publication as a Draft Standard. > > ------------------------------------------------------------------------------- > COMMENT > ------------------------------------------------------------------------------- > > Section 1, paragraph 2 > > When OPSAWG was considering [I-D.ietf-opsawg-rfc7125-update] which > > updates [RFC7125], the WG realized that some other parts of the IANA > > IP Flow Information Export (IPFIX) registry [IANA-IPFIX] were not up- > > to-date. Indeed, since its initial creation in 2007, some IPFIX > > Information Elements (IEs) are no longer adequately specified (while > > they were at some point in time in the past). This document intends > > to update the IANA registry and bring some consistency among the > > entries of the registry. > > > For a specification to "no longer adequately" specify an IE, the underlying > specification for IPFIX must have changed. If that is so, can you cite what > changed? If not, I would just remove the statement. It is not serving any > purpose in this specification. > > [Med] We used to have update to description text of other IEs but those were > removed as the decision was to deprecate them. So, OK to remove this sentence > as well. > > Section 1, paragraph 1 > > As discussed with IANA during the publication process of [RFC9487], > > the "Additional Information" entry in [IANA-IPFIX] should contain a > > link to an existing registry, when applicable, as opposed to having: > > > What happens to the existing registries? Does Section 6 take care of all of > them? Should the link in existing registries not specified in Section 6, also > move to Additional Information? > > [Med] Modifications to existing entries are covered in this doc. [mj] When you say existing entries, do you mean all existing entries currently in the IANA registry? > > Section 3, paragraph 3 > > Since Sections 4-7 are really for the IANA Consideration section, why not > move them there? > > [Med] I prefer to leave them in the core document to record the OLD/NEW; not > only the IANA actions. Thanks. > > Section 4.1.2, paragraph 0 > > - Description: This Information Element describes the forwarding > > status of the flow and any attached reasons. > > IPFIX reduced-size encoding is used as required. > > > I know that you explain what reduced-size encoding in the other IPFIX > documents, but it will be worth repeating it here or refer to that definition > from here. > > [Med] We do already provide the authoritative definition in the para right > before: > > == > The description is also updated to clarify the use of the reduced-size > encoding as per Section 6.2 <https://rfc-editor.org/rfc/rfc7011#section-6.2> > of [RFC7011 > <https://boucadair.github.io/simple-ipfix-fixes/draft-ietf-opsawg-ipfix-fixes.html#RFC7011>]. > == > > Found terminology that should be reviewed for inclusivity; see > https://www.rfc-editor.org/part2/#inclusive_language > <https://www.rfc-editor.org/part2/#inclusive_language> for background and more > guidance: > > * Term "he"; alternatives might be "they", "them", "their" > > [Med] That one is correct as we refer to Andrew. > > * Term "traditional"; alternatives might be "classic", "classical", "common", > "conventional", "customary", "fixed", "habitual", "historic", > "long-established", "popular", "prescribed", "regular", "rooted", > "time-honored", "universal", "widely used", "widespread" > > [Med] We can’t do nothing here because that points to: > > [RFC3022] Srisuresh, P. and K. Egevang, "Traditional IP Network > Address Translator (Traditional NAT)", RFC 3022, > DOI 10.17487/RFC3022, January 2001, > https://www.rfc-editor.org/rfc/rfc3022 > <https://www.rfc-editor.org/rfc/rfc3022>. > > > ------------------------------------------------------------------------------- > 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 > <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. > > Uncited references: [RFC_Errata_1739] and [RFC_Errata_1738]. > > [Med] Fixed. Thanks. > > Reference [RFC2482] to RFC2482, which was obsoleted by RFC6082 (this may be on > purpose). > > [Med] 2482 is cited because this is what we have in an OLD text. We kept it > on purpose in the NEW as this document is about simple fixes and does not > cover major changes or obsoleting existing IEs. > > > Reference [RFC5102] to RFC5102, which was obsoleted by RFC7012 (this may be on > purpose). > > [Med] No change is needed here because we use that ref to remind a context. > > Reference [RFC1631] to RFC1631, which was obsoleted by RFC3022 (this may be on > purpose). > > [Med] 1631 was cited because it was in an OLD text. After thinking about > this, the NEW should use 3022 as that RFC echoed 1631. > > > Document references draft-ietf-opsawg-RFC7125-update, but that has been > published as RFC9565. > > [Med] OK > > Reference [I-D.ietf-opsawg-rfc7125-update] to RFC7125, which was obsoleted by > RFC9565 (this may be on purpose). > > Reference [RFC7125] to RFC7125, which was obsoleted by RFC9565 (this may be on > purpose). > > Reference [RFC4646] to RFC4646, which was obsoleted by RFC5646 (this may be on > purpose). > [Med] 4646 is cited because this is what we have in an OLD text. 5646 is not > used in the NEW for the same reasons as 2482. > > > "Abstract", paragraph 3 > > ions and Management Area Working Group Working Group mailing list > > (opsawg@iet > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^ > This phrase is duplicated. You should probably use "Working Group" only once. > > [Med] We can’t fix that :-) > > Section 6.2.2, paragraph 1 > > NAT event. This IE identifies the type of a NAT event. Examples of NAT > > events > > ^^^^^^^^^ > If "type" is a classification term, "a" is not necessary. Use "type of". (The > phrases "kind of" and "sort of" are informal if they mean "to some extent".). > > [Med] That text was echoed as in RFC8158. > > Section 6.3.1, paragraph 1 > > NAT event. This IE identifies the type of a NAT event. Examples of NAT > > events > > ^^^^^^^^^ > If "type" is a classification term, "a" is not necessary. Use "type of". (The > phrases "kind of" and "sort of" are informal if they mean "to some extent".). > > [Med] Idem > > Section 6.10.1, paragraph 5 > > description of the abstract data type of an IPFIX information element. These > > ^^^^^^^^^^ > If "type" is a classification term, "an" is not necessary. Use "type of". (The > phrases "kind of" and "sort of" are informal if they mean "to some extent".). > > [Med] That text was from RFC5610 > > Section 6.10.2, paragraph 5 > > description of the abstract data type of an IPFIX information element.These > > a > > ^^^^^^^^^^ > If "type" is a classification term, "an" is not necessary. Use "type of". (The > phrases "kind of" and "sort of" are informal if they mean "to some extent".). > > [Med] Idem as the previous one. > > Section 6.20.2, paragraph 1 > > nformation Element identifies the type of a NAT Quota Exceeded event. Values > > ^^^^^^^^^ > If "type" is a classification term, "a" is not necessary. Use "type of". (The > phrases "kind of" and "sort of" are informal if they mean "to some extent".). > > Section 6.20.2, paragraph 2 > > nformation Element identifies the type of a NAT Quota Exceeded event. Values > > ^^^^^^^^^ > If "type" is a classification term, "a" is not necessary. Use "type of". (The > phrases "kind of" and "sort of" are informal if they mean "to some extent".). > > Section 6.21.1, paragraph 2 > > Information Element identifies a type of a NAT Threshold event. Values for > > t > > ^^^^^^^^^ > If "type" is a classification term, "a" is not necessary. Use "type of". (The > phrases "kind of" and "sort of" are informal if they mean "to some extent".). > > Section 6.21.2, paragraph 2 > > Information Element identifies a type of a NAT Threshold event. Values for > > t > > ^^^^^^^^^ > If "type" is a classification term, "a" is not necessary. Use "type of". (The > phrases "kind of" and "sort of" are informal if they mean "to some extent".). > > [Med] same as above. > > Section 6.23.1, paragraph 4 > > | | | table below, and section Section 5.1. | | 2 | PmA | Pe > > ^^^^^^^^^^^^^^^ > Possible typo: you repeated a word. > > [Med] Indeed, but this is what we have in the current registry. This document > fixes that. > > Section 6.24.2, paragraph 1 > > SHOULD have this flag set, and vice-versa.) | +--------+----------+--------- > > ^^^^^^^^^^ > The expression "vice versa" is spelled without hyphens. > > [Med] That’s the current form we have currently in the registry. > > Section 7.2.1, paragraph 1 > > SHOULD have this flag set, and vice-versa.) | +--------+----------+--------- > > ^^^^^^^^^^ > The expression "vice versa" is spelled without hyphens. > > > 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. Mahesh Jethanandani mjethanand...@gmail.com
_______________________________________________ OPSAWG mailing list OPSAWG@ietf.org https://www.ietf.org/mailman/listinfo/opsawg
