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 vpws local and remote identifiers to EVPN-VPWS configuration #930

Merged
merged 8 commits into from
Jul 16, 2024

Conversation

crcampos612
Copy link
Contributor

  • Add local and remote vpws service instance identifier (ethernet-tag) that identifies the local and remote attachment circuit (AC) ends of the emulated service as per RFC 8214.

Change Scope

  • Current connection-point in network-instance model doesn't fit into the VPWS approach where there is no need to explicitly set the remote endpoint address as they are autodiscovered by BGP in respective PE endpoints. Besides, a field to enter the local virtual circuit identifier doesn't exist either.
  • Therefore, the proposal is to extend openconfig-evpn.yang model with those two new for setting both local and remote vpws service identifiers within container evpn/evpn-instances/evpn-instance

Platform Implementations

Examples

CISCO

evpn
 evi 9999
  bgp
   rd 27699:31133
   route-target import 27699:31133
   route-target export 27699:31133

l2vpn
 xconnect group ELINE_VPWS_CISCO_GROUP
  p2p ELINE_VPWS_AC_1
   interface TenGigE0/0/0/15
   neighbor evpn evi 9999 target 111 source 333

JUNIPER

set routing-instances EVPN-VPWS instance-type evpn-vpws
set routing-instances EVPN-VPWS interface ae0.100
set routing-instances EVPN-VPWS route-distinguisher 198.51.100.1:11
set routing-instances EVPN-VPWS vrf-target target:100:11
set routing-instances EVPN-VPWS protocols evpn interface ae0.100 vpws-service-id local 1111
set routing-instances EVPN-VPWS protocols evpn interface ae0.100 vpws-service-id remote 9999

HUAWEI

evpn vpn-instance ELINE_VPWS_AC_1 vpws
route-distinguisher 27699:451123
vpn-target 27699:1123 export-extcommunity
vpn-target 27699:1123 import-extcommunity

evpl instance 9999
evpn binding vpn-instance ELINE_VPWS_AC_1
local-service-id 333 remote-service-id 111

NOKIA

epipe 9999 name "ELINE-VPWS-AC-1" customer 1 create
			bgp
                route-distinguisher 27699:511531
                route-target export target:27699:511531 import target:27699:511531
            exit
            bgp-evpn
                local-ac-name "AC-HEAD-END"
                    eth-tag 333
                exit
                remote-ac-name "AC-TAIL-END"
                    eth-tag 111
                exit
                evi 9999

Extend openconfig-evpn.yang model with those two leafs for setting both local and remote vpws service identifiers within container evpn/evpn-instances/evpn-instance
@crcampos612 crcampos612 requested a review from a team as a code owner July 27, 2023 15:23
@OpenConfigBot
Copy link

OpenConfigBot commented Jul 27, 2023

No major YANG version changes in commit a517d7f

@OpenConfigBot
Copy link

OpenConfigBot commented Jul 27, 2023

Compatibility Report for commit 963ec52:
pyangbind@1b7ee10

@dplore
Copy link
Member

dplore commented Aug 1, 2023

        +--rw evpn
        |  +--rw evpn-instances
        |  |  +--rw evpn-instance* [evi]
        |  |     +--rw evi                     -> ../config/evi
        |  |     +--rw config
        |  |     |  +--rw evi?                      string
        |  |     |  +--rw encapsulation-type?       identityref
        |  |     |  +--rw service-type?             identityref
        |  |     |  +--rw multicast-group?          oc-inet:ip-address
        |  |     |  +--rw multicast-mask?           oc-inet:ip-address
        |  |     |  +--rw replication-mode?         enumeration
        |  |     |  +--rw route-distinguisher?      union
        |  |     |  +--rw control-word-enabled?     boolean
        |  |     |  +--rw local-vpws-service-id?    uint32
        |  |     |  +--rw remote-vpws-service-id?   uint32
        |  |     +--ro state
        |  |     |  +--ro evi?                      string
        |  |     |  +--ro encapsulation-type?       identityref
        |  |     |  +--ro service-type?             identityref
        |  |     |  +--ro multicast-group?          oc-inet:ip-address
        |  |     |  +--ro multicast-mask?           oc-inet:ip-address
        |  |     |  +--ro replication-mode?         enumeration
        |  |     |  +--ro route-distinguisher?      union
        |  |     |  +--ro control-word-enabled?     boolean
        |  |     |  +--ro local-vpws-service-id?    uint32
        |  |     |  +--ro remote-vpws-service-id?   uint32
        |  |     +--rw import-export-policy
        |  |     |  +--rw config
        |  |     |  |  +--rw export-route-target*   union
        |  |     |  |  +--rw import-route-target*   union
        |  |     |  +--ro state
        |  |     |     +--ro export-route-target*   union
        |  |     |     +--ro import-route-target*   union

@dplore
Copy link
Member

dplore commented Aug 1, 2023

Reviewed at OC Operators meeting on Aug 1, 2023. Seems like Huawei and Nokia examples do not include an interface, but Cisco and Juniper do. Is the interface reference required to complete the configuration? Also, this model only allows one VPWS per network instance. Probably there should be a list of of the VPWS?

@jorabada
Copy link

jorabada commented Aug 4, 2023

Reviewed at OC Operators meeting on Aug 1, 2023. Seems like Huawei and Nokia examples do not include an interface, but Cisco and Juniper do. Is the interface reference required to complete the configuration? Also, this model only allows one VPWS per network instance. Probably there should be a list of of the VPWS?

In case of Nokia, our "network-instance" is defined as a generic point-to-point service, where you have interfaces and optionally you can have one interface plus an "evpn-instance". So there is a 1:1 mapping, and the interface is not included in the evpn-instance, but in the network-instance. As you do for the case of MAC-VRFs..

