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

EVPN Multi Home Support #2084

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+744 −3
Open

EVPN Multi Home Support #2084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7df3274
EVPN Multihoming
JaiOCP Sep 30, 2024
79c5e17
EVPN Multihoming
srj102 Dec 11, 2024
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
621 changes: 621 additions & 0 deletions doc/tunnel/SAI-Proposal-EVPN-Multihoming.md
View file
Open in desktop

Large diffs are not rendered by default.

Binary file added doc/tunnel/figures/sai_evpnmh_df.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added doc/tunnel/figures/sai_evpnmh_failover.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added doc/tunnel/figures/sai_evpnmh_singleactive.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added doc/tunnel/figures/sai_evpnmh_splithorizon.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added doc/tunnel/figures/sai_evpnmh_unicast.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
75 changes: 74 additions & 1 deletion inc/saibridge.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ typedef enum _sai_bridge_port_type_t
/** Bridge tunnel port */
SAI_BRIDGE_PORT_TYPE_TUNNEL,

/** Bridge nexthop group port */

/** Nexthop group should be of type bridge port */
SAI_BRIDGE_PORT_TYPE_BRIDGE_PORT_NEXT_HOP_GROUP,
Copy link

Choose a reason for hiding this comment

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

Can we have this named as SAI_BRIDGE_PORT_TYPE_NEXT_HOP_GROUP ?

Copy link

Choose a reason for hiding this comment

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

This is specific to NHG of type bridgeport and hence this name.

Copy link

Choose a reason for hiding this comment

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

@JaiOCP Please comment if the extra BRIDGE_PORT in the name can be dropped ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rational for naming it like is to be explicit in intent that the NHG type is BRIDGE_PORT. Though the attributes are for bridge port object and it may seem redundant but the NHG are of many type and one used here is of type bridge port.

It would be good to clarify this in comments to avoid such confusion.

Copy link

Choose a reason for hiding this comment

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

Updated comments.

} sai_bridge_port_type_t;

/**
Expand Down Expand Up @@ -181,7 +185,7 @@ typedef enum _sai_bridge_port_attr_t
* @type sai_object_id_t
* @flags MANDATORY_ON_CREATE | CREATE_AND_SET
* @objects SAI_OBJECT_TYPE_BRIDGE
* @condition SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_SUB_PORT or SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_1D_ROUTER or SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_TUNNEL
* @condition SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_SUB_PORT or SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_1D_ROUTER or SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_TUNNEL or SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_BRIDGE_PORT_NEXT_HOP_GROUP
*/
SAI_BRIDGE_PORT_ATTR_BRIDGE_ID,

Expand Down Expand Up @@ -281,6 +285,75 @@ typedef enum _sai_bridge_port_attr_t
*/
SAI_BRIDGE_PORT_ATTR_SELECTIVE_COUNTER_LIST,

/**
* @brief Associated nexthop group id
*
* @type sai_object_id_t
* @flags MANDATORY_ON_CREATE | CREATE_ONLY
* @objects SAI_OBJECT_TYPE_NEXT_HOP_GROUP
* @condition SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_BRIDGE_PORT_NEXT_HOP_GROUP
*/
SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_NEXT_HOP_GROUP_ID,
Copy link

Choose a reason for hiding this comment

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

Same as above comment, can we name it as SAI_BRIDGE_PORT_ATTR_NEXT_HOP_GROUP_ID ?

Copy link

Choose a reason for hiding this comment

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

Same response as above.


/**
* @brief Indicates if the bridge port is set to drop the broadcast, unknown unicast and multicast traffic
*
* When set to true, egress BUM traffic will be dropped
*
* @type bool
* @flags CREATE_AND_SET
* @default false
* @validonly SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_PORT or SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_SUB_PORT or SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_TUNNEL
*/
SAI_BRIDGE_PORT_ATTR_TUNNEL_TERM_BUM_TX_DROP,

/**
* @brief Indicates if the bridge port is set to drop the ingress traffic
*
* When set to true, all ingress traffic will be dropped
*
* @type bool
* @flags CREATE_AND_SET
* @default false
* @validonly SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_PORT or SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_SUB_PORT or SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_TUNNEL
*/
SAI_BRIDGE_PORT_ATTR_RX_DROP,

