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:
Tue, 9 Sep 2025 10:51:25 +0200

Download raw body.

Thread
On Sat, Aug 30, 2025 at 01:28:45PM +0000, David Hill wrote:
> On 8/30/25 6:23 AM, Stefan Sperling wrote:
> > Ok by me, though I spotted one issue noted below which should be
> > fixed before commit:

> > > +		switch (ss.ss_family) {
> > > +		case AF_INET:
> > > +			ptb->dummybuf_len = 1500-28;
> > 
> > Missing a "break;" here.
> > 
> 
> Good catch!  We could get the interface MTU instead of hardcoding 1500, but
> that would require ioctl, so I left it as is.

It seems this hasn't been committed yet?

Still OK by me. Meanwhile I have tested it, too. Works as expected for me.

> 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 13:25:30 -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,45 @@ 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;
> +			break;
> +		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 +1313,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 +1399,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;
> 
>