Skip to content

CLI backward compatible fix for issue #768#797

Open
paaguti wants to merge 2 commits into
esnet:masterfrom
paaguti:master
Open

CLI backward compatible fix for issue #768#797
paaguti wants to merge 2 commits into
esnet:masterfrom
paaguti:master

Conversation

@paaguti
Copy link
Copy Markdown

@paaguti paaguti commented Sep 24, 2018

PLEASE NOTE the following text from the iperf3 license. Submitting a
pull request to the iperf3 repository constitutes "[making]
Enhancements available...publicly":

You are under no obligation whatsoever to provide any bug fixes, patches, or
upgrades to the features, functionality or performance of the source code
("Enhancements") to anyone; however, if you choose to make your Enhancements
available either publicly, or directly to Lawrence Berkeley National
Laboratory, without imposing a separate written license agreement for such
Enhancements, then you hereby grant the following license: a non-exclusive,
royalty-free perpetual license to install, use, modify, prepare derivative
works, incorporate into other computer software, distribute, and sublicense
such enhancements or derivative works thereof, in binary and source code form.

The complete iperf3 license is available in the LICENSE file in the
top directory of the iperf3 source tree.

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

  • Issues fixed (if any):
    iperf3 3.5 TCP option -n not working #768

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

Provide a backwards compatible approach to using the -n option.
-n burst leaves multisend untouched
-n burst/multisend sets the value for multisend.
It's like with the -b flag

This work is done in the context of the MAMI Project, which is receiving funding from the European Union’s Horizon 2020 research and innovation programme under grant agreement No 688421.

@paaguti
Copy link
Copy Markdown
Author

paaguti commented Sep 24, 2018

The second alternative directly writes the multisend field in the test. This is what I did originally and it seems to be more reliable than setting the burst.
It seems that the code that sets the multisend for the sending loop needs further investigation ...
(sorry :( )

@bmah888
Copy link
Copy Markdown
Contributor

bmah888 commented Sep 27, 2018

Hi @paaguti...thanks for the pull request. I didn't quite understand your last comment...are you saying that you need to do some more work on this code change?

@paaguti
Copy link
Copy Markdown
Author

paaguti commented Sep 28, 2018

Hi,

first I tried setting test->settings->burst, it looked more intuitive for me. But I discovered that it doesn't prevent multiple bursts. Therefore I went back to my original rationale and set test->multisend and everything works as expected (no more changes needed) .

I have left both steps in the GitHub, because I think it would be nice to understand and document the behaviour depending on both variables.

They seem interdependent, because when test->settings->burst not zero, it will control the send loop the same way as test->multisend does, but there are more implications I don't quite understand yet.

Maybe I'm just confusing you with my thoughts, rumblings and ways to arrive to knowledge... sorry if it is the case.

@bmah888 bmah888 linked an issue Sep 17, 2020 that may be closed by this pull request
@bmah888
Copy link
Copy Markdown
Contributor

bmah888 commented Sep 17, 2020

I haven't forgotten about this. The PR is actually larger than it has to be, because there are a number of whitespace-only changes in the code...all of the significant changes are really in the argument-processing section of iperf_api.c. Because there were merge conflicts, I started playing with a change that's just the argument processing. But I noticed I still wasn't getting exactly deterministic results...they seem "better" in an informal way. I did try rearranging some of the logic in send loop to see if that makes a difference, and I'm not sure. Also I added support for -k, similar to what was in the original PR for -n.

The result of my changes are on the issue-768 branch. There's some diagnostic code in there as well that would eventually go away.

@bmah888
Copy link
Copy Markdown
Contributor

bmah888 commented Sep 17, 2020

Stumbled across #982 while trying to test this.

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.

iperf3 3.5 TCP option -n not working

2 participants