Skip to content

Fix interface error/discard percentage calculation exceeding 100% #346

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

c-kr
Copy link
Contributor

@c-kr c-kr commented Jun 5, 2025

Problem Description

My customer encountered this issue on several firewall switches with WAN interfaces where the plugin reported error values exceeding 4000%. This mathematical impossibility indicated a bug in the percentage calculation logic.

Root Cause Analysis

The current calculation uses only successful packets as the denominator:

100 * $self->{delta_ifInErrors} / $self->{delta_ifInPkts}

However, according to RFC 2863, the ifInPkts/ifOutPkts counters represent only successfully received packets, excluding errored/discarded packets. This means:

  • Total packets on the wire = Successful packets + Error packets
  • Current formula: Errors / Successful_packets * 100
  • Correct formula: Errors / (Successful_packets + Errors) * 100

Note: This analysis was assisted by AI and should be verified against the full RFC specification.

Solution

This PR corrects the mathematical formula for both error and discard percentage calculations by including the errors/discards in the denominator.

Related Issue

This fix addresses issue #307 opened by @andytlc68, who independently identified the same problem and proposed an identical solution. The issue already has community support with additional users confirming the problem, indicating this affects at least 3+ users in different environments.

Testing

  • ✅ Successfully tested in customer environment
  • ✅ Error rates > 100% eliminated
  • ✅ Mathematical results now bounded to 0-100% range

Disclaimer and Considerations

⚠️ Please review carefully before merging:

This fix modifies a core component of the plugin. While I've implemented this to the best of my knowledge and successfully tested it in our customer environment, I cannot provide absolute guarantee of correctness due to several considerations:

  1. Potential Side Effects: This change affects core interface calculations and may have implications for other parts of the plugin that weren't immediately apparent.

  2. Scope Limitation: I've only been able to verify this issue with error rates, but applied the same fix to discard calculations based on logical consistency.

  3. Calculation Precision: The new calculation may still have edge cases or imprecision issues (e.g., lack of distinction between unicast/broadcast/multicast traffic).

  4. Vendor Implementation: It's possible that only certain vendors incorrectly implement the SNMP specification, and this change might affect others differently.

Recommendation: Please conduct thorough testing across different network equipment vendors and gather community feedback before merging this PR.

Fixes #307

- Correct mathematical formula to include errors/discards in denominator
- Prevents error rates > 100% on high-error interfaces
- Affects both input/output error and discard calculations
- Resolves issue lausser#307

The original calculation used only successful packets as denominator,
but RFC 2863 defines that ifInPkts/ifOutPkts counters exclude
errored/discarded packets, leading to mathematically incorrect
percentages when error rates are high.
@lausser
Copy link
Owner

lausser commented Jun 6, 2025

sounds correct. i am on holiday right now, will check and test next week.

@lausser
Copy link
Owner

lausser commented Jun 12, 2025

It's even
100 * $self->{delta_ifInErrors} / ($self->{delta_ifInPkts} + $self->{delta_ifInErrors} + $self->{delta_ifInDiscards})
unfortunately i am waiting for a customer feedback who seems to be on vacation too, i need it before i can release.

@lausser
Copy link
Owner

lausser commented Jun 26, 2025

Unfortunately it takes time...still waiting for a customer who is testing the current work-in-progress

@c-kr
Copy link
Contributor Author

c-kr commented Jun 27, 2025

Sure, no hurry. We also need to upgrade to mod-gearman Go worker first before we can use the latest nwc-health with ePN support

@lausser lausser merged commit 13f6893 into lausser:master Jul 16, 2025
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.

Interface Errors percentage over 100%
2 participants