INT extensions for domain specific features #54

rsivakolundu · 2018-09-28T18:27:22Z

No description provided.

Updated the location of Domain Specific ID and DS Flags

telemetry/specs/INT.mdk

Removed the "S" bit in the first part of the header. Added Domain Specific Field.

Corrected typo

Corrected typo : requring - requiring

telemetry/specs/INT.mdk

Removed the "S" bit.

telemetry/specs/INT.mdk

Deleted Version 1 header. Modified the examples to include the extended headers.

jklr · 2019-10-31T05:25:07Z

telemetry/specs/INT.mdk

-  except if bit 6 is set, which requires 8 bytes of metadata. Per-hop
-  metadata length is set accordingly at the INT source.
-
+  Bits 0 - 13 are Baseline INT Instructions and Bit 14, Domain Specific Instruction.


"Bit 14 is Domain ..."

minor nit, let's correct.

Also bit 14 should be added to the list of bits above.

Removed Bit 14 and added text to address Domain Specific processing

jklr

Do we want to leave some notes how it's related to the Domain Specific Extension of the report format?

Removed Bit 14: Domain Specific Instruction

- Removed "Rep" - Removed special "Synthetic/Probe sections"; replaced with clarification of using C bit for such traffic - Allow Transit nodes to do OAM generation and NW Event Detection - Allow Transit nodes to be INT Source for the Domain-specific parts - Some minor cleanup, length fixes in examples, etc.

mickeyspiegel

I see two major differences from the IETF's IOAM namespace beyond the additional fields:

When a transit node receives an INT packet with a Domain Specific ID that it does not recognize, is it still allowed to add its metadata to the packet?
Does the Domain Specific ID have any bearing on the semantics and/or metadata units? In the IETF this seems to be the main point of namespace.
It would be good to hash this out further, either aligning with the IETF or clarifying at least to ourselves if not in the spec, why should this be different?

telemetry/specs/INT.mdk

mickeyspiegel · 2020-01-08T21:50:38Z

telemetry/specs/INT.mdk

   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-   |      Instruction Bitmap       |            Reserved           |
+   |         DS Instruction        |            DS Flags           |     


Nit: whitespace at the end of the line

You added whitespace at the end of the line. I thought it should be removed.

telemetry/specs/INT.mdk

mickeyspiegel · 2020-01-09T00:13:12Z

telemetry/specs/INT.mdk

-  except if bit 6 is set, which requires 8 bytes of metadata. Per-hop
-  metadata length is set accordingly at the INT source.
-
+  Bits 0 - 13 are Baseline INT Instructions and Bit 14, Domain Specific Instruction.


Also bit 14 should be added to the list of bits above.

telemetry/specs/INT.mdk

mickeyspiegel · 2020-01-09T01:11:25Z

telemetry/specs/INT.mdk

@@ -1141,9 +1174,11 @@ INT Metadata Header and Metadata Stack, followed by TCP payload:
    0                   1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-   | Ver=1 | 0 |0|0|0|     Reserved      | HopML=2 |RemainingHopC=6|
+   | Ver=2 | 0 |0|0|0|0|   Reserved      | HopML=2 |RemainingHopC=6|


I don't see any definition of a new flag bit after the "M" bit.
The only change here should be "Ver=1" --> "Ver=2".

What Flag bit after M are you referring to?

This change adds a fourth 0 before the Reserved field. That does not belong.

mickeyspiegel · 2020-01-09T01:11:55Z

telemetry/specs/INT.mdk

@@ -1196,9 +1231,11 @@ INT Metadata Header and Metadata Stack:
    0                   1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-   | Ver=1 | 0 |0|0|0|     Reserved      | HopML=2 |RemainingHopC=5|
+   | Ver=2 | 0 |0|0|0|0|   Reserved      | HopML=2 |RemainingHopC=5|


I don't see any definition of a new flag bit after the "M" bit.
The only change here should be "Ver=1" --> "Ver=2".

mickeyspiegel · 2020-01-09T01:12:11Z

telemetry/specs/INT.mdk

@@ -1253,9 +1290,11 @@ INT Metadata Header and Metadata Stack:
    0                   1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-   | Ver=1 | 0 |0|0|0|     Reserved      | HopML=2 |RemainingHopC=5|
+   | Ver=2 | 0 |0|0|0|0|   Reserved      | HopML=2 |RemainingHopC=5|


I don't see any definition of a new flag bit after the "M" bit.
The only change here should be "Ver=1" --> "Ver=2".

Removed the new flag bit. We need to discuss the need for a bit to indicate the VxLAN vs VxLAN GPE encap of the incoming packet. Do we need that indication in the header for the sink node processing.

We're addressing the GPE discussion and flag in #58.
Let's merge this PR first into mater and rebase #58.

telemetry/specs/INT.mdk

Updated INT pre-2.0

telemetry/specs/INT.mdk

mickeyspiegel

I see two major differences from the IETF's IOAM namespace beyond the additional fields:

When a transit node receives an INT packet with a Domain Specific ID that it does not recognize, is it still allowed to add its metadata to the packet?
Does the Domain Specific ID have any bearing on the semantics and/or metadata units? In the IETF this seems to be the main point of namespace.
It would be good to hash this out further, either aligning with the IETF or clarifying at least to ourselves if not in the spec, why should this be different?

Remaining changes are minor.
Remove some duplicate text.
Fix typos.
Fix packet format changes that no longer apply.
Check packet length in example.

telemetry/specs/INT.mdk

mickeyspiegel · 2020-01-20T01:23:10Z

telemetry/specs/INT.mdk

@@ -1141,9 +1174,11 @@ INT Metadata Header and Metadata Stack, followed by TCP payload:
    0                   1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
-   | Ver=1 | 0 |0|0|0|     Reserved      | HopML=2 |RemainingHopC=6|
+   | Ver=2 | 0 |0|0|0|0|   Reserved      | HopML=2 |RemainingHopC=6|


This change adds a fourth 0 before the Reserved field. That does not belong.

mickeyspiegel · 2020-01-20T01:28:15Z

telemetry/specs/INT.mdk

@@ -1194,9 +1212,11 @@ INT Metadata Header and Metadata Stack, followed by TCP payload:
    0                   1                   2                   3


The length above in the shim header does not seem correct.

telemetry/specs/INT.mdk

jklr · 2020-01-23T02:13:37Z

@rsivakolundu
I wonder if we can prioritize this PR and merge first than others.
The change of this PR is applied to every INT mode (old and new) and the examples.
If the key section is updated and merged into master, I can update the other examples (under different transports) and add new examples accordingly.

Corrected typos found by Mickey and other changes requested.

jklr · 2020-01-28T19:07:09Z

Thanks @rsivakolundu for the fixes. They look good to me.
Can you pls comment on @mickeyspiegel 's questions below?
I put my answers.

I see two major differences from the IETF's IOAM namespace beyond the additional fields:

When a transit node receives an INT packet with a Domain Specific ID that it does not recognize, is it still allowed to add its metadata to the packet?

[JK] I didn't think through but the answer depends on the Yang-model-based query mechanism.
Yes, if the collector can ping each device and get the metadata semantics.
No, if not.
Agree we better clarify this in the spec.

Does the Domain Specific ID have any bearing on the semantics and/or metadata units? In the IETF this seems to be the main point of namespace.
It would be good to hash this out further, either aligning with the IETF or clarifying at least to ourselves if not in the spec, why should this be different?

[JK] associating the domain ID to the metadata semantics seems a good idea to me.

jklr · 2020-01-29T02:34:16Z

Let's merge it and address the domain ID questions in a separate PR on top of master.

rsivakolundu added 7 commits June 21, 2018 15:03

Update INT.mdk

3771848

Update INT.mdk

10355fd

Update INT.mdk

fe346e4

Update INT.mdk

0ea8228

Update INT.mdk

fd1d540

Update INT.mdk

5244e63

Update INT.mdk

4921765

Updated the location of Domain Specific ID and DS Flags

mickeyspiegel requested changes Dec 5, 2018

View reviewed changes

rsivakolundu added 3 commits December 5, 2018 15:56

Update INT.mdk

7f16f73

Removed the "S" bit in the first part of the header. Added Domain Specific Field.

Update INT.mdk

ede50c6

Corrected typo

Update INT.mdk

e206567

Corrected typo : requring - requiring

mickeyspiegel reviewed Dec 6, 2018

