TCP congestion control algorithm support for client

bmah888 commented 10 years ago

From susant.sahani on December 05, 2013 09:38:49

Explanation of new feature Added TCP Congestion control support. -C, --linux-congestion set TCP congestion control algorithm (Linux only)

Attached patch

Thanks, Susant

Attachment: tcp_congestion.patch

Original issue: http://code.google.com/p/iperf/issues/detail?id=121

bmah888 commented 10 years ago

From bltierney@es.net on December 06, 2013 06:12:32

thanks for the patch!

Owner: bmah@es.net

bmah888 commented 10 years ago

From bltierney@es.net on December 10, 2013 06:39:48

Susant: I went ahead and made you a committer, so you can add this if you'd like.

bmah888 commented 10 years ago

From jef.poskanzer on December 10, 2013 06:53:34

Just a couple of comments on the patch:

The references to test->congestion do not need to be ifdeffed.
The references to TCP_CONGESTION in iperf_tcp.c probably do need to be ifdeffed.
Make sure the flag gives an error on on-linux systems; it's ifdeffed in longopts but not in getopt_long or the switch cases.
The IESETCONGESTION case in iperf_error.c should probably set perr = 1.
Don't forget to add it to the man page, and the wiki copy of the man page.

bmah888 commented 10 years ago

From jef.poskanzer on December 10, 2013 15:26:17

Couple more notes on this change:

Not sure we want to transmit the setting from client to server, the other alternative is to specify it separately on the two sides.
If we do want to transmit it, we need to strdup the value client side as well as server side, and free it. Also we should set client_flag=1.
Not sure sending sizeof(test->congestion) to setsockopt is right - that's the size of the pointer. Shouldn't it be strlen instead?

bmah888 commented 10 years ago

From ssahani@redhat.com on December 12, 2013 02:22:53

Modified the patch

The references to test->congestion do not need to be ifdeffed. Done
The references to TCP_CONGESTION in iperf_tcp.c probably do need to be ifdeffed. Done
Make sure the flag gives an error on on-linux systems; it's ifdeffed in longopts but not in getopt_long or the switch cases. Done
The IESETCONGESTION case in iperf_error.c should probably set perr = 1. Not sure .. setting perr =1 resulting in wrong error code .. "File not found"
Not sure we want to transmit the setting from client to server, the other alternative is to specify it separately on the two sides.

If it's about JSON setting then The API itself doing a strdup void cJSON_AddItemToObject( cJSON object, const char string, cJSON *item ) { if ( ! item ) return; if ( item->string ) cJSON_free( item->string ); item->string = cJSON_strdup( string ); cJSON_AddItemToArray( object, item ); }
Not sure sending sizeof(test->congestion) to setsockopt is right - that's the size of the pointer. Shouldn't it be strlen instead? Modified

Attachment: tcp_congestion.patch

bmah888 commented 10 years ago

From jef.poskanzer on December 12, 2013 08:37:36

Thanks.

The cJSON library does do a strdup, but then it frees the item when cJSON_Delete is called, so you can't save that copy and have to make your own. So, get_parameters should strdup congestion, therefore iperf_free_test should free it, and therefore iperf_parse_arguments should strdup it too. That last part is the only thing still missing in this version of the patch.

I'm not sure what the problem is with perr. The setsockopt man page says it sets errno so that should work. Oh wait, you're calling close - that probably overwrites errno even if it succeeds. I guess all the other setsockopt calls (and the bind call) have the same problem and no one ever noticed.

Oh and the flag still needs to be added to the man page.

bmah888 commented 10 years ago

From ssahani@redhat.com on December 12, 2013 08:54:27

Thanks for reviewing . iperf_parse_arguments it's doing a malloc

I believe I added the free part

iperf_free_test

if(test->congestion)
free(test->congestion);

bmah888 commented 10 years ago

From jef.poskanzer on December 12, 2013 08:58:59

Oh right, iperf_parse_arguments is doing a malloc and strncpy. My eyes were looking for strdup and missed that. Ok.

bmah888 commented 10 years ago

From jef.poskanzer on December 13, 2013 17:23:58

Although this is assigned to Bruce I will probably do it later tonight. Now that we have hashed it out, it's just a matter of applying the patch and testing, and I want to log another hour for this week.

bmah888 commented 10 years ago

From jef.poskanzer on December 13, 2013 20:04:03

Ok, added.

Someone needs to test whether it's actually working though! First step would be to find a list of valid congestion control algorithm names.

I put in code to save & restore errno across the close() calls. However it's still giving "file not found". I put in a printf to be sure - yep, that's really the errno. Is it possible that's the actual error code returned when you ask for an algorithm that doesn't exist?

bmah888 commented 10 years ago

From ssahani@redhat.com on December 13, 2013 21:06:16

Thanks for committing .

Here is a doc speaks about the list . http://linuxgazette.net/135/pfeiffer.html In My Fedora 20 cubic is default : $ cat /proc/sys/net/ipv4/tcp_congestion_control cubic Also with RHEL 6.X

I think It can be added to wiki how to figure out what are the available congestion control algo . This is what I could pick from the kernel makefile

sus@max ipv4]$ uname -a Linux max 3.11.10-300.fc20.x86_64 #1 SMP Fri Nov 29 19:16:48 UTC 2013 x86_64 x86_64 x86_64 GNU/Linux

