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

Fix rcv-timeout issue because of Nread timeout #1744

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

davidBar-On
Copy link
Contributor

@davidBar-On davidBar-On commented Aug 13, 2024

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

  • Issues fixed (if any): None

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

Fix an issue with --rcv-timeout - did not work with value over 10 seconds (my first PR for a problem I found at work as iperf3 user 😄). This is because Nread() timeout after 10 seconds, but iperf_recv_mt() collected statistics even when receiving 0 bytes. Therefore, it was assumed that data blocks where received and so receive did not timeout.

@davidBar-On
Copy link
Contributor Author

@bmah888, as this is a bug fix (although not critical), just making sure it was not overlooked.

@bmah888
Copy link
Contributor

bmah888 commented Sep 13, 2024

Thanks for the PR!

Documenting how I reproduced this bug. With a built from tip of the master branch:

server% iperf3 --server --rcv-timeout=2000
client% iperf3 --client server.example.com --time=60

Suspend the client process after a few seconds, observe the timeout on the server side after 2 seconds (approximately).

server% iperf3 --server --rcv-timeout=15000
client% iperf3 --client server.example.com --time=60

Suspend the client process after a few seconds, the server will never timeout and also will never terminate the test.

@bmah888
Copy link
Contributor

bmah888 commented Sep 13, 2024

With this PR, testing the above scenario results in the server terminating successfully.

Also tested this scenario:

server% iperf3 --server
client% iperf3 --client server.example.com --time=60 --rcv-timeout=15000

After a few seconds, suspend the server side process. Without this PR, the client will not honor the --rcv-timeout parameter, and it runs to the end of the --time parameter and hangs.

With the PR, the --rcv-timeout parameter works correctly.

This looks good for a merge.

@bmah888 bmah888 merged commit 36deba4 into esnet:master Sep 13, 2024
3 checks passed
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.

2 participants