Adding support for configuration of ECMP of next hops for a static LSP #1138

Shashank-arista · 2024-06-23T13:31:49Z

Change Scope

(M) release/models/mpls/openconfig-mpls-static.yang
(M) release/models/mpls/openconfig-mpls.yang

Currently, model supports configuration of only a single next hop for a given LSP name. We are adding support to configure multiple next hops by adding a new container next-hops to support backward compatibility. The incoming-label and metric will be used from existing config itself as its common to all next-hops.

     +--rw network-instance* [name]
        +--rw mpls
           +--rw lsps
              +--rw static-lsps
                 +--rw static-lsp* [name]
                    +--rw name       -> ../config/name
                    +--rw config
                    |  +--rw name?   string
                    +--ro state
                    |  +--ro name?   string
                    +--rw ingress
                    |  +--rw config
                    |  |  +--rw incoming-label?   oc-mplst:mpls-label
                    |  |  x--rw next-hop?         inet:ip-address
                    |  |  x--rw push-label?       oc-mplst:mpls-label
                    |  |  +--rw interface?        -> /oc-if:interfaces/interface/name
                    |  |  +--rw subinterface?     -> /oc-if:interfaces/interface[oc-if:name=current()/../interface]/subinterfaces/subinterface/index
                    |  |  +--rw metric?           uint8
                    |  +--ro state
                    |  |  +--ro incoming-label?   oc-mplst:mpls-label
                    |  |  x--ro next-hop?         inet:ip-address
                    |  |  x--ro push-label?       oc-mplst:mpls-label
                    |  |  +--ro interface?        -> /oc-if:interfaces/interface/name
                    |  |  +--ro subinterface?     -> /oc-if:interfaces/interface[oc-if:name=current()/../interface]/subinterfaces/subinterface/index
                    |  |  +--ro metric?           uint8
                    |  +--rw lsp-next-hops                                                                                                                                   [36/9557]
                    |     +--rw lsp-next-hop* [index]
                    |        +--rw index     -> ../config/index
                    |        +--rw config
                    |        |  +--rw index?          uint32
                    |        |  +--rw ip-address?     inet:ip-address
                    |        |  +--rw push-label?     oc-mplst:mpls-label
                    |        |  +--rw interface?      -> /oc-if:interfaces/interface/name
                    |        |  +--rw subinterface?   -> /oc-if:interfaces/interface[oc-if:name=current()/../interface]/subinterfaces/subinterface/index
                    |        +--ro state
                    |           +--ro index?          uint32
                    |           +--ro ip-address?     inet:ip-address
                    |           +--ro push-label?     oc-mplst:mpls-label
                    |           +--ro interface?      -> /oc-if:interfaces/interface/name
                    |           +--ro subinterface?   -> /oc-if:interfaces/interface[oc-if:name=current()/../interface]/subinterfaces/subinterface/index
                    +--rw transit
                    |  +--rw config
                    |  |  +--rw incoming-label?   oc-mplst:mpls-label
                    |  |  x--rw next-hop?         inet:ip-address
                    |  |  x--rw push-label?       oc-mplst:mpls-label
                    |  |  +--rw interface?        -> /oc-if:interfaces/interface/name
                    |  |  +--rw subinterface?     -> /oc-if:interfaces/interface[oc-if:name=current()/../interface]/subinterfaces/subinterface/index
                    |  |  +--rw metric?           uint8
                    |  +--ro state
                    |  |  +--ro incoming-label?   oc-mplst:mpls-label
                    |  |  x--ro next-hop?         inet:ip-address
                    |  |  x--ro push-label?       oc-mplst:mpls-label
                    |  |  +--ro interface?        -> /oc-if:interfaces/interface/name
                    |  |  +--ro subinterface?     -> /oc-if:interfaces/interface[oc-if:name=current()/../interface]/subinterfaces/subinterface/index
                    |  |  +--ro metric?           uint8
                    |  +--rw lsp-next-hops
                    |     +--rw lsp-next-hop* [index]
                    |        +--rw index     -> ../config/index
                    |        +--rw config
                    |        |  +--rw index?          uint32
                    |        |  +--rw ip-address?     inet:ip-address
                    |        |  +--rw push-label?     oc-mplst:mpls-label
                    |        |  +--rw interface?      -> /oc-if:interfaces/interface/name
                    |        |  +--rw subinterface?   -> /oc-if:interfaces/interface[oc-if:name=current()/../interface]/subinterfaces/subinterface/index
                    |        +--ro state
                    |           +--ro index?          uint32
                    |           +--ro ip-address?     inet:ip-address
                    |           +--ro push-label?     oc-mplst:mpls-label
                    |           +--ro interface?      -> /oc-if:interfaces/interface/name
                    |           +--ro subinterface?   -> /oc-if:interfaces/interface[oc-if:name=current()/../interface]/subinterfaces/subinterface/index
                    +--rw egress
                       +--rw config
                       |  +--rw incoming-label?   oc-mplst:mpls-label
                       |  x--rw next-hop?         inet:ip-address
                       |  x--rw push-label?       oc-mplst:mpls-label
                       |  +--rw interface?        -> /oc-if:interfaces/interface/name
                       |  +--rw subinterface?     -> /oc-if:interfaces/interface[oc-if:name=current()/../interface]/subinterfaces/subinterface/index
                       |  +--rw metric?           uint8
                       +--ro state
                       |  +--ro incoming-label?   oc-mplst:mpls-label
                       |  x--ro next-hop?         inet:ip-address
                       |  x--ro push-label?       oc-mplst:mpls-label
                       |  +--ro interface?        -> /oc-if:interfaces/interface/name
                       |  +--ro subinterface?     -> /oc-if:interfaces/interface[oc-if:name=current()/../interface]/subinterfaces/subinterface/index
                       |  +--ro metric?           uint8
                       +--rw lsp-next-hops
                          +--rw lsp-next-hop* [index]
                             +--rw index     -> ../config/index
                             +--rw config
                             |  +--rw index?          uint32
                             |  +--rw ip-address?     inet:ip-address
                             |  +--rw push-label?     oc-mplst:mpls-label
                             |  +--rw interface?      -> /oc-if:interfaces/interface/name
                             |  +--rw subinterface?   -> /oc-if:interfaces/interface[oc-if:name=current()/../interface]/subinterfaces/subinterface/index
                             +--ro state
                                +--ro index?          uint32
                                +--ro ip-address?     inet:ip-address
                                +--ro push-label?     oc-mplst:mpls-label
                                +--ro interface?      -> /oc-if:interfaces/interface/name
                                +--ro subinterface?   -> /oc-if:interfaces/interface[oc-if:name=current()/../interface]/subinterfaces/subinterface/index