My suggestion would be to do something similar to the MAC-VRF with EVPN model, that is, an EVPN-VPWS service would be configured as follows:

  • add a network-instance type L2P2P, with one interface
  • add an evpn-instance, which includes the local and remote vpws-service-id

To satisfy models that support multiple VPWS per "routing-instance" (e.g. JunOS), we could define a new NETWORK_INSTANCE_TYPE, e.g. MULTI-L2P2P (or similar), and then for that type we could include the interface in the evpn-instance...

My two cents..

@oscargdd
Copy link
Contributor

oscargdd commented Oct 3, 2023

This PR was discussed in the public OC special meeting. An alternative proposal was discussed in which connection points were used and enhanced to be able to have multiple point-to-point connections in the same network instance reusing the same EVPN instance. All vendors implementing EVPN-VPWS support the one-to-one mapping case between point-to-point service, while not all support the case of reusing the EVPN insance. The consensus was that is was better to decouple both functions. Hence, with this PR, ONE Network instance will represent ONE point to point service with ONE EVPN instance (route distinguishsçer and target). The proposed solution in this PR was reasonable and implementable for the vendors present in the call. For the "multi Point to point connection" use case, a new type of network instance needs to be created. This will be done in a separate PR triggered by the need of that use case.

@dplore dplore self-assigned this Oct 3, 2023
@dplore dplore added the last-call PR that is in final review before merging. label Oct 3, 2023
@dplore
Copy link
Member

dplore commented Oct 10, 2023

Last call expires today. Since there are no new comments, I will merge this.

/gcbrun

dplore
dplore previously approved these changes Oct 10, 2023
@dplore
Copy link
Member

dplore commented Oct 10, 2023

@oscargdd can you resolve the merge conflicts?

@wlin212
Copy link

wlin212 commented Oct 11, 2023

A P2P service establish by VPWS is between a pair of ACs. Hence, it's essential to specify the local interface associated with a P2P service within an EVPN VPWS instance.  Each EVPN VPWS instance can accommodate one or more P2P services, provided each P2P service has its unique AC and its local VPWS service ID.  Therefore, it is crucial to ensure that we can incorporate multiple P2P services under the same EVPN VPWS instance.
 
Additionally, please ensure the inclusion of 'flow-label-enabled' for the EVPN VPWS instance.

I'm also curious about the 'multicast-group,' 'multicast-mask,' and 'replication-mode' attributes within an EVPN VPWS instance as these attributes do not apply to VPWS P2P services.  it might be worthwhile to remove them, but please let me know if I've overlooked any specific relevance or context.
 

@oscargdd
Copy link
Contributor

h EVPN VPWS instance can accommodate one or more P2P services, provided each P2P service has its unique AC and its local VPWS service ID.  Therefore, it is crucial to ensure that we can incorporate multiple P2P services under the same EVPN VPWS instance.

HI, in the public openconfig call we agreed on limiting the functionality of this use case to the one-to-one mapping case, which is the most common. There was another proposal, to add in another PR in which , with a different network instance type you can support multiple P2P over the same EVPN instance. Not all vendors support this way of working, that is why we decided to keep them separate.

@dplore
Copy link
Member

dplore commented Nov 28, 2023

Reviewed in today's in OC Operators meeting. Operators did not have a use case for 'flow-label-enabled' for the EVPN VPWS instance at this time. It seems like this could be added in a future PR, with the necessary references and operational use cases described.

@oscargdd can you respond regarding the presence of multicast-group multicast-mask and replication-mode leafs now that we are adding (local|remote)-vpws-service-id?

@dplore
Copy link
Member

dplore commented Apr 30, 2024

Re-reviewed in April 30, 2024 OC operators meeting. @oscargdd to resolve conflicts and respond on the multicast related leafs. Then I think this is ready to merge.

@dplore
Copy link
Member

dplore commented May 14, 2024

/gcbrun

@dplore
Copy link
Member

dplore commented May 28, 2024

@oscargdd care to comment on the multicast leaves? That is the only open issue.

@dplore
Copy link
Member

dplore commented Jul 16, 2024

/gcbrun

@dplore
Copy link
Member

dplore commented Jul 16, 2024

/gcbrun

@oscargdd
Copy link
Contributor

@wlin212 As you mention, the presence of multicast-group multicast-mask do not apply for the EVPN-VPWS use case. In order not to complicate, when the services is evpn-vpws, these leafs are ignored by the implementation, as they are out of scope and not related.

@dplore
Copy link
Member

dplore commented Jul 16, 2024

Re-reviewed in July 16, 2024 OC Operators meeting without objection and rough consensus that the multicast question can be handled via a separate pull request.

@dplore dplore merged commit d23641e into openconfig:master Jul 16, 2024
14 checks passed
abamberger-arista pushed a commit to abamberger-arista/openconfig-public that referenced this pull request Aug 7, 2024
…nconfig#930)

* Update openconfig-evpn.yang with vpws identifiers

Extend openconfig-evpn.yang model with those two leafs for setting both local and remote vpws service identifiers within container evpn/evpn-instances/evpn-instance

---------

Co-authored-by: Óscar González de Dios <[email protected]>
sallylsy pushed a commit to sallylsy/OC_public that referenced this pull request Sep 5, 2024
…nconfig#930)

* Update openconfig-evpn.yang with vpws identifiers

Extend openconfig-evpn.yang model with those two leafs for setting both local and remote vpws service identifiers within container evpn/evpn-instances/evpn-instance

---------

Co-authored-by: Óscar González de Dios <[email protected]>
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
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants