Skip to content

[Nexthop][fboss2-dev] Add config and delete traffic-counter CLI - #1521

Open
vybhav-nexthop wants to merge 1 commit into
facebook:mainfrom
nexthop-ai:traffic-counter-config
Open

[Nexthop][fboss2-dev] Add config and delete traffic-counter CLI#1521
vybhav-nexthop wants to merge 1 commit into
facebook:mainfrom
nexthop-ai:traffic-counter-config

Conversation

@vybhav-nexthop

@vybhav-nexthop vybhav-nexthop commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Pre-submission checklist

  • I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running pip install -r requirements-dev.txt && pre-commit install
  • pre-commit run

Summary

Background: what a traffic counter is and how it flows through the system

The switch ASIC has hardware counter registers. A TrafficCounter config entry asks for one and gives it a name; the types field says whether it tallies packets, bytes, or both:

{"name": "acl-drops", "types": [PACKETS, BYTES]}

On its own it counts nothing, because nothing says which packets to count. That link comes from a traffic policy: an ACL is the classifier, a MatchToAction entry binds the ACL to an action, and the action can carry counter: "acl-drops", a plain string naming the counter. Config apply resolves the name and ties the ACL's TCAM rule to the counter register (the ActionCounter attribute on the SAI ACL entry).

From then on the ASIC counts by itself: a packet matches the rule, the register increments, no software in the hot path. The agent periodically polls the register over SAI (SaiAclTableManager::updateStats) and publishes the value as per-name fb303 stats (acl-drops.packets, acl-drops.bytes)

What: Adds the traffic-counter config surface to fboss2-dev, both create and delete, plus an end-to-end integration test.

  • fboss2-dev config traffic-counter <name> <types> creates or updates a named counter in sw.trafficCounters, where <types> is a comma separated list of PACKETS,BYTES. Every wanted type has to be named: a collapsed alias for "all of them" stops meaning that as soon as a cfg::CounterType is added, and an optional type argument would let a command line silently narrow a counter that is already collecting both.
  • fboss2-dev delete traffic-counter <name> removes a named counter. It refuses while a dataPlaneTrafficPolicy or cpuTrafficPolicy match action still references the counter, and names the referring matchers, since deleting it would leave a dangling reference the agent rejects.

Both operations are hitless.

Why: sw.trafficCounters had no CLI. The delete refusal guards against the agent's ApplyThriftConfig rejection ("Invalid config: No counter named ...") that a dangling reference would cause.

How: Both leaves follow the fboss2 CmdHandler/Traits single-positional-arg pattern. The delete refusal scans the two live TrafficPolicyConfig homes (dataPlaneTrafficPolicy and cpuTrafficPolicy.trafficPolicy) for MatchAction.counter, the same set the agent validates. The deprecated globalEgressTrafficPolicy_DEPRECATED has no agent consumers and is not scanned.

Sample usage

$ fboss2-dev config traffic-counter cpu-high-count PACKETS,BYTES
Successfully created traffic counter 'cpu-high-count' with type 'PACKETS,BYTES'
$ fboss2-dev delete traffic-counter cpu-high-count
Traffic counter 'cpu-high-count' is still referenced by: cpuTrafficPolicy matcher 'cpu-high'. Remove the referencing match action(s) first.
$ fboss2-dev delete traffic-counter port-drop-count
Successfully deleted traffic counter 'port-drop-count'

Test Plan

Validation ran in our internal tree, which carries this same change. The upstream tree itself was not built locally; this PR's CI is the build verification.

Unit (cmd_config_test):

[----------] 5 tests from CmdConfigTrafficCounterTestFixture
[----------] 7 tests from CmdDeleteTrafficCounterTestFixture
[  PASSED  ] 12 tests.

Delete coverage includes arg validation, unreferenced delete, cpu- and dataplane-policy referenced-counter refusal, and absent-counter refusal.

Integration (ConfigTrafficCounterTest, run on a DUT via internal CI):

  • CreateThenDelete: programs a counter, commits (hitless), verifies it appears in the running config with the expected types, then deletes it, commits again, and verifies the running config returns to baseline. The fixture deletes a leftover test counter first, so a run that died between create and delete leaves the next run a clean config instead of blocking it.

Review Findings

No issues found above confidence threshold (fboss-review, 5 reviewers + verifier).

@vybhav-nexthop
vybhav-nexthop requested review from a team as code owners August 17, 2026 07:16
@meta-cla meta-cla Bot added the CLA Signed label Aug 17, 2026

@joseph5wu joseph5wu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the both support and adjust the integration-test

Comment thread fboss/cli/fboss2/commands/config/traffic_counter/CmdConfigTrafficCounter.cpp Outdated
Comment thread fboss/cli/fboss2/test/integration_test/ConfigTrafficCounterTest.cpp Outdated
@vybhav-nexthop
vybhav-nexthop force-pushed the traffic-counter-config branch from 89fd9c1 to ce5f6ca Compare August 20, 2026 09:27
fboss2-dev config traffic-counter <name> <types> creates or updates a
named entry in sw.trafficCounters, where types is a comma separated list
of PACKETS,BYTES. The handler upserts by counter name: an existing
counter's types are updated in place, otherwise a new TrafficCounter is
appended. Every wanted type has to be named: a collapsed alias for "all
of them" stops meaning that as soon as a cfg::CounterType is added, and
an optional type argument lets a command line silently narrow a counter
that is already collecting both. Types are stored in a fixed order so the
config does not depend on the order they were typed in.

fboss2-dev delete traffic-counter <name> removes a named counter from
sw.trafficCounters. It refuses while a dataPlaneTrafficPolicy or
cpuTrafficPolicy match action still references the counter, naming the
referring matchers: deleting it would leave a dangling reference that
ApplyThriftConfig rejects ("Invalid config: No counter named ..."). The
deprecated globalEgressTrafficPolicy_DEPRECATED has no agent consumers
and is not scanned.

Both operations are hitless: trafficCounters is consumed at ACL
apply-time in ApplyThriftConfig::updateAclsImpl() and does not require
an agent restart on its own.

ConfigTrafficCounterTest exercises the create+delete round trip against
a real agent: config traffic-counter <name> PACKETS,BYTES -> hitless
commit -> verify it appears in the running config with types [PACKETS,
BYTES] -> delete traffic-counter <name> -> hitless commit -> verify the
running config returns to baseline. The fixture deletes a leftover test
counter before each case, so a run that died between create and delete
leaves the next run a clean config instead of blocking it. The
absent-counter refusal is left to the unit tests, which cover it
directly.

Unit tests cover arg validation, the create/update/no-op execution
paths, unreferenced delete, cpu- and dataplane-policy referenced-counter
refusal, and absent-counter refusal.
@vybhav-nexthop
vybhav-nexthop force-pushed the traffic-counter-config branch from ce5f6ca to ec9f184 Compare August 20, 2026 09:49
@vybhav-nexthop

Copy link
Copy Markdown
Contributor Author

Please remove the both support and adjust the integration-test

done

@meta-codesync

meta-codesync Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

@joseph5wu has imported this pull request. If you are a Meta employee, you can view this in D117038083.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants