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.

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 aft encapsulation-headers #1153

Open
wants to merge 41 commits into
base: master
Choose a base branch
from
Open

Add aft encapsulation-headers #1153

wants to merge 41 commits into from

Conversation

dplore
Copy link
Member

@dplore dplore commented Jul 18, 2024

Change Scope

  • Add AFT container representing next-hop encapsulation headers. A list of encap-headers is defined which is used to represent a stack of one or more packets added to any packet matching a next-hop.

  • Add containers for GRE, IP, UDP and MPLS encapsulation

  • Deprecate the existing AFT encapsulation containers for ip-in-ip and gre and MPLS.

  • Deprecate pop-top-label and move to encap-header/mpls list

Use cases

  • One operational use case for these nexthops is for an SDN controller to use gRIBI to program match and action rules that cause packets to be encapsulated in MPLS in UDP packets. The SDN controller chooses the encapsulation stack.
  • In another, future operational use case, policy-forwarding might be used to define encapsulations which are representing in the afts model upon instantiation into the forwarding tables of the system. (Note, at this moment, changes to the policy-forwarding model to support multiple, arbitrary encapsulations is not in scope for this PR.)

Platform Implementations

  • RFC 7510 defines the MPLS in UDP feature specification.
  • Cisco IOS XR supports MPLS in UDP.
  • JunOS nexthop-based-dynamic-tunnels supports MPLS in UDP.

Tree view

Note that deprecated leafs are marked with an 'x'.

we are at: /network-instances/network-instance/afts/

diff -U 10 ~/master-tree.txt ~/aft-encap-tree.txt
[... snip ...]
         |  +--ro next-hops
         |  |  +--ro next-hop* [index]
         |  |     +--ro index            -> ../state/index
         |  |     +--ro state
         |  |     |  +--ro index?                       uint64
         |  |     |  +--ro programmed-index?            uint64
         |  |     |  +--ro ip-address?                  oc-inet:ip-address
         |  |     |  +--ro mac-address?                 oc-yang:mac-address
-        |  |     |  +--ro pop-top-label?               boolean
-        |  |     |  +--ro pushed-mpls-label-stack*     oc-mplst:mpls-label
-        |  |     |  +--ro encapsulate-header?          oc-aftt:encapsulation-header-type
+        |  |     |  x--ro pop-top-label?               boolean
+        |  |     |  x--ro pushed-mpls-label-stack*     oc-mplst:mpls-label-stack
+        |  |     |  x--ro encapsulate-header?          oc-aftt:encapsulation-header-type
         |  |     |  +--ro decapsulate-header?          oc-aftt:encapsulation-header-type
         |  |     |  +--ro origin-protocol?             identityref
         |  |     |  +--ro lsp-name?                    string
         |  |     |  +--ro counters
         |  |     |  |  +--ro packets-forwarded?   oc-yang:counter64
         |  |     |  |  +--ro octets-forwarded?    oc-yang:counter64
-        |  |     |  +--ro vni-label?                   oc-evpn-types:evi-id
-        |  |     |  +--ro tunnel-src-ip-address?       oc-inet:ip-address
+        |  |     |  x--ro vni-label?                   oc-evpn-types:evi-id
+        |  |     |  x--ro tunnel-src-ip-address?       oc-inet:ip-address
         |  |     |  +--ro oc-aftni:network-instance?   oc-ni:network-instance-ref
-        |  |     +--ro ip-in-ip
-        |  |     |  +--ro state
+        |  |     x--ro ip-in-ip
+        |  |     |  x--ro state
         |  |     |     +--ro src-ip?   oc-inet:ip-address
         |  |     |     +--ro dst-ip?   oc-inet:ip-address
-        |  |     +--ro gre
-        |  |     |  +--ro state
+        |  |     x--ro gre
+        |  |     |  x--ro state
         |  |     |     +--ro src-ip?   oc-inet:ip-address
         |  |     |     +--ro dst-ip?   oc-inet:ip-address
         |  |     |     +--ro ttl?      uint8
+        |  |     +--ro encap-headers
+        |  |     |  +--ro encap-header* [index]
+        |  |     |     +--ro index    -> ../state/index
+        |  |     |     +--ro state
+        |  |     |     |  +--ro index?   uint8
+        |  |     |     |  +--ro type?    oc-aftt:encapsulation-header-type
+        |  |     |     +--ro gre
+        |  |     |     |  +--ro state
+        |  |     |     |     +--ro src-ip?   oc-inet:ip-address
+        |  |     |     |     +--ro dst-ip?   oc-inet:ip-address
+        |  |     |     |     +--ro ttl?      uint8
+        |  |     |     +--ro ipv4
+        |  |     |     |  +--ro state
+        |  |     |     |     +--ro src-ip?   oc-inet:ip-address
+        |  |     |     |     +--ro dst-ip?   oc-inet:ip-address
+        |  |     |     +--ro ipv6
+        |  |     |     |  +--ro state
+        |  |     |     |     +--ro src-ip?   oc-inet:ip-address
+        |  |     |     |     +--ro dst-ip?   oc-inet:ip-address
+        |  |     |     +--ro mpls
+        |  |     |     |  +--ro state
+        |  |     |     |     +--ro traffic-class?             oc-mplst:mpls-tc
+        |  |     |     |     +--ro pop-top-label?             boolean
+        |  |     |     |     +--ro pushed-mpls-label-stack*   oc-mplst:mpls-label-stack
+        |  |     |     +--ro udp
+        |  |     |     |  +--ro state
+        |  |     |     |     +--ro src-ip?         oc-inet:ip-address
+        |  |     |     |     +--ro dst-ip?         oc-inet:ip-address
+        |  |     |     |     +--ro ip-dscp?        oc-inet:dscp
+        |  |     |     |     +--ro src-udp-port?   oc-inet:port-number
+        |  |     |     |     +--ro dst-udp-port?   oc-inet:port-number
+        |  |     |     |     +--ro ip-ttl?         uint8
+        |  |     |     +--ro vxlan
+        |  |     |        +--ro state
+        |  |     |           +--ro vni-label?               oc-evpn-types:evi-id
+        |  |     |           +--ro tunnel-src-ip-address?   oc-inet:ip-address

@OpenConfigBot
Copy link

OpenConfigBot commented Jul 18, 2024

No major YANG version changes in commit a85fa26

@dplore dplore marked this pull request as ready for review July 18, 2024 19:52
@dplore dplore requested a review from a team as a code owner July 18, 2024 19:52
release/models/aft/openconfig-aft-common.yang Outdated Show resolved Hide resolved
release/models/aft/openconfig-aft-common.yang Outdated Show resolved Hide resolved
release/models/aft/openconfig-aft-common.yang Show resolved Hide resolved
release/models/aft/openconfig-aft-common.yang Outdated Show resolved Hide resolved
release/models/aft/openconfig-aft-common.yang Outdated Show resolved Hide resolved
@dplore
Copy link
Member Author

dplore commented Aug 28, 2024

should the idea proposed in #1038 (comment) be followed in this PR? pushed-mpls-label-stack in mpls-in-udp container is same as pushed-mpls-label-stack in next-hop's state container. If we follow the idea in the above link this can be avoided & In near future when ip-in-udp needs to be supported then if we rename current mpls-in-udp container to udp container then it can be easily extended without adding yet another container.

Ok, so something like leaf-list encapsulate-stack = [MPLS, UDP]

we are at: /network-instances/network-instance/afts/next-hops/next-hop/

         |  +--ro next-hops
         |  |  +--ro next-hop* [index]
         |  |    +--ro encapsulate-stack
         |  |      +--ro encap-type    (leaflist of type encapsulation-header-type ordered by user )

And then the fields for each encap in the stack would need to be set
     |  |    +--ro encap-headers
     |  |      +--ro encap-header  [index]
     |  |        +--leaf1 for encap type1
     |  |        +--leaf2 for encap type1
     |  |        +--leaf1 for encap type2
     |  |        +--leaf2 for encap type2
... etc ...

One then sets the appropriate leaves for each item in the encapsulate-stack. (an MPLS encap would set the MPLS things, GRE would set the GRE things, etc...)

How does that sound?

@akalluru1
Copy link
Contributor

akalluru1 commented Aug 29, 2024

should the idea proposed in #1038 (comment) be followed in this PR? pushed-mpls-label-stack in mpls-in-udp container is same as pushed-mpls-label-stack in next-hop's state container. If we follow the idea in the above link this can be avoided & In near future when ip-in-udp needs to be supported then if we rename current mpls-in-udp container to udp container then it can be easily extended without adding yet another container.

Ok, so something like leaf-list encapsulate-stack = [MPLS, UDP]

we are at: /network-instances/network-instance/afts/next-hops/next-hop/

         |  +--ro next-hops
         |  |  +--ro next-hop* [index]
         |  |    +--ro encapsulate-stack
         |  |      +--ro encap-type    (leaflist of type encapsulation-header-type ordered by user )

And then the fields for each encap in the stack would need to be set
     |  |    +--ro encap-headers
     |  |      +--ro encap-header  [index]
     |  |        +--leaf1 for encap type1
     |  |        +--leaf2 for encap type1
     |  |        +--leaf1 for encap type2
     |  |        +--leaf2 for encap type2
... etc ...

One then sets the appropriate leaves for each item in the encapsulate-stack. (an MPLS encap would set the MPLS things, GRE would set the GRE things, etc...)

How does that sound?

