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

Add iperf example #87

Merged
merged 1 commit into from
Jan 29, 2024
Merged

Conversation

dimstav23
Copy link

@dimstav23 dimstav23 commented Jan 12, 2024

This PR adds the iperf example.
It provides a Makefile that clones and builds the latest stable version (3.16) of iperf as of now.
Note that it places the iperf3 executable and the libiperf shared library in the created install directory.
It further includes the Gramine manifest template and a top level README that provides instructions on how to compile and run the iperf server (both natively and in Gramine --- with and without SGX) and the iperf client (natively) to get the desired measurements.


This change is Reviewable

@dimstav23 dimstav23 force-pushed the dimstav23/add-iperf branch from 406cbaf to 7754d5e Compare January 12, 2024 23:06
@dimakuv dimakuv force-pushed the dimstav23/add-iperf branch 3 times, most recently from 20922fe to a1be28c Compare January 15, 2024 09:05
Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 4 files at r1, 1 of 1 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimstav23)

a discussion (no related file):
For other reviewers: @dimstav23 was working with me on this PR. I already did the first round of review internally.

I also took the liberty of squashing his initial 6 commits into a single commit.

Also, there were a couple bugs in the first revision, so I fixed them too.


a discussion (no related file):
Results of my localhost tests (just making sure that Gramine works, and it has ok-ish perf):

  • Native:
gramineproject/examples/iperf$ LD_LIBRARY_PATH=./install ./install/iperf3 -s
Accepted connection from 127.0.0.1, port 58698
[  5] local 127.0.0.1 port 5201 connected to 127.0.0.1 port 58700
[ ID] Interval           Transfer     Bitrate
[  5]   0.00-1.00   sec  7.41 GBytes  63.6 Gbits/sec
...
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bitrate
[  5]   0.00-10.00  sec  73.3 GBytes  63.0 Gbits/sec                  receiver

gramineproject/examples/iperf$ LD_LIBRARY_PATH=./install ./install/iperf3 -c localhost -p 5201
Connecting to host localhost, port 5201
[  5] local 127.0.0.1 port 58700 connected to 127.0.0.1 port 5201
[ ID] Interval           Transfer     Bitrate         Retr  Cwnd
[  5]   0.00-1.00   sec  7.41 GBytes  63.6 Gbits/sec    0   2.06 MBytes
...
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec  73.3 GBytes  63.0 Gbits/sec    0             sender
[  5]   0.00-10.00  sec  73.3 GBytes  63.0 Gbits/sec                  receiver
  • Gramine-direct:
gramineproject/examples/iperf$ gramine-direct iperf3
Accepted connection from 127.0.0.1, port 58702
[  5] local 127.0.0.1 port 5201 connected to 127.0.0.1 port 58704
[ ID] Interval           Transfer     Bitrate
[  5]   0.00-1.00   sec  7.72 GBytes  66.3 Gbits/sec
...
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bitrate
[  5]   0.00-10.00  sec  77.3 GBytes  66.4 Gbits/sec                  receiver

gramineproject/examples/iperf$ LD_LIBRARY_PATH=./install ./install/iperf3 -c localhost -p 5201
Connecting to host localhost, port 5201
[  5] local 127.0.0.1 port 58704 connected to 127.0.0.1 port 5201
[ ID] Interval           Transfer     Bitrate         Retr  Cwnd
[  5]   0.00-1.00   sec  7.73 GBytes  66.3 Gbits/sec    0   2.62 MBytes
...
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec  77.3 GBytes  66.4 Gbits/sec    0             sender
[  5]   0.00-10.00  sec  77.3 GBytes  66.4 Gbits/sec                  receiver
  • Gramine-SGX:
gramineproject/examples/iperf$ gramine-sgx iperf3
Accepted connection from 127.0.0.1, port 58706
[  5] local 127.0.0.1 port 5201 connected to 127.0.0.1 port 58708
[ ID] Interval           Transfer     Bitrate
[  5]   0.00-1.00   sec  3.67 GBytes  31.5 Gbits/sec
...
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bitrate
[  5]   0.00-10.00  sec  36.3 GBytes  31.2 Gbits/sec                  receiver

gramineproject/examples/iperf$ LD_LIBRARY_PATH=./install ./install/iperf3 -c localhost -p 5201
Connecting to host localhost, port 5201
[  5] local 127.0.0.1 port 58708 connected to 127.0.0.1 port 5201
[ ID] Interval           Transfer     Bitrate         Retr  Cwnd
[  5]   0.00-1.00   sec  3.68 GBytes  31.6 Gbits/sec    0   1.37 MBytes
...
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec  36.3 GBytes  31.2 Gbits/sec    0             sender
[  5]   0.00-10.00  sec  36.3 GBytes  31.2 Gbits/sec                  receiver

Gramine-SGX shows 50% of native throughput, which is what I would expect.



iperf/Makefile line 21 at r3 (raw file):

$(IPERF_DIR)/configure:
	git clone $(IPERF_REPO)
	cd $(IPERF_DIR) && git checkout $(IPERF_VERSION) && ./configure && make

Other reviewers: please note that here we use git clone && git checkout <tag>. I don't remember if there are any objections against this way of downloading the code?

Alternatively, we can easily replace with a classic "download from GitHub Releases page".


iperf/README.md line 8 at r3 (raw file):

# Prerequisites

`iperf` has no prerequisites ([source](https://github.com/esnet/iperf?tab=readme-ov-file#prerequisites)).

That's a bit weird that the official iperf repo doesn't list any dependencies, because iperf does have non-libc dependencies:

$ ldd ./install/iperf3
 libcrypto.so.1.1 => /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1 (0x00007fc8d6bb3000)
 libsctp.so.1 => /usr/lib/x86_64-linux-gnu/libsctp.so.1 (0x00007fc8d6bae000)
 ...

I guess these two libs are more or less always installed on distros, so no need to specify them explicitly?

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @dimstav23)

a discussion (no related file):
Add the explicit BSD-3 license, see #90


@dimstav23
Copy link
Author

dimstav23 commented Jan 19, 2024

Reviewable status: all files reviewed, 3 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @dimstav23)

a discussion (no related file): Add the explicit BSD-3 license, see #90

Added the BSD-3 license.

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimstav23)

a discussion (no related file):

Previously, dimstav23 (Dimitris) wrote…

Added the BSD-3 license.

Thanks


Copy link
Member

@woju woju left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv and @dimstav23)


iperf/Makefile line 21 at r3 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Other reviewers: please note that here we use git clone && git checkout <tag>. I don't remember if there are any objections against this way of downloading the code?

Alternatively, we can easily replace with a classic "download from GitHub Releases page".

Tags are as mutable as branches, both are just refs in git. Of course this is vulnerable to anyone capable of pushing to the repo of mitming HTTPS. Unless you're verifying signature over either tag or commit, which you're not, you should be checking out SHA1. Code as it is here is bad example.

But this is all unneeded. Why can't we just use iperf or iperf3 installed from distro packages?

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @dimakuv and @dimstav23)


iperf/Makefile line 21 at r3 (raw file):

Previously, woju (Wojtek Porczyk) wrote…

Tags are as mutable as branches, both are just refs in git. Of course this is vulnerable to anyone capable of pushing to the repo of mitming HTTPS. Unless you're verifying signature over either tag or commit, which you're not, you should be checking out SHA1. Code as it is here is bad example.

But this is all unneeded. Why can't we just use iperf or iperf3 installed from distro packages?

I will take care of this now.

But I want to do it via "download from GitHub Releases page". I think it's much better to have a specific version of the tool, instead of the unknown distro package version.

@dimstav23
Copy link
Author

dimstav23 commented Jan 25, 2024

Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ) (waiting on @dimakuv and @dimstav23)

iperf/Makefile line 21 at r3 (raw file):

Previously, woju (Wojtek Porczyk) wrote…
I will take care of this now.

But I want to do it via "download from GitHub Releases page". I think it's much better to have a specific version of the tool, instead of the unknown distro package version.

I also tried to adapt it to use the Ubuntu 22.04.3 LTS package binary.
However I hit on this error.
Note: tested with gramine-direct using gramine v1.6.

Update: I saw that @dimakuv modified the fetching with the SHA256 addition :)

dimakuv
dimakuv previously approved these changes Jan 25, 2024
Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 3 of 3 files at r6, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @dimstav23 and @woju)


iperf/Makefile line 21 at r3 (raw file):

Previously, dimstav23 (Dimitris) wrote…

I also tried to adapt it to use the Ubuntu 22.04.3 LTS package binary.
However I hit on this error.
Note: tested with gramine-direct using gramine v1.6.

Thanks @dimstav23 for explanations.

Anyway, done now.

Copy link
Member

@woju woju left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r5, 2 of 3 files at r6, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @dimstav23)


iperf/Makefile line 21 at r3 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Thanks @dimstav23 for explanations.

Anyway, done now.

Ah, yes, if there's technical reason, then OK, but please describe the problem in README and say that for iperf3 to work in Gramine, you need to recompile it to disable usage of that flag. What I want to avoid is people who will use iperf from distro rediscovering this problem the hard way.

dimakuv
dimakuv previously approved these changes Jan 26, 2024
Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r7, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @dimstav23)


iperf/Makefile line 21 at r3 (raw file):

Previously, woju (Wojtek Porczyk) wrote…

Ah, yes, if there's technical reason, then OK, but please describe the problem in README and say that for iperf3 to work in Gramine, you need to recompile it to disable usage of that flag. What I want to avoid is people who will use iperf from distro rediscovering this problem the hard way.

Done.

It got even more interesting. This same problem was hit on Microsoft's WSL, so iperf authors fixed it in v3.10:

On Ubuntu 22.04, however, we have v3.9: https://packages.ubuntu.com/jammy/iperf3

Copy link
Contributor

@kailun-qin kailun-qin left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r5, 1 of 3 files at r6, 2 of 2 files at r7, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @dimstav23)


