Re: [OPSAWG] AD review of draft-ietf-opsawg-ipfix-fixes

[Mahesh Jethanandani <[e-mail]>]

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, [e-mail] 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 <[e-mail] <mailto:[e-mail]>> 
> Envoyé : dimanche 14 avril 2024 05:21
> À : [e-mail] <mailto:[e-mail]>
> Cc : [e-mail] <mailto:[e-mail]>
> 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
> [e-mail] <mailto:[e-mail]>
>  
>  
>  
>  
> 
>  
> ____________________________________________________________________________________________________________
> 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
[e-mail]