Skip to content
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

Prevent UDP packet count and operations overflow #1536

Merged
merged 1 commit into from
Jul 3, 2023

Conversation

davidBar-On
Copy link
Contributor

  • Version of iperf3 (or development branch, such as master or
    3.1-STABLE) to which this pull request applies:
    master

  • Issues fixed (if any):

  • UDP loss data not calculated correctly?  #1534

  • Brief description of code changes (suitable for use as a commit message):

Make all variables related to UDP packets numbers and related calculations int64_t. Although --udp-counters-64bit options allows UDP packet number to be larger than 2^32 bits, several of these related variables were defined as int which is 32 nits in several environments. Note that int64_t is used and not uint64_t to keep sign compatibility with the replaced int (assuming that the 63 bits limit is large enough).

In addition, report_bw_udp/report_sum_bw_udp... definitions in iperf_locale.c were changed to use %PRIu64 instead of %d when printing packets counts. Note that the related reportCSV_bw_udp_format was not changed, as I am not sure how it is used.

@bmah888
Copy link
Contributor

bmah888 commented Jul 3, 2023

Thanks for the PR, this looks good to me. I've done some testing on it, but not enough to get the counters past 32-bit integers. Regarding your comment about using signed 64-bit integers, I think if we were writing this code from scratch using uint64_t variables would be best, but I agree with your rationale of a least-impact change and just sticking with signed variables for now.

@bmah888 bmah888 merged commit c2fe76c into esnet:master Jul 3, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants