Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gre and mpls containers under next-hops aft entry state. Add ttl and tos under gre, mpls and ip-in-ip aft entry state for telemetry. #879

Closed
wants to merge 7 commits into from
+122 −8
Closed

Add gre and mpls containers under next-hops aft entry state. Add ttl and tos under gre, mpls and ip-in-ip aft entry state for telemetry. #879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
68a9e7e
* (M) aft/openconfig-aft-common.yang
romeyod May 25, 2023
dd3774f
Addressed review comments
romeyod Jun 6, 2023
af0a0c9
Adding auto-size
romeyod Jul 13, 2023
b131e42
Add gre and mpls containers under next-hops aft entry state. Add ttl …
romeyod Aug 7, 2023
478746d
remove nhg-name and auto-size
romeyod Aug 7, 2023
1113d66
changed ttl and tos type to uint8
romeyod Aug 21, 2023
a69ea22
updated documentation for default value
romeyod Aug 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 59 additions & 1 deletion release/models/aft/openconfig-aft-common.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,15 @@ submodule openconfig-aft-common {
"Submodule containing definitions of groupings that are re-used
across multiple contexts within the AFT model.";

oc-ext:openconfig-version "2.3.0";
oc-ext:openconfig-version "2.4.0";

revision "2023-05-25" {
description
"Add gre and mpls containers under next-hops aft entry state.
Add ttl and tos under gre, mpls and ip-in-ip aft entry state
for telemetry.";
reference "2.4.0";
}

revision "2023-04-19" {
description
Expand Down Expand Up @@ -204,6 +212,33 @@ submodule openconfig-aft-common {
description
"State parameters relating to IP-in-IP encapsulation.";
uses aft-common-entry-nexthop-ipip-state;
uses aft-common-entry-nexthop-encap-outer-header-state;
}
}

container gre {
description
"When specified, the packet has an GRE header applied to it before
forwarding to the specified next-hop.";

container state {
config false;
description
"State parameters relating to GRE encapsulation.";
uses aft-common-entry-nexthop-encap-outer-header-state;
}
}

container mpls {
description
"When specified, the packet has an MPLS header applied to it before
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If multiple labels are getting added and each label has its own tos/ttl values then what should be the values?
Should these mpls specific leaves tos/ttl kept under "pushed-mpls-label-stack"?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@romeyod any comments on this suggestion?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed following up on this, let me get back to you about this. [ my understanding is this TTL/TOS is for the next-hop and need to clarify what it means wrt mpls labels ]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@krishna-juniper Thanks for the suggestion

pushed-mpls-label-stack is of type oc-mplst:mpls-label which is also used in other models. Example: popped-mpls-label-stack, start-label-value more occurences

Instead of adding ttl/tos to oc-mplst:mpls-label and affecting unrelated models, we can do the following:
define a typedef encap-outer-header-state with ttl and tos
and
in place of mpls container, under next-hop/state at the same level as pushed-mpls-label-stack we define

leaf-list pushed-mpls-label-header-state {
    type encap-outer-header-state;
}

This new leaf-list pushed-mpls-label-header-state will have ordered entries with tos/ttl corresponding to pushed-mpls-label-stack

Let me know what you think about this proposal.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach assumes implicit correlation between labels and ttl/tos. (labels coming from pushed-mpls-label-stack, ttl/tos coming from pushed-mpls-label-header-state).
By adding label stack as part of new container(container for encap-outer-header-state + pushed labels) we can avoid this implicit correlation at the cost of redundant information.
what are your thoughts on this approach?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this approach supported on any system? Please can we provide multi-vendor examples if we are going to make such a change?

Note that changes around how the MPLS label stack is being pushed have implications in gRIBI and are backwards compatible. I'd like to make sure we're supporting a use case that is actually supported here, since it also has overhead for those that don't care about this.

I also don't really like this typedef being a single leaf -- i think we rather need to think about how this is actually user friendly to parse.

Copy link
Contributor Author

@romeyod romeyod Aug 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you @robshakir. We should not be make existing pushed-mpls-label-stack backward incompatible in anyway. Hence I proposed a new leaf/leaf list pushed-mpls-label-header-state that focuses only on the ttl/tos use case.

I also don't really like this typedef being a single leaf -- i think we rather need to think about how this is actually user friendly to parse.
I don't completely understand this comment. The new typedef encap-outer-header-state will have ttl and tos leafs.

I do think we need to work together and come up with a more user friendly layout. Will appreciate any inputs or existing precedence references.

forwarding to the specified next-hop.";

container state {
config false;
description
"State parameters relating to MPLS encapsulation.";
uses aft-common-entry-nexthop-encap-outer-header-state;
}
}

Expand Down Expand Up @@ -379,6 +414,29 @@ submodule openconfig-aft-common {
}
}

grouping aft-common-entry-nexthop-encap-outer-header-state {
description
"Outer header fields of encapsulation applied on a next-hop";

leaf ttl {
type uint8;
description
"This leaf reflects the configured/default TTL value that is used in the
outer header during packet encapsulation. When this leaf is not set,
the TTL value of the inner packet is copied over as the outer packet's
TTL value during encapsulation.";
}

leaf tos {
type uint8;
description
"This leaf reflects the configured/default TOS value that is used in the
outer header during packet encapsulation. When this leaf is not set,
the TOS value of the inner packet is copied over as the outer packet's
TOS value during encapsulation.";
}
}

grouping aft-common-install-protocol {
description
"Grouping for a common reference to the protocol which
Expand Down
10 changes: 9 additions & 1 deletion release/models/aft/openconfig-aft-ethernet.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,15 @@ submodule openconfig-aft-ethernet {
"Submodule containing definitions of groupings for the abstract
forwarding tables for Ethernet.";

oc-ext:openconfig-version "2.3.0";
oc-ext:openconfig-version "2.4.0";

revision "2023-05-25" {
description
"Add gre and mpls containers under next-hops aft entry state.
Add ttl and tos under gre, mpls and ip-in-ip aft entry state
for telemetry.";
reference "2.4.0";
}

revision "2023-04-19" {
description
Expand Down
10 changes: 9 additions & 1 deletion release/models/aft/openconfig-aft-ipv4.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,15 @@ submodule openconfig-aft-ipv4 {
"Submodule containing definitions of groupings for the abstract
forwarding tables for IPv4.";

oc-ext:openconfig-version "2.3.0";
oc-ext:openconfig-version "2.4.0";

revision "2023-05-25" {
description
"Add gre and mpls containers under next-hops aft entry state.
Add ttl and tos under gre, mpls and ip-in-ip aft entry state
for telemetry.";
reference "2.4.0";
}

revision "2023-04-19" {
description
Expand Down
10 changes: 9 additions & 1 deletion release/models/aft/openconfig-aft-ipv6.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,15 @@ submodule openconfig-aft-ipv6 {
"Submodule containing definitions of groupings for the abstract
forwarding tables for IPv6.";

oc-ext:openconfig-version "2.3.0";
oc-ext:openconfig-version "2.4.0";

revision "2023-05-25" {
description
"Add gre and mpls containers under next-hops aft entry state.
Add ttl and tos under gre, mpls and ip-in-ip aft entry state
for telemetry.";
reference "2.4.0";
}

revision "2023-04-19" {
description
Expand Down
10 changes: 9 additions & 1 deletion release/models/aft/openconfig-aft-mpls.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,15 @@ submodule openconfig-aft-mpls {
"Submodule containing definitions of groupings for the abstract
forwarding table for MPLS label forwarding.";

oc-ext:openconfig-version "2.3.0";
oc-ext:openconfig-version "2.4.0";

revision "2023-05-25" {
description
"Add gre and mpls containers under next-hops aft entry state.
Add ttl and tos under gre, mpls and ip-in-ip aft entry state
for telemetry.";
reference "2.4.0";
}

revision "2023-04-19" {
description
Expand Down
10 changes: 9 additions & 1 deletion release/models/aft/openconfig-aft-pf.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,15 @@ submodule openconfig-aft-pf {
fields other than the destination address that is used in
other forwarding tables.";

oc-ext:openconfig-version "2.3.0";
oc-ext:openconfig-version "2.4.0";

revision "2023-05-25" {
description
"Add gre and mpls containers under next-hops aft entry state.
Add ttl and tos under gre, mpls and ip-in-ip aft entry state
for telemetry.";
reference "2.4.0";
}

revision "2023-04-19" {
description
Expand Down
10 changes: 9 additions & 1 deletion release/models/aft/openconfig-aft-state-synced.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,15 @@ submodule openconfig-aft-state-synced {
"Submodule containing definitions of groupings for the state
synced signals corresponding to various abstract forwarding tables.";

oc-ext:openconfig-version "2.3.0";
oc-ext:openconfig-version "2.4.0";

revision "2023-05-25" {
description
"Add gre and mpls containers under next-hops aft entry state.
Add ttl and tos under gre, mpls and ip-in-ip aft entry state
for telemetry.";
reference "2.4.0";
}

revision "2023-04-19" {
description
Expand Down
10 changes: 9 additions & 1 deletion release/models/aft/openconfig-aft.yang
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,15 @@ module openconfig-aft {
is referred to as an Abstract Forwarding Table (AFT), rather than
the FIB.";

oc-ext:openconfig-version "2.3.0";
oc-ext:openconfig-version "2.4.0";

revision "2023-05-25" {
description
"Add gre and mpls containers under next-hops aft entry state.
Add ttl and tos under gre, mpls and ip-in-ip aft entry state
for telemetry.";
reference "2.4.0";
}

revision "2023-04-19" {
description
Expand Down
Loading