Platform Implementations

Arista: https://www.arista.com/en/um-eos/eos-mpls-commands#xx1141474
CLI configuration: mpls static top-label top_tag[ bgp peer [peer IP]] [DEST_INTF] NEXTHOP_ADDR ACTION [PRIORITY]

LimeHat · 2024-06-25T04:27:32Z

I would propose to align this with the AFT model and introduce the concept of next-hop-groups.
cc @robshakir, any thoughts on this?

Also, I'm not entirely clear on the support backward compatibility thing. The tree in the description doesn't look like a backward-compatible change to me. Could you elaborate on that?

Shashank-arista · 2024-06-25T04:36:50Z

I would propose to align this with the AFT model and introduce the concept of next-hop-groups. cc @robshakir, any thoughts on this?

Also, I'm not entirely clear on the support backward compatibility thing. The tree in the description doesn't look like a backward-compatible change to me. Could you elaborate on that?

By backward compatibility, I meant the leaves model under (egress | transit | ingress)/config to configure a single next-hop are not being removed to change into a list. Instead, we are introducing a new container for the list of next-hops.

LimeHat · 2024-06-25T04:46:43Z

Ah, I see what you mean.

I don't think it makes too much sense to keep the two (redundant/conflicting) methods in the long term, and the usual procedure of marking the old leafs as deprecated should be used.

Shashank-arista · 2024-06-25T04:54:23Z

Ah, I see what you mean.

I don't think it makes too much sense to keep the two (redundant/conflicting) methods in the long term, and the usual procedure of marking the old leafs as deprecated should be used.

