| Message ID | 20240307213913.2511954-1-trond.myklebust@hammerspace.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | nfsd: allow more than 64 backlogged connections | expand |
On Thu, 7 Mar 2024 at 22:57, <trondmy@gmail.com> wrote: > > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > When creating a listener socket to be handed to /proc/fs/nfsd/portlist, > we currently limit the number of backlogged connections to 64. Since > that value was chosen in 2006, the scale at which data centres operate > has changed significantly. Given a modern server with many thousands of > clients, a limit of 64 connections can create bottlenecks, particularly > at at boot time. > By converting to using an argument of -1, we allow the backlog to be set > by the default value in /proc/sys/net/core/somaxconn. Could you please port this to the Linux 6.6 LTS branch too? Ced
On Thu, 2024-03-07 at 16:39 -0500, trondmy@gmail.com wrote: > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > When creating a listener socket to be handed to /proc/fs/nfsd/portlist, > we currently limit the number of backlogged connections to 64. Since > that value was chosen in 2006, the scale at which data centres operate > has changed significantly. Given a modern server with many thousands of > clients, a limit of 64 connections can create bottlenecks, particularly > at at boot time. > By converting to using an argument of -1, we allow the backlog to be set > by the default value in /proc/sys/net/core/somaxconn. > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > --- > utils/nfsd/nfssvc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c > index 46452d972407..c781054dbdae 100644 > --- a/utils/nfsd/nfssvc.c > +++ b/utils/nfsd/nfssvc.c > @@ -205,7 +205,7 @@ nfssvc_setfds(const struct addrinfo *hints, const char *node, const char *port) > rc = errno; > goto error; > } > - if (addr->ai_protocol == IPPROTO_TCP && listen(sockfd, 64)) { > + if (addr->ai_protocol == IPPROTO_TCP && listen(sockfd, -1)) { > xlog(L_ERROR, "unable to create listening socket: " > "errno %d (%m)", errno); > rc = errno; It does look like the kernel casts the value to unsigned int before trying to interpret it, but that doesn't seem to be documented anywhere that I can find. It's certainly not in the manpage There is this in /usr/include/bits/socket.h: /* Maximum queue length specifiable by listen. */ #define SOMAXCONN 4096 ...but I guess that's problematic if you set "somaxconn" sysctl higher. I wonder if SOMAXCONN should be redefined as "(int)UINT_MAX" in the UAPI headers?
On Fri, 2024-03-08 at 11:54 -0500, Jeff Layton wrote: > On Thu, 2024-03-07 at 16:39 -0500, trondmy@gmail.com wrote: > > From: Trond Myklebust <trond.myklebust@hammerspace.com> > > > > When creating a listener socket to be handed to > > /proc/fs/nfsd/portlist, > > we currently limit the number of backlogged connections to 64. > > Since > > that value was chosen in 2006, the scale at which data centres > > operate > > has changed significantly. Given a modern server with many > > thousands of > > clients, a limit of 64 connections can create bottlenecks, > > particularly > > at at boot time. > > By converting to using an argument of -1, we allow the backlog to > > be set > > by the default value in /proc/sys/net/core/somaxconn. > > > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > > --- > > utils/nfsd/nfssvc.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c > > index 46452d972407..c781054dbdae 100644 > > --- a/utils/nfsd/nfssvc.c > > +++ b/utils/nfsd/nfssvc.c > > @@ -205,7 +205,7 @@ nfssvc_setfds(const struct addrinfo *hints, > > const char *node, const char *port) > > rc = errno; > > goto error; > > } > > - if (addr->ai_protocol == IPPROTO_TCP && > > listen(sockfd, 64)) { > > + if (addr->ai_protocol == IPPROTO_TCP && > > listen(sockfd, -1)) { > > xlog(L_ERROR, "unable to create listening > > socket: " > > "errno %d (%m)", errno); > > rc = errno; > > It does look like the kernel casts the value to unsigned int before > trying to interpret it, but that doesn't seem to be documented > anywhere > that I can find. It's certainly not in the manpage > > There is this in /usr/include/bits/socket.h: > > /* Maximum queue length specifiable by listen. */ > #define SOMAXCONN 4096 > > ...but I guess that's problematic if you set "somaxconn" sysctl > higher. > I wonder if SOMAXCONN should be redefined as "(int)UINT_MAX" in the > UAPI > headers? Fair enough. I'll respin with SOMAXCONN. It looks as if that is what POSIX expects us to use as well.
diff --git a/utils/nfsd/nfssvc.c b/utils/nfsd/nfssvc.c index 46452d972407..c781054dbdae 100644 --- a/utils/nfsd/nfssvc.c +++ b/utils/nfsd/nfssvc.c @@ -205,7 +205,7 @@ nfssvc_setfds(const struct addrinfo *hints, const char *node, const char *port) rc = errno; goto error; } - if (addr->ai_protocol == IPPROTO_TCP && listen(sockfd, 64)) { + if (addr->ai_protocol == IPPROTO_TCP && listen(sockfd, -1)) { xlog(L_ERROR, "unable to create listening socket: " "errno %d (%m)", errno); rc = errno;