Hi,
Thanks for this document. Here are my AD review comments for
draft-ietf-opsawg-ipfix-srv6-srh-07. There is an obvious disclaimer here that
I'm not an IPFIX expert, and I appreciate that one of the authors is ;-).
Moderate level comments:
(1) p 4, sec 3. IPFIX Information Elements
srhIPv6Section
Exposes the SRH and its TLVs as specified in Section 2 of
[RFC8754] as series of n octets in IPFIX.
It isn't clear to me exactly what this is? Does this cover the whole SRH, or
just the "SRH TLVs" section? From looking at the examples, I think that this
exposes the whole SRH, in which case, I think that the description could be
clearer.
Please can you also update the description in the IANA section.
(2) p 12, sec 5.11. srhSegmentIPv6EndpointBehavior
ElementID: TBD11
Description: The 16-bit SRv6 Endpoint behavior. Assigned values and
their meanings are provided in the "SRV6 Endpoint Behavior" IANA
registry [IANA-IPFIX].
Is this linking to the right reference? It looks like this should be the SRv6
IANA registry rather than the IPFIX registry.
Minor level comments:
(3) p 4, sec 3. IPFIX Information Elements
srhSegmentIPv6BasicList
Ordered basicList [RFC6313] of zero or more 128-bit IPv6 addresses
in the SRH that represents the SRv6 segment list. As specified in
Section 2 of [RFC8754], the Segment List is encoded starting from
the last segment of the SR Policy. That is, the first element of
the Segment List (Segment List[0]) contains the last segment of
the SR Policy, the second element contains the penultimate segment
of the SR Policy, and so on.
In terms of naming, I note that the "IPv6" seems to turn up in difference
places in the IEs defined in this draft. It might be awkward to change, but
wouldn't the names be more consistent (e.g., with RFC8754), if they put IPv6
all in the same place? I was going to suggest putting them at the start (e.g.,
ipv6srh...), but looking at https://www.iana.org/assignments/ipfix/ipfix.xhtml,
it looks like the "IPv6" is often put at the end of the name.
E.g., srhSegmentIPv6BasicList -> srhSegmentBasicListIPv6, etc.
Note, I'm happy to defer to Benoit's judgement as an IPFIX expert here ...
(4) p 4, sec 3. IPFIX Information Elements
srhSegmentIPv6LocatorLength
The number of significant bits. Together with srhSegmentIPv6 it
enables the calculation of the SRv6 Locator.
I suggest "The number of significant bits" is reworded to something like: "The
IPv6 location length specified as the number of significant bits."
If you update this, then please can you make a similar clarification for the
IANA entry description.
(5) p 9, sec 5.6. srhSegmentIPv6ListSection
Description: The SRH Segment List as defined in section 2 of
[RFC8754] as series of n octets.
Am I right to assume that the length of the array must be a multiple of 16? Is
that worth specifying at all?
(6) p 26, sec Appendix A. IPFIX Encoding Examples
Figure 8: Data Set Encoding Format for SRH Section
Note, I don't really know IPFIX well enough to validate the examples, hence I'm
trusting the authors that they are correct.
Nit level comments:
(7) p 2, sec 1. Introduction
These IEs are used to export SRv6 active segment and its control
plane protocol, the SRv6 segment list, the next SRv6 node and its
type, and then numbers of SRv6 segments left.
"then numbers" -> "the numbers"?
(8) p 4, sec 3. IPFIX Information Elements
srhSegmentIPv6ListSection
Exposes the SRH Segment List as defined in Section 2 of [RFC8754]
as series of n octets in IPFIX.
I suggest "series of n octets" -> "sesies of octets", here and below.
(9) p 12, sec 6.1. SRv6 Segment List
The srhSegmentIPv6BasicList encodes the SRv6 segment list with a
basicList, specified in the IPFIX Structured Data [RFC6313]. This
encoding offers the advantage to the data collection that the
different IPv6 addresses are already structured as a list, without
the need of post processing. However, this method requires some
extra processing on the exporter, to realize the BasicList data
mapping.
Should it be "to the data collector" or is "to the data collection" correct?
(10) p 12, sec 6.1. SRv6 Segment List
The srhSegmentIPv6ListSection, on the other hand, encodes the list of
IPv6 addresses as an octetArray. This doesn't impose any data flow
manipulation on the exporter, facilitating the immediate export.
However, the data collection MUST be able to decode the IPv6
addresses according the SR specifications. Compared to the
srhSegmentIPv6BasicList, the srhSegmentIPv6ListSection flow records
length is slightly reduced.
'according the' -> 'according to the'
Regards,
Rob
_______________________________________________
OPSAWG mailing list
OPSAWG@ietf.org
https://www.ietf.org/mailman/listinfo/opsawg
