Re: [OPSAWG] Opsdir early review of draft-ietf-opsawg-ipfix-fixes-03

2024-01-11 Thread mohamed . boucadair
Hi Qin, 

(apologies for the delay to reply as I was out of office for the last three 
weeks)

Thank you for the review. 

Please see inline. 

Cheers,
Med

> -Message d'origine-
> De : Qin Wu via Datatracker 
> Envoyé : lundi 25 décembre 2023 13:51
> À : ops-...@ietf.org
> Cc : draft-ietf-opsawg-ipfix-fixes@ietf.org; opsawg@ietf.org
> Objet : Opsdir early review of draft-ietf-opsawg-ipfix-fixes-03
> 
> Reviewer: Qin Wu
> Review result: Has Nits
> 
> I have reviewed this document as part of the Operational
> directorate's ongoing effort to review all IETF  documents being
> processed by the IESG.  These comments were written with the
> intent of improving the operational aspects of the IETF drafts.
> Comments that  are not addressed in last call may be included in
> AD reviews during the IESG review.  Document editors and WG
> chairs should treat these  comments just like any other last call
> comments.
> 
> This document Updates IPFIX IANA Registry to fix several issues
> including consistency issues. This update help IANA to better
> structure the content in more consistent way and also help
> automate extraction of values from IANA registry.
> 
> This document is well written and I believe it is ready for
> publication.
> However I have a few comments on the latest version v-03:
> 
> Major issues:
> None
> 
> Minor issues
> 1. Abstract:
> I believe IANA IPFIX registry is associated with all IPFIX
> related RFCs, I am wondering whether update to  IANA IPFIX
> registry indicate update to all these IPFIX related RFC as well
> such as RFC7125,RFC7012 and etc?
> 

[Med] The document does not update RFC7012 because 7012 says explicitly the 
following: 

   [IANA-IPFIX] is now the normative reference for IPFIX Information
   Elements.

For entries that were created by other RFCs (7270, 6759, 8158, 6235, 5477, 
5610, 7014, 7015, 7133, 8038, and 5655), I understand that it might seem 
cleaner to tag that this I-D updates them. However, the description of many of 
these IEs in the IANA registry does not echo exactly what is in the RFC. See, 
for example, forwardingStatus or natEvent IEs. Will need to dig further and 
check this with Benoît, though.

> 2. Section 4.1.1 said:
> “
>   [I-D.ietf-opsawg-ipfix-tcpo-v6eh] specifies a new Information
> Element
>to fix the last issue.
> ”
> It is not clear where or which section the procedure is specified
> from the first glance. If my understanding is correct, the
> solution to address the last issue is to define new IEs to
> address all the ipv6ExtensionHeaders IE limitations rather than
> simply specifying the procedure.

[Med] Yes.

> 
> To better clarify the relation between this document and [I-
> D.ietf-opsawg-ipfix-tcpo-v6eh] and understand where the procedure
> is specified, I propose the following change: s/ [I-D.ietf-
> opsawg-ipfix-tcpo-v6eh] specifies a new Information Element/
> Section 3 of [I-D.ietf-opsawg-ipfix-tcpo-v6eh] specifies a new
> Information Element

[Med] Works for me.

> 
> 3. Section 4.1.1 said:
> “
> Note that some implementations may not be able to export all
> observed  extension headers in a Flow because of a hardware of

[Med] We meant " hardware or" instead of "hardware of". Fixed.


