Re: [libvirt] [PATCH 1/2] RPC: Don't accept client if it would overcommit max_clients

Currently, even if max_client limit is hit, we accept() incoming connection request, but close it immediately. This has disadvantage of not using listen() queue. We should accept() only those clients we know we can serve and let all other wait in the (limited) queue. --- src/rpc/virnetserver.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/rpc/virnetserverservice.c | 9 +++++++++ src/rpc/virnetserverservice.h | 4 ++++ 3 files changed, 53 insertions(+) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index cb770c3..7ea1707 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -315,6 +315,34 @@ static int virNetServerDispatchNewClient(virNetServerServicePtr svc, } +static int +virNetServerDispatchCheck(virNetServerServicePtr svc, + virNetSocketPtr socket, + void *opaque) +{ + virNetServerPtr srv = opaque; + int ret = -1; + + VIR_DEBUG("svc=%p socket=%p opaque=%p", svc, socket, opaque); + + if (srv->nclients >= srv->nclients_max) { + VIR_DEBUG("Not accepting client now due to max_clients setting (%zu)", + srv->nclients_max); + + /* We need to temporarily disable the server services in order to + * prevent event loop calling us over and over again. */ + + virNetServerUpdateServices(srv, false); + + goto cleanup; + } + + ret = 0; +cleanup: + return ret; +} + + static void virNetServerFatalSignal(int sig, siginfo_t *siginfo ATTRIBUTE_UNUSED, void *context ATTRIBUTE_UNUSED) @@ -981,6 +1009,7 @@ int virNetServerAddService(virNetServerPtr srv, virNetServerServiceSetDispatcher(svc, virNetServerDispatchNewClient, + virNetServerDispatchCheck, srv); virObjectUnlock(srv); @@ -1110,6 +1139,8 @@ void virNetServerRun(virNetServerPtr srv) virNetServerClientClose(srv->clients[i]); if (virNetServerClientIsClosed(srv->clients[i])) { virNetServerClientPtr client = srv->clients[i]; + bool update_services; + if (srv->nclients > 1) { memmove(srv->clients + i, srv->clients + i + 1, @@ -1120,7 +1151,16 @@ void virNetServerRun(virNetServerPtr srv) srv->nclients = 0; } + /* Enable services if we can accept a new client. + * The new client can be accepted if we are at the limit. */ + update_services = srv->nclients == srv->nclients_max - 1; + virObjectUnlock(srv); + if (update_services) { + /* Now it makes sense to accept() a new client. */ + VIR_DEBUG("Re-enabling services"); + virNetServerUpdateServices(srv, true); + } virObjectUnref(client); virObjectLock(srv); diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index 632f03d..a47c8f8 100644 --- a/src/rpc/virnetserverservice.c +++ b/src/rpc/virnetserverservice.c @@ -46,6 +46,7 @@ struct _virNetServerService { #endif virNetServerServiceDispatchFunc dispatchFunc; + virNetServerServiceDispatchCheckFunc dispatchCheckFunc; void *dispatchOpaque; }; @@ -74,6 +75,12 @@ static void virNetServerServiceAccept(virNetSocketPtr sock, virNetServerServicePtr svc = opaque; virNetSocketPtr clientsock = NULL; + if (svc->dispatchCheckFunc && + svc->dispatchCheckFunc(svc, sock, svc->dispatchOpaque) < 0) { + /* Accept declined */ + goto cleanup; + } +