From: Theo Buehler <tb@theobuehler.org>
Subject: Re: bgpd: set the socketpair socketbuffer size only once
To: tech@openbsd.org
Date: Tue, 3 Dec 2024 14:36:17 +0100

On Tue, Dec 03, 2024 at 02:28:06PM +0100, Claudio Jeker wrote:
> For good performance it makes sense to bump the socket buffers to 4 times
> the IBUF_READ_SIZE (aka MAX_SOCK_BUF). It makes little sense to do this
> loop dance around those setsockopts. It either works or it will most
> probably always fail.

I've seen such a dance in bgpq4 and was scratching my head.

ok tb

> 
> -- 
> :wq Claudio
> 
> Index: bgpd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.c,v
> diff -u -p -r1.279 bgpd.c
> --- bgpd.c	2 Dec 2024 15:13:57 -0000	1.279
> +++ bgpd.c	3 Dec 2024 13:24:23 -0000
> @@ -1306,29 +1306,23 @@ getsockpair(int pipe[2])
>  		fatal("socketpair");
>  
>  	for (i = 0; i < 2; i++) {
> -		for (bsize = MAX_SOCK_BUF; bsize >= 16 * 1024; bsize /= 2) {
> -			if (setsockopt(pipe[i], SOL_SOCKET, SO_RCVBUF,
> -			    &bsize, sizeof(bsize)) == -1) {
> -				if (errno != ENOBUFS)
> -					fatal("setsockopt(SO_RCVBUF, %d)",
> -					    bsize);
> -				log_warn("setsockopt(SO_RCVBUF, %d)", bsize);
> -				continue;
> -			}
> -			break;
> +		bsize = MAX_SOCK_BUF;
> +		if (setsockopt(pipe[i], SOL_SOCKET, SO_RCVBUF,
> +		    &bsize, sizeof(bsize)) == -1) {
> +			if (errno != ENOBUFS)
> +				fatal("setsockopt(SO_RCVBUF, %d)",
> +				    bsize);
> +			log_warn("setsockopt(SO_RCVBUF, %d)", bsize);
>  		}
>  	}
>  	for (i = 0; i < 2; i++) {
> -		for (bsize = MAX_SOCK_BUF; bsize >= 16 * 1024; bsize /= 2) {
> -			if (setsockopt(pipe[i], SOL_SOCKET, SO_SNDBUF,
> -			    &bsize, sizeof(bsize)) == -1) {
> -				if (errno != ENOBUFS)
> -					fatal("setsockopt(SO_SNDBUF, %d)",
> -					    bsize);
> -				log_warn("setsockopt(SO_SNDBUF, %d)", bsize);
> -				continue;
> -			}
> -			break;
> +		bsize = MAX_SOCK_BUF;
> +		if (setsockopt(pipe[i], SOL_SOCKET, SO_SNDBUF,
> +		    &bsize, sizeof(bsize)) == -1) {
> +			if (errno != ENOBUFS)
> +				fatal("setsockopt(SO_SNDBUF, %d)",
> +				    bsize);
> +			log_warn("setsockopt(SO_SNDBUF, %d)", bsize);
>  		}
>  	}
>  }
>