[Nexthop][fboss2-dev] CLI for config and delete ACL rule and action subcommands

[hillol-nexthop]

Summary

Full ACL rule CLI surface in fboss2-dev — match fields, actions, and whole-rule delete:

fboss2-dev config acl rule <table> <rule> <attr> <value> [<ttl-mask>]
fboss2-dev config acl rule <table> <rule> action <subattr> [<value>]
fboss2-dev delete acl rule <table> <rule>

config acl rule upserts — if the named rule does not exist in the table it is created with the supplied attribute, otherwise the attribute is applied to the existing entry. delete acl rule removes the entire AclEntry (and its MatchAction) from the table.

Match-field attributes

16 setters on AclEntry: source-ip, destination-ip, protocol, source-port, destination-port, dscp, tcp-flags, icmp-type, icmp-code, ip-fragment, ttl, destination-mac, ethertype, vlan, ip-type, packet-lookup-result. The 5-token ttl <value> <mask> form is unique to the setter; without <mask> it defaults to 0xFF.

Action subcommands

action permit | deny                            # AclEntry.actionType
action send-to-queue <0-32767>                  # MatchAction.sendToQueue
action set-dscp <0-63>                          # MatchAction.setDscp
action set-tc <0-7>                             # MatchAction.setTc
action mirror-ingress <name>                    # MatchAction.ingressMirror
action mirror-egress <name>                     # MatchAction.egressMirror
action counter <name>                           # MatchAction.counter
action trap-to-cpu                              # MatchAction.toCpuAction = TRAP
action copy-to-cpu                              # MatchAction.toCpuAction = COPY
action redirect nexthop <ip>                    # MatchAction.redirectToNextHop

permit / deny mutate AclEntry.actionType in place. The rest land on a MatchAction attached via dataPlaneTrafficPolicy.matchToAction keyed by rule name; the CLI finds or creates the MatchToAction entry on demand.

Common

All operations target sw.aclTableGroups (field 56) — the deprecated aclTableGroup (field 45) is not supported. All are HITLESS: AclEntry + MatchAction mutations apply at runtime via processAclTableGroupDelta in SaiAclTableManager; no warmboot-prohibited guard in SaiSwitch.

How to use the CLIs

# Create + set a match field (rule auto-created if absent)
$ fboss2-dev config acl rule AclTable1 my-rule source-ip 10.250.0.0/24
Created and set acl rule 'my-rule' (table 'AclTable1', group 'acl-table-group-ingress') source-ip to 10.250.0.0/24

# Set an action on the same rule
$ fboss2-dev config acl rule AclTable1 my-rule action set-dscp 46
Set acl rule 'my-rule' (table 'AclTable1', group 'acl-table-group-ingress') action to set-dscp

# Redirect form (6-token)
$ fboss2-dev config acl rule AclTable1 my-rule action redirect nexthop 10.10.10.1
Set acl rule 'my-rule' (table 'AclTable1', group 'acl-table-group-ingress') action to redirect

# Remove the whole rule
$ fboss2-dev delete acl rule AclTable1 my-rule
Deleted acl rule 'my-rule' from table 'AclTable1' (group 'acl-table-group-ingress')

# Inspect + commit
$ fboss2-dev config session diff       # shows entry + matchToAction add/remove
$ fboss2-dev config session commit     # HITLESS — no agent restart

Test plan

Unit tests

fboss2_cmd_config_test --gtest_filter='*AclRule*' → 49/49 PASS

CmdConfigAclRuleTestFixture (44):