Sure thanks, will update that. Also regarding next-hops, I have tried to use here similar to Static routes

LimeHat · 2024-06-25T05:08:29Z

I'd be happy if we added the concept of NHGs to static-routes too! That makes a lot of sense.

I think you guys support it as well?
https://www.arista.com/en/um-eos/eos-nexthop-groups

Shashank-arista · 2024-06-25T05:40:41Z

I'd be happy if we added the concept of NHGs to static-routes too! That makes a lot of sense.

I think you guys support it as well? https://www.arista.com/en/um-eos/eos-nexthop-groups

Yes, next-hop-groups is like an addition/enhancement to next-hops, right? We can still configure a list of next-hops and use them directly without configuring next-hop groups right?

In EOS, we can create a new next-hop each time we run a command:
mpls static top-label top_tag [ bgp peer [peer IP]] [DEST_INTF] NEXTHOP_ADDR ACTION [PRIORITY]

Here, we need not use next-hop group

robshakir · 2024-06-25T20:42:24Z

w.r.t next-hop-groups -- the general benefit of these is if we end up with lots of sharing. I don't see that we necessarily must have the same approach between AFT and the data model used for config (for example, we do not have this for static routes).

robshakir · 2024-06-25T20:49:33Z

For reference:

Cisco XR -- allows multiple path statements docs
JUNOS -- allows one next-hop, multiple LSPs possible to the same destination using different "name" docs - this is similar to the current model.
SROS -- allows only one next-hop (docs)[https://infocenter.nokia.com/public/7750SR217R1A/index.jsp?topic=%2Fcom.nokia.MPLS_Guide_21.7.R1%2Fconfiguring_a_s-ai9emdyo7e.html]

@LimeHat @earies @rgwilton -- do you have thoughts here?

LimeHat · 2024-06-25T20:52:09Z

the general benefit of these is if we end up with lots of sharing.

yes, and I think we should allow that.

I don't see that we necessarily must have the same approach between AFT and the data model used for config

Not a must, but a uniform approach would be highly beneficial from an implementation point of view. Otherwise, we need to support different ways of "config" (e.g., for gRIBI vs. CLI), which has to be normalized behind the scenes multiple times/in different ways (e.g., for hw programming, for AFT outputs, etc.).

(for example, we do not have this for static routes).

I'd love to see it for static routes as well. :-)

wenovus · 2024-06-25T21:29:57Z

/gcbrun

OpenConfigBot · 2024-06-25T21:31:21Z

No major YANG version changes in commit 75ac876

earies · 2024-06-26T01:45:47Z

@LimeHat @earies @rgwilton -- do you have thoughts here?

Will revert back shortly - quick passthrough appears to not align well to existing support within our implementation as proposed

Shashank-arista · 2024-06-26T04:40:20Z

w.r.t next-hop-groups -- the general benefit of these is if we end up with lots of sharing. I don't see that we necessarily must have the same approach between AFT and the data model used for config (for example, we do not have this for static routes).

Also Arista implementation doesn't have option to use next-hop group to configure transit labels and its optional for egress labels (https://www.arista.com/en/um-eos/eos-mpls-commands#xx1141474)

wenovus · 2024-06-28T19:50:24Z

/gcbrun

dplore · 2024-07-24T21:18:44Z

To be reviewed on July 30,2024 OC Operators meeting

dplore · 2024-07-24T21:22:44Z

Should this also deprecate the following leafs? This way it's clear the preferred way to configure any/all LSP's is via the next-hops/next-hop subtree.

                    |  |  +--rw interface?        -> /oc-if:interfaces/interface/name
                    |  |  +--rw subinterface?     -> /oc-if:interfaces/interface[oc-if:name=current()/../interface]/subinterfaces/subinterface/index
                    |  |  +--rw metric?           uint8

robshakir · 2024-07-25T00:43:02Z

ygot is failing here because after path compression we have duplicate names -- i.e., next-hops/next-hop and state/next-hop are duplicate names in this generated code. Can we select different naming?

dplore · 2024-07-25T00:51:04Z

next-hops

To avoid a breaking change we can't rename the existing, to be deprecated leaf so we'll need the newly introduced container and list to use something like lsp-next-hops/lsp-next-hop

Shashank-arista · 2024-07-25T05:16:50Z

Should this also deprecate the following leafs? This way it's clear the preferred way to configure any/all LSP's is via the next-hops/next-hop subtree.
                    |  |  +--rw interface?        -> /oc-if:interfaces/interface/name
                    |  |  +--rw subinterface?     -> /oc-if:interfaces/interface[oc-if:name=current()/../interface]/subinterfaces/subinterface/index
                    |  |  +--rw metric?           uint8

Actually, interface and subinterface are marked as deprecated in the changes but they are not getting displayed in pyang due to some issue which is discussed here: #1113

Regarding metric deprecation, incoming-label and metric together is considered as a key for lsp and is same for a route. So we have kept both them under the config as above except others.

dplore · 2024-07-30T16:30:30Z

Reviewed with OC Operators on July 30,2024. The following names were proposed. We'll leave this open a couple more days and then should choose one:

Ideas to resolve conflict due to ygot compression:

lsp-next-hops/lsp-next-hop # most explicit, uses canonical terms, stutters
lsp-nhs/lsp-nh # pretty cryptic still
nhs/nh # too cryptic
next-nodes/next-node # succinct, doesn’t stutter, node is not the canonical term

dplore · 2024-08-02T23:19:38Z

/gcbrun

Shashank-arista · 2024-08-07T05:11:24Z

Reviewed with OC Operators on July 30,2024. The following names were proposed. We'll leave this open a couple more days and then should choose one:

Ideas to resolve conflict due to ygot compression:

lsp-next-hops/lsp-next-hop # most explicit, uses canonical terms, stutters

lsp-nhs/lsp-nh # pretty cryptic still

nhs/nh # too cryptic

next-nodes/next-node # succinct, doesn’t stutter, node is not the canonical term

I would be updating the model using the first option: lsp-next-hops/lsp-next-hop

dplore · 2024-08-08T19:20:48Z

Reviewed with OC Operators on July 30,2024. The following names were proposed. We'll leave this open a couple more days and then should choose one:
Ideas to resolve conflict due to ygot compression:

lsp-next-hops/lsp-next-hop # most explicit, uses canonical terms, stutters

lsp-nhs/lsp-nh # pretty cryptic still

nhs/nh # too cryptic

next-nodes/next-node # succinct, doesn’t stutter, node is not the canonical term

I would be updating the model using the first option: lsp-next-hops/lsp-next-hop

I approve, please do submit the change.

Shashank-arista · 2024-08-12T05:14:16Z

Reviewed with OC Operators on July 30,2024. The following names were proposed. We'll leave this open a couple more days and then should choose one:
Ideas to resolve conflict due to ygot compression:

lsp-next-hops/lsp-next-hop # most explicit, uses canonical terms, stutters

lsp-nhs/lsp-nh # pretty cryptic still

nhs/nh # too cryptic

next-nodes/next-node # succinct, doesn’t stutter, node is not the canonical term

I would be updating the model using the first option: lsp-next-hops/lsp-next-hop

I approve, please do submit the change.

Thanks, I have updated the PR with the change.

dplore · 2024-08-12T21:19:33Z

/gcbrun

dplore · 2024-08-12T22:41:06Z

Regarding operational use case, Google and others on the Operator group voiced support that this is a real use case in use.

Regarding multiple vendor implementations:

Arista static MPLS LSP configuration
Repeating this CLI configuration is the method to define multiple next hops in EOS.
mpls static top-label top_tag[ bgp peer [peer IP]] [DEST_INTF] NEXTHOP_ADDR ACTION [PRIORITY]
Cisco IOS XR has a path command which can be repeated in this example for MPLS over GRE. This looks very similar to the proposed OC model.

Router(config)# mpls static
Router(config-mpls-static)# lsp v4-encap-payload-1
Router(config-mpls-static-lsp)# in-label 10001 allocate per-prefix 111.0.0.1/32
Router(config-mpls-static-lsp)# forward
Router(config-mpls-static-lsp-fwd))# path 1 nexthop tunnel-ip1 out-label 20001
Router(config-mpls-static-lsp-fwd))# path 2 nexthop tunnel-ip2 out-label 20001
Router(config-mpls-static-lsp-fwd))# path 3 nexthop tunnel-ip3 out-label 20001
Router(config-mpls-static-lsp-fwd))# path 4 nexthop tunnel-ip4 out-label 20001
Router(config-mpls-static-lsp-fwd)# commit

