Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: tcpbench - fix ipv6/udp (diff)
To:
David Hill <dhill@mindcry.org>
Cc:
tech@openbsd.org
Date:
Sat, 30 Aug 2025 08:23:00 +0200

Download raw body.

Thread
On Sat, Aug 30, 2025 at 12:53:25AM +0000, David Hill wrote:
> Hello -
> 
> When using tcpbench with IPv6 and UDP, it showed poor performance. Using
> -vvv, I saw it writing 1472 bytes.   With -B 1452 (1500 - 40 - 8) (mtu -
> ipv6 header - udp header) I get proper performance.

Thanks for fixing this! I've seen this happen as well and ended up
using bluhm's udpbench instead due to this problem.

Ok by me, though I spotted one issue noted below which should be
fixed before commit:

> Index: usr.bin/tcpbench/tcpbench.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/tcpbench/tcpbench.c,v
> diff -u -p -r1.73 tcpbench.c
> --- usr.bin/tcpbench/tcpbench.c	30 Dec 2024 21:19:29 -0000	1.73
> +++ usr.bin/tcpbench/tcpbench.c	30 Aug 2025 00:52:08 -0000
> @@ -61,7 +61,6 @@
>  #define DEFAULT_PORT "12345"
>  #define DEFAULT_STATS_INTERVAL 1000 /* ms */
>  #define DEFAULT_BUF (256 * 1024)
> -#define DEFAULT_UDP_PKT (1500 - 28) /* TODO don't hardcode this */
>  #define TCP_MODE !ptb->uflag
>  #define UDP_MODE ptb->uflag
>  #define MAX_FD 1024
> @@ -117,6 +116,7 @@ static void	print_tcp_header(void);
>  static void	list_kvars(void);
>  static void	check_kvar(const char *);
>  static char **	check_prepare_kvars(char *);
> +static void	dummybuf_prepare(struct statctx *);
>  static void	stats_prepare(struct statctx *);
>  static void	summary_display(void);
>  static void	tcp_stats_display(unsigned long long, long double, float,
> @@ -352,8 +352,44 @@ check_prepare_kvars(char *list)
>  }
> 
>  static void
> +dummybuf_prepare(struct statctx *sc)
> +{
> +	/*
> +	 * Rationale,
> +	 * If TCP, use a big buffer with big reads/writes.
> +	 * If UDP, use a big buffer in server and a buffer the size of a
> +	 * ethernet packet.
> +	 */
> +	if (ptb->dummybuf_len == 0 && (ptb->sflag || TCP_MODE))
> +		ptb->dummybuf_len = DEFAULT_BUF;
> +		
> +	if (ptb->dummybuf_len == 0) {
> +		struct sockaddr_storage ss;
> +		socklen_t len = sizeof(ss);
> +
> +		if (getsockname(sc->fd, (struct sockaddr *)&ss, &len) == -1)
> +			err(1, "getsockname");
> +
> +		switch (ss.ss_family) {
> +		case AF_INET:
> +			ptb->dummybuf_len = 1500-28;

Missing a "break;" here.

> +		case AF_INET6:
> +		default:
> +			ptb->dummybuf_len = 1500-48;
> +			break;
> +		}
> +	}
> +
> +	if ((ptb->dummybuf = malloc(ptb->dummybuf_len)) == NULL)
> +		err(1, "malloc");
> +	arc4random_buf(ptb->dummybuf, ptb->dummybuf_len);
> +}
> +
> +static void
>  stats_prepare(struct statctx *sc)
>  {
> +	dummybuf_prepare(sc);
> +
>  	sc->buf = ptb->dummybuf;
>  	sc->buflen = ptb->dummybuf_len;
> 
> @@ -1276,19 +1312,6 @@ main(int argc, char **argv)
>  	if (pledge("stdio id dns inet unix", NULL) == -1)
>  		err(1, "pledge");
> 
> -	/*
> -	 * Rationale,
> -	 * If TCP, use a big buffer with big reads/writes.
> -	 * If UDP, use a big buffer in server and a buffer the size of a
> -	 * ethernet packet.
> -	 */
> -	if (!ptb->dummybuf_len) {
> -		if (ptb->sflag || TCP_MODE)
> -			ptb->dummybuf_len = DEFAULT_BUF;
> -		else
> -			ptb->dummybuf_len = DEFAULT_UDP_PKT;
> -	}
> -
>  	bzero(&hints, sizeof(hints));
>  	hints.ai_family = family;
>  	if (UDP_MODE) {
> @@ -1375,9 +1398,6 @@ main(int argc, char **argv)
> 
>  	/* Init world */
>  	TAILQ_INIT(&sc_queue);
> -	if ((ptb->dummybuf = malloc(ptb->dummybuf_len)) == NULL)
> -		err(1, "malloc");
> -	arc4random_buf(ptb->dummybuf, ptb->dummybuf_len);
> 
>  	timerclear(&mainstats.t_first);
>  	mainstats.floor_mbps = INFINITY;
> 
>