Re: [OPSAWG] [Tsv-art] Tsvart early review of draft-ietf-opsawg-tsvwg-udp-ipfix-03
On Jan 2, 2024, at 9:07 AM, Tommy Pauly via Datatracker wrote: > > Reviewer: Tommy Pauly > Review result: Almost Ready > > Thanks for writing a clear and succinct draft. I only found one issue of note, > around the registration of the new udpOptions Information Element. > > Section 4.1: > The data type used for the “udpOptions” entry is just listed as “unsigned”, > and > is described as being either an unsigned32 or an unsigned64. I understood this to be a bitfield of length 256, which MAY be shorter, and only in those shorter cases could it be UINT32 or UINT64, as per RFC7011. But that doesn’t seem consistent with RFC7011 either. AFAICT, this really should be octetArray, which COULD be shortened as needed. > However, when I > look at the registry at https://www.iana.org/assignments/ipfix/ipfix.xhtml, I > don’t see any entries that use this abstract “unsigned” type, and it is not > listed as an option in the element data types I would think it should have been indicated as “octetArray”. > . Is there a reason this shouldn’t > just be registered as an unsigned64? Yes, IMO - because UDP option Kind values higher than 63 are valid and could be used. > My understanding from > https://www.rfc-editor.org/rfc/rfc7011#section-6.2 is that an unsigned64 can > be > automatically encoded as an unsigned32 if the value is small enough, so the > definition can just use unsigned64. As per above, I don’t see how this helps given the field could be up to 265 bits long. Joe ___ OPSAWG mailing list OPSAWG@ietf.org https://www.ietf.org/mailman/listinfo/opsawg
[OPSAWG] 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. -- 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. 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. - 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. 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. - Section 1.1, "do no correspond" -> "do not correspond" ___ OPSAWG mailing list OPSAWG@ietf.org https://www.ietf.org/mailman/listinfo/opsawg
[OPSAWG] Tsvart early review of draft-ietf-opsawg-tsvwg-udp-ipfix-03
Reviewer: Tommy Pauly Review result: Almost Ready Thanks for writing a clear and succinct draft. I only found one issue of note, around the registration of the new udpOptions Information Element. Section 4.1: The data type used for the “udpOptions” entry is just listed as “unsigned”, and is described as being either an unsigned32 or an unsigned64. However, when I look at the registry at https://www.iana.org/assignments/ipfix/ipfix.xhtml, I don’t see any entries that use this abstract “unsigned” type, and it is not listed as an option in the element data types. Is there a reason this shouldn’t just be registered as an unsigned64? My understanding from https://www.rfc-editor.org/rfc/rfc7011#section-6.2 is that an unsigned64 can be automatically encoded as an unsigned32 if the value is small enough, so the definition can just use unsigned64. ___ OPSAWG mailing list OPSAWG@ietf.org https://www.ietf.org/mailman/listinfo/opsawg