/**
* @brief Indicates if the bridge port is set to drop the egress traffic
*
* When set to true, all egress traffic will be dropped
*
* @type bool
* @flags CREATE_AND_SET
* @default false
* @validonly SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_PORT or SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_SUB_PORT or SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_TUNNEL
*/
SAI_BRIDGE_PORT_ATTR_TX_DROP,

/**
* @brief Associated protection bridge port nexthop group id
*
* The Protection nexthop group type should be SAI_NEXT_HOP_GROUP_TYPE_BRIDGE_PORT
*
* @type sai_object_id_t
* @flags CREATE_AND_SET
* @objects SAI_OBJECT_TYPE_NEXT_HOP_GROUP
* @allownull true
* @default SAI_NULL_OBJECT_ID
*/
SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_PROTECTION_NEXT_HOP_GROUP_ID,
Copy link

Choose a reason for hiding this comment

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

Actually, instead of having the protectin at nexthop group level, having protection for each nexthop(NHG member) might helps.

Assume, VTEP-5, had a nexthop group with members VTEP-{1,2,3,4}. what will be the backup members for this NHG. instead VTEP-1, can be protected with one member from VTEP-{2,3,4} and same way, VTEP-2 can be protected with one of the VTEP from VTEP-{1,3,4}.
Generally, not all members in a give nexthop group will not go down, so under what condition, switchover to backup Nexthop group is Initiated. if backup NHG also constituted from same VTEP list{1,2,3,4} by skipping one member. how does NOS knowns which member will go down

Copy link

Choose a reason for hiding this comment

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

Could you look at section 4.4 Fast Failover workflow and see if it clarifies ?

Copy link

@sathk-marvell sathk-marvell Nov 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

No, it didn't explain how the backup members are constructed. SONIC HLD mentions that we can remove the, member from L2-NHG when VTEP advertises ES_ID Down notification, so that all FDBs learnt on the Ethernet segment will be updated with a single forwading update(NHG member delete). but for that we don't need backup protection next-hop group, simple member removal from Primary NHG, when VTEP advertises ES_ID down notification.

Still not clear why the backup NHG is needed and how it will be constructed

Copy link

@srj102 srj102 Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

4.4 has no VTEP-5 in the diagram.

As mentioned in 4.4,
Step1 - Traffic from Host1 received on LAG1 and fwded to Host2 over LAG2.
This is due to Local bias.
Step2 - When LAG2 fails, Traffic will be sent to a backup NHG comprising of
the remaining members sharing the ES.

This is the reason why the backup exists and is not for the scenario you mention.

This is a different flow from 4.1 which you seem to be referring to.

As to how the backup NHG is constructed and for more queries on the need for this usecase please raise this query in the SONiC EVPN-MH HLD PR.

Copy link

Choose a reason for hiding this comment

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

Thanks for the explaination, it addressed my query


/**
* @brief Trigger a switch-over to backup next hop group
*
* @type bool
* @flags CREATE_AND_SET
* @default false
* @validonly SAI_BRIDGE_PORT_ATTR_TYPE == SAI_BRIDGE_PORT_TYPE_PORT
*/
SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_SET_SWITCHOVER,

/**
* @brief End of attributes
*/
Expand Down
7 changes: 5 additions & 2 deletions inc/sainexthop.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ typedef enum _sai_next_hop_type_t

/** IPMC next hop */
SAI_NEXT_HOP_TYPE_IPMC,

/** Next hop group is for bridge port */
SAI_NEXT_HOP_TYPE_BRIDGE_PORT,
} sai_next_hop_type_t;

/**
Expand All @@ -78,7 +81,7 @@ typedef enum _sai_next_hop_attr_t
*
* @type sai_ip_address_t
* @flags MANDATORY_ON_CREATE | CREATE_ONLY
* @condition SAI_NEXT_HOP_ATTR_TYPE == SAI_NEXT_HOP_TYPE_IP or SAI_NEXT_HOP_ATTR_TYPE == SAI_NEXT_HOP_TYPE_MPLS or SAI_NEXT_HOP_ATTR_TYPE == SAI_NEXT_HOP_TYPE_TUNNEL_ENCAP or SAI_NEXT_HOP_ATTR_TYPE == SAI_NEXT_HOP_TYPE_IPMC
* @condition SAI_NEXT_HOP_ATTR_TYPE == SAI_NEXT_HOP_TYPE_IP or SAI_NEXT_HOP_ATTR_TYPE == SAI_NEXT_HOP_TYPE_MPLS or SAI_NEXT_HOP_ATTR_TYPE == SAI_NEXT_HOP_TYPE_TUNNEL_ENCAP or SAI_NEXT_HOP_ATTR_TYPE == SAI_NEXT_HOP_TYPE_IPMC or SAI_NEXT_HOP_ATTR_TYPE == SAI_NEXT_HOP_TYPE_BRIDGE_PORT
*/
SAI_NEXT_HOP_ATTR_IP,

