EVPN MultiHoming Fast ReRoute for L3VNI-routed traffic - #2332
EVPN MultiHoming Fast ReRoute for L3VNI-routed traffic#2332manamand2020 wants to merge 2 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
6a392e8 to
406fde9
Compare
Signed-off-by: Manas Kumar Mandal <manamand@cisco.com>
|
|
||
| The route lookup resolves to a RIF/neighbor pair, which in turn resolves to the primary bridge port (`SAI_BRIDGE_PORT_TYPE_PORT`) | ||
| for the ES. That bridge port carries `SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_PROTECTION_MODE` and | ||
| `SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_PROTECTION_NEXT_HOP_GROUP_ID`, pointing to a `SAI_NEXT_HOP_GROUP_TYPE_BRIDGE_PORT` |
There was a problem hiding this comment.
Hey Manas, do you know why the next hop type being bridge port? The next hop id is vxlan tunnel, so this sounds a bit inconsistent (I am aware that this is somehow allowed in the SAI, but not sure why)
There was a problem hiding this comment.
This is following the existing section 3.2.1, :
A new nexthop type which will be part of groups of the above group type.
typedef enum _sai_next_hop_type_t
...
/** Next hop group is for bridge port */
SAI_NEXT_HOP_TYPE_BRIDGE_PORT,
} sai_next_hop_type_t;
Maybe the question is why SAI_NEXT_HOP_TYPE_TUNNEL_ENCAP is not being used instead. I think using the SAI_NEXT_HOP_TYPE_BRIDGE_PORT is only allowed for bridge nexthop group case as the comment mentions and implementation can distinguish this clearly.
/** Next hop group is for bridge port */
| SAI_BRIDGE_PORT_PROTECTION_MODE_HARDWARE, | ||
|
|
||
| /** Hardware switchover. Does not switch back to the bridge port once it recovers */ | ||
| SAI_BRIDGE_PORT_PROTECTION_MODE_HARDWARE_NON_REVERTIVE, |
There was a problem hiding this comment.
Will need to make sure the recovery path of non-revertive flavor is more clear.
There was a problem hiding this comment.
Do we need an attribute for delayed recovery when the port is up again?
There was a problem hiding this comment.
Intentionally kept this out of the scope here. I think delay/debouncing should be handled by the Port object itself. Like Link UP and Link Down debouncing. We already have attributes like SAI_PORT_ATTR_LINK_UP_DEBOUNCE_TIMEOUT and there is a proposal for link down in #2284.
For link damping as well we should extend the existing https://github.com/sonic-net/SONiC/blob/master/doc/link_event_damping/Link-event-damping-HLD.md
I see that this defines as switch level attribute to update the oper status using SAI_SWITCH_HOSTIF_OPER_STATUS_UPDATE_MODE_SAI_ADAPTER.
There was a problem hiding this comment.
Manas, I want to push back gently on deferring debounce/damping to the Port object, because I don't think either existing mechanism can gate a hardware switchover.
SAI_REDIS_PORT_ATTR_LINK_EVENT_DAMPING_ALGORITHM is a sairedis custom-range attribute (lib/sairedis.h, = SAI_PORT_ATTR_CUSTOM_RANGE_START). It is evaluated above the ASIC, so it damps the oper-status notification delivered to the NOS and never reaches the selection logic.
The merged SAI_PORT_ATTR_LINK_UP_DEBOUNCE_TIMEOUT is likewise defined in terms of delivery — "0 means no delay time so link up events are immediately delivered as usual" — and PR #2284 describes link-down debounce the same way, as suppressing the notification.
So if debounce/damping is non-zero on an ES member port, the ASIC selects on the raw transition while the NOS sees a damped one, and the two disagree for the damping interval. For sub-microsecond FRR that window is enormous.
Suggestion: state explicitly that hardware protection reacts to a qualified link event, define qualification as the point at which the adapter treats the primary as unavailable, and measure the selection budget from that point. Then either require qualification to be observable by hardware, or require damping/debounce to be zero on ES member ports. Today the proposal leaves the reference point undefined.
There was a problem hiding this comment.
Folding revertive behaviour into the mode enum makes it hard to extend, and it collides with the delayed-recovery question above. Recovery policy is an independent axis from software-vs-hardware selection, so composing them in one enum grows combinatorially — adding delayed recovery would need HARDWARE_NON_REVERTIVE_DELAYED and friends.
A separate SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_PROTECTION_REVERTIVE (bool, @default true) plus, if needed later, a wait-to-restore timeout attribute would keep the mode enum to the selection mechanism and let recovery policy evolve independently. That also gives r12f's delayed-recovery request a natural home without another enum value.
I don't see an existing SAI precedent for encoding revertive in a protection-mode enum; NHG HW protection keeps mode and state separate.
| /** | ||
| * @brief Defines the reason for a bridge port HW protection switchover | ||
| * | ||
| * Reported only for hardware-initiated transitions. A switchover requested |
There was a problem hiding this comment.
This docblock is where the pre-existing SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_SET_SWITCHOVER first acquires hardware-mode meaning, and two interactions look undefined to me.
Manual request vs. autonomous selection in MODE_HARDWARE. If the NOS writes SET_SWITCHOVER = true while the primary is still healthy, revertive hardware is documented to "switch back to the bridge port once it recovers" — but it never left. Does hardware immediately revert, making the request a no-op or a transient blip? Is a manual request sticky until explicitly cleared, or always subordinate to hardware's own selection? Section 5 says the resulting path is readable immediately from PROTECTION_STATE, which only holds if the answer is deterministic.
Re-arming in MODE_HARDWARE_NON_REVERTIVE. The walkthrough says traffic stays on the protection path "until the NOS explicitly reverts it (e.g. via SET_SWITCHOVER)". Two gaps: if the NOS writes false while the primary is still down, is that rejected, or committed and then immediately failed over again? And after a successful revert, is the latch re-armed so the next failure switches over autonomously? The "e.g." also leaves it ambiguous whether SET_SWITCHOVER is the defined mechanism or merely one option.
Header/proposal divergence. The proposal document is considerably more precise than the header here — that false reverts to primary, that the outcome is the return status of set_bridge_port_attribute(), and that PROTECTION_STATE is readable immediately are all stated in section 5 but not in the docblock. Since vendors implement from headers, it would help to pull those sentences into the Doxygen comment.
There was a problem hiding this comment.
Added SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_PROTECTION_ADMIN_MODE and SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_PROTECTION_REVERTIVE to address these concerns. I have also deprecated the SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_SET_SWITCHOVER attribute.
Signed-off-by: Manas Kumar Mandal <manamand@cisco.com>
Adds SAI support for hardware-driven fast reroute (FRR) of L3VNI-routed traffic to a cross-switch multihomed bridge port, extending the existing software-driven EVPN Multihoming fast-failover mechanism (autonomous ASIC switchover + async NOS notification, instead of NOS-driven SET_SWITCHOVER).
Changes
inc/saibridge.h: sai_bridge_port_protection_mode_t (SOFTWARE/HARDWARE) + SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_PROTECTION_MODE; sai_bridge_port_protection_state_t + read-only SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_PROTECTION_STATE; sai_bridge_port_protection_event_t, notification data struct, and callback typedef for HW switchover events.
SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_PROTECTION_REVERTIVE; control revertive behavior for switchover.
sai_bridge_port_protection_admin_mode_t ( SAI_BRIDGE_PORT_ATTR_BRIDGE_PORT_PROTECTION_MODE); allows NOS to control forced switchover.
inc/saiswitch.h: SAI_SWITCH_ATTR_BRIDGE_PORT_HW_PROTECTION_SWITCHOVER_NOTIFY to register the callback (mirrors the existing next-hop-group HW protection notify attribute).
inc/saitunnel.h: SAI_TUNNEL_ATTR_VXLAN_TUNNEL_MAC — inner DMAC for routed traffic re-encapsulated onto a P2P VXLAN tunnel after switchover.
doc/tunnel/SAI-Proposal-EVPN-Multihoming.md: new §3.2.6 spec, §4.6 workflow, Figure 7 object-model diagram.