Skip to content

[Nexthop] Fix queue per host classID race in ACL table group traffic tests - #1498

Open
vvasavada-nexthop wants to merge 1 commit into
facebook:mainfrom
nexthop-ai:fix-queue-per-host-classid-race-in-acl
Open

[Nexthop] Fix queue per host classID race in ACL table group traffic tests#1498
vvasavada-nexthop wants to merge 1 commit into
facebook:mainfrom
nexthop-ai:fix-queue-per-host-classid-race-in-acl

Conversation

@vvasavada-nexthop

Copy link
Copy Markdown

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

The QueuePerHost ACL test basically stores the tag in two separate places for the same host.

neighbor table ---> "IP 1.0.0.10 lives at MAC 00:02:03:04:05:10" -----> use for routed packet
MAC table ----> "MAC 00:02:03:04:05:10 is out port 1" -----> switched

A packet in the test makes two trips:

Trip 1: Routes it by IP towards host 1.0.0.10, tags it using neighbor table, picks a lane, sends it out of port1.
Trip 2: Loops back in. Its destination MAC is now host's MAC, so this time packet is switched, tags it using the MAC table, picks a lane and sends out of port 1 again.

The problem is that addResolvedNeighborWithClassID applies two state updates: resolve the neighbors, then set their class IDs. They both ignored the in and used getProgrammedState. In between, a background component LookupClassUpdater does its own job of tagging the MAC table entries.

So the sequence that happens is:

stateA ---> LookupClassUpdater --> add MAC tags ---> stateB
stateA ---> test: add neighbor tags ---> stateC (MAC tags gone!)
^
|
----> this is a stale state copy as a result of using getProgrammedState which may not have updated yet.

So the packet goes like this:

trip 1 (routed, neighbor tags OK) → 2 packets into each of lanes 0-4
trip 2 (switched, MAC tags ERASED) → no tag, no rule matches
                                                               → all 10 dumped into lane 0
                                                --------------------------------
                                                12, 2, 2, 2, 2 (total 20)

This is evident from the logs

  50273 V0810 02:09:18.780021 30810 AgentAclTableGroupTrafficTests.cpp:269] TestType: IPv4 Traffic Pkts on queue : 0 pkts: 12
  50274 V0810 02:09:18.780030 30810 AgentAclTableGroupTrafficTests.cpp:269] TestType: IPv4 Traffic Pkts on queue : 1 pkts: 2
  50275 V0810 02:09:18.780033 30810 AgentAclTableGroupTrafficTests.cpp:269] TestType: IPv4 Traffic Pkts on queue : 2 pkts: 2
  50276 V0810 02:09:18.780035 30810 AgentAclTableGroupTrafficTests.cpp:269] TestType: IPv4 Traffic Pkts on queue : 3 pkts: 2
  50277 V0810 02:09:18.780037 30810 AgentAclTableGroupTrafficTests.cpp:269] TestType: IPv4 Traffic Pkts on queue : 4 pkts: 2

What should have happened is:

trip 1 (routed, neighbor tags OK) → 2 packets into each of lanes 0-4
trip 2 (switched, MAC tags OK) → 2 packets into each of lanes 0-4
                                                ----------------------------------
                                                4, 4, 4, 4, 4 (total 20)

The fix is to use in (state that was handed in) instead of getProgrammedState.

Test Plan

Was able to reproduce the below AgentAclTableGroupTrafficTest.VerifyQueuePerHostAclTableAndTtlAclTable failure without the fix, but not with the fix.

/src/fboss/agent/test/agent_hw_tests/AgentAclTableGroupTrafficTests.cpp:265
Expected equality of these values:
  pktsOnQueue
    Which is: 4
  2
[ PASSED ] cold_boot.AgentAclTableGroupTrafficTest.VerifyQueuePerHostAclTableAndTtlAclTable (24194 ms)
[ PASSED ] warm_boot.AgentAclTableGroupTrafficTest.VerifyQueuePerHostAclTableAndTtlAclTable (12448 ms)

Other QueuePerHost tests passes with the change.

[ PASSED ] cold_boot.AgentQueuePerHostL2Test.VerifyHostToQueueMappingClassID (17186 ms)
[ PASSED ] warm_boot.AgentQueuePerHostL2Test.VerifyHostToQueueMappingClassID (8577 ms)
[ PASSED ] cold_boot.AgentQueuePerHostTest.VerifyHostToQueueMappingClassIDsAfterResolve (24394 ms)
[ PASSED ] warm_boot.AgentQueuePerHostTest.VerifyHostToQueueMappingClassIDsAfterResolve (16642 ms)
[ PASSED ] cold_boot.AgentQueuePerHostTest.VerifyHostToQueueMappingClassIDsAfterResolveBlock (20489 ms)
[ PASSED ] warm_boot.AgentQueuePerHostTest.VerifyHostToQueueMappingClassIDsAfterResolveBlock (8612 ms)
[ PASSED ] cold_boot.AgentQueuePerHostTest.VerifyHostToQueueMappingClassIDsWithResolve (24345 ms)
[ PASSED ] warm_boot.AgentQueuePerHostTest.VerifyHostToQueueMappingClassIDsWithResolve (16691 ms)
[ PASSED ] cold_boot.AgentQueuePerHostTest.VerifyHostToQueueMappingClassIDsWithResolveBlock (20489 ms)
[ PASSED ] warm_boot.AgentQueuePerHostTest.VerifyHostToQueueMappingClassIDsWithResolveBlock (8586 ms)
[ PASSED ] cold_boot.AgentQueuePerHostTest.VerifyTtldCounter (24295 ms)
[ PASSED ] warm_boot.AgentQueuePerHostTest.VerifyTtldCounter (12717 ms)
[ PASSED ] cold_boot.AgentQueuePerHostRouteTest.VerifyHostToQueueMappingClassID (32406 ms)
[ PASSED ] warm_boot.AgentQueuePerHostRouteTest.VerifyHostToQueueMappingClassID (14855 ms)
[ PASSED ] cold_boot.AgentQueuePerHostRouteTest.VerifyHostToQueueMappingClassIDBlock (28497 ms)
[ PASSED ] warm_boot.AgentQueuePerHostRouteTest.VerifyHostToQueueMappingClassIDBlock (9860 ms)

Signed-off-by: Vishrant Vasavada <vvasavada@nexthop.ai>
@vvasavada-nexthop
vvasavada-nexthop requested a review from a team as a code owner August 10, 2026 22:48
@meta-cla meta-cla Bot added the CLA Signed label Aug 10, 2026
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.

1 participant