Expand All @@ -98,7 +101,7 @@ typedef enum _sai_next_hop_attr_t
* @type sai_object_id_t
* @flags MANDATORY_ON_CREATE | CREATE_ONLY
* @objects SAI_OBJECT_TYPE_TUNNEL
* @condition SAI_NEXT_HOP_ATTR_TYPE == SAI_NEXT_HOP_TYPE_TUNNEL_ENCAP or SAI_NEXT_HOP_ATTR_TYPE == SAI_NEXT_HOP_TYPE_SRV6_SIDLIST
* @condition SAI_NEXT_HOP_ATTR_TYPE == SAI_NEXT_HOP_TYPE_TUNNEL_ENCAP or SAI_NEXT_HOP_ATTR_TYPE == SAI_NEXT_HOP_TYPE_SRV6_SIDLIST or SAI_NEXT_HOP_ATTR_TYPE == SAI_NEXT_HOP_TYPE_BRIDGE_PORT
*/
SAI_NEXT_HOP_ATTR_TUNNEL_ID,

Expand Down
3 changes: 3 additions & 0 deletions inc/sainexthopgroup.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ typedef enum _sai_next_hop_group_type_t
/** Next hop group is ECMP, with members specified with the group */
SAI_NEXT_HOP_GROUP_TYPE_ECMP_WITH_MEMBERS,

/** Next hop group is for bridge port */
SAI_NEXT_HOP_GROUP_TYPE_BRIDGE_PORT,

/* Other types of next hop group to be defined in the future, e.g., WCMP */

} sai_next_hop_group_type_t;
Expand Down
41 changes: 41 additions & 0 deletions inc/saivlan.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,47 @@ typedef enum _sai_vlan_member_attr_t
*/
SAI_VLAN_MEMBER_ATTR_VLAN_TAGGING_MODE,

/**
* @brief Indicates if the bridge port is set to drop the Tunnel Terminated broadcast, unknown unicast and multicast traffic
*
* When set to true, egress BUM traffic will be dropped
* Valid only when the SAI_VLAN_MEMBER_ATTR_BRIDGE_PORT_ID is of type SAI_BRIDGE_PORT_TYPE_PORT.
* Valid only for .1Q bridge ports.
Copy link

Choose a reason for hiding this comment

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

Can the comment be made explicit and state it as below to include both port and lag ?

  • Valid only when the SAI_VLAN_MEMBER_ATTR_BRIDGE_PORT_ID is of type SAI_BRIDGE_PORT_TYPE_PORT

Copy link

Choose a reason for hiding this comment

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

Ack

*
* @type bool
* @flags CREATE_AND_SET
* @default false
*/
SAI_VLAN_MEMBER_ATTR_TUNNEL_TERM_BUM_TX_DROP,

/**
* @brief Indicates if the vlan member is set to drop the ingress traffic
*
* When set to true, ingress traffic will be dropped
*
* Valid only when the SAI_VLAN_MEMBER_ATTR_BRIDGE_PORT_ID is of type SAI_BRIDGE_PORT_TYPE_PORT.
* Valid only for .1Q bridge ports.
*
* @type bool
* @flags CREATE_AND_SET
* @default false
*/
SAI_VLAN_MEMBER_ATTR_RX_DROP,

/**
* @brief Indicates if the vlan member is set to drop the egress traffic
*
* When set to true, egress traffic will be dropped
*
* Valid only when the SAI_VLAN_MEMBER_ATTR_BRIDGE_PORT_ID is of type SAI_BRIDGE_PORT_TYPE_PORT.
* Valid only for .1Q bridge ports.
*
* @type bool
* @flags CREATE_AND_SET
* @default false
*/
SAI_VLAN_MEMBER_ATTR_TX_DROP,

/**
* @brief End of attributes
*/
Expand Down