[doc] SONiC-VPP packet trimming HLD - #2483
Conversation
High-level design for enabling SAI packet trimming on the sonic-vpp data plane: the software admission + trim datapath in the sonic_ext VPP plugin, the SAI attribute translation in sonic-sairedis vslib/vpp, the sonic-mgmt test enablement, dev-VM validation results, and the explicitly deferred scope (asymmetric trimming, ACL/mirror/SRv6 actions, trim counters exposure, and reload/reboot stability toggles). Tracking: sonic-net/sonic-buildimage#25789 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Aaron Bernardino <aaronber@microsoft.com>
Address code-review gap M5: update the SONiC-VPP packet-trimming HLD so it matches what shipped. Replace the stale "gate SWITCH_TRIMMING_CAPABLE to false / not supported" language with the delivered model: - advertise SWITCH_TRIMMING_CAPABLE = true, - narrow the trimming enum-values capability to the single resolution mode the VPP software shim honors (DSCP_VALUE for the DSCP mode, STATIC for the queue mode), and - defer the out-of-scope config variants via conditional_mark (asic_type == vpp) rather than by suppressing the capability. Sections 2 (Scope), 4 (Background), 6.3 (component responsibilities), 14.3 (Enablement Gating) and 15 (Open Items) are reconciled; the previously rewritten 7.12 / 13.1 remain consistent. Tracking: sonic-net/sonic-buildimage#25789 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Aaron Bernardino <aaronber@microsoft.com>
Second accuracy pass over the full HLD against the committed sonic_ext plugin, SAI-VPP translation, and sonic-mgmt changes. Distinguish the pre-change baseline, delivered symmetric core, and deferred target behavior throughout: - Admission model: buffer profiles select trim eligibility; SAI-VPP maps the effective scheduler to a binary unlimited/blocked bucket (UINT64_MAX or 0 rate/capacity) rather than deriving a proportional rate or buffer-profile capacity. - Original-queue classification via a switch-wide DSCP-to-queue table; multicast queues skipped by type, unicast index >= 8 skipped by cap. - Counters: three switch-global plugin summaries only; SAI getStatsExt sourcing and per-port/queue scoping deferred. - Update/create/remove ordering matched to the delivered error propagation (set commits then surfaces the VPP push failure for orchagent retry; create refresh is best effort; remove does not refresh). - Marked FROM_TC, DYNAMIC, ACL disable-trim, GCU config, toggles, persistence, mirroring, and SRv6 as deferred, with matching conditional_mark skips and CLI/warmboot/requirement status updates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Aaron Bernardino <aaronber@microsoft.com>
Replace the incremental v0.1-v0.7 revision log with one v1.0 entry; the working history is not needed in the published design. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Aaron Bernardino <aaronber@microsoft.com>
… original drop) Multi-agent review flagged that the HLD overstated the datapath as performing a shim-level "normal original-packet drop" for non-eligible queues. The delivered admission node never polices a non-eligible or unconfigured queue -- it passes the packet straight through on the interface-output arc to normal port TX -- so forwarding is unchanged and the shim adds no drop of its own. - REQ-3 status: clarify that DROP/unbound queues are simply never made trim-eligible (forwarding unchanged), while DROP_AND_TRIM marks the queue eligible and invokes trim. - §6.2: annotate the flowchart's 'Normal original-packet drop' node as the conceptual/hardware admission model, not shim behavior. - §12: reword the missing-egress-metadata row to passthrough/bypass semantics. - §13.2: document the last-good DSCP-to-queue table retained by SAI-VPP when no port currently binds both QoS maps, and why a stale mapping is benign. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Aaron Bernardino <aaronber@microsoft.com>
|
/azp run |
|
No pipelines are associated with this pull request. |
|
Companion PRs (SONiC-VPP packet trimming, tracked in sonic-net/sonic-buildimage#25789):
Suggested review/merge order: HLD → sonic-platform-vpp → sonic-sairedis → sonic-mgmt. |
There was a problem hiding this comment.
Pull request overview
Adds a new High Level Design (HLD) document under doc/vpp/ describing how SONiC packet trimming is implemented on the SONiC-VPP dataplane (software admission shim on the VPP interface-output arc, trim action behavior, SAI-to-VPP translation, capability advertisement, counters, and deferred items).
Changes:
- Introduces the SONiC-VPP packet trimming HLD with control-plane + dataplane flows and component responsibilities.
- Documents the delivered (symmetric) feature set vs deferred items (asymmetric
FROM_TC, dynamic trim queue resolution, ACL disable-trim, SAI-scoped counters, warmboot validation, etc.). - Captures testing/enablement status and the planned gating approach for
sonic-mgmtonasic_type == vpp.
Suppressed comments (1)
doc/vpp/vpp-packet-trimming-HLD.md:774
- The error-handling section currently states that out-of-range values are silently narrowed/masked, and that unicast queue indices >= 8 are “skipped as SUCCESS”. That combination can make SONiC believe trimming is correctly configured when it is partially ignored, and it can route trimmed packets to a different queue than intended. It would be more robust to validate these inputs and fail the set operation so orchagent can retry/correct the configuration.
| Out-of-range scalar values reaching the vendor layer | The vendor code performs no additional rejection: trim size is narrowed to `u16`, DSCP is masked to six bits by the plugin, and the static trim queue is masked to three bits. The supported control path programs valid values (size 256/4084, DSCP 48, queue 6) |
| Queue is multicast or has an out-of-range unicast index | Multicast queues are skipped by `SAI_QUEUE_ATTR_TYPE` so a colliding multicast index cannot overwrite the corresponding unicast admission slot. Unicast indices at or above `SONIC_EXT_TRIM_MAX_QUEUES` (8) are skipped as SUCCESS. Every eligible lossy queue in the supported configuration is unicast and below the cap ([13.2](#132-design-limitations)) |
| | `SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_RESOLUTION_MODE` | Accept `STATIC`; `DYNAMIC` is omitted from the advertised enum ([7.12](#712-capability-advertisement)) so orchagent never programs it. A stored `DYNAMIC` value is not honored by the datapath (an explicit set is accepted as a no-op) | | ||
| | `SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_INDEX` | Global static trim queue; the plugin stores the low three bits (`queue & 7`). The validated configuration uses queue 6 | |
The two §6 flowcharts were mermaid fenced blocks, which only render in GitHub's rendered-file view (not in the PR diff) and not in editors/wikis without a mermaid plugin. Match the VXLAN HLD format by committing pre-rendered PNG diagrams under doc/vpp/images/ and embedding them with sized, centered <img> tags (width-constrained so they don't dominate the page) plus descriptive alt text. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Aaron Bernardino <aaronber@microsoft.com>
|
/azp run |
|
No pipelines are associated with this pull request. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Aaron Bernardino <aaronber@microsoft.com>
|
/azp run |
|
No pipelines are associated with this pull request. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (2)
doc/vpp/vpp-packet-trimming-HLD.md:661
- The mapping describes accepting an explicitly-set
DYNAMICqueue-resolution mode as a no-op and masking the configured trim-queue index (queue & 7). Both behaviors can silently diverge from the configured SAI state (e.g.,DYNAMICappears set but is ignored; an out-of-range queue wraps to a different queue), which makes misconfiguration hard to detect and troubleshoot. Prefer rejecting unsupported/invalid values with a clear error (and/or logging) rather than silently accepting/masking them.
| `SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_RESOLUTION_MODE` | Accept `STATIC`; `DYNAMIC` is omitted from the advertised enum ([7.12](#712-capability-advertisement)) so orchagent never programs it. A stored `DYNAMIC` value is not honored by the datapath (an explicit set is accepted as a no-op) |
| `SAI_SWITCH_ATTR_PACKET_TRIM_QUEUE_INDEX` | Global static trim queue; the plugin stores the low three bits (`queue & 7`). The validated configuration uses queue 6 |
doc/vpp/vpp-packet-trimming-HLD.md:1016
- This paragraph includes a very specific local-harness/ansible error string that is likely to go stale and isn’t essential to the HLD’s design description. Consider generalizing this to a short note about local-harness limitations and leaving detailed failure signatures in an issue or troubleshooting doc.
The local KVM harness cannot exercise a `conditional_mark`-enforcing `pytest`
run because its `dut_basic_facts` ansible module is incompatible with the
harness ansible-core (`No module named ansible.module_utils.parse_utils`), which
is why in-harness runs use `--ignore-conditional-mark` and select cases with
`-k`. The rule is therefore validated at the plugin-decision level and behaves
What / Why
Adds the High Level Design document for SONiC-VPP packet trimming, describing
how the SAI
DROP_AND_TRIMmodel is realized as a software admission shim onthe VPP
interface-outputfeature arc, the SAI-to-VPP translation, capabilitygating, counters, and the deferred/target items.
Part of the SONiC packet-trimming enablement tracked in
sonic-net/sonic-buildimage#25789.
Changes
doc/vpp/vpp-packet-trimming-HLD.md.🤖 Co-authored with GitHub Copilot.