NHG with Unequal Cost Members - #2302
Closed
JaiOCP wants to merge 12 commits into
Closed
Conversation
Contributor
Author
|
There is a case where NHG need to allocated differently for ECMP and UCMP for highly optimized fabrics.This PR addresses that case.On Jun 22, 2026, at 8:26 PM, sathk-marvell ***@***.***> wrote:
@sathk-marvell commented on this pull request.
In inc/sainexthopgroup.h:
@@ -359,6 +359,19 @@ typedef enum _sai_next_hop_group_attr_t
*/
SAI_NEXT_HOP_GROUP_ATTR_ADMIN_ROLE,
+ /**
+ * @brief This attribute indicates the nexthop group members weights
+ *
+ * NOS must query this attribute for the support.
+ * False: All members are with equal cost
+ * True: Members are configured with unequal cost
+ *
+ * @type bool
+ * @flags CREATE_AND_SET
+ * @default false
+ */
+ SAI_NEXT_HOP_GROUP_ATTR_MEMBERS_WITH_UNEQUAL_WEIGHTS,
@JaiOCP, what is the additional benefit of querying this capability?
If NOS is configured to program unequal paths, NOS can directly add those members to the NHG with the required weights, right? I believe we don't need any additional hardware capability/support to handle unequal-cost paths in this case.
Please let me know if I am missing any specific use case where this capability is required.
—Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
rck-innovium
suggested changes
Jun 25, 2026
Contributor
Author
|
@eddyk-nvidia @rck-innovium @tjchadaga Changes made as per the review. |
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Contributor
Author
|
@eddyk-nvidia @rck-innovium Please approve. This PR is next release candidate |
Signed-off-by: JaiOCP <jai.kumar@broadcom.com> NHG with UECMP Signed-off-by: JaiOCP <jai.kumar@broadcom.com> NHG with Weighted or unWeighted Members Signed-off-by: JaiOCP <jai.kumar@broadcom.com>
…puteproject#2274) This PR introduces two new ACL match fields to the SAI ACL headers: SAI_ACL_TABLE_ATTR_FIELD_ACL_MIRROR_COPY_TYPE: Allows ACL rules to match packets based on whether they are mirror copies and their specific type (Ingress, Egress, or Ingress Or Egress). SAI_ACL_TABLE_ATTR_FIELD_ACL_MIRROR_SESSION_ID: Allows ACL rules to match based on a specific mirror session ID. Signed-off-by: Mobin Mohan <mobinmohan@google.com>
…opencomputeproject#2308) Fixes opencomputeproject#2307 Two fixes in the SAI_STATUS_IS_* helpers in inc/saistatus.h: Compare on the magnitude (SAI_STATUS_CODE(x)) before masking, so the range check is correct for negative status codes on Linux. Previously a non-zero attribute index borrowed across the 16-bit boundary and matched the wrong range (e.g. INVALID_ATTR_VALUE index 3 classified as ATTR_NOT_IMPLEMENTED). No-op on Windows, where SAI_STATUS_CODE is identity. Fix SAI_STATUS_IS_UNKNOWN_ATTRIBUTE, which compared against SAI_STATUS_INVALID_ATTRIBUTE_0 instead of SAI_STATUS_UNKNOWN_ATTRIBUTE_0 (wrong on all platforms).
… fixes (opencomputeproject#2299) Context / motivation This PR is part of the SAIVPP unit-test framework: a Docker harness (docker-sai-test-vpp) that runs the upstream OCP sai_test PTF suite against the real VPP SAI backend (libsaivs) in one container — VPP + saiserver + PTF + veth/AF_PACKET topology. That work is documented in our devdocs under sonic-sairedis/.azure-pipelines/docker-sai-test-vpp/devdocs/ (see progress.md). To run the suite, saiserver (saithriftv2) must build and link against the VPP SAI backend on a bookworm/python3 toolchain. Upstream saithriftv2 only has a vs link line and a python2.7 install path, so it does not build for the vpp platform on bookworm. This is Phase 1, Task 2 of the SAIVPP UT HLD (the saithriftv2 Makefile work). What this change does Build/packaging only, no test logic: test/saithriftv2/Makefile — add a platform=vpp link line that pulls in the 5 VPP libraries SaiVppXlate.c requires (-lvlib -lvlibapi -lvppapiclient -lvlibmemoryclient -lvppinfra) plus -lswsscommon; also add -lswsscommon to the vs line. test/saithriftv2/src/saiserver.cpp — enable SWSS debug logging at startup (swss::Logger to STDOUT), so the harness captures a high-level SAI RPC trace in saiserver.log (used throughout our debugging). debian/python-saithrift.install — install the Python bindings from the python3 dist-packages path so the package builds on bookworm (was hardcoded to python2.7/site-packages). test/saithriftv2/convert_header.py — fix an enum __str__ that did split(".")[1] and threw an IndexError on some enum names; use self.name instead. Scope / risk 4 files, +11/−3 — build flags, a packaging path, a logging init, and a one-line stringification fix. No change to SAI semantics, RPC surface, or test logic. The vpp link line is new (additive); the vs line only gains -lswsscommon.
…pencomputeproject#2300) Context / motivation Part of the SAIVPP unit-test framework (see PR 1 / our docker-sai-test-vpp/devdocs/). When running the OCP sai_test suite against the VPP SAI backend, neighbor- and route-create tests failed at create time with SAI_STATUS_INVALID_PARAMETER (-5): ERROR: meta_generic_validation_create: switch id is NULL for SAI_OBJECT_TYPE_NEIGHBOR_ENTRY Root cause sai_thrift_neighbor_entry_t / sai_thrift_route_entry_t carry a switch_id, but many OCP tests build these entries with only rif_id (+ ip_address / destination) and omit switch_id. The saithriftv2 RPC server copied the thrift switch_id straight through, so meta validation received switch_id == 0 and rejected the create. (Tests that pass through the common-config path, which sets switch_id explicitly, succeeded — only entries that omit it failed.) What this change does meta/templates/sai_rpc_server_helper_functions.tt — custom sai_thrift_parse_neighbor_entry() / sai_thrift_parse_route_entry() that, when the thrift switch_id is 0, fall back to the RPC-global switch_id (the OID returned by sai_thrift_create_switch) and then gSwitchId. (The auto-generated struct copies for these two entry types are skipped in favor of these.) test/saithrift/src/switch_sai.thrift, switch_sai_rpc_server.cpp — the same switch_id field + fallback in the legacy v1 thrift server, for parity. test/sai_test/config/route_configer.py — pass switch_id=self.dut.switch_id explicitly on the route entries the common config builds, so the persisted T0 config is consistent regardless of the fallback. This mirrors how a real ASIC SAI shim behaves (the switch is unambiguous), and unblocks the neighbor/route-create-on-RIF tests. Scope / risk 4 files, +79/−18 — RPC-server parsing for two entry types (+ v1 parity) and explicit switch_id in the test common config. No change to SAI behavior for callers that already pass switch_id — the fallback only triggers when it is 0/omitted. Server-side only; no packet-path or test-logic change. Dependencies None. Related to opencomputeproject#2299, however, their edits are isolated and each target master.
…tdevs for standalone benches (opencomputeproject#2316) Context / motivation Part of the SAIVPP unit-test framework (see the sonic-sairedis docker-sai-test-vpp harness). When the OCP sai_test T0 suite runs against a SAI backend on a standalone bench with no SONiC control plane, there is no teamd to create PortChannel netdevs and no IntfMgr to assign LAG/SVI interface IPs — which some backends (e.g. the VPP virtual switch) rely on for LAG bring-up and routed-to-LAG/SVI forwarding. This adds an opt-in way for the test setup to emulate that. What this change does test/sai_test/config/simulate_sonic.py (new). Opt-in helper, active only when SIMULATE_SONIC=1; every entry point is a no-op otherwise. test/sai_test/config/lag_configer.py — create_lag(). When enabled, create the PortChannel<N> netdev teamd would provide, before the SAI LAG create. test/sai_test/config/route_configer.py — create_router_interface(). When enabled, assign the connected IPs IntfMgr would set on a LAG or VLAN(SVI) router interface. Vendor-neutral by construction. PortChannel netdevs use generic Linux ip … type bond; LAG RIF IPs use ip addr add. A VLAN SVI has no host-interface netdev to mirror from, so its address is programmed via command templates supplied by the caller (SVI_RIF_SET_IP_CMD / SVI_RIF_PROBE_CMD, with {ifname}/{addr} placeholders) — no backend-specific tooling is hardcoded in sai_test. All interface-name prefixes and address patterns are env-overridable. Scope / risk 3 files (one new module + two setUp-path hooks) — test-config helpers only; no change to SAI/backend code or packet semantics. Fully opt-in with default-off: with SIMULATE_SONIC unset, all entry points no-op, so real-HW and other OCP consumers are unchanged unless they opt in. Dependencies None. Independent of opencomputeproject#2299 / opencomputeproject#2300 / opencomputeproject#2301 (edits different functions); targets master.
…ix v4/v6 NHG port-list aliasing (opencomputeproject#2301) Context / motivation Part of the SAIVPP unit-test framework (see PR 1 / our docker-sai-test-vpp/devdocs/, esp. the 6-19 entry). Three independent correctness/robustness fixes in the OCP sai_test config helpers that the suite needs when run against the VPP SAI backend. What this change does test/sai_test/config/port_configer.py — drop bridge_id on create_bridge_port. Passing bridge_id to sai_thrift_create_bridge_port caused a create failure in our backend; the default 1Q bridge is used, so the argument is redundant. Removing it lets bridge-port creation succeed. test/sai_test/config/port_configer.py — env-gated, bounded port bring-up wait. turn_up_and_get_checked_ports() waited per port, serially (retries × sleep) for oper-status UP. On a 32-port topology where oper-status is slow to settle, this is ~60s+ of dead time per common-config build. The wait is now tunable via env (SAI_PORT_UP_RETRIES, SAI_PORT_UP_POLL_INTERVAL, SAI_PORT_UP_SHARED_WAIT) and can poll all ports together in one bounded window. Defaults preserve the original behavior (per-port wait), so real-HW/other OCP consumers are unchanged unless they opt in; only our harness sets the fast values. test/sai_test/config/route_configer.py — give v4/v6 NHGs independent member_port_indexs. create_nexthop_group_by_nexthops() constructed the v4 and v6 NexthopGroup objects sharing one Python list object for member_port_indexs. A mutation on one group (member remove/re-add tests) then corrupted the other's port list (ValueError: list.remove(x): x not in list). Each group now gets its own list(...) copy. (Latent aliasing bug, independent of any harness specifics.) Scope / risk 2 files, +64/−25 — test-config helpers only; no change to SAI/backend code or packet semantics. The port-up change is opt-in via env with original defaults, so it does not alter timing for existing consumers. The bridge_id removal relies on the default 1Q bridge (already how these tests are used). Dependencies None. Related to opencomputeproject#2299 and opencomputeproject#2300, however, their edits are isolated and each target master.
Signed-off-by: Prince George <prgeor@microsoft.com>
…uteproject#2321) Context / motivation Part of the SAIVPP unit-test framework. When OCP sai_test runs on a standalone bench with SIMULATE_SONIC=1, the bench emulates SONiC control-plane setup for LAG/SVI router interfaces. During that setup, a simulated router can emit IPv6 lifecycle frames such as Router Solicitations and MLDv2 reports. These frames share PTF dataplane queues with test traffic and can make strict verify_no_other_packets() assertions fail even though the test's TCP/UDP forwarding behavior is correct. The sonic-sairedis harness previously used PTF --relax globally, which disabled all unexpected-packet checks and masked this issue. The harness now scopes --relax to positive flooding tests only, so the shared simulation helper needs a narrow, opt-in way to isolate known startup control traffic without suppressing actual dataplane failures. What this change does test/sai_test/config/simulate_sonic.py. Adds an opt-in PTF filter enabled only when SIMULATE_SONIC=1 and a caller provides SIMULATE_SONIC_IPV6_CONTROL_SRC_MAC. Exact filtering. Discards only IPv6 ICMP Router Solicitations (type 133 to ff02::2) and MLDv2 reports (type 143 to ff02::16, including the standard Hop-by-Hop header) emitted from that configured source MAC. No vendor hardcoding. The caller supplies the source MAC. The shared helper does not contain VPP-specific MAC addresses or commands. Linux IPv6 setup ordering. Configures accept_dad=0, accept_ra=0, autoconf=0, and router_solicitations=0 before re-enabling IPv6 on a simulated LAG host interface, preventing a DAD/router-discovery race. Scope / risk One shared test-config helper. No SAI headers, backend code, or production SONiC runtime behavior changes. Default-off: with SIMULATE_SONIC unset, or without a valid source MAC, the helper installs no filter and retains existing behavior. Narrow by construction: ordinary IPv4/IPv6 traffic, other ICMPv6 types, and matching control frames from other source MACs stay visible to PTF strict verification. Validated behavior: unit-level boundary checks used captured Router Solicitation and MLDv2 frames and confirmed that ordinary traffic is retained. A standalone VPP harness rerun recovered all eleven classes previously failing solely on background IPv6 lifecycle traffic. Dependencies None. The change extends the existing SIMULATE_SONIC helper implemented in opencomputeproject#2316 and is independent of the prior SIMULATE_SONIC PR once that PR is merged. The sonic-sairedis VPP harness enables the option only in its separate Phase 3 integration branch. Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>z
* add SAI definitions for optical circuit switch Signed-off-by: Nathan Ni <nathan.ni@coherent.com>
Summary: This is similar to opencomputeproject#1158, opencomputeproject#1407, opencomputeproject#1430. Those PRs added label attribute for LAG/virtual router, counter, ACL Counter. This PR adds similar label attribute for ACL entries. Adding a label attribute that can be used to uniquely identify ACL entry object during warmboot. This attribute is considered as user data attached to the object. Signed-off-by: Shrikrishna (Shri) Khare <skhare@meta.com> Signed-off-by: Jasmeet Bagga <jasmeetbagga@meta.com> Signed-off-by: Ron He <zecheng@meta.com> Co-authored-by: Ron He <zecheng@meta.com>
kcudnik
reviewed
Aug 13, 2026
Contributor
Author
|
Fair enough :) will change it to unspecified.I was still living in my karnaugh map :)On Aug 12, 2026, at 9:24 PM, Kamil Cudnik ***@***.***> wrote:
@kcudnik commented on this pull request.
In inc/sainexthopgroup.h:
@@ -141,6 +141,22 @@ typedef struct _sai_next_hop_group_hw_protection_switchover_notification_data_t
sai_object_list_t failed_next_hop_groups;
} sai_next_hop_group_hw_protection_switchover_notification_data_t;
+/**
+ * @brief Next hop group is configured with weights or not
+ */
+typedef enum _sai_next_hop_group_members_weight_t
+{
+ /** This is for legacy platforms where this attribute is don't care */
+ SAI_NEXT_HOP_GROUP_MEMBERS_WEIGHT_DONT_CARE,
maybe change to SAI_NEXT_HOP_GROUP_MEMBERS_WEIGHT_IGNORED ? or NOT_APPLICABLE or UNSPECIFIED:? this souds more user friendly :)
—Reply to this email directly, view it on GitHub, or unsubscribe.Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Contributor
Author
|
Opened a new PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
12 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR brings in support for high performant fabric where NHG is created with prior information about the members with weights.
NOS can use sai_query_attribute_capability() to find out if the platform supports such behavior or not. Legacy implementation will always return the status as not supported attribute.