> software limit  (see, e.g., [I-D.ietf-6man-eh-limits].
> ”
> What is the reason for some implementation may not be able to
> export all observed extension headers in a Flow? Software limit,
> hardware limit or hardware/software limit, here the proposed
> change: s/a hardware of software limit/a hardware or software
> limit
> 

[Med] ACK. 

> 4.
> Section 4.2.2 said:
> "
> 4.2.2.  Update the Description of the tcpOptions IE
> 
>This document requests IANA to update the description of the
>tcpOptions IE in the IANA IPFIX registry [IANA-IPFIX] as
> follows:
> 
> "
> Section 3 said:
> "
> The current forwardingStatus entry in [IANA-IPFIX] deviates from
> what
>is provided in [RFC7270].
> 
> "
> Section 4 said:
> "
> This document requests IANA to update the description of the
>following entries in [IANA-IPFIX].
> 
> "
> You can see some places use "the IANA IPFIX registry [IANA-
> IPFIX]", some places use "IANA-IPFIX", it is not consistent.
> 
> 

[Med] OK to use consistent wording. Fixed this one and other nits at 
https://github.com/boucadair/simple-ipfix-fixes/commit/757e674afa455e18619a44fd8d322c8dfb4bb770.

Ce message et ses pieces jointes peuvent contenir des informations 
confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou cop

[OPSAWG] Opsdir early review of draft-ietf-opsawg-ipfix-fixes-03

2023-12-25 Thread Qin Wu via Datatracker
Reviewer: Qin Wu
Review result: Has Nits

I have reviewed this document as part of the Operational directorate's ongoing
effort to review all IETF
 documents being processed by the IESG.  These comments were written with the
 intent of improving the operational aspects of the IETF drafts. Comments that
 are not addressed in last call may be
included in AD reviews during the IESG review.  Document editors and WG chairs
should treat these
 comments just like any other last call comments.

This document Updates IPFIX IANA Registry to fix several issues including
consistency issues. This update help IANA to better structure the content in
more consistent way and also help automate extraction of values from IANA
registry.

This document is well written and I believe it is ready for publication.
However I have a few comments on the latest version v-03:

Major issues:
None

Minor issues
1. Abstract:
I believe IANA IPFIX registry is associated with all IPFIX  related RFCs, I am
wondering whether update to
 IANA IPFIX registry indicate update to all these IPFIX related RFC as well
 such as RFC7125,RFC7012 and etc?

2. Section 4.1.1 said:
“
  [I-D.ietf-opsawg-ipfix-tcpo-v6eh] specifies a new Information Element
   to fix the last issue.
”
It is not clear where or which section the procedure is specified from the
first glance. If my understanding is correct, the solution to address the last
issue is to define new IEs to address all the ipv6ExtensionHeaders IE
limitations rather than simply specifying the procedure.

To better clarify the relation between this document and
[I-D.ietf-opsawg-ipfix-tcpo-v6eh] and understand where the procedure is
specified, I propose the following change: s/ [I-D.ietf-opsawg-ipfix-tcpo-v6eh]
specifies a new Information Element/ Section 3 of
[I-D.ietf-opsawg-ipfix-tcpo-v6eh] specifies a new Information Element

3. Section 4.1.1 said:
“
Note that some implementations may not be able to export all observed
 extension headers in a Flow because of a hardware of software limit
 (see, e.g., [I-D.ietf-6man-eh-limits].
”
What is the reason for some implementation may not be able to export all
observed extension headers in a Flow? Software limit, hardware limit or
hardware/software limit, here the proposed change: s/a hardware of software
limit/a hardware or software limit

4.
Section 4.2.2 said:
"
4.2.2.  Update the Description of the tcpOptions IE

   This document requests IANA to update the description of the
   tcpOptions IE in the IANA IPFIX registry [IANA-IPFIX] as follows:

"
Section 3 said:
"
The current forwardingStatus entry in [IANA-IPFIX] deviates from what
   is provided in [RFC7270].

"
Section 4 said:
"
This document requests IANA to update the description of the
   following entries in [IANA-IPFIX].

"
You can see some places use "the IANA IPFIX registry [IANA-IPFIX]",
some places use "IANA-IPFIX", it is not consistent.



___
OPSAWG mailing list
OPSAWG@ietf.org
https://www.ietf.org/mailman/listinfo/opsawg