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

From: "Frank Brockners (fbrockne)" <[email-masked]>
Date: Thu, 1 Aug 2019 10:59:11 +0000

Hi Tom,

Thanks for your comments. Couple of thoughts:

* Bit 7 / Opaque State Snapshot:

Per the discussion in the WG meeting in Montreal: The field could indeed lead to variable length data being inserted into the packet. That said, the envisioned use would be that for a specific deployment or IOAM domain, the "length" would be a fixed value so that a parser could be preconfigured in a proper way. Those could even be specified as part of an IOAM profile (draft-mizrahi-ippm-ioam-profile-00). 
For the next revision of the draft, we should add a deployment consideration for "Opaque State Snapshot" to reflect this discussion.

* Bit 23 / Checksum complement:

The idea for the checksum complement was indeed what you mention: Avoid the need to update the packet's UDP checksum by an intermediate entity, which isn't be able to update the UDP checksum itself. One can argue whether these nodes would exist - per what you say, but from what I remember, this option was added to be on the safe side. The field was added way back (https://github.com/inband-oam/ietf/pull/41). I'm hoping that Tal could shed some further light it.

* Short / wide format of fields:

You note that "Having separate fields for different sizes of the same information awkward and inefficient." - thought these fields don't necessarily need to carry the same content. Both fields (short and wide) could indeed be present in the same packet and can complement each other. Given that the term "interface" isn't further defined, what an IOAM domain is going to associate with interface is flexible, e.g. "interface_short" could be an identifier for the physical interface, whereas "interface_wide" could be an identifier for a logical sub-interface of that physical interface.
Per your suggestion - given that we use 2 bits, we could consider making things more flexible by using all of the four potential values of the 2-bit number (like your 16/32/64 example). The question is: Is this needed? I'd appreciate additional opinions.

* 24 bits Trace type 

Per your suggestion, reserving a bit to allow for future scalability makes sense - and bit 23 would be an obvious choice; which would in turn mean that we'd need to assign a different bit for checksum complement. If everyone else is fine with this change, we can include this change in -07.

Thanks again, Frank

> -----Original Message-----
> From: ippm <[email-masked]> On Behalf Of Tom Herbert
> Sent: Dienstag, 30. Juli 2019 17:20
> To: IETF IPPM WG <[email-masked]>
> Subject: Re: [ippm] Comments on draft-ietf-ippm-ioam-data-06
> 
> On Wed, Jul 24, 2019 at 4:07 PM Tom Herbert <[email-masked]> 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
> 
> _______________________________________________
> ippm mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/ippm

---

From: Tom Herbert <[email-masked]>
Date: Sat, 3 Aug 2019 12:42:40 -0700

> For the next revision of the draft, we should add a deployment consideration for "Opaque State Snapshot" to reflect this discussion.
Hi Frank,

Preconfiguring a fixed length across a domain seems like a can of
worms to me. Maybe some domain has reason to use different lengths
simultaneously, or what happens the day that some domain changes
length? My comment wasn't in opposition of variable length opaque
data, it is a question of where the data should be placed in the
packet. I juat think it should follow any defined flag-fields (you
might want to look at how GUE handles private data which immediate
follows the flag-fields).

> The idea for the checksum complement was indeed what you mention: Avoid the need to update the packet's UDP checksum by an intermediate entity, which isn't be able to update the UDP checksum itself…

Yes, I believe that it could be useful in the UDP case. My concern is
the placement of the field so that it skips over bits yet to be
defined. This makes backwards compatibility difficult when new fields
are defined that deployed devices won't understand and know their
lengths to find the offset of checksum complement field.

> You note that "Having separate fields for different sizes of the same information awkward and inefficient." - thought these fields don't necessarily need to carry the same content. Both fields (sho…

Okay, but I wonder how that will work for interoperability. i.e. is
there a problem if different implementations apply different semantics
to fields?

> Per your suggestion, reserving a bit to allow for future scalability makes sense - and bit 23 would be an obvious choice; which would in turn mean that we'd need to assign a different bit for check…

One other minor suggestion for the checksum field is to define it as
32 bits instead of just 16 since the field size is 32 bits regardless.
This might save some folding operations when writing it. For instance,
if a 32 bit field is set with initial value of zero, we would just
need to set the checksum field to the not of the value being set
(assuming checksum field is also initialized zero, otherwise it's a
one's complement add)-- no need to fold to sixteen bits. Of course, if
an implementation really wants to fold the result to sixteen bits it
can and still results in the same effect.

Tom

---

From: "Frank Brockners (fbrockne)" <[email-masked]>
Date: Mon, 5 Aug 2019 07:07:31 +0000

Hi Tom,

> From: Tom Herbert <[email-masked]>
> Sent: Samstag, 3. August 2019 21:43
> To: Frank Brockners (fbrockne) <[email-masked]>
> Cc: IETF IPPM WG <[email-masked]>
> > Per the discussion in the WG meeting in Montreal: The field could indeed lead
> to variable length data being inserted into the packet. That said, the envisioned
> use would be that for a specific deployment or IOAM domain, the "length"
> would be a fixed value so that a parser could be preconfigured in a proper way.
> Those could even be specified as part of an IOAM profile (draft-mizrahi-ippm-
> ioam-profile-00).
> > For the next revision of the draft, we should add a deployment consideration
> for "Opaque State Snapshot" to reflect this discussion.
> >
> Hi Frank,
> 
> Preconfiguring a fixed length across a domain seems like a can of worms to me.
> Maybe some domain has reason to use different lengths simultaneously, or
> what happens the day that some domain changes length? My comment wasn't
> in opposition of variable length opaque data, it is a question of where the data
> should be placed in the packet. I juat think it should follow any defined flag-
> fields (you might want to look at how GUE handles private data which
> immediate follows the flag-fields).

... FB: Not sure I follow the "can of worms" statement. Choosing a length is just an optimization. 
It would allow you to pre-compute things - so that during operations, you'd just check validity instead of doing the actual calculations.

Thanks for the pointer to GUE. Ordering the bits so that parsing becomes easier makes sense. So you seem to suggest that we make "opaque state snapshot" bit 22 (assuming that we're reserve 23 for future extension and move checksum - as a fixed size field further up)?

> > The idea for the checksum complement was indeed what you mention: Avoid
> the need to update the packet's UDP checksum by an intermediate entity, which
> isn't be able to update the UDP checksum itself. One can argue whether these
> nodes would exist - per what you say, but from what I remember, this option
> was added to be on the safe side. The field was added way back
> (https://github.com/inband-oam/ietf/pull/41). I'm hoping that Tal could shed
> some further light it.
> 
> Yes, I believe that it could be useful in the UDP case. My concern is the
> placement of the field so that it skips over bits yet to be defined. This makes
> backwards compatibility difficult when new fields are defined that deployed
> devices won't understand and know their lengths to find the offset of checksum
> complement field.
> 
> >
> > You note that "Having separate fields for different sizes of the same
> information awkward and inefficient." - thought these fields don't necessarily
> need to carry the same content. Both fields (short and wide) could indeed be
> present in the same packet and can complement each other. Given that the term
> "interface" isn't further defined, what an IOAM domain is going to associate
> with interface is flexible, e.g. "interface_short" could be an identifier for the
> physical interface, whereas "interface_wide" could be an identifier for a logical
> sub-interface of that physical interface.
> 
> Okay, but I wonder how that will work for interoperability. i.e. is there a
> problem if different implementations apply different semantics to fields?

...FB: This is why IOAM has namespaces. IOAM does not define any semantics of the fields carried - this means that the semantics can (and are also expected to) change from namespace to namespace.  

> 
> > Per your suggestion - given that we use 2 bits, we could consider making things
> more flexible by using all of the four potential values of the 2-bit number (like
> your 16/32/64 example). The question is: Is this needed? I'd appreciate
> additional opinions.
> >
> > * 24 bits Trace type
> >
> > Per your suggestion, reserving a bit to allow for future scalability makes sense
> - and bit 23 would be an obvious choice; which would in turn mean that we'd
> need to assign a different bit for checksum complement. If everyone else is fine
> with this change, we can include this change in -07.
> 
> This might save some folding operations when writing it. For instance, if a 32 bit
> field is set with initial value of zero, we would just need to set the checksum field
> to the not of the value being set (assuming checksum field is also initialized zero,
> otherwise it's a one's complement add)-- no need to fold to sixteen bits. Of
> course, if an implementation really wants to fold the result to sixteen bits it can
> and still results in the same effect.

...FB: Sounds reasonable - but I'd like to hear other opinions as well - especially Tal's who wrote the initial patch for checksum complement.

Thanks, Frank

> > > > These are some comments on the draft that are motivated while
> > > > working on the IOAM hackathon project.
> > > >
> > > > From the draft:
> > > >
> > > > 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.
> > > >
> > > > 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)?
> > > >
> > > > 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:
> > > >
> > > > 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.
> > > >
> > > > 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.