From: David Hill <dhill@mindcry.org>
Subject: Re: tcpbench - fix ipv6/udp (diff)
To: tech@openbsd.org
Date: Sat, 30 Aug 2025 13:28:45 +0000



On 8/30/25 6:23 AM, Stefan Sperling wrote:
> 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.
> 

Good catch!  We could get the interface MTU instead of hardcoding 1500, 
but that would require ioctl, so I left it as is.

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;