Not without further measurement of all applications.

Imagine if this gets commited, and causes kernel memory shortage
and deadlock.

Shall we all then point fingers at you for experimenting live?
I know I will.

Claudio Jeker <[email protected]> wrote:

> I noticed that by default the send and recv socket buffers for
> socketpair(2) is rather low (4k IIRC). The result is a fairly inefficent
> write/read behaviour on the imsg sockets. Increasing SO_SNDBUF and
> SO_RCVBUF seems to help increase the data sent and received per syscall.
> 
> Another option would be to make the default socketbuffer watermarks for
> socketpair(2) a bit less limited. Then all imsg users would benefit at the
> same time.
> -- 
> :wq Claudio
> 
> Index: bgpd.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.c,v
> retrieving revision 1.219
> diff -u -p -r1.219 bgpd.c
> --- bgpd.c    29 May 2019 08:48:00 -0000      1.219
> +++ bgpd.c    4 Jun 2019 09:47:18 -0000
> @@ -46,6 +46,7 @@ int         send_filterset(struct imsgbuf *, st
>  int          reconfigure(char *, struct bgpd_config *);
>  int          dispatch_imsg(struct imsgbuf *, int, struct bgpd_config *);
>  int          control_setup(struct bgpd_config *);
> +static void  getsockpair(int [2]);
>  int          imsg_send_sockets(struct imsgbuf *, struct imsgbuf *);
>  
>  int                   cflags;
> @@ -203,12 +204,8 @@ main(int argc, char *argv[])
>  
>       log_info("startup");
>  
> -     if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK,
> -         PF_UNSPEC, pipe_m2s) == -1)
> -             fatal("socketpair");
> -     if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK,
> -         PF_UNSPEC, pipe_m2r) == -1)
> -             fatal("socketpair");
> +     getsockpair(pipe_m2s);
> +     getsockpair(pipe_m2r);
>  
>       /* fork children */
>       rde_pid = start_child(PROC_RDE, saved_argv0, pipe_m2r[1], debug,
> @@ -1073,18 +1070,54 @@ handle_pollfd(struct pollfd *pfd, struct
>       return (0);
>  }
>  
> +static void
> +getsockpair(int pipe[2])
> +{
> +     int bsize, i;
> +
> +     if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK,
> +         PF_UNSPEC, pipe) == -1)
> +             fatal("socketpair");
> +
> +     /* increase socketpair buffers */
> +     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);
> +                             continue;
> +                     }
> +                     break;
> +             }
> +             if (bsize != MAX_SOCK_BUF)
> +                     log_warn("non optimal SO_RCVBUF size of %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);
> +                             continue;
> +                     }
> +                     break;
> +             }
> +             if (bsize != MAX_SOCK_BUF)
> +                     log_warn("non optimal SO_SNDBUF size of %d", bsize);
> +     }
> +}
> +
>  int
>  imsg_send_sockets(struct imsgbuf *se, struct imsgbuf *rde)
>  {
>       int pipe_s2r[2];
>       int pipe_s2r_ctl[2];
>  
> -     if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK,
> -          PF_UNSPEC, pipe_s2r) == -1)
> -             return (-1);
> -     if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC | SOCK_NONBLOCK,
> -          PF_UNSPEC, pipe_s2r_ctl) == -1)
> -             return (-1);
> +     getsockpair(pipe_s2r);
> +     getsockpair(pipe_s2r_ctl);
>  
>       if (imsg_compose(se, IMSG_SOCKET_CONN, 0, 0, pipe_s2r[0],
>           NULL, 0) == -1)
> Index: bgpd.h
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
> retrieving revision 1.386
> diff -u -p -r1.386 bgpd.h
> --- bgpd.h    17 Jun 2019 13:35:42 -0000      1.386
> +++ bgpd.h    17 Jun 2019 13:56:15 -0000
> @@ -49,7 +49,7 @@
>  #define      MIN_HOLDTIME                    3
>  #define      READ_BUF_SIZE                   65535
>  #define      RT_BUF_SIZE                     16384
> -#define      MAX_RTSOCK_BUF                  (2 * 1024 * 1024)
> +#define      MAX_SOCK_BUF                    (2 * 1024 * 1024)
>  #define      MAX_COMM_MATCH                  3
>  
>  #define      BGPD_OPT_VERBOSE                0x0001
> Index: kroute.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
> retrieving revision 1.236
> diff -u -p -r1.236 kroute.c
> --- kroute.c  6 May 2019 09:49:26 -0000       1.236
> +++ kroute.c  3 Jun 2019 20:16:59 -0000
> @@ -236,7 +236,7 @@ kr_init(void)
>           &default_rcvbuf, &optlen) == -1)
>               log_warn("%s: getsockopt SOL_SOCKET SO_RCVBUF", __func__);
>       else
> -             for (rcvbuf = MAX_RTSOCK_BUF;
> +             for (rcvbuf = MAX_SOCK_BUF;
>                   rcvbuf > default_rcvbuf &&
>                   setsockopt(kr_state.fd, SOL_SOCKET, SO_RCVBUF,
>                   &rcvbuf, sizeof(rcvbuf)) == -1 && errno == ENOBUFS;
> 

Reply via email to