Re: [ippm] Alvaro Retana's No Objection on draft-ietf-ippm-ioam-data-12: (with COMMENT)

[Tal Mizrahi <[e-mail]>]

Hi Alvaro,

Thanks for the comments.
We are currently working on an updated version of the draft, including
text edits that address the comments you noted below.

Please see the responses and suggested text updates below, and please
let us know if there are any further comments.

Cheers,
Tal.

On Tue, Mar 23, 2021 at 7:58 PM Alvaro Retana via Datatracker
<[e-mail]> wrote:
>
> Alvaro Retana has entered the following ballot position for
> draft-ietf-ippm-ioam-data-12: No Objection
>
> 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/
>
>
>
> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
>
> I have several significant issues to bring up.  Individually, none of them
> raise to the point of a DISCUSS, so I'm balloting No Objection.
>
> (1) §5.3: "Any IOAM-Namespace MUST interpret the IOAM-Option-Types and
> associated IOAM-Data-Fields per the definition in this document."  This
> sentence seems to not say much beyond requiring compliance with this document,
> which is obvious since this document is the one defining the IOAM-Namespaces.
>

Suggested text edit:
OLD:
Any IOAM-Namespace MUST interpret the IOAM-Option-
Types and associated IOAM-Data-Fields per the definition in this
document.  IOAM-Namespaces group nodes to support different
deployment approaches
NEW:
The IOAM-Option-Types and associated IOAM-Data-Fields are interpreted
as defined in this document, regardless of the value of the IOAM-Namespace.
However, IOAM-Namespaces group nodes to support different
deployment approaches


> (2) §5.3: "IOAM-Namespace identifiers MUST be present and populated in all
> IOAM-Option-Types."   What does "populated" mean?  I assumed that it meant that
> the field has to have a value in it, but then I found out that the
> Default-Namespace-ID is 0x0000.  This seems to mean that the receiver can't
> tell the difference between the sender forgetting to populate the field and the
> default.  IOW, it seems to me that requiring that the field be populated
> doesn't serve a specific need and cannot be normatively enforced.
>

Suggested text edit:
OLD:
IOAM-Namespace identifiers MUST be present and populated in all
IOAM-Option-Types.
NEW:
The IOAM-Namespace field is included in all the IOAM-Option-Types
defined in this document, and MUST be included in all future
IOAM-Option-Types.


> (3) §5.3: Given the example, I don't think this statement is true:
>
>    "...node identifiers might not be unique for other organizational reasons,
>    such as after a merger of two formerly separated organizations), the
>    combination of node_id and Namespace-ID will always be unique."
>
> It seems to me that for the same reason that merged organizations can end up
> with overlapping node_ids, they can also end up with overlapping Namespace-IDs.
>  If not deployed in an overlapping way (on the same set of nodes), it seems
> that it may be ok to have the same Namespace-ID in multiple places.
>
> This deployment consideration should be better explained in this document.  I
> took a quick look at draft-brockners-opsawg-ioam-deployment and this item is
> not covered there either.
>

The Node ID disambiguation is an example in the context of the
NamespaceID, and is not necessarily the main use case of the Namespace
ID. Therefore, additional text in the deployment draft is probably not
required. However, your point that the current text needs to be
clarified is well taken, and a proposed text update is presented
below.

Suggested text edit:
OLD:
IOAM-Namespaces provide additional context for IOAM-Data-Fields
and thus ensure that IOAM-Data-Fields are unique and can be
interpreted properly by management stations or network
controllers.  While, for example, the node identifier field
(node_id, see below) does not need to be unique in a deployment
(e.g. if an operator wishes to use different node identifiers for
different IOAM layers, even within the same device; or node
identifiers might not be unique for other organizational reasons,
such as after a merger of two formerly separated organizations),
the combination of node_id and Namespace-ID will always be unique.
NEW:
IOAM-Namespaces provide additional context for IOAM-Data-Fields
and thus can be used to ensure that IOAM-Data-Fields are unique and are
interpreted properly by management stations or network
controllers.  For example, the node identifier field
(node_id, see below) does not need to be unique in a deployment
(e.g. if an operator wishes to use different node identifiers for
different IOAM layers, even within the same device; or node
identifiers might not be unique for other organizational reasons,
such as after a merger of two formerly separated organizations),
the Namespace-ID can be used as a context identifier, such that
the combination of node_id and Namespace-ID will always be unique.


> (4) I think that the encapsulation-independent pieces of
> draft-brockners-opsawg-ioam-deployment would be better placed in this document.
>

The deployment draft is aimed at discussing various deployment
considerations and any other aspects that are not strictly
data-field-related (and belong in the data draft). Therefore, our
feeling is that the encapsulation independent aspects belong in the
deployment draft.

> (5) §5.4: "Any deployment MAY choose to configure and support one or both of
> the following options."  This sentence looks like a statement of fact and not a
> normative statement. s/MAY/may
>

Suggested text edit:
OLD:
Any deployment MAY choose to
configure and support one or both of the following options.
NEW:
A deployment can choose to configure and support one or both of the
following options.

> (6) §5.4.1: "The Namespace-ID value of 0x0000 is defined as the
> "Default-Namespace-ID" (see Section 5.3) and MUST be known to all the nodes
> implementing IOAM."  The second part ("MUST be known...") is not needed because
> this document is defining the default value...
>

Although this part of the sentence might be omitted, we prefer to
leave it there to clarify the point.

> (7) §5.4.1:
>
>       An IOAM encapsulating node MUST set NodeLen.
>
>       A node receiving an IOAM Pre-allocated or Incremental Trace-Option
>       relies on the NodeLen value, or it can ignore the NodeLen value
>       and calculate the node length from the IOAM-Trace-Type bits (see
>       below).
>
> Assuming that the NodeLen is not 0 (i.e. set), when can the receiver ignore it?
>  If NodeLen is 0 (not set), what should the receiver do?   The text above
> requires NodeLen to be set, but then it makes its use optional.
>

Right, this was also raised by Ben Kaduk in his IESG review, and our
suggestion is to remove the second part of the paragraph: "or it can
ignore the NodeLen value..."

> (8) §5.4.1: "RemainingLen: ...the sender MAY set the initial value of
> RemainingLen according to the number of node data bytes allowed before
> exceeding the MTU. ... When node data is added, the node MUST decrease
> RemainingLen by the amount of data added."
>
> This text includes a normative inconsistency.  If the sender doesn't initialize
> the "value of RemainingLen according to the number of node data bytes allowed"
> (because it is optional!), and the transit node does "decrease RemainingLen by
> the amount of data added" (because it is required), then the value won't
> reflect the data space remaining and a downstream node may add enough bits to
> overflow -- or not add anything because it considered the overflow imminent.
>

We suggest the following update:
OLD:
      Given that the sender knows the path MTU (PMTU), the sender MAY
      set the initial value of RemainingLen according to the number of
      node data bytes allowed before exceeding the MTU.  Subsequent
      nodes can carry out a simple comparison between RemainingLen and
      NodeLen, along with the length of the "Opaque State Snapshot" if
      applicable, to determine whether or not data can be added by this
      node.  When node data is added, the node MUST decrease
      RemainingLen by the amount of data added.
NEW:
      The sender MUST assign the initial value of the RemainingLen field.
      The sender MAY calculate the value of the RemainingLen field by
      computing the number of node data bytes allowed before exceeding
      the path MTU (PMTU), given that the PMTU is known to the sender.
Subsequent
      nodes can carry out a simple comparison between RemainingLen and
      NodeLen, along with the length of the "Opaque State Snapshot" if
      applicable, to determine whether or not data can be added by this
      node.  When node data is added, the node MUST decrease
      RemainingLen by the amount of data added.

> (9) §5.4.1: "If RemainingLen in a pre-allocated trace option exceeds the length
> of the option, as specified in the preceding header, then the node MUST NOT add
> any fields."   I didn't find "the length of the option" defined anywhere.  Did
> I miss it?
>

Suggested text:
OLD
If RemainingLen in a pre-allocated trace option exceeds the length
of the option, as specified in the preceding header, then the node MUST NOT add
any fields.
NEW
If RemainingLen in a pre-allocated trace option exceeds the length
of the option, as specified in the lower layer header (which is
not within the scope of this document), then the node MUST NOT add
any fields.

> (10) §5.4.1:
>
>     Bit 12-21  Undefined.  An IOAM encapsulating node MUST set the
>                value of each of these bits to 0.  If an IOAM transit
>                node receives a packet with one or more of these bits set
>                to 1, it MUST either:
>
>                1.  Add corresponding node data filled with the reserved
>                    value 0xFFFFFFFF, after the node data fields for the
>                    IOAM-Trace-Type bits defined above, such that the
>                    total node data added by this node in units of
>                    4-octets is equal to NodeLen, or
>
> This first option assumes that all undefined data fields will be 4-octets long.
>  But if future data fields are defined to be 8-octets (and the transit node
> doesn't understand them) then 0xFFFFFFFF won't be enough and there will be a
> misalignment.  IOW, I don't think it is possible to require this behavior --
> unless it is also required that all future data fields have to be 4-octets long.
>

Right, we agree that the right way to resolve this is by mandating
future node data fields to be 4-octets long.
We suggest the following update:

OLD:
      Bit 12-21  Undefined. An IOAM encapsulating node MUST set the
               value of each of these bits to 0.  If an IOAM transit
               node receives a packet with one or more of these bits set
               to 1, it MUST either:
NEW:
      Bit 12-21  Undefined. These values are available for future
               assignment in the IOAM Trace-Type Registry (Section 8.2).
               Every future node data field corresponding to one of these
               bits MUST be 4-octets long.
               An IOAM encapsulating node MUST set the
               value of each of these bits to 0.  If an IOAM transit
               node receives a packet with one or more of these bits set
               to 1, it MUST either:

> (11) Some of the values to be included in the data fields (§5.4.*) are not well
> defined and could result in nodes reporting inconsistent measurements.
> Specifically, transit delay and queue depth.  It would be ideal if the document
> provided some guidance for the implementation/use.
>

We propose to add the following text to Section 5.4:

NEW:
It should be noted that the semantics of some of the node data fields
that are defined below, such as the queue depth and buffer occupancy,
are implementation specific. This approach is intended to allow IOAM
nodes with various different architectures.

> (12) §8.7: "Upon a new allocation request, the responsible AD will appoint a
> designated expert, who will review the allocation request."   This sentence is
> not needed: the assignment happens only once and not when whenever "a new
> allocation request" comes up.

Suggested text:
OLD:
Upon a new allocation request, the responsible AD will
appoint a designated expert, who will review
NEW:
Upon a new allocation request, a designated expert will review