[vpp] CoPP dataplane enablement HLD #2539
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| # SONiC-VPP CoPP Dataplane Enablement — HLD | ||
|
|
||
| ## Revisions | ||
|
|
||
| | Rev | Date | Author(s) | Changes | | ||
| |-----|------|-----------|---------| | ||
| | 1.0 | 2026-09-02 | nhegde-microsoft | Initial HLD for `copp_punt_policer` design. | | ||
| | 1.1 | 2026-09-10 | nhegde-microsoft | Added `copp_ip2me_policer` plugin (IP2ME/SNMP/SSH enforcement on `ip4-punt`); updated status table. | | ||
|
|
||
| --- | ||
|
|
||
| ## Background | ||
|
|
||
| [sonic-buildimage#25801](https://github.com/sonic-net/sonic-buildimage/issues/25801) asks to enable Control Plane Policing (CoPP) testing for the `t1-lag-vpp` topology (`vms-kvm-vpp-t1-lag` testbed) on the SONiC-VPP KVM testbed. | ||
|
Contributor
There was a problem hiding this comment. I think the design should not limit the scope to t1-lag topology. We should at least include t0 to make sure it is compatible. we don't want to redesign it to work for both t1 and t0
Author
There was a problem hiding this comment. Fair point, I did not make it generic enough to cover t0. I believe the primary difference is the VLAN/bridge setup in t0 versus the L3 routed ports in t1-lag. t0 still uses the same linux-cp tap pairing, so I think this should work just fine, but I'd like to run a quick test first. I will update here and in the document when done. Thanks for the suggestion. |
||
|
|
||
| CoPP on real ASICs classifies control-plane protocols, traps them to the CPU, groups traps under trap-groups, and rate-limits each group with a policer. On SONiC-VPP, the SAI `config plane` already worked end-to-end before this effort (`orchagent`'s `CoppOrch` issues normal SAI calls, accepted and stored by `saivpp`) — what was missing was the `dataplane enforcement`: no VPP mechanism actually rate-limited or even punted most CoPP-relevant traffic to the CPU. | ||
|
|
||
| ### Control-plane protocols in scope | ||
|
|
||
| SONiC's default CoPP config (`copp_cfg.j2`) traps the following protocols on this platform (`show copp config` on `vlab-vpp-01`): | ||
|
|
||
| | Protocol / trap | Trap group | CIR/CBS (pps) | Notes | | ||
| |---|---|---|---| | ||
| | ARP request / response (`arp_req`, `arp_resp`) | `queue4_group2` | 600 | Addressed by this effort (device-input plugin) | | ||
| | LACP (`lacp`) | `queue4_group1` | 600 | Addressed by this effort | | ||
| | LLDP (`lldp`) | `queue4_group3` | 100 | Addressed by this effort | | ||
| | UDLD (`udld`) | `queue4_group3` | 100 | Addressed by this effort | | ||
| | TTL_ERROR (default trap group, IPv4 TTL-expiry) | (implicit default group) | 600 | Addressed by this effort | | ||
| | BGP / BGPv6 (`bgp`, `bgpv6`) | `queue4_group1` | 600 | Already worked pre-effort (rides `ip4-unicast`/`ip6-unicast`) | | ||
| | DHCP / DHCPv6 (`dhcp`, `dhcpv6`) | `queue4_group3` | 100 | Already worked pre-effort | | ||
| | IP2ME (`ip2me`) | `queue1_group1` | 600 | Already worked pre-effort (IP-destined-to-router traffic) | | ||
| | SNMP / SSH | `queue1_group1` (same as IP2ME) | 600 | Addressed by this effort (`copp_ip2me_policer` plugin) | | ||
| | Neighbor discovery (`neigh_discovery`) | `queue4_group2` | 600 | Already worked pre-effort | | ||
|
|
||
| ARP, LACP, LLDP, UDLD, TTL_ERROR are ethertype/L2-level control-plane traffic on `linux-cp`-paired, L3-routed ports, which never reaches any of VPP's existing classify-based policing arcs. BGP/DHCP/IP2ME/neighbor-discovery are IP-layer traffic that were already targeted by VPP's `ip4-unicast`/`ip6-unicast` policer-classify feature prior to this effort. | ||
|
|
||
| ## Requirements | ||
|
|
||
| | # | Requirement | | ||
| |---|-------------| | ||
| | REQ-1 | Creating a SAI `POLICER` object must program an equivalent policer in the VPP dataplane (CIR/CBS/PIR/PBS, meter type, mode, conform/exceed/violate actions), not just store the attributes. | | ||
| | REQ-2 | Creating a SAI `HOSTIF_TRAP` for a given `trap_type` must cause matching control-plane traffic (ARP, BGP, LACP, LLDP, DHCP/DHCPv6, UDLD, TTL_ERROR, IP2ME/SNMP/SSH) to be classified and punted to the CPU via the existing TAP/genetlink punt path. | | ||
| | REQ-3 | Traffic punted for a trap must first pass through the VPP policer bound to that trap's `HOSTIF_TRAP_GROUP` (`SAI_HOSTIF_TRAP_GROUP_ATTR_POLICER`), so excess traffic is dropped (or marked, per `SAI_POLICER_ATTR_RED_PACKET_ACTION`) rather than delivered to the CPU. | | ||
| | REQ-4 | Removing/disabling a trap at runtime (`test_add_new_trap`, `test_remove_trap`) must add/remove the corresponding classify/punt binding immediately, with no swss/syncd restart required. **Validated** — both pass live. | | ||
| | REQ-5 | SAI `getStats`/`getStatsExt` on a `POLICER` object must return live counters (`SAI_POLICER_STAT_GREEN/YELLOW/RED_PACKETS/BYTES`) sourced from VPP's policer conform/exceed/violate counters, not stubbed zeros. | | ||
| | REQ-6 | Trap/trap-group/policer configuration must persist and be re-applied after `config save` + reboot, matching existing SONiC CoPP semantics. | | ||
| | REQ-7 | The feature must not regress existing ACL, FDB, or routing dataplane behavior in `saivpp` — new code is additive (new object-type dispatch cases + new files), following the existing `SwitchVpp` extension pattern. | | ||
| | REQ-8 | Underlying testbed/harness issues that currently prevent the packet-injection subtests from even running (PTF auth, DUT service stability under VPP CPU load) must be resolved, since they block validation of REQ-1..REQ-6 regardless of SAI correctness. | | ||
|
|
||
| ## Design | ||
| ### `copp_punt_policer` VPP plugin | ||
|
|
||
| **Why a new plugin, not VPP's existing classify/policer features:** VPP already ships a classify-based policer feature (`policer-classify`), but it only runs on three feature arcs — `l2-input` (bridged L2 traffic), `ip4-unicast`, `ip6-unicast`. This project's ports are `linux-cp`-paired, L3-routed ports: ARP/LACP/LLDP/UDLD traffic on them never traverses any of those three arcs — `ethernet-input` dispatches it directly to protocol-specific nodes (`arp-input`, `linux-cp-punt-xc`) that punt straight to each port's TAP. | ||
| **What we built:** a self-contained VPP plugin, `copp_punt_policer` registering one feature node on `device-input`. It performs the following tasks: | ||
|
|
||
| 1. **Classify**: parses the raw 14-byte Ethernet header and looks up a small, bounded ethertype→policer-name table. TTL_ERROR is matched by ethertype `0x0800` (IPv4) **plus** an additional condition requiring the IP header's TTL ≤ 1. | ||
|
nhegde-microsoft marked this conversation as resolved.
|
||
| 2. **Police**: applies VPP's existing `vnet_police_packet()` token-bucket primitive against the same VPP policer object `SwitchVppPolicer.cpp` already creates from SAI `POLICER` attributes — no new metering implementation. Uses a fixed 256-byte reference packet length since VPP's pps→token-bucket conversion assumes that fixed size internally. | ||
|
nhegde-microsoft marked this conversation as resolved.
|
||
| 3. **Deliver**: for a conforming packet, sets the buffer's TX interface directly to the mapped linux-cp TAP and dispatches straight to `interface-output`. Resolves against the raw ingress port for every ethertype uniformly, matching what this project's PTF test harness (`ptf_nn_agent`) actually observes. | ||
|
nhegde-microsoft marked this conversation as resolved.
|
||
|
|
||
| SAI wiring (`SwitchVppHostifTrap.cpp`) mirrors the existing per-trap dispatch pattern used for bookkeeping: on `createHostifTrap`/`setHostifTrap`/`setHostifTrapGroup`, resolve the trap's bound policer to its VPP policer name and call the plugin's `copp_punt_policer_bind` API — switch-wide, since the plugin auto-enables its feature on every interface as it's created (`VNET_SW_INTERFACE_ADD_DEL_FUNCTION`), no per-port bind needed. | ||
|
nhegde-microsoft marked this conversation as resolved.
Contributor
There was a problem hiding this comment. I have concern with adding this node to device input, which will affect every packets. Most of the protocols listed (except UDLD and TTL expiry requiring investigation) are already punted to control plane today. Can we add the policer to the punt path? For example, in the interface output arc of tap interface?
Author
There was a problem hiding this comment. I did have the same concern but it seemed like the best option while I was trying this out. I also ran some tests and found the added delay was ~50ns per packet and the protocol match list was small (<16). The advantage was that this would work for any topology. |
||
|
|
||
| **Node graph:** | ||
|
|
||
| ``` | ||
| Before (bypasses policer-classify entirely): | ||
| device-input -> ethernet-input -> arp-input / linux-cp-punt-xc -> TAP | ||
|
|
||
| After: | ||
| device-input -> copp_punt_policer -> police -> { interface-output(TAP) | drop } | ||
| ``` | ||
|
|
||
| **Scope and cost notes:** | ||
|
|
||
| - **Tagged/VLAN frames are out of scope.** This project's ports are untagged L3-routed access ports; no VLAN sub-interface case exists on this testbed. The fixed 14-byte parse assumes untagged Ethernet — an 802.1Q-tagged frame's ethertype (and, for TTL_ERROR, the IPv4 TTL) would be read from the wrong offset and misclassified. Handling tagged frames is not attempted by this design. | ||
| - **Metering is pps-based, not byte-rate.** SONiC CoPP CIR/CBS on this platform are configured in pps, not bytes/sec, so the policing decision is deliberately packet-size-independent — the fixed 256-byte reference length passed to `vnet_police_packet()` is purely VPP's internal pps-to-token-bucket calibration constant, not a byte-accounting choice, and does not scale with real frame size. This is why `test_policer_mtu[BGP]` passes identically at 64/1514/4096B. This is orthogonal to the byte-oriented `SAI_POLICER_STAT_*_BYTES` counters (REQ-5): those still report real packet lengths for statistics purposes; only the policing *verdict* uses the fixed 256. | ||
| - **The node consumes matched, conforming packets.** A single `vlib_buffer_enqueue_to_next` per packet redirects it straight to `interface-output`/TAP; there is no double-punt. This means a trapped ARP packet's normal path (`arp-input`) is skipped, and VPP's own ARP-learning side effect on that path is lost for punted traffic — the equivalent function is expected to happen at the CPU/Linux side (kernel ARP handling on the TAP), matching the existing `linux-cp` model. | ||
| - **Fast-path cost for non-punted traffic.** Every packet on every port pays a 14-byte Ethernet header read plus a linear scan of a small, bounded table (`COPP_PUNT_POLICER_MAX_ENTRIES` = 16 today); no policer/counter work happens unless an entry matches. The lookup is linear, not hashed — acceptable given the table's small, static size, but noted here as a known optimization opportunity if entry count grows materially. | ||
|
|
||
| ### `copp_ip2me_policer` VPP plugin (IP2ME/SNMP/SSH) | ||
|
|
||
| **Why not `copp_punt_policer` or VPP's classify feature:** IP2ME/SNMP/SSH traffic has no ethertype to | ||
| match on (`copp_punt_policer` runs on `device-input`, pre-routing) — it is identified by destination IP | ||
| *after* routing. VPP's built-in classify-based policer only meters on interfaces it is explicitly bound | ||
| to, and since a shared classify table matches purely on destination IP, IP2ME traffic for interface A's | ||
| address can arrive via interface B — requiring binding on every L3 interface, which proved fragile | ||
| (binding-scope bugs, missing reply handler, watchdog stalls) during initial implementation. | ||
|
|
||
| **What we built:** a second plugin, `copp_ip2me_policer`, registered on the `ip4-punt` feature arc | ||
| (global, always-on, reached only after `ip4-lookup`/`ip4-local` already decide a packet is host-bound — | ||
| so no per-interface binding is ever needed). It tracks router-owned IPv4 addresses (added/removed as SAI | ||
| router-interface addresses change) and meters matches with the existing SAI-created policer via | ||
| `vnet_police_packet()`, the same primitive `copp_punt_policer` uses. Conforming/unmatched packets fall | ||
| through unchanged to `ip4-punt-redirect`; exceed/violate packets go to `ip4-drop`. | ||
|
|
||
| SAI wiring: `createHostifTrap`/`setHostifTrapGroup` for `SAI_HOSTIF_TRAP_TYPE_IP2ME` bind the shared | ||
| policer via `copp_ip2me_policer_bind`; router-interface IPv4 address add/remove calls | ||
| `copp_ip2me_policer_addr_add_del`. Both replace an earlier `ip4-policer-classify`-based implementation | ||
| (per-interface bind, deferred work queue) that is removed by this change. | ||
|
|
||
| ## Alternate Designs Considered | ||
|
|
||
| Two earlier enforcement designs were built, deployed, and disproven before landing on the design above. | ||
|
|
||
| 1. **Linux `tc` ingress policer on each port's hostif TAP device.** Linux delivers a copy of every received frame to `AF_PACKET` sniffers and PTF harness reads punted traffic off the TAP via a raw `AF_PACKET`/`SOCK_RAW` socket. But this happens _before_ the ingress qdisc/`tc filter` chain gets a chance to run. | ||
| 2. **VPP-native classify table bound via `policer_classify_set_interface(..., l2_table_index)`.** ARP/LACP/LLDP/UDLD traffic on the `linux-cp`-paired L3-routed ports are not caught by the policer bindings that are bound to the `l2-input` feature arc. | ||
|
|
||
| ## Status | ||
|
|
||
| All CoPP tests pass on `vlab-vpp-01` (testbed `vms-kvm-vpp-t1-lag`), including `test_policer_mtu` for | ||
| every protocol (IP2ME/SNMP/SSH/BGP): | ||
|
|
||
| | Test | Protocol | Result | | ||
| |---|---|---| | ||
| | `test_verify_copp_configuration_cli` | (config-plane, no traffic) | ✅ PASS | | ||
| | `test_policer[ARP]` | ARP | ✅ PASS | | ||
| | `test_policer[LACP]` | LACP | ✅ PASS | | ||
| | `test_policer[LLDP]` | LLDP | ✅ PASS | | ||
| | `test_policer[UDLD]` | UDLD | ✅ PASS | | ||
| | `test_policer[Default]` | TTL_ERROR | ✅ PASS | | ||
| | `test_policer[DHCP]` | DHCP | ✅ PASS | | ||
| | `test_policer[DHCP6]` | DHCPv6 | ✅ PASS | | ||
| | `test_trap_config_save_after_reboot` | (config persistence) | ✅ PASS | | ||
| | `test_policer_mtu[IP2ME]` (64/1514B) | IP2ME | ✅ PASS | | ||
| | `test_policer_mtu[SNMP]` (64/1514B) | SNMP | ✅ PASS | | ||
| | `test_policer_mtu[SSH]` (64/1514B) | SSH | ✅ PASS | | ||
| | `test_policer_mtu[BGP]` (64/1514/4096B) | BGP | ✅ PASS | | ||
| | `test_add_new_trap` | BGP (dynamic add) | ✅ PASS | | ||
| | `test_remove_trap[delete_feature_entry]` | BGP (dynamic remove) | ✅ PASS | | ||
| | `test_remove_trap[disable_feature_status]` | BGP (dynamic remove) | ✅ PASS | | ||
| | `test_trap_neighbor_miss` | (neighbor miss) | ✅ SKIPPED (t0 only) | | ||
|
|
||
| ## Key files changed | ||
|
|
||
| | Repo | File | Change | | ||
| |---|---|---| | ||
| | `sonic-platform-vpp` (`platform/vpp` submodule) | `vppbld/plugins/copp_punt_policer/{copp_punt_policer.c,.h,.api,_node.c,CMakeLists.txt}` | New VPP plugin: device-input classify+police+direct-to-TAP, incl. TTL_ERROR IPv4-TTL match support | | ||
| | `sonic-platform-vpp` (`platform/vpp` submodule) | `vppbld/plugins/copp_ip2me_policer/{copp_ip2me_policer.c,.h,.api,_node.c,CMakeLists.txt}` | New VPP plugin: `ip4-punt`-arc IP2ME/SNMP/SSH classify+police | | ||
| | `sonic-sairedis` | `vslib/vpp/SwitchVppHostifTrap.cpp` | Per-trap-type ethertype/TTL match-key table, plugin bind/unbind wiring, default-trap-group tracking fix | | ||
| | `sonic-sairedis` | `vslib/vpp/vppxlate/SaiVppXlate.c` / `.h` | `vpp_copp_punt_policer_bind()`/`_get_counters()` VAPI wrappers, extended with `match_ip4_ttl_expiring`; also fixed a missing `POLICER_CLASSIFY_SET_INTERFACE_REPLY` client reply-handler registration (caused an infinite CPU-spin retry loop on IP2ME L3-interface classify bind) | | ||
| | `sonic-sairedis` | `vslib/vpp/SwitchVpp{.cpp,.h}`, `SwitchVppHostifTrap.cpp`, `SwitchVppPolicer.cpp`, `SwitchVppRif.cpp`, `vppxlate/SaiVppXlate.{c,h}` | Removed the earlier `ip4-policer-classify`-based IP2ME implementation; added `vpp_copp_ip2me_policer_{addr_add_del,bind,get_counters}()` VAPI wrappers and call sites | | ||
| | `sonic-mgmt` | `tests/common/plugins/conditional_mark/tests_mark_conditions_sonic_vpp.yaml` | Lift `copp` skip for `asic_type in ['vpp']`; unskip `test_add_new_trap`/`test_remove_trap` | | ||
| | `sonic-mgmt` | `ansible/roles/test/files/ptftests/py3/copp_tests.py` | Added `'vpp'` to `BGPTest.check_constraints()`'s asic_type exception list (same treatment as `broadcom`/`marvell-teralynx`); added `ip_src` to `IP2METest`/`SNMPTest` packet construction (previously defaulted to an unroutable address, failing VPP's uRPF check) | | ||
|
|
||
| ## References | ||
|
|
||
| - [sonic-buildimage#25801](https://github.com/sonic-net/sonic-buildimage/issues/25801) | ||
| - `sonic-sairedis` branch `copp-vpp-enablement` (SAI POLICER/HOSTIF_TRAP/HOSTIF_TRAP_GROUP wiring) | ||
| - `sonic-platform-vpp` branch `copp-vpp-enablement` (`copp_punt_policer` VPP plugin) | ||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so far sonic-ext plugin is used for multiple features for sonic. Please consider if it is better to use the same plugin. The benefit is we can avoid too many plugins. But we lose the flexibility to enable/disable a feature through plugin infra. Currently, we use API in the sonic-ext plugin to enable/disable individual feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point. Let me look into feasibility of moving this into sonic-ext.