From: Stuart Henderson <stu@spacehopper.org>
Subject: Re: iperf3: unbreak rcctl with IPv6 link-local bind/source addresses
To: Klemens Nanni <kn@openbsd.org>
Cc: ports <ports@openbsd.org>
Date: Thu, 30 Oct 2025 18:48:12 +0000

On 2025/10/30 17:49, Klemens Nanni wrote:
> If the argument contains a '%', the code will modify argv[] in place,
> which causes rc.subr(8)'s pexp aka. pgrep(1) to mismatch and thus
> rcctl(8) to report failure despite the service running fine:
> 
> 	$ rcctl get iperf3 flags
> 	-6 --bind fe80::1%vport0
> 	$ rcctl check iperf3
> 	iperf3(failed)
> 	$ pgrep -fl iperf3
> 	33091 /usr/local/bin/iperf3 -s -D -6 --bind fe80::1

ok I guess.

> Funnily, the code knows that:
> 
> 	 * Modifies the string pointed to by spec in-place due to the use of
> 		  * strtok(3). The caller should strdup(3) or otherwise copy the string
> 	 * if an unmodified copy is needed.

suppose that comment should be removed too?

seems worth trying it upstream and see if they'll take it?

> 	 */
> 	int
> 	iperf_parse_hostname(struct iperf_test *test, char *spec, char **p, char **p1) {
> 
> 
> Simply pass a copy to that function to keep scope identifies in argv[]:
> 
> 	$ rcctl check iperf3
> 	iperf3(ok)
> 	$ pgrep -fl iperf3
> 	98863 /usr/local/bin/iperf3 -s -D -6 --bind fe80::1%vport0
> 
> --client works the same as --bind, so fix it as well.
> 
> Feedback? OK?
> 
> 
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/net/iperf3/Makefile,v
> diff -u -p -r1.21 Makefile
> --- Makefile	26 Dec 2024 18:12:38 -0000	1.21
> +++ Makefile	30 Oct 2025 17:08:08 -0000
> @@ -3,6 +3,7 @@ COMMENT=	tool to measure maximum achieva
>  V=		3.18
>  DISTNAME=	iperf-${V}
>  PKGNAME=	iperf3-${V}
> +REVISION=	0
>  
>  SHARED_LIBS +=	iperf                5.0      # 0.0
>  
> Index: patches/patch-src_iperf_api_c
> ===================================================================
> RCS file: /cvs/ports/net/iperf3/patches/patch-src_iperf_api_c,v
> diff -u -p -r1.12 patch-src_iperf_api_c
> --- patches/patch-src_iperf_api_c	26 Dec 2024 18:12:38 -0000	1.12
> +++ patches/patch-src_iperf_api_c	30 Oct 2025 17:31:59 -0000
> @@ -1,9 +1,64 @@
> -Default to IPv4.
> +- Pass a copy of argv[] to prevent changes by strtok(3),
> +  otherwise rc.subr(8) pexp might not match and cause rcctl failure.
> +- Default to IPv4.
>  
>  Index: src/iperf_api.c
>  --- src/iperf_api.c.orig
>  +++ src/iperf_api.c
> -@@ -2998,7 +2998,7 @@ iperf_defaults(struct iperf_test *testp)
> +@@ -1243,7 +1243,7 @@ iperf_parse_arguments(struct iperf_test *test, int arg
> +                 }
> + 		iperf_set_test_role(test, 's');
> +                 break;
> +-            case 'c':
> ++            case 'c': {
> +                 if (test->role == 's') {
> +                     i_errno = IESERVCLIENT;
> +                     return -1;
> +@@ -1251,7 +1251,8 @@ iperf_parse_arguments(struct iperf_test *test, int arg
> + 		iperf_set_test_role(test, 'c');
> + 		iperf_set_test_server_hostname(test, optarg);
> + 
> +-                if (iperf_parse_hostname(test, optarg, &p, &p1)) {
> ++                char *arg = strdup(optarg);
> ++                if (iperf_parse_hostname(test, arg, &p, &p1)) {
> + #if defined(HAVE_SO_BINDTODEVICE)
> +                     /* Get rid of the hostname we saved earlier. */
> +                     free(iperf_get_test_server_hostname(test));
> +@@ -1262,7 +1263,9 @@ iperf_parse_arguments(struct iperf_test *test, int arg
> +                     return -1;
> + #endif /* HAVE_SO_BINDTODEVICE */
> +                 }
> ++                free(arg);
> +                 break;
> ++            }
> +             case 'u':
> +                 set_protocol(test, Pudp);
> + 		client_flag = 1;
> +@@ -1374,10 +1377,11 @@ iperf_parse_arguments(struct iperf_test *test, int arg
> + 		client_flag = 1;
> +                 break;
> + 
> +-            case 'B':
> ++            case 'B': {
> +                 iperf_set_test_bind_address(test, optarg);
> + 
> +-                if (iperf_parse_hostname(test, optarg, &p, &p1)) {
> ++                char *arg = strdup(optarg);
> ++                if (iperf_parse_hostname(test, arg, &p, &p1)) {
> + #if defined(HAVE_SO_BINDTODEVICE)
> +                     /* Get rid of the hostname we saved earlier. */
> +                     free(iperf_get_test_bind_address(test));
> +@@ -1388,7 +1392,9 @@ iperf_parse_arguments(struct iperf_test *test, int arg
> +                     return -1;
> + #endif /* HAVE_SO_BINDTODEVICE */
> +                 }
> ++                free(arg);
> +                 break;
> ++            }
> + #if defined (HAVE_SO_BINDTODEVICE)
> +             case OPT_BIND_DEV:
> +                 iperf_set_test_bind_dev(test, optarg);
> +@@ -2998,7 +3004,7 @@ iperf_defaults(struct iperf_test *testp)
>       testp->stats_interval = testp->reporter_interval = 1;
>       testp->num_streams = 1;
>   
>