• Match-field arg validation: argValidation_badArity, argValidation_unknownAttr, argValidation_extraTokenOnlyForTtl, argValidation_outOfRange, argValidation_badIp, argValidation_badMac, argValidation_protocolKeyword, argValidation_ipFragmentEnum, argValidation_etherTypeNumericOrName
• Action arg validation: argValidation_unknownActionSubattr, argValidation_actionPermitNoExtra, argValidation_actionRequiresValue, argValidation_actionRangeChecks, argValidation_actionRedirectShape
• Match-field mutation: ruleAutoCreated, tableNotFound, setSourceIp, setDestinationIp, setProtocol, setSourcePort, setDestinationPort, setDscp, setTcpFlags, setIcmpType, setIcmpCode, setIpFragment, setTtlDefaultMask, setTtlExplicitMask, setDestinationMac, setEtherType, setVlan, setIpType, setPacketLookupResult
• Action mutation: setActionPermit, setActionDeny, setActionSendToQueue, setActionSetDscp, setActionSetTc, setActionMirrorIngress, setActionMirrorEgress, setActionCounter, setActionTrapToCpu, setActionCopyToCpu, setActionRedirectNexthop

CmdDeleteAclRuleTestFixture (5):

• argValidation_badArity, argValidation_validArity, tableNotFound, ruleNotFound, deleteRule

Integration tests

fboss2_integration_test --gtest_filter='*AclRule*' on a real DUT → 25/25 PASS

SetUpTestSuite snapshots /etc/coop/agent.conf and unions the first AclTable's qualifiers list with every match-field qualifier exercised by the suite, then restarts fboss_sw_agent. TearDownTestSuite restores the snapshot and restarts. Each test creates the rule on demand via config acl rule; per-test TearDown deletes it via delete acl rule.

Match-field (17):

• SetSourceIp, SetDestinationIp, SetProtocol, SetSourcePort, SetDestinationPort, SetDscp, SetTcpFlags, SetIcmpType, SetIcmpCode, SetIpFragment, SetTtlDefaultMask, SetTtlExplicitMask, SetDestinationMac, SetEtherType, SetVlan, SetIpType, SetPacketLookupResult

Action (7):

• SetActionPermit, SetActionDeny, SetActionSendToQueue, SetActionSetDscp, SetActionSetTc, SetActionTrapToCpu, SetActionCopyToCpu

Delete (1):

• DeleteRule — creates the entry via config acl rule, commits, asserts present, then delete acl rule, commits, asserts gone.

action mirror-ingress, action mirror-egress, action counter, and action redirect nexthop are intentionally unit-test-only — SAI rejects entries that reference an undefined mirror session, unprovisioned counter, or unresolvable nexthop, and provisioning that supporting state on the DUT is out of scope for this PR. The CLI is verified to construct the right config delta in unit tests.

[joseph5wu]

Recently I introduced this per-test agent cold boot to Fboss2IntegrationTestRunner
b108a87
to make sure each single fboss2_integration_test run will alwasy start with a clean state.
And in our FBOSS internal test tool(Netcastle), I also implemented something similar

The goal is to keep our gtest here only focus on testing the functionality of our services/fboss2,
while our test infrastructure/orchestrator(like Netcastle or run_test.py) can deal with the setup/cleanup test suite logic. Please consider to simplify the test to use what we provide in run_test.py

[hillol-nexthop]

I have removed the snapshotting part but I need to keep the residue for injecting ACL qualifiers. I know run_test.py accepts custom config for a test but when I want to run all the integration tests, I can't specify custom config for a particular suite. The current method takes whatever base config you provide and adds the qualifiers to facilitate ACL tests to run which I find much more convenient.
It deviates a bit from the convention but makes the test self-sustainable. Let me know if you are okay with the tradeoff.

[joseph5wu]

Please address the magic number and acl unrelated test services orchestration logic comments.

[joseph5wu]

Let's define all these magic number as constants in the header.
I noticed you also directly use these numbers in the CmdConfigAclRule.cpp which makes me concern if we need to change the syntax of one of the attributes, it might not be as easy to locate the places we need to adjust

[joseph5wu]

This is why we need to define these magic numbers.

[joseph5wu]

Magic numbers!!!

[joseph5wu]

I highly recommend not to include this logic in the integration tests.
Instead we should consider to use the test orchestrator like run_test.py or our internal Netcastle
to handle the agent services or collecting logs since these should be the test orchestrator's responsibility.

Let's remove this logic out from this PR and have this PR only focus on ACL related implementation