Juniper: As @robshakir pointed out, Juniper appears to support this in concept, but the implementation is via multiple LSPs to the same destination using different "name" docs
Nokia SRLinux appears to support multiple next hops for static lsp

@LimeHat I think adding next-hop-group is also an option. That could be via a separate PR. As Rob pointed out, the precedent with IP static routes uses only a nexthop. So I think this PR follows that precedent which is ok. Internally a NOS is free to construct an AFT using groups to deliver this configured functionality.

dplore · 2024-08-12T22:41:17Z

Last call for comment Aug 20th, 2024

Shashank-arista · 2024-08-13T04:52:11Z

Regarding operational use case, Google and others on the Operator group voiced support that this is a real use case in use.

Regarding multiple vendor implementations, it seems there are three, although they are achieved in different ways. Cisco and Arista seem to both support configuring multiple next-hops for a static LSP:

Cisco has a path command which can be repeated in this example for MPLS over GRE. This looks very similar to the proposed OC model.
Router(config)# mpls static
Router(config-mpls-static)# lsp v4-encap-payload-1
Router(config-mpls-static-lsp)# in-label 10001 allocate per-prefix 111.0.0.1/32
Router(config-mpls-static-lsp)# forward
Router(config-mpls-static-lsp-fwd))# path 1 nexthop tunnel-ip1 out-label 20001
Router(config-mpls-static-lsp-fwd))# path 2 nexthop tunnel-ip2 out-label 20001
Router(config-mpls-static-lsp-fwd))# path 3 nexthop tunnel-ip3 out-label 20001
Router(config-mpls-static-lsp-fwd))# path 4 nexthop tunnel-ip4 out-label 20001
Router(config-mpls-static-lsp-fwd)# commit
Arista static MPLS LSP configuration
@Shashank-arista , is repeating this CLI configuration the method to define multiple next hops in EOS? It was not clear to me from the docs.
mpls static top-label top_tag[ bgp peer [peer IP]] [DEST_INTF] NEXTHOP_ADDR ACTION [PRIORITY]

As @robshakir pointed out, Juniper appears to support this in concept, but the implementation is via multiple LSPs to the same destination using different "name" docs

@LimeHat I think adding next-hop-group is also an option. That could be via a separate PR. As Rob pointed out, the precedent with IP static routes uses only a nexthop. So I think this PR follows that precedent which is ok. Internally a NOS is free to construct an AFT using groups to deliver this configured functionality.

@dplore, yes, repeating the CLI configuration is the method to define multiple next hops in EOS.

dplore · 2024-08-15T00:38:24Z

Updated the implementation references, including adding a reference to SRLinux.

Shashank-arista · 2024-08-23T06:17:37Z

Last call for comment Aug 20th, 2024

Gentle reminder!

dplore · 2024-08-26T17:13:36Z

/gcbrun

Co-authored-by: Darren Loher <[email protected]>

Shashank-arista requested a review from a team as a code owner June 23, 2024 13:31

OpenConfigBot added the non-breaking label Jun 25, 2024

Shashank-arista force-pushed the mplsEcmp branch 2 times, most recently from 9ee7059 to 6bdbc24 Compare June 26, 2024 05:22

Shashank-arista mentioned this pull request Jun 26, 2024

Using status substatement for uses statement #1140

Open

dplore self-assigned this Jul 24, 2024

Shashank-arista force-pushed the mplsEcmp branch from 6bdbc24 to c406910 Compare August 1, 2024 08:31

Augment mpls static model to support ECMP of next hops

039ee2a

Shashank-arista force-pushed the mplsEcmp branch from c406910 to 039ee2a Compare August 12, 2024 05:08

Merge branch 'master' into mplsEcmp

75ac876

dplore approved these changes Aug 26, 2024

View reviewed changes

dplore merged commit b492f67 into openconfig:master Aug 26, 2024
14 checks passed

sallylsy pushed a commit to sallylsy/OC_public that referenced this pull request Sep 5, 2024

Augment mpls static model to support ECMP of next hops (openconfig#1138)

c9fd959

Co-authored-by: Darren Loher <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for configuration of ECMP of next hops for a static LSP #1138

Adding support for configuration of ECMP of next hops for a static LSP #1138

Shashank-arista commented Jun 23, 2024 •

edited

Loading

LimeHat commented Jun 25, 2024

Shashank-arista commented Jun 25, 2024

LimeHat commented Jun 25, 2024

Shashank-arista commented Jun 25, 2024

LimeHat commented Jun 25, 2024

Shashank-arista commented Jun 25, 2024

robshakir commented Jun 25, 2024

robshakir commented Jun 25, 2024

LimeHat commented Jun 25, 2024 •

edited

Loading

wenovus commented Jun 25, 2024

OpenConfigBot commented Jun 25, 2024 •

edited

Loading

earies commented Jun 26, 2024

Shashank-arista commented Jun 26, 2024 •

edited

Loading

wenovus commented Jun 28, 2024

dplore commented Jul 24, 2024

dplore commented Jul 24, 2024

robshakir commented Jul 25, 2024

dplore commented Jul 25, 2024 •

edited

Loading

Shashank-arista commented Jul 25, 2024 •

edited

Loading

dplore commented Jul 30, 2024

dplore commented Aug 2, 2024

Shashank-arista commented Aug 7, 2024

dplore commented Aug 8, 2024

Shashank-arista commented Aug 12, 2024

dplore commented Aug 12, 2024

dplore commented Aug 12, 2024 •

edited

Loading

dplore commented Aug 12, 2024 •

edited

Loading

Shashank-arista commented Aug 13, 2024

dplore commented Aug 15, 2024

Shashank-arista commented Aug 23, 2024

dplore commented Aug 26, 2024

Adding support for configuration of ECMP of next hops for a static LSP #1138

Adding support for configuration of ECMP of next hops for a static LSP #1138

Conversation

Shashank-arista commented Jun 23, 2024 • edited Loading

Change Scope

Platform Implementations

LimeHat commented Jun 25, 2024

Shashank-arista commented Jun 25, 2024

LimeHat commented Jun 25, 2024

Shashank-arista commented Jun 25, 2024

LimeHat commented Jun 25, 2024

Shashank-arista commented Jun 25, 2024

robshakir commented Jun 25, 2024

robshakir commented Jun 25, 2024

LimeHat commented Jun 25, 2024 • edited Loading

wenovus commented Jun 25, 2024

OpenConfigBot commented Jun 25, 2024 • edited Loading

earies commented Jun 26, 2024

Shashank-arista commented Jun 26, 2024 • edited Loading

wenovus commented Jun 28, 2024

dplore commented Jul 24, 2024

dplore commented Jul 24, 2024

robshakir commented Jul 25, 2024

dplore commented Jul 25, 2024 • edited Loading

Shashank-arista commented Jul 25, 2024 • edited Loading

dplore commented Jul 30, 2024

dplore commented Aug 2, 2024

Shashank-arista commented Aug 7, 2024

dplore commented Aug 8, 2024

Shashank-arista commented Aug 12, 2024

dplore commented Aug 12, 2024

dplore commented Aug 12, 2024 • edited Loading

dplore commented Aug 12, 2024 • edited Loading

Shashank-arista commented Aug 13, 2024

dplore commented Aug 15, 2024

Shashank-arista commented Aug 23, 2024

dplore commented Aug 26, 2024

Shashank-arista commented Jun 23, 2024 •

edited

Loading

LimeHat commented Jun 25, 2024 •

edited

Loading

OpenConfigBot commented Jun 25, 2024 •

edited

Loading

Shashank-arista commented Jun 26, 2024 •

edited

Loading

dplore commented Jul 25, 2024 •

edited

Loading

Shashank-arista commented Jul 25, 2024 •

edited

Loading

dplore commented Aug 12, 2024 •

edited

Loading

dplore commented Aug 12, 2024 •

edited

Loading