iperf/README.md line 9 at r7 (raw file):

`iperf` has no prerequisites
([source](https://github.com/esnet/iperf?tab=readme-ov-file#prerequisites)).

Can we pin this to a specific commit/tag?

Code quote:

https://github.com/esnet/iperf?tab=readme-ov-file#prerequisites

iperf/README.md line 14 at r7 (raw file):

We build `iperf` from source because on current Ubuntu distros (we tested Ubuntu
22.04), the `iperf3` package has a version 3.9 which is built with

why not simply -> ...because Ubuntu 22.04 has iperf3 v3.9 in its package repositories which is built ...

Code quote:

We build `iperf` from source because on current Ubuntu distros (we tested Ubuntu
22.04), the `iperf3` package has a version 3.9 which is built with

iperf/README.md line 27 at r7 (raw file):

implement congestion control algorithm. Thus, iperf 3.10+ prebuilt packages
should work under Gramine without problems. See [release
notes](https://github.com/esnet/iperf/blob/master/RELNOTES.md#iperf-310-2021-05-26)

ditto (pin to specific commit/tag)

Code quote:

https://github.com/esnet/iperf/blob/master/RELNOTES.md#iperf-310-2021-05-26

iperf/README.md line 33 at r7 (raw file):

## Building for Linux

Run `make` in the root directory.

current?

Code quote:

root

iperf/README.md line 37 at r7 (raw file):

## Building for SGX

Run `make SGX=1` (non-debug) or `make SGX=1 DEBUG=1` (debug) in the root

ditto

Code quote:

root

iperf/README.md line 72 at r7 (raw file):

- `-s, --server`: run in server mode
- `-1, --one-off`: handle one client connection then exit
- `--idle-timeout #`: restart idle server after # seconds in case it got stuck

The option(s) here may also be version-dependent. I suggest that we also note this somewhere.

Code quote:

--idle-timeout

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @dimakuv and @kailun-qin)


-- commits line 4 at r8:
TODO: Add myself as co-author.


iperf/README.md line 9 at r7 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

Can we pin this to a specific commit/tag?

Done


iperf/README.md line 14 at r7 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

why not simply -> ...because Ubuntu 22.04 has iperf3 v3.9 in its package repositories which is built ...

Done


iperf/README.md line 27 at r7 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

ditto (pin to specific commit/tag)

Done


iperf/README.md line 33 at r7 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

current?

Done


iperf/README.md line 37 at r7 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

ditto

Done


iperf/README.md line 72 at r7 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

The option(s) here may also be version-dependent. I suggest that we also note this somewhere.

Done

@dimakuv dimakuv self-requested a review January 26, 2024 15:16
Copy link
Member

@woju woju left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r7, 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions, "fixup! " found in commit messages' one-liners (waiting on @dimakuv and @kailun-qin)


iperf/Makefile line 21 at r3 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Done.

It got even more interesting. This same problem was hit on Microsoft's WSL, so iperf authors fixed it in v3.10:

On Ubuntu 22.04, however, we have v3.9: https://packages.ubuntu.com/jammy/iperf3

Debian 12 has 3.12 and Ubuntu 24.04 will have at least 3.15 (which is what's in noble right now: https://launchpad.net/ubuntu/+source/iperf3. So IDK, either you merge as is with a TODO bug filed to be fixed after 24.04, or you can test on bookworm or mantic (23.10) and just write that in README. I'm OK with either case.

Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 7 unresolved discussions, "fixup! " found in commit messages' one-liners (waiting on @kailun-qin)


iperf/Makefile line 21 at r3 (raw file):

Previously, woju (Wojtek Porczyk) wrote…

Debian 12 has 3.12 and Ubuntu 24.04 will have at least 3.15 (which is what's in noble right now: https://launchpad.net/ubuntu/+source/iperf3. So IDK, either you merge as is with a TODO bug filed to be fixed after 24.04, or you can test on bookworm or mantic (23.10) and just write that in README. I'm OK with either case.

Done, added a TODO github issue: #92

Copy link
Contributor

@kailun-qin kailun-qin left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, "fixup! " found in commit messages' one-liners (waiting on @dimakuv)

Co-authored-by: Dmitrii Kuvaiskii <dmitrii.kuvaiskii@intel.com>
Signed-off-by: Dimitrios Stavrakakis <dimitrios.stavrakakis@intel.com>
Signed-off-by: Dmitrii Kuvaiskii <dmitrii.kuvaiskii@intel.com>
@dimakuv dimakuv force-pushed the dimstav23/add-iperf branch from bb3e49e to d5b0168 Compare January 29, 2024 10:36
Copy link

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


-- commits line 4 at r8:

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

TODO: Add myself as co-author.

Done

@dimakuv dimakuv merged commit d5b0168 into gramineproject:master Jan 29, 2024
2 checks passed
@dimstav23 dimstav23 deleted the dimstav23/add-iperf branch January 29, 2024 11:52
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.

4 participants