Re: [ippm] Lars Eggert's Discuss on draft-ietf-ippm-ioam-data-12: (with DISCUSS and COMMENT)

["Frank Brockners (fbrockne)" <[e-mail]>]

Hi Lars,

Thanks a lot for your detailed review - and sorry for the delay (PTO & Easter holidays..). Please see inline.

> -----Original Message-----
> From: Lars Eggert via Datatracker <[e-mail]>
> Sent: Donnerstag, 25. März 2021 13:30
> To: The IESG <[e-mail]>
> Cc: [e-mail]; [e-mail]; [e-mail];
> Al Morton <[e-mail]>; [e-mail]
> Subject: Lars Eggert's Discuss on draft-ietf-ippm-ioam-data-12: (with DISCUSS
> and COMMENT)
> 
> Lars Eggert has entered the following ballot position for
> draft-ietf-ippm-ioam-data-12: Discuss
> 
> 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.)
> 
> 
> Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> for more information about IESG DISCUSS and COMMENT positions.
> 
> 
> The document, along with other ballot positions, can be found here:
> https://datatracker.ietf.org/doc/draft-ietf-ippm-ioam-data/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> Section 5.4, paragraph 6, discuss:
> >    A particular implementation of IOAM MAY choose to support only one of
> >    the two trace option types.  In the event that both options are
> 
> Not requiring at least one mandatory-to-implement trace option type is highly
> problematic, since it creates two incompatible flavors of this standard.
> Preventing bifurcation seems to trump the desire for allowing (minor?)
> optimizations.

...FB: There seems to be some confusion and potential misunderstanding here: The two different Trace Option-Types would not lead to two incompatible flavors of the standard:
Each Trace Option-Type fills in the exact same IOAM data-fields - i.e. " node data list [i]" looks the exact same whether pre-allocated or incremental trace option types are used.
Each Trace Option-Type just handles a different “bucket” in the packet. 
And each Trace Option-Type caters for a different type of transit node: Software-based forwarders and Hardware-based forwarders. The differences between the two are not negligible. Memcopy is expensive for a software forwarder, as is a pointer-lookup in the packet for a hardware forwarder. The fact that there are two Option-Types that cater for different types of transit nodes is the result of long discussions. The two different ways to carry the data-fields is to ensure that IOAM can be deployed on a wide variety of devices. I'm afraid, that if we'd drop one variant, or make one variant mandatory, all we'd get is a slowed down adoption of the standard, because we would restrict IOAM tracing to either software forwarders or hardware forwarders.

That said, one thing that we should underline in the document is the fact that encap and decap nodes SHOULD be capable of inserting and removing both Trace-Option types. That way, there won’t be any compatibility challenge at all. Transit nodes of different type (HW or SW) would write the information into "their" type of bucket. Extracting the information at the decap node would be common in any case.

> 
> 
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Section 1, paragraph 4, comment:
> >    IOAM use cases and mechanisms have expanded as this document matured,
> >    resulting in additional flags and options that could trigger creation
> >    of additional packets dedicated to OAM.  The term IOAM continues to
> >    be used for such mechanisms, in addition to the "in-situ" mechanisms
> >    that motivated this terminology.
> 
> Suggest to rephrase this expanded view on IAM in a way that does not tie the
> description to the time period during which this soon-to-be-archival document
> was edited.

...FB: Thanks. Good suggestion - which we'll reflect in the next revision.

> 
> Section 5.2, paragraph 6, comment:
> >    A transit node MUST ignore IOAM-Option-Types that it does not
> >    understand.  A transit node MUST NOT add new IOAM-Option-Types to a
> >    packet, MUST NOT remove IOAM-Option-Types from a packet, and MUST
> NOT
> >    change the IOAM-Data-Fields of an IOAM Edge-to-Edge Option-Type.
> 
> I'm surprised that IOAM data isn't authenticated or even integrity-protected at
> all. Relying on RFC2119 language alone seems a pretty weak protection.

...FB: Integrity protection of IOAM data fields is handled in a sister document: draft-brockners-ippm-ioam-data-integrity.

