Skip to content

Fix drop-check in test_acl_drop and test_acl_egress_drop for test_drop_counters.py - #27815

Open
AkeelAli wants to merge 1 commit into
sonic-net:masterfrom
AkeelAli:fix-acl-drop-test
Open

AkeelAli wants to merge 1 commit into
sonic-net:masterfrom
AkeelAli:fix-acl-drop-test

Conversation

@AkeelAli

@AkeelAli AkeelAli commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Description of PR

Summary:

Fixes a latent integer-parsing bug in the drop-counter test helpers that caused ensure_no_l2_drops / ensure_no_l3_drops to silently skip any interface whose RX_DRP / RX_ERR reached the thousands (comma-formatted by portstat -j / intfstat -j). Because PKT_NUMBER = 1000, these checks have effectively been no-ops at the exact packet count they validate. Also declares VPP (x86_64-kvm_x86_64-r0) as an ACL/L2 combined-counter platform, since VPP charges ACL denies to the interface drop counter (RX_DRP).

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605

Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO):
Failure type:

Tested branch

  • master
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605
  • N/A

Test result

Previously, this failure resulted in a warning log:

drop_packets/test_drop_counters.py::test_acl_drop[port_channel_members-vlab-vpp-01]
-------------------------------- live log call ---------------------------------
01/09/2026 17:21:40 drop_counters.ensure_no_l2_drops   L0088 WARNING| Unable to verify L2 drops on iface Ethernet28
invalid literal for int() with base 10: '1,000'
PASSED                                                                    [ 83%]

drop_packets/test_drop_counters.py::test_acl_drop[vlan_members-vlab-vpp-01] SKIPPED [ 84%]

drop_packets/test_drop_counters.py::test_acl_drop[rif_members-vlab-vpp-01]
-------------------------------- live log call ---------------------------------
01/09/2026 17:22:48 drop_counters.ensure_no_l2_drops   L0088 WARNING| Unable to verify L2 drops on iface Ethernet84
invalid literal for int() with base 10: '1,000'
PASSED                                                                    [ 85%]

drop_packets/test_drop_counters.py::test_acl_egress_drop[port_channel_members-vlab-vpp-01]
-------------------------------- live log call ---------------------------------
01/09/2026 17:24:39 drop_counters.ensure_no_l2_drops   L0088 WARNING| Unable to verify L2 drops on iface Ethernet28
invalid literal for int() with base 10: '1,000'
PASSED                                                                    [ 86%]

drop_packets/test_drop_counters.py::test_acl_egress_drop[vlan_members-vlab-vpp-01] SKIPPED [ 87%]

drop_packets/test_drop_counters.py::test_acl_egress_drop[rif_members-vlab-vpp-01]
-------------------------------- live log call ---------------------------------
01/09/2026 17:26:31 drop_counters.ensure_no_l2_drops   L0088 WARNING| Unable to verify L2 drops on iface Ethernet112
invalid literal for int() with base 10: '1,000'
PASSED

With this change, the check is skipped for sonic-vpp and only the ACL check remains.

Approach

What is the motivation for this PR?

ensure_no_l2_drops / ensure_no_l3_drops parse counters with int(value[RX_DRP]), but portstat -j formats values >= 1000 as "1,000". At PKT_NUMBER=1000 the int() call throws, the interface is skipped, and the check silently passes. On VPP this hid that ACL denies increment RX_DRP.

How did you do it?

Strip commas before int() (as verify_drop_counters already does), and add VPP (x86_64-kvm_x86_64-r0) to the acl_l2 list.

How did you verify/test it?

On VPP t1-lag: with only the comma fix, test_acl_drop and test_acl_egress_drop flip to FAILED (RX_DRP ~1000); with the acl_l2 entry added, they pass legitimately while the per-ACE counter check still runs.

Any platform specific information?

  • The acl_l2 entry is VPP-only.
  • The drop_counters.py comma fix is platform-agnostic and corrects a check that was silently no-op at PKT_NUMBER=1000 everywhere. We may need to add other platforms as needed to the list.

Supported testbed topology if it's a new test case?

N/A - existing tests, validated on t1-lag.

Documentation

Signed-off-by: AkeelAli <701916+AkeelAli@users.noreply.github.com>
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@nhegde-microsoft nhegde-microsoft 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.

Excellent find! And agreed on the approach of including VPP in the list of platforms that combine ACL and L2 drops.

@AkeelAli

AkeelAli commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@lolyu @nhegde-microsoft how do we want to proceed here?

Adding VPP to acl_l2 skips the RX_DRP check in test_drop_counters.py::test_acl_drop / test_acl_egress_drop, so the (fixed) tests now pass. If that's the direction we want to go (and accept that ACL drops land in RX_DRP on VPP), we just need to update test_upstream_standby_rx_drop_check to likewise skip the check for any acl_l2 platforms.

If not, we would go with sonic-net/sonic-platform-vpp#280 and have the standby report no RX_DRP. I would use this current PR to simply fix the test_drop_counters.py test in this case. Let me know how you'd like to proceed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants