Skip to content

fix burst mode throttle checking #898

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

Merged
merged 1 commit into from
Oct 1, 2019
Merged

Conversation

olichtne
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):

While investigating a different iperf issue with udp stream tests I noticed that using -b 0/1000 results in iperf sending 1000 packets and then just idle for the remainder of the test duration.
The full command I was using to reproduce the issue:
iperf3 -c 192.168.101.2 -B 192.168.101.1 -b0/1000 -u -t 10 -l 1400

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

When burst mode is configured for unlimited rate (-b0) but with a
specific packet burst value, iperf only sends packets once, after that
the iperf_check_throttle function gets called and sets green_light=0
due to the rate value being 0 and average calculated rate always being
higher than that.

This can be fixed by moving the "don't throttle" condition
directly inside the iperf_check_throttle function.

Signed-off-by: Ondrej Lichtner [email protected]

When burst mode is configured for unlimited rate (-b0) but with a
specific packet burst value (e.g. /1000), iperf only sends packets once,
after that the iperf_check_throttle function gets called and sets
green_light=0 due to the rate value being 0 and average calculated rate
always being higher than 0.

The iperf_check_throttle function is designed to be skipped in case the
target rate is unlimited or if a specific burst value was configured,
however this skip is only utilized in one place where the function is
called leading to the situation above.

This can be fixed by moving the "skip throttling" condition directly
inside the iperf_check_throttle function.

Signed-off-by: Ondrej Lichtner <[email protected]>
@bmah888
Copy link
Contributor

bmah888 commented Aug 8, 2019

Thanks for the PR. I'm trying to remember what are the semantics exactly of doing burst mode on an unlimited-rate UDP transfer? In other words what's the difference between -b0 and -b0/1000? Without having thought about it a lot, it feels like both of them are supposed to spew packets out as fast as they can, so the idea of a burst size doesn't really mean anything.

@olichtne
Copy link
Contributor Author

olichtne commented Aug 9, 2019

Yes, I also think that both are supposed to "spew packets out as fast as they can" which is the reason for my pull request. Which I assume means that the iperf_check_throttle function shouldn't be getting called with this combination of parameters.

On master, the condition for "should I do throttling" is only in one place - before calling the throttle function and in the send burst loop. However, the throttling function is also called after this loop at which point it calculates that the target rate '0' (as configured through -b 0) has been exceeded and red-lights the generation stream after the first burst.

This means that the -b 0/1000 combination basically only sends 1000 packets in a single burst and then does nothing until an end-condition is met, in case of an end condition for number of packets sent that just means it'll loop until infinity; in case a duration it'll wait until the expiration.

@olichtne
Copy link
Contributor Author

olichtne commented Aug 9, 2019

I'll just repeat that this is related to the issue #899 where I can workaround slow udp generation by using the -b 0/1000 arguments together if I have this fix. I reported the issue separately as it might mean that a different fix is required for that, but if not then this does work as a workaround.

@bmah888
Copy link
Contributor

bmah888 commented Aug 9, 2019

Got it. I'm going to have to find some time to stare at this code for a little while (as well as your bug report in #899). Thanks.

@bmah888 bmah888 self-assigned this Aug 9, 2019
Copy link
Contributor

@bmah888 bmah888 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I finally get it. There are multiple calls to iperf_check_throttle(). Your point was that only one of them had the "skip throttling" condition around it, and it makes more sense to put the "skip throttling" check inside iperf_check_throttle() itself.

I tested this and it seemed to work correctly.

I wonder if another way to do this would have been to force burst to 0 if the rate was 0 when starting a new test. Your way is a little more robust, however.

Will merge this shortly.

@bmah888 bmah888 merged commit 80353b0 into esnet:master Oct 1, 2019
@olichtne olichtne deleted the fix_burst_mode branch October 2, 2019 18:25
@olichtne olichtne restored the fix_burst_mode branch October 2, 2019 18:27
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