From 40162e41f4dcfa2db67ae05a5b644fd74e702b41 Mon Sep 17 00:00:00 2001 From: AkeelAli <701916+AkeelAli@users.noreply.github.com> Date: Tue, 1 Sep 2026 08:59:22 -0400 Subject: [PATCH 1/3] Latest t0 drop_counters.py changes Signed-off-by: AkeelAli <701916+AkeelAli@users.noreply.github.com> --- docs/HLD/vpp-port-drop-stats.md | 191 ++++++++++++++++++ rules/vpp.mk | 2 +- ...tats-track-original-member-interface.patch | 87 -------- ...tats-track-original-member-interface.patch | 111 ++++++++++ vppbld/patches/series | 6 +- 5 files changed, 307 insertions(+), 90 deletions(-) create mode 100644 docs/HLD/vpp-port-drop-stats.md delete mode 100644 vppbld/patches/0008-bond-drop-stats-track-original-member-interface.patch create mode 100644 vppbld/patches/0008-drop-stats-track-original-member-interface.patch diff --git a/docs/HLD/vpp-port-drop-stats.md b/docs/HLD/vpp-port-drop-stats.md new file mode 100644 index 00000000..236e21e2 --- /dev/null +++ b/docs/HLD/vpp-port-drop-stats.md @@ -0,0 +1,191 @@ +# VPP Port Drop Stats HLD + +## Table of Contents + +1. [Revisions](#revisions) +2. [Scope](#scope) +3. [Problem Statement](#problem-statement) +4. [Solution](#solution) +5. [Design Details](#design-details) +6. [Code References](#code-references) +7. [Expected Behavior](#expected-behavior) +8. [Risks](#risks) + +## Revisions + +| Rev | Date | Author(s) | +|-----|------|-----------| +| v0.1 | 08/31/2026 | Akeel Ali | + +## Scope + +This document describes the high level design for counting interface drops +against the original physical ingress port on the SONiC VPP platform. + +The change is implemented as a VPP dataplane patch, +`vppbld/patches/0008-drop-stats-track-original-member-interface.patch`, applied +to the bundled VPP source used by the `syncd-vpp` image. It ensures that a drop +is also counted on the physical member port's `drops` counter when the RX +interface index has been rewritten by LAG (bonding) or SVI/BVI forwarding. It is +purely a VPP dataplane change: there are no SAI, orchagent, SONiC CLI, or +database-schema changes, and interfaces whose RX index is not rewritten are +unaffected. + +## Problem Statement + +VPP's `error-drop` node runs `interface_drop_punt()`, which increments the +per-interface `drops` counter (`/interfaces//drops`, +`VNET_INTERFACE_COUNTER_DROP`) for the packet's RX interface index +`sw_if_index[VLIB_RX]`. sairedis maps this `drops` counter to +`SAI_PORT_STAT_IF_IN_DISCARDS` (RX_DRP in `show interfaces counters`). + +Two forwarding features rewrite `sw_if_index[VLIB_RX]` before a packet can reach +the drop path: + +1. **LAG / bonding** — `bond_sw_if_idx_rewrite()` replaces the member's RX index + with the bond interface's index. +2. **SVI / VLAN routing** — `l2_to_bvi()` replaces the RX index with the index of + the bridge domain's BVI (Bridge Virtual Interface) to route out of the bridge + domain. + +As a result, a drop on a LAG member or a VLAN (bridge-domain) member is counted +against the LAG or BVI, not the physical member, so the ingress port shows zero +drops — losing per-port visibility exactly in the aggregated topologies where it +matters most. The original port is unrecoverable at drop time (a LAG +load-balances across members, a BVI has many members), so it must be captured at +the rewrite and carried on the buffer to the drop node. + +This gap is exercised by the sonic-mgmt drop-counter tests +(`tests/drop_packets/test_drop_counters.py`), whose `tx_dut_ports` fixture +parametrizes the ingress port over `port_channel_members`, `vlan_members`, and +`rif_members`. Counting LAG-member drops against the member covers the +`port_channel_members` case on the `t1-lag-vpp` topology, and counting SVI/BVI +member drops against the member covers the `vlan_members` case on the `t0-vpp` +topology. The `rif_members` case needs no special handling, since a plain L3 RIF +port has no RX rewrite. + +A previous iteration stored the index in `opaque2` (VPP's second per-buffer +scratch metadata area) and treated `0` as "not set", zeroing it in `bond_input`. +That was fragile: `opaque2` is not reset per packet, so packets that skip +`bond_input` could read a stale non-zero value; `0` is also a valid `sw_if_index` +(`local0`); and the `member -> LAG -> BVI` chain recorded the LAG instead of the +physical member. + +## Solution + +Capture the original RX interface index on the buffer at each rewrite site, mark +it valid with a dedicated buffer flag, and have `interface_drop_punt()` add a +second counter increment against it. + +Validity is signaled by a buffer flag rather than a sentinel value because +`b->flags` is reset on every buffer allocation (copied from the pool template) +while `opaque2` is not. The flag therefore provides correct per-packet validity +at near-zero fast-path cost, with no extra initialization node, and it composes +correctly with buffer clones used for flood/replication. + +## Design Details + +### Buffer storage and validity flag (`src/vnet/buffer.h`) + +- A `u32 orig_rx_sw_if_index` field is carved from the tail `unused[]` words of + `vnet_buffer_opaque2_t`, so the struct does not grow: + + ```c + u32 orig_rx_sw_if_index; + u32 unused[5]; + ``` + +- Buffer flag bit 27 (previously `AVAIL9`) is repurposed as + `VNET_BUFFER_F_ORIG_RX_SW_IF_VALID` and removed from + `VNET_BUFFER_FLAGS_ALL_AVAIL`. The highest available bit is chosen to minimize + rebase collisions with upstream, which allocates from the lowest free bit. + +### Capture at the LAG rewrite (`src/vnet/bonding/node.c`) + +In both branches of `bond_sw_if_idx_rewrite()`, before the RX index is +overwritten with the bond index: + +```c +b->flags |= VNET_BUFFER_F_ORIG_RX_SW_IF_VALID; +vnet_buffer2 (b)->orig_rx_sw_if_index = vnet_buffer (b)->sw_if_index[VLIB_RX]; +``` + +### Capture at the BVI rewrite (`src/vnet/l2/l2_bvi.h`) + +In `l2_to_bvi()`, record the ingress member before the BVI rewrite, but only if +nothing has recorded it yet, so a bonded packet keeps its physical member instead +of the LAG: + +```c +if (!(b0->flags & VNET_BUFFER_F_ORIG_RX_SW_IF_VALID)) + { + b0->flags |= VNET_BUFFER_F_ORIG_RX_SW_IF_VALID; + vnet_buffer2 (b0)->orig_rx_sw_if_index = + vnet_buffer (b0)->sw_if_index[VLIB_RX]; + } +``` + +### Second increment at drop (`src/vnet/interface_output.c`) + +In `interface_drop_punt()`, after the existing super-interface increment, walk the +buffers just counted and, gated on the flag, increment the original member port: + +```c +u32 orig_off = frame->n_vectors - n_left - count; +for (u32 j = 0; j < count; j++) + { + vlib_buffer_t *ob = bufs[orig_off + j]; + if (!(ob->flags & VNET_BUFFER_F_ORIG_RX_SW_IF_VALID)) + continue; + u32 orig = vnet_buffer2 (ob)->orig_rx_sw_if_index; + if (orig != sw_if_index[0]) + vlib_increment_simple_counter (cm, thread_index, orig, 1); + } +``` + +The flag check prevents reading a stale `opaque2` word, and the +`orig != sw_if_index[0]` check avoids double-counting when the original equals the +interface already counted. + +`interface_drop_punt()` is shared with the `error-punt` node, so this same `cm` +increment also bumps the member's `punt` counter; because sairedis does not export +`punt` to a SAI port stat, only the `drops` / in-discards case is user-visible. + +## Code References + +The implementation is the VPP dataplane patch +`vppbld/patches/0008-drop-stats-track-original-member-interface.patch`. + +| Area | File (patched VPP source) | +|------|---------------------------| +| Buffer field + validity flag | `src/vnet/buffer.h` | +| Capture at LAG member ingress | `src/vnet/bonding/node.c` (`bond_sw_if_idx_rewrite`) | +| Capture at SVI/BVI rewrite | `src/vnet/l2/l2_bvi.h` (`l2_to_bvi`) | +| Second drop increment | `src/vnet/interface_output.c` (`interface_drop_punt`) | + +## Expected Behavior + +1. A drop on a LAG member port is counted on both the LAG and the physical + member port (`SAI_PORT_STAT_IF_IN_DISCARDS`). +2. A drop on a VLAN (bridge-domain) member port is counted on both the BVI and + the physical member port. +3. For a `member -> LAG -> BVI` chain, the drop is counted against the physical + member, not the intermediate LAG. +4. Traffic on ports whose RX index is not rewritten is counted exactly as before. +5. The drop appears on the physical member's `SAI_PORT_STAT_IF_IN_DISCARDS`, + satisfying the `port_channel_members` case (`t1-lag-vpp`) via the LAG capture + and the `vlan_members` case (`t0-vpp`) via the SVI/BVI capture in + `tests/drop_packets/test_drop_counters.py`; the `rif_members` case is + unaffected. + +## Risks + +The main risk with this solution is that it relies on non-upstream buffer state — a scarce +per-buffer flag bit (bit 27, formerly `AVAIL9`) plus a `u32` in `opaque2` — so it +is fragile across VPP rebases. It permanently consumes one of VPP's limited +`AVAIL` flag bits and must be re-checked on every rebase: if upstream later +assigns bit 27, the patch has to move to another free bit. The flag also guards +only against stale reads across packets, not against another node overwriting the +`orig_rx_sw_if_index` word in `opaque2` for the same packet between the RX rewrite +and the drop; that word is carved from the tail `unused[]` words that nothing +touches today, so that case is low risk. diff --git a/rules/vpp.mk b/rules/vpp.mk index 1d59ed0c..739c82c6 100644 --- a/rules/vpp.mk +++ b/rules/vpp.mk @@ -7,7 +7,7 @@ VPP_VERSION_BASE = 2606 # https://packages.buildkite.com/sonic-vpp/vpp; if the suffix isn't bumped, # downstream sonic-buildimage builds will silently pull stale debs that # pre-date the new patch series and end up with VPP/SAI CRC drift. -VPP_VERSION = $(VPP_VERSION_BASE)-0.5 +VPP_VERSION = $(VPP_VERSION_BASE)-0.6 VPP_VERSION_SONIC = $(VPP_VERSION)+b1sonic1 VPP_SRC_PATH = platform/vpp/vppbld diff --git a/vppbld/patches/0008-bond-drop-stats-track-original-member-interface.patch b/vppbld/patches/0008-bond-drop-stats-track-original-member-interface.patch deleted file mode 100644 index 552f8167..00000000 --- a/vppbld/patches/0008-bond-drop-stats-track-original-member-interface.patch +++ /dev/null @@ -1,87 +0,0 @@ -diff --git a/src/vnet/bonding/node.c b/src/vnet/bonding/node.c -index 88dc0c875..3a6b0c8a3 100644 ---- a/src/vnet/bonding/node.c -+++ b/src/vnet/bonding/node.c -@@ -91,6 +91,9 @@ bond_sw_if_idx_rewrite (vlib_main_t * vm, vlib_node_runtime_t * node, - && !packet_is_cdp (eth) - && (ethertype != htons (ETHERNET_TYPE_802_1_LLDP)))) - { -+ /* Save original member interface before rewrite */ -+ vnet_buffer2 (b)->orig_rx_sw_if_index = -+ vnet_buffer (b)->sw_if_index[VLIB_RX]; - /* Change the physical interface to bond interface */ - vnet_buffer (b)->sw_if_index[VLIB_RX] = bond_sw_if_index; - return; -@@ -111,6 +114,9 @@ bond_sw_if_idx_rewrite (vlib_main_t * vm, vlib_node_runtime_t * node, - && (ethertype != htons (ETHERNET_TYPE_CDP)) - && (ethertype != htons (ETHERNET_TYPE_802_1_LLDP)))) - { -+ /* Save original member interface before rewrite */ -+ vnet_buffer2 (b)->orig_rx_sw_if_index = -+ vnet_buffer (b)->sw_if_index[VLIB_RX]; - /* Change the physical interface to bond interface */ - vnet_buffer (b)->sw_if_index[VLIB_RX] = bond_sw_if_index; - return; -@@ -229,6 +235,13 @@ VLIB_NODE_FN (bond_input_node) (vlib_main_t * vm, - sw_if_index[2] = vnet_buffer (b[2])->sw_if_index[VLIB_RX]; - sw_if_index[3] = vnet_buffer (b[3])->sw_if_index[VLIB_RX]; - -+ /* Clear orig_rx_sw_if_index; will be set by bond_sw_if_idx_rewrite -+ only for packets whose sw_if_index[VLIB_RX] is rewritten. */ -+ vnet_buffer2 (b[0])->orig_rx_sw_if_index = 0; -+ vnet_buffer2 (b[1])->orig_rx_sw_if_index = 0; -+ vnet_buffer2 (b[2])->orig_rx_sw_if_index = 0; -+ vnet_buffer2 (b[3])->orig_rx_sw_if_index = 0; -+ - x |= sw_if_index[0] ^ last_member_sw_if_index; - x |= sw_if_index[1] ^ last_member_sw_if_index; - x |= sw_if_index[2] ^ last_member_sw_if_index; -@@ -315,6 +328,7 @@ VLIB_NODE_FN (bond_input_node) (vlib_main_t * vm, - while (n_left) - { - sw_if_index[0] = vnet_buffer (b[0])->sw_if_index[VLIB_RX]; -+ vnet_buffer2 (b[0])->orig_rx_sw_if_index = 0; - bond_update_next (vm, node, &last_member_sw_if_index, sw_if_index[0], - &bond_sw_if_index, b[0], &next_index, &error); - next[0] = next_index; -diff --git a/src/vnet/buffer.h b/src/vnet/buffer.h -index 0ff3bf07d..22eaa9bde 100644 ---- a/src/vnet/buffer.h -+++ b/src/vnet/buffer.h -@@ -509,7 +509,11 @@ typedef struct - u8 loop_counter; - - u8 unused8; -- u32 unused[6]; -+ /* Original RX sw_if_index before bond/other rewrites. -+ Set by bond-input so that error-drop/error-punt can also -+ count drops against the physical member interface. */ -+ u32 orig_rx_sw_if_index; -+ u32 unused[5]; - } vnet_buffer_opaque2_t; - - #define vnet_buffer2(b) ((vnet_buffer_opaque2_t *) (b)->opaque2) -diff --git a/src/vnet/interface_output.c b/src/vnet/interface_output.c -index f7d65619f..8817157bb 100644 ---- a/src/vnet/interface_output.c -+++ b/src/vnet/interface_output.c -@@ -1048,6 +1048,19 @@ interface_drop_punt (vlib_main_t * vm, - if (sw_if0->sup_sw_if_index != sw_if_index[0]) - vlib_increment_simple_counter - (cm, thread_index, sw_if0->sup_sw_if_index, count); -+ -+ /* Also count against the original member interface if the -+ RX sw_if_index was rewritten (e.g. by bond-input). */ -+ { -+ u32 orig_off = frame->n_vectors - n_left - count; -+ for (u32 j = 0; j < count; j++) -+ { -+ vlib_buffer_t *ob = bufs[orig_off + j]; -+ u32 orig = vnet_buffer2 (ob)->orig_rx_sw_if_index; -+ if (orig && orig != sw_if_index[0]) -+ vlib_increment_simple_counter (cm, thread_index, orig, 1); -+ } -+ } - } - - vnet_interface_main_t *im = &vnm->interface_main; diff --git a/vppbld/patches/0008-drop-stats-track-original-member-interface.patch b/vppbld/patches/0008-drop-stats-track-original-member-interface.patch new file mode 100644 index 00000000..10f5d8fa --- /dev/null +++ b/vppbld/patches/0008-drop-stats-track-original-member-interface.patch @@ -0,0 +1,111 @@ +diff --git a/src/vnet/bonding/node.c b/src/vnet/bonding/node.c +index 88dc0c8..3985a88 100644 +--- a/src/vnet/bonding/node.c ++++ b/src/vnet/bonding/node.c +@@ -91,6 +91,10 @@ bond_sw_if_idx_rewrite (vlib_main_t * vm, vlib_node_runtime_t * node, + && !packet_is_cdp (eth) + && (ethertype != htons (ETHERNET_TYPE_802_1_LLDP)))) + { ++ /* Save original member interface before the RX rewrite */ ++ b->flags |= VNET_BUFFER_F_ORIG_RX_SW_IF_VALID; ++ vnet_buffer2 (b)->orig_rx_sw_if_index = ++ vnet_buffer (b)->sw_if_index[VLIB_RX]; + /* Change the physical interface to bond interface */ + vnet_buffer (b)->sw_if_index[VLIB_RX] = bond_sw_if_index; + return; +@@ -111,6 +115,10 @@ bond_sw_if_idx_rewrite (vlib_main_t * vm, vlib_node_runtime_t * node, + && (ethertype != htons (ETHERNET_TYPE_CDP)) + && (ethertype != htons (ETHERNET_TYPE_802_1_LLDP)))) + { ++ /* Save original member interface before the RX rewrite */ ++ b->flags |= VNET_BUFFER_F_ORIG_RX_SW_IF_VALID; ++ vnet_buffer2 (b)->orig_rx_sw_if_index = ++ vnet_buffer (b)->sw_if_index[VLIB_RX]; + /* Change the physical interface to bond interface */ + vnet_buffer (b)->sw_if_index[VLIB_RX] = bond_sw_if_index; + return; +diff --git a/src/vnet/buffer.h b/src/vnet/buffer.h +index 3cef5e6..efcd447 100644 +--- a/src/vnet/buffer.h ++++ b/src/vnet/buffer.h +@@ -41,7 +41,7 @@ + _ (24, AVAIL6, "avail6", 1) \ + _ (25, AVAIL7, "avail7", 1) \ + _ (26, AVAIL8, "avail8", 1) \ +- _ (27, AVAIL9, "avail9", 1) ++ _ (27, ORIG_RX_SW_IF_VALID, "orig-rx-sw-if-valid", 1) + + /* + * Please allocate the FIRST available bit, redefine +@@ -52,7 +52,7 @@ + #define VNET_BUFFER_FLAGS_ALL_AVAIL \ + (VNET_BUFFER_F_AVAIL1 | VNET_BUFFER_F_AVAIL2 | VNET_BUFFER_F_AVAIL3 | \ + VNET_BUFFER_F_AVAIL4 | VNET_BUFFER_F_AVAIL5 | VNET_BUFFER_F_AVAIL6 | \ +- VNET_BUFFER_F_AVAIL7 | VNET_BUFFER_F_AVAIL8 | VNET_BUFFER_F_AVAIL9) ++ VNET_BUFFER_F_AVAIL7 | VNET_BUFFER_F_AVAIL8) + + #define VNET_BUFFER_FLAGS_VLAN_BITS \ + (VNET_BUFFER_F_VLAN_1_DEEP | VNET_BUFFER_F_VLAN_2_DEEP) +@@ -508,7 +508,12 @@ typedef struct + u8 loop_counter; + + u8 unused8; +- u32 unused[6]; ++ /* Original RX sw_if_index before an RX rewrite (bond-input or the ++ l2-to-bvi / SVI path). Valid only when ++ VNET_BUFFER_F_ORIG_RX_SW_IF_VALID is set; lets error-drop/error-punt ++ also count drops against the physical member interface. */ ++ u32 orig_rx_sw_if_index; ++ u32 unused[5]; + } vnet_buffer_opaque2_t; + + #define vnet_buffer2(b) ((vnet_buffer_opaque2_t *) (b)->opaque2) +diff --git a/src/vnet/interface_output.c b/src/vnet/interface_output.c +index f7d6561..a8596f6 100644 +--- a/src/vnet/interface_output.c ++++ b/src/vnet/interface_output.c +@@ -1048,6 +1048,22 @@ interface_drop_punt (vlib_main_t * vm, + if (sw_if0->sup_sw_if_index != sw_if_index[0]) + vlib_increment_simple_counter + (cm, thread_index, sw_if0->sup_sw_if_index, count); ++ ++ /* Also count against the original member interface if the RX ++ sw_if_index was rewritten (bond-input or l2-to-bvi). Gated on ++ the per-buffer flag so a stale opaque2 value is never used. */ ++ { ++ u32 orig_off = frame->n_vectors - n_left - count; ++ for (u32 j = 0; j < count; j++) ++ { ++ vlib_buffer_t *ob = bufs[orig_off + j]; ++ if (!(ob->flags & VNET_BUFFER_F_ORIG_RX_SW_IF_VALID)) ++ continue; ++ u32 orig = vnet_buffer2 (ob)->orig_rx_sw_if_index; ++ if (orig != sw_if_index[0]) ++ vlib_increment_simple_counter (cm, thread_index, orig, 1); ++ } ++ } + } + + vnet_interface_main_t *im = &vnm->interface_main; +diff --git a/src/vnet/l2/l2_bvi.h b/src/vnet/l2/l2_bvi.h +index 1087190..188c7bd 100644 +--- a/src/vnet/l2/l2_bvi.h ++++ b/src/vnet/l2/l2_bvi.h +@@ -82,6 +82,17 @@ l2_to_bvi (vlib_main_t * vlib_main, + u8 *l3h = vlib_buffer_get_current (b0); + u16 ethertype = clib_net_to_host_u16 (*(u16 *) (l3h - 2)); + ++ /* SVI (VLAN) routing via a bridge-domain BVI: record the ingress ++ member port before the BVI rewrite so error-drop/error-punt also ++ count against the physical port. Skip if an earlier rewrite (e.g. ++ bond) already recorded the member, so it survives a ++ member->LAG->BVI chain. */ ++ if (!(b0->flags & VNET_BUFFER_F_ORIG_RX_SW_IF_VALID)) ++ { ++ b0->flags |= VNET_BUFFER_F_ORIG_RX_SW_IF_VALID; ++ vnet_buffer2 (b0)->orig_rx_sw_if_index = ++ vnet_buffer (b0)->sw_if_index[VLIB_RX]; ++ } + /* Set the input interface to be the BVI interface */ + vnet_buffer (b0)->sw_if_index[VLIB_RX] = bvi_sw_if_index; + vnet_buffer (b0)->sw_if_index[VLIB_TX] = ~0; diff --git a/vppbld/patches/series b/vppbld/patches/series index 43f96210..8f3ea16a 100644 --- a/vppbld/patches/series +++ b/vppbld/patches/series @@ -11,8 +11,10 @@ 0005-version-support-custom-version-override.patch # 6. Fix curl conflicting -o and -O output options 0006-fix-curl-conflicting-output-options.patch -# 7. Track drop stats against original bond member interface -0008-bond-drop-stats-track-original-member-interface.patch +# 7. Track drop stats against the original RX member interface (bond members +# and VLAN/SVI bridge members via the BVI) so drops/punts are also counted +# on the physical ingress port +0008-drop-stats-track-original-member-interface.patch # 9. Add ipip mp2p tunnel 0009-ipip-add-mp2p-ipip-tunnel.patch # 10. ip: add no-class-e-drop startup config option to suppress class E drop route From d024bd6c1d0fa99237b215d7be1ce27433cdb7e1 Mon Sep 17 00:00:00 2001 From: AkeelAli <701916+AkeelAli@users.noreply.github.com> Date: Tue, 1 Sep 2026 10:48:25 -0400 Subject: [PATCH 2/3] Update documentation Signed-off-by: AkeelAli <701916+AkeelAli@users.noreply.github.com> --- docs/HLD/vpp-port-drop-stats.md | 77 ++++++++++++++++----------------- 1 file changed, 38 insertions(+), 39 deletions(-) diff --git a/docs/HLD/vpp-port-drop-stats.md b/docs/HLD/vpp-port-drop-stats.md index 236e21e2..47f73b39 100644 --- a/docs/HLD/vpp-port-drop-stats.md +++ b/docs/HLD/vpp-port-drop-stats.md @@ -9,7 +9,7 @@ 5. [Design Details](#design-details) 6. [Code References](#code-references) 7. [Expected Behavior](#expected-behavior) -8. [Risks](#risks) +8. [Appendix](#appendix) ## Revisions @@ -39,8 +39,8 @@ per-interface `drops` counter (`/interfaces//drops`, `sw_if_index[VLIB_RX]`. sairedis maps this `drops` counter to `SAI_PORT_STAT_IF_IN_DISCARDS` (RX_DRP in `show interfaces counters`). -Two forwarding features rewrite `sw_if_index[VLIB_RX]` before a packet can reach -the drop path: +Two forwarding features in VPP rewrite the RX interface index +`sw_if_index[VLIB_RX]` before a packet can reach the drop path: 1. **LAG / bonding** — `bond_sw_if_idx_rewrite()` replaces the member's RX index with the bond interface's index. @@ -48,28 +48,17 @@ the drop path: the bridge domain's BVI (Bridge Virtual Interface) to route out of the bridge domain. -As a result, a drop on a LAG member or a VLAN (bridge-domain) member is counted +As a result, a drop on a LAG member or a VLAN member is counted against the LAG or BVI, not the physical member, so the ingress port shows zero -drops — losing per-port visibility exactly in the aggregated topologies where it -matters most. The original port is unrecoverable at drop time (a LAG -load-balances across members, a BVI has many members), so it must be captured at -the rewrite and carried on the buffer to the drop node. - -This gap is exercised by the sonic-mgmt drop-counter tests -(`tests/drop_packets/test_drop_counters.py`), whose `tx_dut_ports` fixture -parametrizes the ingress port over `port_channel_members`, `vlan_members`, and -`rif_members`. Counting LAG-member drops against the member covers the -`port_channel_members` case on the `t1-lag-vpp` topology, and counting SVI/BVI -member drops against the member covers the `vlan_members` case on the `t0-vpp` -topology. The `rif_members` case needs no special handling, since a plain L3 RIF -port has no RX rewrite. - -A previous iteration stored the index in `opaque2` (VPP's second per-buffer -scratch metadata area) and treated `0` as "not set", zeroing it in `bond_input`. -That was fragile: `opaque2` is not reset per packet, so packets that skip -`bond_input` could read a stale non-zero value; `0` is also a valid `sw_if_index` -(`local0`); and the `member -> LAG -> BVI` chain recorded the LAG instead of the -physical member. +drops. This gap is exercised by the sonic-mgmt drop-counter tests +(`tests/drop_packets/test_drop_counters.py`), whose tests consequently fail when +parametrized over `port_channel_members` and `vlan_members` in the `t1-lag-vpp` +and `t0-vpp` topologies. + +To address this gap, the original port must be captured at the rewrite and +carried on the packet buffer to the drop node, where the counter increment can +then be applied against the original physical member port in addition to the +rewritten LAG or BVI interface. ## Solution @@ -127,7 +116,7 @@ if (!(b0->flags & VNET_BUFFER_F_ORIG_RX_SW_IF_VALID)) ### Second increment at drop (`src/vnet/interface_output.c`) -In `interface_drop_punt()`, after the existing super-interface increment, walk the +In `interface_drop_punt()`, after the existing LAG/BVI interface increment, walk the buffers just counted and, gated on the flag, increment the original member port: ```c @@ -166,9 +155,9 @@ The implementation is the VPP dataplane patch ## Expected Behavior 1. A drop on a LAG member port is counted on both the LAG and the physical - member port (`SAI_PORT_STAT_IF_IN_DISCARDS`). -2. A drop on a VLAN (bridge-domain) member port is counted on both the BVI and - the physical member port. + member port. +2. A drop on a VLAN member port is counted on both the BVI and the physical + member port. 3. For a `member -> LAG -> BVI` chain, the drop is counted against the physical member, not the intermediate LAG. 4. Traffic on ports whose RX index is not rewritten is counted exactly as before. @@ -178,14 +167,24 @@ The implementation is the VPP dataplane patch `tests/drop_packets/test_drop_counters.py`; the `rif_members` case is unaffected. -## Risks - -The main risk with this solution is that it relies on non-upstream buffer state — a scarce -per-buffer flag bit (bit 27, formerly `AVAIL9`) plus a `u32` in `opaque2` — so it -is fragile across VPP rebases. It permanently consumes one of VPP's limited -`AVAIL` flag bits and must be re-checked on every rebase: if upstream later -assigns bit 27, the patch has to move to another free bit. The flag also guards -only against stale reads across packets, not against another node overwriting the -`orig_rx_sw_if_index` word in `opaque2` for the same packet between the RX rewrite -and the drop; that word is carved from the tail `unused[]` words that nothing -touches today, so that case is low risk. +## Appendix + +A previous version of this patch only stored the index in `opaque2` and treated +`0` as "not set", zeroing it in `bond_input`. That was fragile: `opaque2` is not +reset per packet, so packets that skip `bond_input` could read a stale non-zero +value. It was also costlier on the fast path: since `opaque2` is not reset per +buffer allocation, the sentinel had to be written on every packet to establish a +known "not set" state (and to be fully correct would have needed a reset on every +path reaching the drop node, not just `bond_input`). The current design (buffer +flag plus a dedicated `opaque2` word) addresses those issues — `b->flags` is +already reset on every buffer allocation, so per-packet validity comes for free. + +This approach however has the disadvantage that it relies on non-upstream buffer +state — a scarce per-buffer flag bit (bit 27, formerly `AVAIL9`) plus a `u32` in +`opaque2` — so it is fragile across VPP rebases. It permanently consumes one of +VPP's limited `AVAIL` flag bits and must be re-checked on every rebase: +if upstream later assigns bit 27, the patch has to move to another free bit. +The flag also guards only against stale reads across packets, not against another +node overwriting the `orig_rx_sw_if_index` word in `opaque2` for the same packet +between the RX rewrite and the drop; that word is carved from the tail `unused[]` +words that nothing touches today, so that case is low risk. From e3bd1563fb5664e3a65375cd759c7480a691fe28 Mon Sep 17 00:00:00 2001 From: AkeelAli <701916+AkeelAli@users.noreply.github.com> Date: Thu, 10 Sep 2026 11:36:59 -0400 Subject: [PATCH 3/3] New patchless design to support LAG and BVI Signed-off-by: AkeelAli <701916+AkeelAli@users.noreply.github.com> --- docs/HLD/vpp-port-drop-stats.md | 190 ---------------- ...tats-track-original-member-interface.patch | 111 ---------- vppbld/patches/series | 4 - vppbld/plugins/sonic_ext/CMakeLists.txt | 1 + .../sonic_ext/drop_member_stats_node.c | 203 ++++++++++++++++++ 5 files changed, 204 insertions(+), 305 deletions(-) delete mode 100644 docs/HLD/vpp-port-drop-stats.md delete mode 100644 vppbld/patches/0008-drop-stats-track-original-member-interface.patch create mode 100644 vppbld/plugins/sonic_ext/drop_member_stats_node.c diff --git a/docs/HLD/vpp-port-drop-stats.md b/docs/HLD/vpp-port-drop-stats.md deleted file mode 100644 index 47f73b39..00000000 --- a/docs/HLD/vpp-port-drop-stats.md +++ /dev/null @@ -1,190 +0,0 @@ -# VPP Port Drop Stats HLD - -## Table of Contents - -1. [Revisions](#revisions) -2. [Scope](#scope) -3. [Problem Statement](#problem-statement) -4. [Solution](#solution) -5. [Design Details](#design-details) -6. [Code References](#code-references) -7. [Expected Behavior](#expected-behavior) -8. [Appendix](#appendix) - -## Revisions - -| Rev | Date | Author(s) | -|-----|------|-----------| -| v0.1 | 08/31/2026 | Akeel Ali | - -## Scope - -This document describes the high level design for counting interface drops -against the original physical ingress port on the SONiC VPP platform. - -The change is implemented as a VPP dataplane patch, -`vppbld/patches/0008-drop-stats-track-original-member-interface.patch`, applied -to the bundled VPP source used by the `syncd-vpp` image. It ensures that a drop -is also counted on the physical member port's `drops` counter when the RX -interface index has been rewritten by LAG (bonding) or SVI/BVI forwarding. It is -purely a VPP dataplane change: there are no SAI, orchagent, SONiC CLI, or -database-schema changes, and interfaces whose RX index is not rewritten are -unaffected. - -## Problem Statement - -VPP's `error-drop` node runs `interface_drop_punt()`, which increments the -per-interface `drops` counter (`/interfaces//drops`, -`VNET_INTERFACE_COUNTER_DROP`) for the packet's RX interface index -`sw_if_index[VLIB_RX]`. sairedis maps this `drops` counter to -`SAI_PORT_STAT_IF_IN_DISCARDS` (RX_DRP in `show interfaces counters`). - -Two forwarding features in VPP rewrite the RX interface index -`sw_if_index[VLIB_RX]` before a packet can reach the drop path: - -1. **LAG / bonding** — `bond_sw_if_idx_rewrite()` replaces the member's RX index - with the bond interface's index. -2. **SVI / VLAN routing** — `l2_to_bvi()` replaces the RX index with the index of - the bridge domain's BVI (Bridge Virtual Interface) to route out of the bridge - domain. - -As a result, a drop on a LAG member or a VLAN member is counted -against the LAG or BVI, not the physical member, so the ingress port shows zero -drops. This gap is exercised by the sonic-mgmt drop-counter tests -(`tests/drop_packets/test_drop_counters.py`), whose tests consequently fail when -parametrized over `port_channel_members` and `vlan_members` in the `t1-lag-vpp` -and `t0-vpp` topologies. - -To address this gap, the original port must be captured at the rewrite and -carried on the packet buffer to the drop node, where the counter increment can -then be applied against the original physical member port in addition to the -rewritten LAG or BVI interface. - -## Solution - -Capture the original RX interface index on the buffer at each rewrite site, mark -it valid with a dedicated buffer flag, and have `interface_drop_punt()` add a -second counter increment against it. - -Validity is signaled by a buffer flag rather than a sentinel value because -`b->flags` is reset on every buffer allocation (copied from the pool template) -while `opaque2` is not. The flag therefore provides correct per-packet validity -at near-zero fast-path cost, with no extra initialization node, and it composes -correctly with buffer clones used for flood/replication. - -## Design Details - -### Buffer storage and validity flag (`src/vnet/buffer.h`) - -- A `u32 orig_rx_sw_if_index` field is carved from the tail `unused[]` words of - `vnet_buffer_opaque2_t`, so the struct does not grow: - - ```c - u32 orig_rx_sw_if_index; - u32 unused[5]; - ``` - -- Buffer flag bit 27 (previously `AVAIL9`) is repurposed as - `VNET_BUFFER_F_ORIG_RX_SW_IF_VALID` and removed from - `VNET_BUFFER_FLAGS_ALL_AVAIL`. The highest available bit is chosen to minimize - rebase collisions with upstream, which allocates from the lowest free bit. - -### Capture at the LAG rewrite (`src/vnet/bonding/node.c`) - -In both branches of `bond_sw_if_idx_rewrite()`, before the RX index is -overwritten with the bond index: - -```c -b->flags |= VNET_BUFFER_F_ORIG_RX_SW_IF_VALID; -vnet_buffer2 (b)->orig_rx_sw_if_index = vnet_buffer (b)->sw_if_index[VLIB_RX]; -``` - -### Capture at the BVI rewrite (`src/vnet/l2/l2_bvi.h`) - -In `l2_to_bvi()`, record the ingress member before the BVI rewrite, but only if -nothing has recorded it yet, so a bonded packet keeps its physical member instead -of the LAG: - -```c -if (!(b0->flags & VNET_BUFFER_F_ORIG_RX_SW_IF_VALID)) - { - b0->flags |= VNET_BUFFER_F_ORIG_RX_SW_IF_VALID; - vnet_buffer2 (b0)->orig_rx_sw_if_index = - vnet_buffer (b0)->sw_if_index[VLIB_RX]; - } -``` - -### Second increment at drop (`src/vnet/interface_output.c`) - -In `interface_drop_punt()`, after the existing LAG/BVI interface increment, walk the -buffers just counted and, gated on the flag, increment the original member port: - -```c -u32 orig_off = frame->n_vectors - n_left - count; -for (u32 j = 0; j < count; j++) - { - vlib_buffer_t *ob = bufs[orig_off + j]; - if (!(ob->flags & VNET_BUFFER_F_ORIG_RX_SW_IF_VALID)) - continue; - u32 orig = vnet_buffer2 (ob)->orig_rx_sw_if_index; - if (orig != sw_if_index[0]) - vlib_increment_simple_counter (cm, thread_index, orig, 1); - } -``` - -The flag check prevents reading a stale `opaque2` word, and the -`orig != sw_if_index[0]` check avoids double-counting when the original equals the -interface already counted. - -`interface_drop_punt()` is shared with the `error-punt` node, so this same `cm` -increment also bumps the member's `punt` counter; because sairedis does not export -`punt` to a SAI port stat, only the `drops` / in-discards case is user-visible. - -## Code References - -The implementation is the VPP dataplane patch -`vppbld/patches/0008-drop-stats-track-original-member-interface.patch`. - -| Area | File (patched VPP source) | -|------|---------------------------| -| Buffer field + validity flag | `src/vnet/buffer.h` | -| Capture at LAG member ingress | `src/vnet/bonding/node.c` (`bond_sw_if_idx_rewrite`) | -| Capture at SVI/BVI rewrite | `src/vnet/l2/l2_bvi.h` (`l2_to_bvi`) | -| Second drop increment | `src/vnet/interface_output.c` (`interface_drop_punt`) | - -## Expected Behavior - -1. A drop on a LAG member port is counted on both the LAG and the physical - member port. -2. A drop on a VLAN member port is counted on both the BVI and the physical - member port. -3. For a `member -> LAG -> BVI` chain, the drop is counted against the physical - member, not the intermediate LAG. -4. Traffic on ports whose RX index is not rewritten is counted exactly as before. -5. The drop appears on the physical member's `SAI_PORT_STAT_IF_IN_DISCARDS`, - satisfying the `port_channel_members` case (`t1-lag-vpp`) via the LAG capture - and the `vlan_members` case (`t0-vpp`) via the SVI/BVI capture in - `tests/drop_packets/test_drop_counters.py`; the `rif_members` case is - unaffected. - -## Appendix - -A previous version of this patch only stored the index in `opaque2` and treated -`0` as "not set", zeroing it in `bond_input`. That was fragile: `opaque2` is not -reset per packet, so packets that skip `bond_input` could read a stale non-zero -value. It was also costlier on the fast path: since `opaque2` is not reset per -buffer allocation, the sentinel had to be written on every packet to establish a -known "not set" state (and to be fully correct would have needed a reset on every -path reaching the drop node, not just `bond_input`). The current design (buffer -flag plus a dedicated `opaque2` word) addresses those issues — `b->flags` is -already reset on every buffer allocation, so per-packet validity comes for free. - -This approach however has the disadvantage that it relies on non-upstream buffer -state — a scarce per-buffer flag bit (bit 27, formerly `AVAIL9`) plus a `u32` in -`opaque2` — so it is fragile across VPP rebases. It permanently consumes one of -VPP's limited `AVAIL` flag bits and must be re-checked on every rebase: -if upstream later assigns bit 27, the patch has to move to another free bit. -The flag also guards only against stale reads across packets, not against another -node overwriting the `orig_rx_sw_if_index` word in `opaque2` for the same packet -between the RX rewrite and the drop; that word is carved from the tail `unused[]` -words that nothing touches today, so that case is low risk. diff --git a/vppbld/patches/0008-drop-stats-track-original-member-interface.patch b/vppbld/patches/0008-drop-stats-track-original-member-interface.patch deleted file mode 100644 index 10f5d8fa..00000000 --- a/vppbld/patches/0008-drop-stats-track-original-member-interface.patch +++ /dev/null @@ -1,111 +0,0 @@ -diff --git a/src/vnet/bonding/node.c b/src/vnet/bonding/node.c -index 88dc0c8..3985a88 100644 ---- a/src/vnet/bonding/node.c -+++ b/src/vnet/bonding/node.c -@@ -91,6 +91,10 @@ bond_sw_if_idx_rewrite (vlib_main_t * vm, vlib_node_runtime_t * node, - && !packet_is_cdp (eth) - && (ethertype != htons (ETHERNET_TYPE_802_1_LLDP)))) - { -+ /* Save original member interface before the RX rewrite */ -+ b->flags |= VNET_BUFFER_F_ORIG_RX_SW_IF_VALID; -+ vnet_buffer2 (b)->orig_rx_sw_if_index = -+ vnet_buffer (b)->sw_if_index[VLIB_RX]; - /* Change the physical interface to bond interface */ - vnet_buffer (b)->sw_if_index[VLIB_RX] = bond_sw_if_index; - return; -@@ -111,6 +115,10 @@ bond_sw_if_idx_rewrite (vlib_main_t * vm, vlib_node_runtime_t * node, - && (ethertype != htons (ETHERNET_TYPE_CDP)) - && (ethertype != htons (ETHERNET_TYPE_802_1_LLDP)))) - { -+ /* Save original member interface before the RX rewrite */ -+ b->flags |= VNET_BUFFER_F_ORIG_RX_SW_IF_VALID; -+ vnet_buffer2 (b)->orig_rx_sw_if_index = -+ vnet_buffer (b)->sw_if_index[VLIB_RX]; - /* Change the physical interface to bond interface */ - vnet_buffer (b)->sw_if_index[VLIB_RX] = bond_sw_if_index; - return; -diff --git a/src/vnet/buffer.h b/src/vnet/buffer.h -index 3cef5e6..efcd447 100644 ---- a/src/vnet/buffer.h -+++ b/src/vnet/buffer.h -@@ -41,7 +41,7 @@ - _ (24, AVAIL6, "avail6", 1) \ - _ (25, AVAIL7, "avail7", 1) \ - _ (26, AVAIL8, "avail8", 1) \ -- _ (27, AVAIL9, "avail9", 1) -+ _ (27, ORIG_RX_SW_IF_VALID, "orig-rx-sw-if-valid", 1) - - /* - * Please allocate the FIRST available bit, redefine -@@ -52,7 +52,7 @@ - #define VNET_BUFFER_FLAGS_ALL_AVAIL \ - (VNET_BUFFER_F_AVAIL1 | VNET_BUFFER_F_AVAIL2 | VNET_BUFFER_F_AVAIL3 | \ - VNET_BUFFER_F_AVAIL4 | VNET_BUFFER_F_AVAIL5 | VNET_BUFFER_F_AVAIL6 | \ -- VNET_BUFFER_F_AVAIL7 | VNET_BUFFER_F_AVAIL8 | VNET_BUFFER_F_AVAIL9) -+ VNET_BUFFER_F_AVAIL7 | VNET_BUFFER_F_AVAIL8) - - #define VNET_BUFFER_FLAGS_VLAN_BITS \ - (VNET_BUFFER_F_VLAN_1_DEEP | VNET_BUFFER_F_VLAN_2_DEEP) -@@ -508,7 +508,12 @@ typedef struct - u8 loop_counter; - - u8 unused8; -- u32 unused[6]; -+ /* Original RX sw_if_index before an RX rewrite (bond-input or the -+ l2-to-bvi / SVI path). Valid only when -+ VNET_BUFFER_F_ORIG_RX_SW_IF_VALID is set; lets error-drop/error-punt -+ also count drops against the physical member interface. */ -+ u32 orig_rx_sw_if_index; -+ u32 unused[5]; - } vnet_buffer_opaque2_t; - - #define vnet_buffer2(b) ((vnet_buffer_opaque2_t *) (b)->opaque2) -diff --git a/src/vnet/interface_output.c b/src/vnet/interface_output.c -index f7d6561..a8596f6 100644 ---- a/src/vnet/interface_output.c -+++ b/src/vnet/interface_output.c -@@ -1048,6 +1048,22 @@ interface_drop_punt (vlib_main_t * vm, - if (sw_if0->sup_sw_if_index != sw_if_index[0]) - vlib_increment_simple_counter - (cm, thread_index, sw_if0->sup_sw_if_index, count); -+ -+ /* Also count against the original member interface if the RX -+ sw_if_index was rewritten (bond-input or l2-to-bvi). Gated on -+ the per-buffer flag so a stale opaque2 value is never used. */ -+ { -+ u32 orig_off = frame->n_vectors - n_left - count; -+ for (u32 j = 0; j < count; j++) -+ { -+ vlib_buffer_t *ob = bufs[orig_off + j]; -+ if (!(ob->flags & VNET_BUFFER_F_ORIG_RX_SW_IF_VALID)) -+ continue; -+ u32 orig = vnet_buffer2 (ob)->orig_rx_sw_if_index; -+ if (orig != sw_if_index[0]) -+ vlib_increment_simple_counter (cm, thread_index, orig, 1); -+ } -+ } - } - - vnet_interface_main_t *im = &vnm->interface_main; -diff --git a/src/vnet/l2/l2_bvi.h b/src/vnet/l2/l2_bvi.h -index 1087190..188c7bd 100644 ---- a/src/vnet/l2/l2_bvi.h -+++ b/src/vnet/l2/l2_bvi.h -@@ -82,6 +82,17 @@ l2_to_bvi (vlib_main_t * vlib_main, - u8 *l3h = vlib_buffer_get_current (b0); - u16 ethertype = clib_net_to_host_u16 (*(u16 *) (l3h - 2)); - -+ /* SVI (VLAN) routing via a bridge-domain BVI: record the ingress -+ member port before the BVI rewrite so error-drop/error-punt also -+ count against the physical port. Skip if an earlier rewrite (e.g. -+ bond) already recorded the member, so it survives a -+ member->LAG->BVI chain. */ -+ if (!(b0->flags & VNET_BUFFER_F_ORIG_RX_SW_IF_VALID)) -+ { -+ b0->flags |= VNET_BUFFER_F_ORIG_RX_SW_IF_VALID; -+ vnet_buffer2 (b0)->orig_rx_sw_if_index = -+ vnet_buffer (b0)->sw_if_index[VLIB_RX]; -+ } - /* Set the input interface to be the BVI interface */ - vnet_buffer (b0)->sw_if_index[VLIB_RX] = bvi_sw_if_index; - vnet_buffer (b0)->sw_if_index[VLIB_TX] = ~0; diff --git a/vppbld/patches/series b/vppbld/patches/series index 8f3ea16a..cde61ef5 100644 --- a/vppbld/patches/series +++ b/vppbld/patches/series @@ -11,10 +11,6 @@ 0005-version-support-custom-version-override.patch # 6. Fix curl conflicting -o and -O output options 0006-fix-curl-conflicting-output-options.patch -# 7. Track drop stats against the original RX member interface (bond members -# and VLAN/SVI bridge members via the BVI) so drops/punts are also counted -# on the physical ingress port -0008-drop-stats-track-original-member-interface.patch # 9. Add ipip mp2p tunnel 0009-ipip-add-mp2p-ipip-tunnel.patch # 10. ip: add no-class-e-drop startup config option to suppress class E drop route diff --git a/vppbld/plugins/sonic_ext/CMakeLists.txt b/vppbld/plugins/sonic_ext/CMakeLists.txt index 4d9d170a..76191b72 100644 --- a/vppbld/plugins/sonic_ext/CMakeLists.txt +++ b/vppbld/plugins/sonic_ext/CMakeLists.txt @@ -22,6 +22,7 @@ add_vpp_plugin(sonic_ext l2_trap_fixup_node.c l2_vlan_filter_node.c ip2me_node.c + drop_member_stats_node.c cli.c API_FILES diff --git a/vppbld/plugins/sonic_ext/drop_member_stats_node.c b/vppbld/plugins/sonic_ext/drop_member_stats_node.c new file mode 100644 index 00000000..4b4c01e5 --- /dev/null +++ b/vppbld/plugins/sonic_ext/drop_member_stats_node.c @@ -0,0 +1,203 @@ +/* + * Copyright (c) 2026 SONiC-VPP contributors + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#include + +#include +#include +#include + +/* + * sonic-ext-drop-member-stats + * + * Feature node on the core "error-drop" arc. When a packet's RX + * sw_if_index has been rewritten to a LAG (bond-input) or SVI/VLAN + * (l2-to-bvi) interface by membership forwarding, VPP's + * interface_drop_punt() charges the drop to the LAG/BVI, not the + * physical member -- so the member's SAI_PORT_STAT_IF_IN_DISCARDS + * stays zero. This node adds a second increment against the original + * wire-ingress member, recovered from the sonic-ext-capture cookie. + */ + +typedef struct +{ + u32 orig_rx_sw_if_index; + u32 rewritten_rx_sw_if_index; + u8 counted; +} sonic_ext_drop_member_stats_trace_t; + +static u8 * +format_sonic_ext_drop_member_stats_trace (u8 *s, va_list *args) +{ + CLIB_UNUSED (vlib_main_t * vm) = va_arg (*args, vlib_main_t *); + CLIB_UNUSED (vlib_node_t * node) = va_arg (*args, vlib_node_t *); + sonic_ext_drop_member_stats_trace_t *t = + va_arg (*args, sonic_ext_drop_member_stats_trace_t *); + s = format (s, + "SONIC-EXT-DROP-MEMBER-STATS: orig-rx %u rewritten-rx %u %s", + t->orig_rx_sw_if_index, t->rewritten_rx_sw_if_index, + t->counted ? "COUNTED" : "skip"); + return s; +} + +#define foreach_sonic_ext_drop_member_stats_error \ + _ (COUNTED, "member drop counted against physical port") + +typedef enum +{ +#define _(sym, str) SONIC_EXT_DROP_MEMBER_STATS_ERROR_##sym, + foreach_sonic_ext_drop_member_stats_error +#undef _ + SONIC_EXT_DROP_MEMBER_STATS_N_ERROR, +} sonic_ext_drop_member_stats_error_t; + +static char *sonic_ext_drop_member_stats_error_strings[] = { +#define _(sym, str) str, + foreach_sonic_ext_drop_member_stats_error +#undef _ +}; + +VLIB_NODE_FN (sonic_ext_drop_member_stats_node) +(vlib_main_t *vm, vlib_node_runtime_t *node, vlib_frame_t *frame) +{ + vnet_main_t *vnm = vnet_get_main (); + vlib_simple_counter_main_t *dcm = + vec_elt_at_index (vnm->interface_main.sw_if_counters, + VNET_INTERFACE_COUNTER_DROP); + u32 thread_index = vm->thread_index; + u32 n_left_from, *from; + vlib_buffer_t *bufs[VLIB_FRAME_SIZE], **b; + u16 next_index = 0; + u32 n_counted = 0; + + from = vlib_frame_vector_args (frame); + n_left_from = frame->n_vectors; + vlib_get_buffers (vm, from, bufs, n_left_from); + b = bufs; + + /* interface_drop_punt() starts the arc on the first buffer only and moves + * the whole frame to a single next, so the remaining buffers carry no valid + * arc state -- resolve the next once, from that first buffer. */ + vnet_feature_next_u16 (&next_index, bufs[0]); + + while (n_left_from > 0) + { + sonic_ext_buffer_opaque_t *seb = sonic_ext_buffer (b[0]); + u32 rx = vnet_buffer (b[0])->sw_if_index[VLIB_RX]; + u32 orig = seb->orig_rx_sw_if_index; + u8 counted = 0; + + /* Count the drop on the original RX if it was rewritten to a bond/BVI. */ + if (seb->magic == SONIC_EXT_BUFFER_MAGIC && orig != rx && + (sonic_ext_phy_is_bond (rx) || sonic_ext_phy_is_bvi (rx))) + { + vlib_increment_simple_counter (dcm, thread_index, orig, 1); + n_counted++; + counted = 1; + } + + if (PREDICT_FALSE ((node->flags & VLIB_NODE_FLAG_TRACE) && + (b[0]->flags & VLIB_BUFFER_IS_TRACED))) + { + sonic_ext_drop_member_stats_trace_t *t = + vlib_add_trace (vm, node, b[0], sizeof (*t)); + t->orig_rx_sw_if_index = orig; + t->rewritten_rx_sw_if_index = rx; + t->counted = counted; + } + + b += 1; + n_left_from -= 1; + } + + vlib_buffer_enqueue_to_single_next (vm, node, from, next_index, + frame->n_vectors); + + if (n_counted) + vlib_node_increment_counter ( + vm, node->node_index, SONIC_EXT_DROP_MEMBER_STATS_ERROR_COUNTED, + n_counted); + + return frame->n_vectors; +} + +VLIB_REGISTER_NODE (sonic_ext_drop_member_stats_node) = { + .name = "sonic-ext-drop-member-stats", + .vector_size = sizeof (u32), + .format_trace = format_sonic_ext_drop_member_stats_trace, + .type = VLIB_NODE_TYPE_INTERNAL, + .n_errors = ARRAY_LEN (sonic_ext_drop_member_stats_error_strings), + .error_strings = sonic_ext_drop_member_stats_error_strings, + /* Feature-arc node: share the error-drop arc's next-node table and + * use vnet_feature_next() to advance toward the terminal drop. */ + .sibling_of = "error-drop", +}; + +VNET_FEATURE_INIT (sonic_ext_drop_member_stats, static) = { + .arc_name = "error-drop", + .node_name = "sonic-ext-drop-member-stats", + .runs_before = VNET_FEATURES ("drop"), +}; + +/* + * Enable on every interface. The error-drop arc dispatches per- + * interface with a coarse first-buffer/single-next model, so a drop + * frame is only steered through this node if the feature is enabled on + * the interface the arc happens to dispatch with. Enabling everywhere + * guarantees every drop frame is observed; the node then walks all + * buffers and gates each individually. Interface deletion tears the + * feature down automatically, so there is no del handling. + */ +static void +sonic_ext_drop_member_stats_enable (u32 sw_if_index) +{ + /* The add/del hook and the boot-time walk below can both cover the same + * interface, and enabling is not idempotent in VPP: a second enable appends + * the node to the arc config again, so the frame would count twice. */ + if (vnet_feature_is_enabled ("error-drop", "sonic-ext-drop-member-stats", + sw_if_index) > 0) + return; + + vnet_feature_enable_disable ("error-drop", "sonic-ext-drop-member-stats", + sw_if_index, 1 /* enable */, 0, 0); +} + +static clib_error_t * +sonic_ext_drop_member_stats_sw_if_add_del (vnet_main_t *vnm, u32 sw_if_index, + u32 is_add) +{ + if (is_add) + sonic_ext_drop_member_stats_enable (sw_if_index); + return 0; +} + +VNET_SW_INTERFACE_ADD_DEL_FUNCTION (sonic_ext_drop_member_stats_sw_if_add_del); + +static clib_error_t * +sonic_ext_drop_member_stats_loop_enter (vlib_main_t *vm) +{ + vnet_main_t *vnm = vnet_get_main (); + vnet_interface_main_t *im = &vnm->interface_main; + vnet_sw_interface_t *si; + + /* Catch interfaces that already existed before the add/del hook was + * registered (local0 and anything created during early boot). */ + pool_foreach (si, im->sw_interfaces) + { + sonic_ext_drop_member_stats_enable (si->sw_if_index); + } + return 0; +} + +VLIB_MAIN_LOOP_ENTER_FUNCTION (sonic_ext_drop_member_stats_loop_enter);