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

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

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;
+    }
+
     if (virNetSocketAccept(sock, &clientsock) < 0)
         goto cleanup;
 
@@ -419,9 +426,11 @@ virNetTLSContextPtr virNetServerServiceGetTLSContext(virNetServerServicePtr svc)
 
 void virNetServerServiceSetDispatcher(virNetServerServicePtr svc,
                                       virNetServerServiceDispatchFunc func,
+                                      virNetServerServiceDispatchCheckFunc checkFunc,
                                       void *opaque)
 {
     svc->dispatchFunc = func;
+    svc->dispatchCheckFunc = checkFunc;
     svc->dispatchOpaque = opaque;
 }
 
diff --git a/src/rpc/virnetserverservice.h b/src/rpc/virnetserverservice.h
index 1ece503..e9e5389 100644
--- a/src/rpc/virnetserverservice.h
+++ b/src/rpc/virnetserverservice.h
@@ -36,6 +36,9 @@ enum {
 typedef int (*virNetServerServiceDispatchFunc)(virNetServerServicePtr svc,
                                                virNetSocketPtr sock,
                                                void *opaque);
+typedef int (*virNetServerServiceDispatchCheckFunc)(virNetServerServicePtr svc,
+                                                    virNetSocketPtr sock,
+                                                    void *opaque);
 
 virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename,
                                                  const char *service,
@@ -77,6 +80,7 @@ virNetTLSContextPtr virNetServerServiceGetTLSContext(virNetServerServicePtr svc)
 
 void virNetServerServiceSetDispatcher(virNetServerServicePtr svc,
                                       virNetServerServiceDispatchFunc func,
+                                      virNetServerServiceDispatchCheckFunc checkFunc,
                                       void *opaque);
 
 void virNetServerServiceToggle(virNetServerServicePtr svc,
-- 
1.8.1.5

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list