Conversation
- Add ERSPAN mirroring to create/removeMirrorSession backed by gre tunnel - Extend MirrorSessionInfo struct with gre tunnel and monitor pin fields - Support SAI_ACL_ENTRY_ATTR_ACTION_MIRROR_INGRESS/EGRESS action - Add acl_table_get_ip_version to avoid duplicating address-less mirror rules (supports MIRROR/MIRRORv6 tables as separate tables) - Add gre tunnel and sonic-ext plugin bindings and types to SaiVppXlate - Maintain oid/neighbour IP/MAC map in SwitchVppNbr to resolve ERSPAN monitor port nexthop from DST_MAC - Move getLagFromPort to protected scope in SwitchStateBase to use in mirror monitor resolution path Signed-off-by: Taran Dwivedula <tdwivedu@cisco.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
aaronber0614
left a comment
aaronber0614
left a comment
There was a problem hiding this comment.
Review note (blocking):
The ERSPAN monitor pin currently installs the collector destination /32 or /128 in VRF 0 with is_multipath=false. This uses the same VPP API FIB source as the ordinary SAI route: adding the pin replaces the existing exact route, and removing the pin deletes that route without restoring it. Two mirror sessions using the same collector also share this prefix, so removing either session can break the other. Please give the pin independent ownership, such as a dedicated outer FIB, or preserve and restore the existing route with shared lifetime management.
The pin update is also reported as successful when route programming fails. applyErspanMonitor() sets monitor_pinned before calling pinErspanMonitor(), ignores both removal and addition failures, and setMirrorSession() persists the attribute regardless. Please propagate these failures and update the stored state only after the dataplane transition succeeds.
Finally, m_port_neighbor_mac is maintained only during neighbor create/remove. SAI_NEIGHBOR_ENTRY_ATTR_DST_MAC_ADDRESS updates currently bypass this index. When MirrorOrch subsequently updates the mirror session DST MAC, resolution uses the stale MAC, removes the working pin, and falls back to FIB/ECMP forwarding. Please handle neighbor MAC SET by updating both the VPP adjacency and this index.
The current Build and BuildAsan failures are caused by the expected dependency on the new gre_tunnel_add_del_v3 API from sonic-net/sonic-platform-vpp#283. This PR should be rerun after that dependency lands and its VPP packages are available.
Reviewed by AI agent on behalf of @aaronber0614.
- Add normalize_rule_ip_family to avoid auto reject, as dst family is inferred to be IP4 when not present, leading to mismatch and reject if src-only IP6 rule is provided. - Fix a double free caused by not setting aces to NULL after freeing it - Solves several test cases from test_everflow_ipv6.py, and fixes the syncd crash exposed by those tests Signed-off-by: Taran Dwivedula <tdwivedu@cisco.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
4 participants
Description of PR
Summary:
Addresses sonic-net/sonic-buildimage#25780 and sonic-net/sonic-buildimage#26047. Implements the feature described in Phase II in this HLD: https://github.com/yue-fred-gao/SONiC/blob/13c854acd86063cd9e11beacd2945515f0b28ac6/doc/vpp/vpp-mirror.md
Type of change
Approach
What is the motivation for this PR?
Enable Everflow feature for dataplane testing.
How did you verify/test it?
Passes 8/8 everflow/test_everflow_per_interface.py tests on both t0 and t1-lag topologies.
Passes 48/48 applicable everflow/test_everflow_testbed.py tests (EgressAclIngressMirror class skipped, dscp_with_policer tests skipped due to out of scope, frwd_with_bkg_trf skipped due to an open issue) on t0 and t1-lag topologies.
Skips are pending merge from these PRs:
IngressAclEgressMirror tests require dest route configuration skip to be removed, which is pending in this PR:
VPP patches, sonic-ext additions, and plugin additions are required, added in this PR: