Re: [ippm] Comments on draft-ietf-ippm-ioam-data-06

[Tom Herbert <[e-mail]>]

On Wed, Jul 24, 2019 at 4:07 PM Tom Herbert <[e-mail]> wrote:
>
> Hello,
>
> These are some comments on the draft that are motivated while working
> on the IOAM hackathon project.
>
> From the draft:
>
> "Bit 7    When set indicates presence of variable length Opaque State
> Snapshot field."
>
> It seems like this would put a variable data field in the middle of
> fixed length fields of the bit vector. Also, it seems like the length
> could vary in each node. Both of these are harsh on a parser.
>
> If variable data is needed, I suggest that it should immediately
> follow the last flag field. The reserved field byte in the trace
> header might be used to hold the length of the variable data.
>
> From the draft:
>
> "Bit 23 When set indicates presence of the Checksum Complement node data."
>
> I don't understand why this is needed. As I understand it, this is to
> offset a change in the UDP payload so that the UDP checksum remains
> correct. Since the node processing this already had to parse into UDP,
> why not just adjust the UDP checksum itself (like NAT does for
> instance)?
>
> Skipping bits when allocating in the bit vector is also problematic.
> In order to determine the offset of the Nth flag field, a node needs
> to know the lengths of the 0..N-1 fields, but the lengths of N+1 field
> and on are relevant. This is important for backwards compatibility as
> new flags are defined. For instance, when bit 12 is defined, a legacy
> implementation that needs to set the checksum complement at bit 23
> would be unable to determine the offset of the checksum complement.
>
RFC7820 describes the need for checksum complement to be after the
field being set. I do believe that skipping bits in the vector is
problematic as described above. This also might preclude extending the
bit vector as proposed below.

Tom

> Side note, per RFC7605:
>
> "It is important to recognize that any interpretation of port numbers
> -- except at the endpoints -- may be incorrect"
>
> This might be a nuisance if just reading UDP payload that is
> misinterpreted, but modifying misinterpreted UDP data, which could
> happen if IOAM data being set is encapsulated in UDP, would be
> systematic data corruption. Very bad!
>
> From the draft:
>
> "Bit 1    When set indicates presence of ingress_if_id and
> egress_if_id (short format) in the node data."
>
> and
>
> "Bit 9    When set indicates presence of ingress_if_id and
> egress_if_id in wide format in the node data."
>
> Having separate fields for different sizes of the same information
> awkward and inefficient. Consider that both bit 1 and bit 9 might be
> simultaneously set and two different values could be reported. In GUE
> we allow flags to be group to allow different sizes for a field and
> that might be useful here. For example, for ingress and egress ID a
> two bit flag could be defined where 00 indicates field not present, 01
> indicates 16 bit IDs, 10 indicates 32 bit IDs, 11 indicates 64 bit
> IDs.
>
> "IOAM-Trace-Type:  A 24-bit identifier which specifies which data
> types are used in this node data list."
>
> With more than half already allocated it seems like the bits could be
> exhausted relatively quickly especially if IOAM is a rousing success
> and people apply their wildest imagination as to what data to collect.
> I suggest to reserve the last bit of the vector. This bit will
> indicate a field is present that itself contains another set of flags.
> And in turn the expansion field's last bit can indicate another
> expansion field and so on.
>
> Tom