Re-,

Thanks Éric for the review.

Please check the candidate changes at: 
https://author-tools.ietf.org/api/iddiff?url_1=https://boucadair.github.io/ipfix-tcpoptions-and-v6eh/draft-ietf-opsawg-ipfix-tcpo-v6eh.txt&url_2=https://boucadair.github.io/ipfix-tcpoptions-and-v6eh/eric-vyncke-ietf-review/draft-ietf-opsawg-ipfix-tcpo-v6eh.txt

See more comments inline.

Cheers,
Med

> -----Message d'origine-----
> De : Éric Vyncke via Datatracker <nore...@ietf.org>
> Envoyé : mardi 2 juillet 2024 14:16
> À : The IESG <i...@ietf.org>
> Cc : draft-ietf-opsawg-ipfix-tcpo-v...@ietf.org; opsawg-
> cha...@ietf.org; opsawg@ietf.org; thomas.g...@swisscom.com;
> thomas.g...@swisscom.com
> Objet : Éric Vyncke's Yes on draft-ietf-opsawg-ipfix-tcpo-v6eh-
> 16: (with COMMENT)
> 
> 
> Éric Vyncke has entered the following ballot position for
> draft-ietf-opsawg-ipfix-tcpo-v6eh-16: Yes
> 
> When responding, please keep the subject line intact and reply to
> all email addresses included in the To and CC lines. (Feel free
> to cut this introductory paragraph, however.)
> 
> 
> 
> # Éric Vyncke, INT AD, comments for draft-ietf-opsawg-ipfix-tcpo-
> v6eh-16
> 
> Thank you for the work put into this document.
> 
> Please find below one blocking some non-blocking COMMENT points
> (but replies
> would be appreciated even if only for my own education), and one
> nit.
> 
> Special thanks to Thomas Graf for the shepherd's detailed write-
> up including
> the WG consensus and the justification of the intended status.
> 
> I hope that this review helps to improve the document,
> 
> Regards,
> 
> -éric
> 
> # COMMENTS (non-blocking)
> 
> ## Section 3
> 
> Suggest using the IE acronym rather than "Information Element" in
> the
> subsections of this section.
> 

[Med] Yes, we need to be consistent with the use IE (but don't say it loudly as 
Benoît may be around :-)) 

> ## Section 3.1
> 
> s/Type of an IPv6 extension header observed *in packets* of this
> Flow./Type of
> an IPv6 extension header observed *in at least one packet* of
> this Flow./ ? (or
> "in some packets")

[Med] OK

> 
> ## Section 3.2
> 
> I was wondering about this IE and the previous one as they looked
> quite atomic
> (plus the use of "consecutive") until I read the specification of
> ipv6ExtensionHeaderTypeCountList. Suggest adding a forward
> reference to section
> 3.4 and add some text that those 2 IEs can only occur in
> ipv6ExtensionHeaderTypeCountList. E.g., "This atomic IE MAY only
> occur in
> ipv6ExtensionHeaderTypeCountList as specified in section 3.4".

[Med] Idem as for the udp spec; I prefer to not cite the sections in the 
description. Went with the following:

NEW:
This IE is reported, e.g., in the ipv6ExtensionHeaderTypeCountList IE.

> 
> ## Section 3.3
> 
> It took me to read until section 8.4.1 to understand the the bit
> number is
> *NOT* the extension header type per RFC 8200. It is really
> confusing with `The
> "No Next Header" (59) value` text, which I also read as bit 59.
> 
> Strongly suggest adding a note on the value referring to
> NEW_IPFIX_IPv6EH_SUBREGISTRY not only in "Additional Information"
> but in
> "Description" (as it is really normative).

[Med] good catch. Went with "s/59/bit 2" for consistency 

We do already have the following in the description:

CURRENT:
  The IPv6 extension header associated with each bit is provided in
  [NEW_IPFIX_IPv6EH_SUBREGISTRY].

> 
> ## Section 3.4
> 
> `How an implementation disambiguates between unknown upper-layer
> protocols vs.
> extension headers is not IPFIX-specific` is true but why not
> specifying the
> expected behavior of the collector at least?

[Med] I don't think that's a good idea as an IPv6 implementation has to already 
support some mechanism to characterize unknown EH/Transport headers/etc. 

 Citing RFC 8883 as
> an example,
> does not really help in a PS. Alternatively, the IANA
> NEW_IPFIX_IPv6EH_SUBREGISTRY could redefine UNK as "unknown
> extension or
> transport header".

[Med] I can live with this. Updated the IANA section. Thanks.

> 
> ## Sections 3.5 & 3.6
> 
> Excellent idea :-) Thanks
> 
> ## Missing RH Type ?
> 
> It would be really to be able to also export the Routing Header
> type, perhaps
> in an optional new IE following ipv6ExtensionHeaderType in the
> list or by
> adding more values in NEW_IPFIX_IPv6EH_SUBREGISTRY (cfr also
> section 8.4.2 `For
> example, a registration may request two bits for a new EH to
> cover specific
> behaviors or uses of that EH.`)?

[Med] I understand that it is tempting to define this as well. However, we had 
to restrict the scope (not cover routing types and DST/HbB options) especially 
that there are already IEs covering specific routing types (SR, for example).

> 
> ## Section 4.1
> 
> `The value of tcpOptionsFull IE may be encoded in fewer octets`
> should it
> rather be a "SHOULD" (with explanations about the consequences of
> bypassing the
> SHOULD) ?

[Med] We spent some cycles about that one with DE before converging on current 
wording. The main argument from the DE was that MAY was used in the base spec 
and we should be consistent with that. 

> 
> s/The presence of tcpSharedOptionExID16List or
> tcpSharedOptionExID32List IEs is
> an indication that a shared TCP option (Kind=253 or 254) is
> observed in a
> Flow./The presence of tcpSharedOptionExID16List (Kind=253) or
> tcpSharedOptionExID32List (Kind=254) IEs is an indication that a
> shared TCP
> option is observed in a Flow./ ?

[Med] OLD is more accurate as the kind is in reference to the TCP options, not 
IEs.

> 
> ## Sections 4.2 and 4.3
> 
> Same comment as in sections 3.2 and 3.3.

[Med] Added a note.

> 
> ## Section 5
> 
> Should it be an "Implementation Consideration" ?

[Med] The last part is more ops. Changed the title to "Implementation and 
Operational Considerations"

> 
> # NITS (non-blocking / cosmetic)
> 
> ## Section 3.4
> 
> Should plural form be used for "header" in the example ?
> 

[Med] Right. Fixed.

> 

____________________________________________________________________________________________________________
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
To unsubscribe send an email to opsawg-le...@ietf.org

Reply via email to