Thanks Darren for the quick response! Couple of clarifications:

  1. In your proposal does encap-headers container reside inside next-hop container or is it parallel to next-hops container?
  2. What is the purpose of index in encap-header list?
    2.1 Will it be somehow referred in the encapsulate-stack container?
    2.2 Is the plan to reuse the entries in encap-header list across next-hop entries?
  3. Starting with ip-in-ip encap & even for gre encap explicit containers have been created under next-hop container to store all the relevant encap information, would it be worth considering the same model under encap-header entries?
  4. If we introduce this model Is the plan to deprecate, following the usual deprecation procedure, existing encapsulate-header leaf, ip-in-ip & gre containers under next-hop list? or Use encapsulate-stack & encap-headers only when there is encap stacking is involved, i.e., For existing supported single encaps will still reside in next-hop list entry?

@dplore
Copy link
Member Author

dplore commented Sep 6, 2024

Here's a better defined, but still conceptual, tree which may answer your questions.

For backwards compatibility we will deprecate the existing containers for gre and ip-in-ip. The new schema would become the preferred schema.

we are at	/network-instances/network-instance/afts/

     |  +--ro next-hops
     |  |  +--ro next-hop* [index]
     |  |  |  +--ro encap-headers
     |  |  |    +--ro encap-header* [index]  (list of type encapsulation-header-type)
     |  |  |      +--ro state
     |  |  |        +--ro index       (an integer identifying the order of encap)
     |  |  |        +--ro type        (encapsulation-header-type, such as IP, GRE, MPLS, UDP)
     |  |  |      +--ro ip-encap      (container when encapsulation-header-type = IP)
     |  |  |        +--ro state
     |  |  |          +--ro src-ip
     |  |  |          +--ro dst-ip
     |  |  |          +--ro ttl
     |  |  |          +--ro dscp
     |  |  |      +--ro gre-encap     (container when encapsulation-header-type = GRE)
     |  |  |        +--ro state
     |  |  |          +--ro src-ip
     |  |  |          +--ro dst-ip
     |  |  |          +--ro ttl
     |  |  |      +--ro mpls-encap    (container when encapsulation-header-type = MPLS)
     |  |  |        +--ro state
     |  |  |          +--ro pushed_label_stack*
     |  |  |      +--ro udp-encap     (container when encapsulation-header-type = UDP)
     |  |  |        +--ro state
     |  |  |          +--ro src-ip
     |  |  |          +--ro dst-ip
     |  |  |          +--ro src-port
     |  |  |          +--ro dst-port
     |  |  |          +--ro ttl
     |  |   |          +--ro dscp

@dplore dplore changed the title Add aft encap mpls-in-udp paths Add aft encapsulation-headers Sep 6, 2024
Copy link
Contributor

@akalluru1 akalluru1 left a comment

Choose a reason for hiding this comment

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

  1. pop-top-label should also be deprecated and should be moved under mpls-encap container? lsp-name should also move to mpls encap?
  2. Doesn't encapsulate-header leaf under next-hop state container need to be deprecated as the encap-headers container has been added?
  3. As the MPLS related leaves are moving to its own encap it would be better to move vni-label & tunnel-src-ip-address fields into a new VXLAN encap container?

@dplore
Copy link
Member Author

dplore commented Sep 10, 2024

  1. pop-top-label should also be deprecated and should be moved under mpls-encap container? lsp-name should also move to mpls encap?
  2. Doesn't encapsulate-header leaf under next-hop state container need to be deprecated as the encap-headers container has been added?
  3. As the MPLS related leaves are moving to its own encap it would be better to move vni-label & tunnel-src-ip-address fields into a new VXLAN encap container?

Items 1 and 2, done.

Regarding item 3, does it make sense to just deprecate these leaves and have the aft indicate an encap-headers/encap-header list with MAC in UDP ? I guess most people would probably expect to see vxlan encap specified versus UDP with a optional VNI-ID header value.

@akalluru1
Copy link
Contributor

akalluru1 commented Sep 12, 2024

I don't have enough knowledge on VxLAN about your question on item 3. But the changes you've made in the latest commit, i.e., creation of a new container for VxLAN encap makes sense & kind of preserves the old VXLAN encapsulation enum as well.

@dplore dplore added the last-call PR that is in final review before merging. label Sep 12, 2024
@dplore
Copy link
Member Author

dplore commented Sep 12, 2024

Last-call for comments! This is planned to merge on Sep 26,2024.

@rgwilton @nandanarista @earies @LimeHat for comments.

Copy link

@nandanarista nandanarista left a comment

Choose a reason for hiding this comment

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

The overall hierarchy LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
last-call PR that is in final review before merging. non-breaking
Projects
Status: last-call
Development

Successfully merging this pull request may close these issues.

5 participants