obj-$(CONFIG_TCP_CONG_BIC) += tcp_bic.o obj-$(CONFIG_TCP_CONG_CUBIC) += tcp_cubic.o obj-$(CONFIG_TCP_CONG_WESTWOOD) += tcp_westwood.o obj-$(CONFIG_TCP_CONG_HSTCP) += tcp_highspeed.o obj-$(CONFIG_TCP_CONG_HYBLA) += tcp_hybla.o obj-$(CONFIG_TCP_CONG_HTCP) += tcp_htcp.o obj-$(CONFIG_TCP_CONG_VEGAS) += tcp_vegas.o obj-$(CONFIG_TCP_CONG_VENO) += tcp_veno.o obj-$(CONFIG_TCP_CONG_SCALABLE) += tcp_scalable.o obj-$(CONFIG_TCP_CONG_LP) += tcp_lp.o obj-$(CONFIG_TCP_CONG_YEAH) += tcp_yeah.o obj-$(CONFIG_TCP_CONG_ILLINOIS) += tcp_illinois.o

bmah888 commented 10 years ago

From jef.poskanzer on December 13, 2013 21:14:06

Cool. I confirmed that my test hosts accept cubic and htcp. Don't know how to verify that they actually do anything different. Perhaps we just have to accept that part on faith.

bmah888 commented 10 years ago

From vdasgupt@redhat.com on December 13, 2013 22:24:19

To check which algorithms are supported by the running kernel, the following command can be used :

$ cat /boot/config-uname -r |grep CONFIG_TCP_CONG

CONFIG_TCP_CONG_ADVANCED=y CONFIG_TCP_CONG_BIC=m CONFIG_TCP_CONG_CUBIC=y CONFIG_TCP_CONG_WESTWOOD=m CONFIG_TCP_CONG_HTCP=m CONFIG_TCP_CONG_HSTCP=m CONFIG_TCP_CONG_HYBLA=m CONFIG_TCP_CONG_VEGAS=m CONFIG_TCP_CONG_SCALABLE=m CONFIG_TCP_CONG_LP=m CONFIG_TCP_CONG_VENO=m CONFIG_TCP_CONG_YEAH=m CONFIG_TCP_CONG_ILLINOIS=m

The default is usually cubic as Susant pointed out.

$ cat /proc/sys/net/ipv4/tcp_congestion_control cubic

-Vivek

bmah888 commented 10 years ago

From bltierney@es.net on December 18, 2013 13:49:17

Labels: Milestone-3.0-Release

bmah888 commented 10 years ago

From bltierney@es.net on December 18, 2013 13:50:49

can folks help test this? Does it work as is should?

bmah888 commented 10 years ago

From bltierney@es.net on December 20, 2013 15:18:28

Seems to work fine, but I wonder if we could have a more user-friendly error message.

Currently its this: iperf3: error - unable to set TCP_CONGESTION: No such file or directory

better might be this: iperf3: error - unable to set TCP_CONGESTION: Congestion control XXX not supported on this host

bmah888 commented 10 years ago

From ssahani@redhat.com on December 22, 2013 04:21:29

The error number ENOENT is getting set when the congestion algo not found .

#define ENOENT           2      /* No such file or directory */

This is the reason why it's not giving correct error. So the saved error number actually carrying the same value what is set by setsockopt failure irrespective of the close() success.

To print the CC algo while it failed a struct iperf_test object is required to access in iperf_strerror(). I think it's bit better than file not found . In this case can't trust on perror which would be confusion for end user.

"Supplied congestion control algorithm not supported on this host"

Attachment: cc_err.diff

bmah888 commented 10 years ago

From ssahani@redhat.com on December 22, 2013 06:26:19

This is a way to test CC http://www.linuxfoundation.org/collaborate/workgroups/networking/tcp_testing#Congestion_Control

bmah888 commented 10 years ago

From ssahani@redhat.com on December 22, 2013 22:57:58

I dint figured out there was a ; by mistake . I changed it now .

Attachment: cc_err.diff

bmah888 commented 10 years ago

From bmah@es.net on January 02, 2014 13:51:21

Committed the patch from Comment 19 in 74da0cfee476, thanks!

bmah888 commented 10 years ago

From bmah@es.net on January 03, 2014 13:31:59

As far as I can tell from reading through the issue comments, all the work for this enhancement is done. Susant and Brian, do you agree? If so I'll close this issue.

bmah888 commented 10 years ago

From bltierney on January 04, 2014 07:32:18

Yep, I agree it can be closed. Susant?

bmah888 commented 10 years ago

From ssahani@redhat.com on January 04, 2014 08:59:10

Yes please . Thank you !

bmah888 commented 10 years ago

From bmah@es.net on January 04, 2014 20:21:49

By consensus we're declaring victory.

Status: Fixed

FossPrime commented 4 years ago

iperf3: error - unable to set TCP_CONGESTION: Supplied congestion control algorithm not supported on this host

Windows WSL with debian lacks the feature... there is no way to disable it... iperf client becomes useless :/

r-or commented 4 years ago

iperf3: error - unable to set TCP_CONGESTION: Supplied congestion control algorithm not supported on this host

Windows WSL with debian lacks the feature... there is no way to disable it... iperf client becomes useless :/

It works, if you enable WSL2 and install a new distro / convert your distro to WSL2: https://docs.microsoft.com/en-us/windows/wsl/install-win10#step-2---update-to-wsl-2

CanadianJeff commented 4 years ago

is upgrading to WSL2 required or can this be fixed to also support legact WSL1?

Foxtaur commented 3 years ago

Why is inability to set TCP_CONGESTION considered a fatal error? It seems friendlier to issue a warning, instead.

I don't think the response ought to be "get rid of your current Linux (WSL1) in order to use our app." As it seems like the app should work just fine if the option wasn't set.

davidBar-On commented 3 years ago

@Foxtaur, see the discussion in issue #1061 (that is still open about this issue) for why why WSL1 no-congestion is not properly supported by iperf3.

esnet / iperf

TCP congestion control algorithm support for client #121