Re: [Tsv-art] Tsvart early review of draft-ietf-opsawg-ipfix-tcpo-v6eh-05

[[e-mail]]

Hi Wes, 

Thank you for the review. 

The changes to take into account your review can be seen at: https://github.com/boucadair/ipfix-tcpoptions-and-v6eh/commit/8bd7d7f92180a8eeaeb9ce13c0028a5c698b1738

Please see inline for more context. 

Cheers,
Med

> -----Message d'origine-----
> De : Wesley Eddy via Datatracker <[e-mail]>
> Envoyé : mardi 2 janvier 2024 19:20
> À : [e-mail]
> Cc : [e-mail];
> [e-mail]
> Objet : Tsvart early review of draft-ietf-opsawg-ipfix-tcpo-v6eh-
> 05
> 
> Reviewer: Wesley Eddy
> Review result: Ready with Issues
> 
> Comments:
> 
> - The document is well-written and easy to read.
> 
> - Section 6 is really nice and helpful!
> 
> Issues:
> 
> - The way an implementation understands the TCP ExIDs may benefit
> from slightly more explanation:
>   -- In 4.2 and 4.3, is the idea that the implementation is just
> sampling the
>   16 or 32 bits following the experimental option kind being
> indicated, and
>   assuming those 2 or 4 bytes to be ExIDs?  From Section 6.2, I
> got the sense
>   that the implementation is aware of particular ExID values
> specifically, to
>   know if they should be reported as 2 or 4 byte values.

[Med] 2-byte IDs are reported in tcpSharedOptionExID16 while 4-byte IDs are reported in tcpSharedOptionExID32. 

 -- Will
> any values
>   present be reported, or only those which show up in the IANA
> registry?  I
>   assume any values will be reported, even if they are not
> registered ExIDs,
>   since the registry changes over time, and implementations
> probably don't grab
>   periodic updates of it.

[Med] Any observed ID must be reported independent of whether the value is registered or not. Made "s/Observed/Any observed" in the relevant sections.

> 
> Questions:
> 
> - This may be alright, but it seemed to me like for
> interoperability there should be some way to indicate what an
> implementation of this IE is doing with regard to this text in
> Section 3.1 (where maybe "may" should be "MAY"?):
> 
>       Several extension header chains may be observed in a Flow.
> These
>       extension headers may be aggregated in one single
>       ipv6ExtensionHeadersFull Information Element or be exported
> in
>       separate ipv6ExtensionHeadersFull IEs, one for each
> extension
>       header chain.
> 

[Med] I went with this change:  s/may be aggregated/MAY be aggregated

> - In Section 3.3, it seems backwards to me that this Limit IE
> being True means that no limitation was applied, whereas False
> means that it was limited.  If the WG and implementers are okay
> with this, I'm not questioning it, but it seems odd, so I just
> wanted to make sure this was the intention.
> 

[Med] ACK.

> Nits:
> 
> - The first paragraph in section 1 should probably mention the
> specific RFC(s) for the specified IEs with the noted problems,
> since it sounds from the beginning paragraphs of section 3 and 4
> like some of those are already being addressed by the separate
> ipfix-fixes document.

[Med] Added a reference to the IANA registry as that is the normative ref for these IEs as per the following from RFC7012:


   [IANA-IPFIX] is now the normative reference for IPFIX Information
   Elements.  When [RFC5102] was published, it defined, in its
   Section 5, the initial contents of that registry.

> 
> - Section 1.1, "do no correspond" -> "do not correspond"

[Med] 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.