View reviewed changes

telemetry/specs/INT.mdk Outdated Show resolved Hide resolved

mickeyspiegel reviewed Dec 6, 2018

View reviewed changes

telemetry/specs/INT.mdk Outdated Show resolved Hide resolved

Update INT.mdk

fba9dff

Removed the "S" bit.

jklr reviewed Dec 6, 2018

View reviewed changes

telemetry/specs/INT.mdk Outdated Show resolved Hide resolved

telemetry/specs/INT.mdk Outdated Show resolved Hide resolved

telemetry/specs/INT.mdk Outdated Show resolved Hide resolved

rsivakolundu and others added 3 commits December 6, 2018 14:40

Update INT.mdk

a68275a

Deleted Version 1 header. Modified the examples to include the extended headers.

Merge branch 'master' into INT-Extensions-for-Domain-Specific-Features

df6c5ed

Merge branch 'master' into INT-Extensions-for-Domain-Specific-Features

7c83a83

jklr reviewed Oct 31, 2019

View reviewed changes

rsivakolundu and others added 2 commits December 11, 2019 13:12

Update INT.mdk

721d3c8

Removed Bit 14: Domain Specific Instruction

mickeyspiegel requested changes Jan 9, 2020

View reviewed changes

rsivakolundu added 3 commits January 16, 2020 14:48

Update INT.mdk

b36643d

Merge branch 'INT-Extensions-for-Domain-Specific-Features' into patch-2

310c922

Merge pull request #2 from orr101/patch-2

de12740

Updated INT pre-2.0

mickeyspiegel requested changes Jan 17, 2020

View reviewed changes

telemetry/specs/INT.mdk Outdated Show resolved Hide resolved

telemetry/specs/INT.mdk Outdated Show resolved Hide resolved

telemetry/specs/INT.mdk Outdated Show resolved Hide resolved

telemetry/specs/INT.mdk Outdated Show resolved Hide resolved

jklr mentioned this pull request Jan 17, 2020

revise INT over UDP #63

Merged

rsivakolundu added 2 commits January 19, 2020 04:54

Update INT.mdk

9977496

Merge branch 'master' into INT-Extensions-for-Domain-Specific-Features

08a1eb8

mickeyspiegel reviewed Jan 20, 2020

View reviewed changes

telemetry/specs/INT.mdk Outdated Show resolved Hide resolved

mickeyspiegel requested changes Jan 20, 2020

View reviewed changes

Update INT.mdk

742ed02

Corrected typos found by Mickey and other changes requested.

mickeyspiegel approved these changes Jan 28, 2020

View reviewed changes

jklr merged commit 90f3006 into p4lang:master Jan 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

INT extensions for domain specific features #54

INT extensions for domain specific features #54

rsivakolundu commented Sep 28, 2018

jklr Oct 31, 2019

jklr Jan 2, 2020

mickeyspiegel Jan 9, 2020

rsivakolundu Jan 16, 2020

jklr left a comment •

edited

mickeyspiegel left a comment

mickeyspiegel Jan 8, 2020

mickeyspiegel Jan 17, 2020

mickeyspiegel Jan 9, 2020

mickeyspiegel Jan 9, 2020

rsivakolundu Jan 16, 2020

mickeyspiegel Jan 20, 2020

mickeyspiegel Jan 9, 2020

mickeyspiegel Jan 9, 2020

rsivakolundu Jan 28, 2020

jklr Jan 28, 2020

mickeyspiegel left a comment

mickeyspiegel Jan 20, 2020

mickeyspiegel Jan 20, 2020

rsivakolundu Jan 28, 2020

jklr commented Jan 23, 2020

jklr commented Jan 28, 2020

jklr commented Jan 29, 2020

		@@ -1194,9 +1212,11 @@ INT Metadata Header and Metadata Stack, followed by TCP payload:
		0 1 2 3

INT extensions for domain specific features #54

INT extensions for domain specific features #54

Conversation

rsivakolundu commented Sep 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jklr left a comment • edited

Choose a reason for hiding this comment

mickeyspiegel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mickeyspiegel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jklr commented Jan 23, 2020

jklr commented Jan 28, 2020

jklr commented Jan 29, 2020

jklr left a comment •

edited