> 
> Section 5.3, paragraph 9, comment:
> >    Namespace identifiers allow devices which are IOAM capable to
> >    determine:
> >
> >    o  whether IOAM-Option-Type(s) need to be processed by a device: If
> >       the Namespace-ID contained in a packet does not match any
> >       Namespace-ID the node is configured to operate on, then the node
> >       MUST NOT change the contents of the IOAM-Data-Fields.
> >
> >    o  which IOAM-Option-Type needs to be processed/updated in case there
> >       are multiple IOAM-Option-Types present in the packet.  Multiple
> >       IOAM-Option-Types can be present in a packet in case of
> >       overlapping IOAM-Domains or in case of a layered IOAM deployment.
> >
> >    o  whether IOAM-Option-Type(s) has to be removed from the packet,
> >       e.g. at a domain edge or domain boundary.
> 
> I'll note that cryptographically authenticating IOM data would probably result in
> a system that wouldn't need the concept of namespaces, because keys would
> automatically serve that purpose. (A device can't update an IOAM data item if it
> doesn't have the key to authenticate the update with.)

...FB: Looking at the way you interpreted the meaning of namespaces, we'll probably need to add some additional explanation of  the role of namespaces. The role of namespaces is to provide additional context to IOAM data fields - something that would be quite difficult to tie to a key which is likely going to be per node and potentially even per packet, depending on what scheme we'd use. The draft is pretty explicit about namespaces: “IOAM-Namespaces add further context to IOAM-Option-Types and associated IOAM-Data-Fields.", i.e. they are not intended as e.g. a tenant-identifier or similar. Looks like we need to think of some even more explicit wording here – maybe using some examples that explain the use of namespaces. I’m thinking of something simple such as “For Namespace 1, a “buffer” might be defined in units of “bytes” whereas for Namespace 2, a “buffer” might be defined in units of “packets”. This would likely also help with other questions, such as “why doesn’t the spec define an explicit unit for e.g. “buffers”.

> 
> Section 5.4, paragraph 11, comment:
> >    o  Time of day when the packet was processed by the node as well as
> >       the transit delay.  Different definitions of processing time are
> >       feasible and expected, though it is important that all devices of
> >       an in-situ OAM domain follow the same definition.
> 
> I think "important" is an understatement, this seems required? Also, capturing
> time-of-day seems to require synchronized clocks.

...FB: As usual, the answer is "in depends". If one is just interested in capturing jitter, then one might also do with non-synchronized clocks. 

> 
> Section 5.4.2.12, paragraph 2, comment:
> > 5.4.2.12.  buffer occupancy
> >
> >    The "buffer occupancy" field is a 4-octet unsigned integer field.
> >    This field indicates the current status of the occupancy of the
> >    common buffer pool used by a set of queues.  The units of this field
> >    are implementation specific.  Hence, the units are interpreted within
> >    the context of an IOAM-Namespace and/or node-id if used.  The authors
> >    acknowledge that in some operational cases there is a need for the
> >    units to be consistent across a packet path through the network,
> >    hence it is RECOMMENDED for implementations to use standard units
> >    such as Bytes.
> 
> There are other "standard units" here, such as packets. You'd need to
> recommend a specific standard unit and not just give an example.

...FB: The topic of "units" for IOAM data fields has been discussed quite a bit and not prescribing units is the result of this discussion. The approach that the draft takes is to "avoid specifying a unit unless you have to", because it is quite easy to translate units in backend analytics systems. As long as one properly understands which unit is used for a particular field, backend systems can easily interpret the data and there is no need to prescribe a particular unit. So the draft acknowledges the fact that different devices provide for different units to measure buffer occupancy.

> 
> Section 5.5, paragraph 3, comment:
> >    o  Random: Unique identifier for the packet (e.g., 64-bits allow for
> >       the unique identification of 2^64 packets).
> 
> If this identifier is supposed to be unique, it can't be random. And if it's random,
> it will only be able to statistically uniquely identify a much smaller number of
> packets (birthday paradox).

...FB: Good point. The next rev will avoid the use of "random" and be more specific. 

> 
> Section 6.3, paragraph 11, comment:
> >       Microseconds: specifies the fractional portion of the number of
> >       seconds since the epoch.
> >
> >       + Size: 32 bits.
> >
> >       + Units: the unit is microseconds.  The value of this field is in
> >       the range 0 to (10^6)-1.
> 
> Given that the max. value for microseconds is 999999, using a 32-bit field leaves
> the top eight bits unused.

...FB: True. Are you suggesting that we'd rather make the top 8-bits "reserved" because they will never be used anyway?

> 
> -------------------------------------------------------------------------------
> All comments below are very minor change suggestions that you may choose to
> incorporate in some way (or ignore), as you see fit. There is no need to let me
> know what you did with these suggestions.
> 
> Matt Joras' Gen-ART review
> (https://mailarchive.ietf.org/arch/msg/gen-art/vzngkYWy-W-
> f0PHqAPNRlyNSwnw/)
> contains several other nits that I wanted to make sure you were aware of.
> 
> "Abstract", paragraph 2, nit:
> -    protocols such as NSH, Segment Routing, Geneve, IPv6 (via extension
> -                                                        ^^^^^^^^^^^^^^^
> -    header), or IPv4.  In-situ OAM can be used to complement OAM
> -   ---------
> -    mechanisms based on e.g.  ICMP or other types of probe packets.
> -                            ^
> +    protocols such as NSH, Segment Routing, Geneve, IPv6,
> +                                                        ^
> +    or IPv4.  In-situ OAM can be used to complement OAM
> +    mechanisms based on, e.g., ICMP or other types of probe packets.
> +                       +     ^
> 
> Section 1, paragraph 2, nit:
> -    in [RFC7799] IOAM could be portrayed as Hybrid Type 1.  IOAM
> -    mechanisms can be leveraged where mechanisms using e.g.  ICMP do not
> -                                                           ^
> +    in [RFC7799], IOAM could be portrayed as Hybrid Type 1.  IOAM
> +                +
> +    mechanisms can be leveraged where mechanisms using, e.g., ICMP, do not
> +                                                      +     ^     +
> 
> Section 4, paragraph 3, nit:
> -    expected that each such encapsulation will be defined in the relevant
> -                                           ^ ^
> +    expected that each such encapsulation would be defined in the relevant
> +                                           ^^ ^
> 
> Section 4, paragraph 4, nit:
> -    IOAM data does not leak beyond the edge of an IOAM domain using,for
> +    IOAM data does not leak beyond the edge of an IOAM domain using, for
> +                                                                    +
> 
> Section 4, paragraph 4, nit:
> -    processing (e.g.  load-balancing schemes based on packet length could
> -                    ^
> -    be impacted by the increased packet size due to IOAM), path MTU (i.e.
> +    processing (e.g., load-balancing schemes based on packet length could
> +                    ^
> +    be impacted by the increased packet size due to IOAM), path MTU (i.e.,
> +
> + +
> 
> Section 4, paragraph 4, nit:
> -    message handling (i.e. in case of IPv6, IOAM support for ICMPv6 Echo
> +    message handling (i.e., in case of IPv6, IOAM support for ICMPv6 Echo
> +                          +
> 
> Section 4, paragraph 7, nit:
> -    are encapsulated into "parent" protocols, like e.g., NSH or IPv6 is
> +    are encapsulated into "parent" protocols, like, e.g., NSH or IPv6 is
> +                                                  +
> 
> Section 4, paragraph 8, nit:
> -    the-night model, i.e. IOAM-Data-Fields in one layer are independent
> +    the-night model, i.e., IOAM-Data-Fields in one layer are independent
> +                         +
> 
> Section 5.1, paragraph 3, nit:
> -    different categories.  In IOAM these categories are referred to as
> +    different categories.  In IOAM, these categories are referred to as
> +                                  +
> 
> Section 5.2, paragraph 2, nit:
> -    transit nodes".  The role of a node (i.e. encapsulating, transit,
> +    transit nodes".  The role of a node (i.e., encapsulating, transit,
> +                                             +
> 
> Section 5.2, paragraph 3, nit:
> -    called the "IOAM encapsulating node", whereas a device which removes
> -           ^^^
> -    an IOAM-Option-Type is referred to as the "IOAM decapsulating node".
> -                                          ^^^
> +    called an "IOAM encapsulating node", whereas a device which removes
> +           ^^
> +    an IOAM-Option-Type is referred to as an "IOAM decapsulating node".
> +                                          ^^
> 
> Section 5.2, paragraph 7, nit:
> -    means that an IOAM node which is e.g. an IOAM-decapsulating node for
> +    means that an IOAM node which is, e.g., an IOAM-decapsulating node for
> +                                    +     +
> 
> Section 5.3, paragraph 6, nit:
> -    assigned range is intended to be domain specific, and managed by the
> -                                           ^
> +    assigned range is intended to be domain-specific, and managed by the
> +                                           ^
> 
> Section 5.3, paragraph 9, nit:
> -       e.g. at a domain edge or domain boundary.
> +       e.g., at a domain edge or domain boundary.
> +           +
> 
> Section 5.4, paragraph 2, nit:
> -    deployment all nodes in an IOAM-Domain would participate in IOAM and
> +    deployment, all nodes in an IOAM-Domain would participate in IOAM and
> +              +
> 
> Section 5.4, paragraph 2, nit:
> -    ways to deal with situations where the PMTU was underestimated, i.e.
> +    ways to deal with situations where the PMTU was underestimated, i.e.,
> +
> + +
> 
> Section 5.4, paragraph 10, nit:
> -       i.e. ingress interface.
> +       i.e., ingress interface.
> +           +
> 
> Section 5.4, paragraph 11, nit:
> -       i.e. egress interface.
> +       i.e., egress interface.
> +           +
> 
> Section 5.4.1, paragraph 2, nit:
> -    i.e. an IOAM transit node MUST NOT modify the Namespace-ID, NodeLen,
> +    i.e., an IOAM transit node MUST NOT modify the Namespace-ID, NodeLen,
> +        +
> 
> Section 5.4.2.2, paragraph 6, nit:
> -    with ingressing or egressing packets, i.e. ingress_if_id could
> +    with ingressing or egressing packets, i.e., ingress_if_id could
> +                                              +
> 
> Section 5.6, paragraph 9, nit:
> -                encapsulating node e.g. by n-tuple based classification
> +                encapsulating node, e.g., by n-tuple based classification
> +                                  +     +
> 
> Section 5.6, paragraph 10, nit:
> -                encapsulating node e.g. by n-tuple based classification
> +                encapsulating node, e.g., by n-tuple based classification
> +                                  +     +

...FB: Thanks much. We'll fix all of those in the next rev (-12).

Thanks again for all your comments, Frank
> 
>