[PATCH] winbindd: control number of winbindd's client connections

Uri Simchoni urisimchoni at gmail.com
Mon Jul 13 19:34:12 UTC 2015


Hi,

Attached pls find version 6 of this patch set, based on Volker's version:
- Added bug report - https://bugzilla.samba.org/show_bug.cgi?id=11397
- Squashed Volker's structural changes
- Added three patches (7-10) that utilize DLIST_PROMOTE to keep the
client list sorted and make list scanning O(1)
- The first patch which self-modifies the FD limit is there for the
time being - awaiting comments. Indeed, the reason I did it that way
was that smbd is already doing it.

Thanks!
Uri


On Mon, Jul 13, 2015 at 11:04 AM, Volker Lendecke
<Volker.Lendecke at sernet.de> wrote:
> Hi!
>
> On Mon, Jul 06, 2015 at 09:40:32PM +0300, Uri Simchoni wrote:
>> Attached pls find V4 of this patch, handling the case where winbindd
>> runs out of file descriptors during heavy load. This patch takes a
>> different approach suggested by Volker:
>> 1. The assumption is that we can estimate a limit on the number of
>> winbindd clients (estimate number of processes using winbindd, at most
>> 1 client per process).
>> 2. This limit should be configured into "winbind max clients"
>> 3. Winbindd shall modify its fd limit accordingly
>
> I'm not 100% sure I like winbind to modify that limit
> itself. Isn't that more a task of the surrounding init
> scripts? I know we do it in smbd too, so this is more a
> request for other opinions.
>
>> 4. Add code that closes the winbindd side of the socket immediately if
>> the client closes the socket (up until now, while a request was queued
>> or being processed, the client socket was not sampled) - This ensures
>> one socket per client
>
> These patches look really good. I have some very minor
> cosmetic comments.
>
>> 5. Take from previous patch the changes in client side - avoid retries
>> 6. Take from previous patch the change to enforce "winbind request
>> timeout" at all times, not just when the number of client connections
>> exceeds the limit.
>
> Here I'm not sure how much load this will bring. Walking
> 10.000 winbind connections every 5 seconds might be
> visible. I haven't really looked -- is it possible to sort
> the client list according to what we're looking for using
> DLIST_PROMOTE?
>
>> While the approach of the previous patch set a hard limit on number of
>> file descriptors, at the cost of suspending new connection acceptance,
>> this patch set never stops accepting connections, at the cost of
>> staying with the soft limit (but has provisions to ensure that a
>> carefully planned system will not reach the limit).
>>
>> The patch set:
>> V4 1/7 - Set winbindd process file descriptor limit according to
>> "winbind max clients" and "winbind max domain connections"
>> V4 2/7 - Monitor client socket when request is queued / processed
>> V4 3/7+4/7 - Refine 2/7 by emitting differnet debug messages according
>> to type of client activity while socket is being monitored
>> V4 5/7 - Same as V3 5/6 (changes to client side)
>> V4 6/7 - Same as V3 6/6 (request timeout)
>> V4 7/7 - Change to documentation, clarifying that "winbind max
>> clients" is not a hard limit.
>
> Attached find a patchset with a few Reviewed-by's. Two
> patches in between are "my comments", feel free to squash.
>
> Thanks a lot for looking at this!
>
> Volker
>
> --
> SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
> phone: +49-551-370000-0, fax: +49-551-370000-9
> AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
> http://www.sernet.de, mailto:kontakt at sernet.de
-------------- next part --------------
From 87b31b0e13a0b9c33124087be07c7122acff05fd Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Mon, 22 Jun 2015 06:38:04 +0300
Subject: [PATCH v6 01/10] winbindd: set file descriptor limit according to
 configuration

Set the winbindd process file descriptor limit according to
the values that affect it in the configuration:
- Maximum number of clients
- Number of outgoing connections per domain

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397

Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>

VL -- I'm not sure here. See mail. Comments?
---
 source3/include/local.h     |  4 ++++
 source3/winbindd/winbindd.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/source3/include/local.h b/source3/include/local.h
index 5963eb0..7f97d4e 100644
--- a/source3/include/local.h
+++ b/source3/include/local.h
@@ -204,4 +204,8 @@
 /* Maximum size of RPC data we will accept for one call. */
 #define MAX_RPC_DATA_SIZE (15*1024*1024)
 
+/* A guestimate of how many domains winbindd will be contacting */
+#ifndef WINBIND_MAX_DOMAINS_HINT
+#define WINBIND_MAX_DOMAINS_HINT 10
+#endif
 #endif
diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index 4ffe79c..defc9cc 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -48,6 +48,7 @@
 
 static bool client_is_idle(struct winbindd_cli_state *state);
 static void remove_client(struct winbindd_cli_state *state);
+static void winbindd_setup_max_fds(void);
 
 static bool opt_nocache = False;
 static bool interactive = False;
@@ -145,6 +146,7 @@ static bool reload_services_file(const char *lfile)
 
 	reopen_logs();
 	load_interfaces();
+	winbindd_setup_max_fds();
 
 	return(ret);
 }
@@ -1130,6 +1132,35 @@ char *get_winbind_priv_pipe_dir(void)
 	return state_path(WINBINDD_PRIV_SOCKET_SUBDIR);
 }
 
+static void winbindd_setup_max_fds(void)
+{
+	int num_fds = MAX_OPEN_FUDGEFACTOR;
+	int actual_fds;
+
+	num_fds += lp_winbind_max_clients();
+	/* Add some more to account for 2 sockets open
+	   when the client transitions from unprivileged
+	   to privileged socket
+	*/
+	num_fds += lp_winbind_max_clients() / 10;
+
+	/* Add one socket per child process
+	   (yeah there are child processes other than the
+	   domain children but only domain children can vary
+	   with configuration
+	*/
+	num_fds += lp_winbind_max_domain_connections() *
+		   (lp_allow_trusted_domains() ? WINBIND_MAX_DOMAINS_HINT : 1);
+
+	actual_fds = set_maxfiles(num_fds);
+
+	if (actual_fds < num_fds) {
+		DEBUG(1, ("winbindd_setup_max_fds: Information only: "
+			  "requested %d open files, %d are available.\n",
+			  num_fds, actual_fds));
+	}
+}
+
 static bool winbindd_setup_listeners(void)
 {
 	struct winbindd_listen_state *pub_state = NULL;
-- 
1.9.1


From 824ac59aa66228af1f7264bc69f127adc6d7adfa Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Thu, 25 Jun 2015 08:59:20 +0300
Subject: [PATCH v6 02/10] winbindd: cleanup client connection if the client
 closes the connection

This patch allows for early cleanup of client connections if the client
has given up.
Before this patch, any received request would be processed, and then only
upon transmitting the result to the client would winbindd find out the
client is no longer with us, possibly leading to a situation where the
same client tries over and over and increases the number of client
connections.

This patch monitors the client socket for readability while the request
is being processed, and closes the client connection if the socket
becomes readable. The client is not supposed to be writing anything to
the socket while it is waiting, so readability means either that the client
has closed the connection, or that it has broken the protocol.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397

Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 source3/winbindd/winbindd.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index defc9cc..a03b5b4 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -42,6 +42,7 @@
 #include "source4/lib/messaging/irpc.h"
 #include "source4/lib/messaging/messaging.h"
 #include "lib/param/param.h"
+#include "lib/async_req/async_sock.h"
 
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_WINBIND
@@ -809,11 +810,15 @@ static void request_finished(struct winbindd_cli_state *state);
 
 static void winbind_client_request_read(struct tevent_req *req);
 static void winbind_client_response_written(struct tevent_req *req);
+static void winbind_client_activity(struct tevent_req *req);
 
 static void request_finished(struct winbindd_cli_state *state)
 {
 	struct tevent_req *req;
 
+	/* free client socket monitoring request */
+	TALLOC_FREE(state->io_req);
+
 	TALLOC_FREE(state->request);
 
 	req = wb_resp_write_send(state, winbind_event_context(),
@@ -966,9 +971,32 @@ static void winbind_client_request_read(struct tevent_req *req)
 		remove_client(state);
 		return;
 	}
+
+	req = wait_for_read_send(state, winbind_event_context(), state->sock);
+	if (req == NULL) {
+		DEBUG(0, ("winbind_client_request_read[%d:%s]:"
+			  " wait_for_read_send failed - removing client\n",
+			  (int)state->pid, state->cmd_name));
+		remove_client(state);
+		return;
+	}
+	tevent_req_set_callback(req, winbind_client_activity, state);
+	state->io_req = req;
+
 	process_request(state);
 }
 
+static void winbind_client_activity(struct tevent_req *req)
+{
+	struct winbindd_cli_state *state =
+	    tevent_req_callback_data(req, struct winbindd_cli_state);
+	int err;
+
+	wait_for_read_recv(req, &err);
+
+	remove_client(state);
+}
+
 /* Remove a client connection from client connection list */
 
 static void remove_client(struct winbindd_cli_state *state)
-- 
1.9.1


From 7c9ce7edf2e41210ecc1ec83d9ffe7ca537913b5 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Thu, 25 Jun 2015 09:46:24 +0300
Subject: [PATCH v6 03/10] async_req: check for errors when monitoring socket
 for readability

Add an option to wait_for_read_send(), so that the request, upon
calling back, report whether the socket actually contains data
or is in EOF/error state. EOF is signalled via the EPIPE error.

This is useful for clients which do not expect data to arrive but
wait for readability to detect a closed socket (i.e. they do not
intend to actually read the socket when it's readable). Actual data
arrival would indicate a bug in this case, so the check can
be used to print an error message.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397

Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>
---
 lib/async_req/async_sock.c  | 45 ++++++++++++++++++++++++++++++++++++++++++---
 lib/async_req/async_sock.h  |  4 ++--
 source3/smbd/process.c      |  4 ++--
 source3/winbindd/winbindd.c |  3 ++-
 4 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/lib/async_req/async_sock.c b/lib/async_req/async_sock.c
index d2cda15..bc3780c 100644
--- a/lib/async_req/async_sock.c
+++ b/lib/async_req/async_sock.c
@@ -534,6 +534,8 @@ ssize_t read_packet_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
 
 struct wait_for_read_state {
 	struct tevent_fd *fde;
+	int fd;
+	bool check_errors;
 };
 
 static void wait_for_read_cleanup(struct tevent_req *req,
@@ -544,8 +546,8 @@ static void wait_for_read_done(struct tevent_context *ev,
 			       void *private_data);
 
 struct tevent_req *wait_for_read_send(TALLOC_CTX *mem_ctx,
-				      struct tevent_context *ev,
-				      int fd)
+				      struct tevent_context *ev, int fd,
+				      bool check_errors)
 {
 	struct tevent_req *req;
 	struct wait_for_read_state *state;
@@ -562,6 +564,9 @@ struct tevent_req *wait_for_read_send(TALLOC_CTX *mem_ctx,
 	if (tevent_req_nomem(state->fde, req)) {
 		return tevent_req_post(req, ev);
 	}
+
+	state->fd = fd;
+	state->check_errors = check_errors;
 	return req;
 }
 
@@ -581,10 +586,44 @@ static void wait_for_read_done(struct tevent_context *ev,
 {
 	struct tevent_req *req = talloc_get_type_abort(
 		private_data, struct tevent_req);
+	struct wait_for_read_state *state =
+	    tevent_req_data(req, struct wait_for_read_state);
+	ssize_t nread;
+	char c;
 
-	if (flags & TEVENT_FD_READ) {
+	if ((flags & TEVENT_FD_READ) == 0) {
+		return;
+	}
+
+	if (!state->check_errors) {
 		tevent_req_done(req);
+		return;
 	}
+
+	nread = recv(state->fd, &c, 1, MSG_PEEK);
+
+	if (nread == 0) {
+		tevent_req_error(req, EPIPE);
+		return;
+	}
+
+	if ((nread == -1) && (errno == EINTR)) {
+		/* come back later */
+		return;
+	}
+
+	if ((nread == -1) && (errno == ENOTSOCK)) {
+		/* Ignore this specific error on pipes */
+		tevent_req_done(req);
+		return;
+	}
+
+	if (nread == -1) {
+		tevent_req_error(req, errno);
+		return;
+	}
+
+	tevent_req_done(req);
 }
 
 bool wait_for_read_recv(struct tevent_req *req, int *perr)
diff --git a/lib/async_req/async_sock.h b/lib/async_req/async_sock.h
index 1b76fab..abbf822 100644
--- a/lib/async_req/async_sock.h
+++ b/lib/async_req/async_sock.h
@@ -53,8 +53,8 @@ ssize_t read_packet_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx,
 			 uint8_t **pbuf, int *perrno);
 
 struct tevent_req *wait_for_read_send(TALLOC_CTX *mem_ctx,
-				      struct tevent_context *ev,
-				      int fd);
+				      struct tevent_context *ev, int fd,
+				      bool check_errors);
 bool wait_for_read_recv(struct tevent_req *req, int *perr);
 
 #endif
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index c83f3bc..4d047a9 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -2846,7 +2846,7 @@ static struct tevent_req *smbd_echo_read_send(
 	state->ev = ev;
 	state->xconn = xconn;
 
-	subreq = wait_for_read_send(state, ev, xconn->transport.sock);
+	subreq = wait_for_read_send(state, ev, xconn->transport.sock, false);
 	if (tevent_req_nomem(subreq, req)) {
 		return tevent_req_post(req, ev);
 	}
@@ -2921,7 +2921,7 @@ static void smbd_echo_read_waited(struct tevent_req *subreq)
 		}
 
 		subreq = wait_for_read_send(state, state->ev,
-					    xconn->transport.sock);
+					    xconn->transport.sock, false);
 		if (tevent_req_nomem(subreq, req)) {
 			return;
 		}
diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index a03b5b4..07faadf 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -972,7 +972,8 @@ static void winbind_client_request_read(struct tevent_req *req)
 		return;
 	}
 
-	req = wait_for_read_send(state, winbind_event_context(), state->sock);
+	req = wait_for_read_send(state, winbind_event_context(), state->sock,
+				 false);
 	if (req == NULL) {
 		DEBUG(0, ("winbind_client_request_read[%d:%s]:"
 			  " wait_for_read_send failed - removing client\n",
-- 
1.9.1


From 8e52b26dfd61c79e775912a7283665ec1c1527a8 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Thu, 25 Jun 2015 10:12:37 +0300
Subject: [PATCH v6 04/10] winbindd: verify that client has closed the
 connection

A recent change was to remove a client if the client socket
has become readable. In this change, a check is added to
determine the source of the readbility (actual readability,
closed connection, or some other error), and a suitable
debug message is printed.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397

Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>
---
 source3/winbindd/winbindd.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index 07faadf..fb47781 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -973,7 +973,7 @@ static void winbind_client_request_read(struct tevent_req *req)
 	}
 
 	req = wait_for_read_send(state, winbind_event_context(), state->sock,
-				 false);
+				 true);
 	if (req == NULL) {
 		DEBUG(0, ("winbind_client_request_read[%d:%s]:"
 			  " wait_for_read_send failed - removing client\n",
@@ -992,8 +992,28 @@ static void winbind_client_activity(struct tevent_req *req)
 	struct winbindd_cli_state *state =
 	    tevent_req_callback_data(req, struct winbindd_cli_state);
 	int err;
+	bool has_data;
 
-	wait_for_read_recv(req, &err);
+	has_data = wait_for_read_recv(req, &err);
+
+	if (has_data) {
+		DEBUG(0, ("winbind_client_activity[%d:%s]:"
+			  "unexpected data from client - removing client\n",
+			  (int)state->pid, state->cmd_name));
+	} else {
+		if (err == EPIPE) {
+			DEBUG(6, ("winbind_client_activity[%d:%s]: "
+				  "client has closed connection - removing "
+				  "client\n",
+				  (int)state->pid, state->cmd_name));
+		} else {
+			DEBUG(2, ("winbind_client_activity[%d:%s]: "
+				  "client socket error (%s) - removing "
+				  "client\n",
+				  (int)state->pid, state->cmd_name,
+				  strerror(err)));
+		}
+	}
 
 	remove_client(state);
 }
-- 
1.9.1


From 8daf73bb7f4c95bfd617a786de73fec0b17b6453 Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Wed, 3 Jun 2015 00:36:27 +0300
Subject: [PATCH v6 05/10] winbind client: avoid vicious cycle created by
 client retry

This patch cancels the retry policy of the winbind client.

When winbindd fails to respond to a request within 30 seconds,
the winbind client closes the connection and retries up to 10
times.

In some cases, delayed response is a result of multiple
requests from multiple clients piling up on the winbind domain
child process. Retrying just piles more and more requests,
creating a vicious cycle.

Even in the case of a single request taking long to complete,
there's no point in retrying because the retry request would just
wait for the current request to complete. Better to wait patiently.

There's one possible benefit in the retry, namely that winbindd typically
caches the results, and therefore a retry might take a cached result, so
the net effect of the retry may be to increase the timeout to 300 seconds.
But a more straightforward way to have a 300 second timeout is to modify the
timeout. Therefore the timeout is modified from 30 seconds to 300 seconds

(IMHO 300 seconds is too much, but we have "winbind rquest timeout"
with a default of 60 to make sure the request completes or fails
within 60 seconds)

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397

Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>
Reviewed-by: Jeremy Allison <jra at samba.org>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 nsswitch/wb_common.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/nsswitch/wb_common.c b/nsswitch/wb_common.c
index 3194df1..139f010 100644
--- a/nsswitch/wb_common.c
+++ b/nsswitch/wb_common.c
@@ -535,7 +535,7 @@ static int winbind_read_sock(struct winbindd_context *ctx,
 
 		if (ret == 0) {
 			/* Not ready for read yet... */
-			if (total_time >= 30) {
+			if (total_time >= 300) {
 				/* Timeout */
 				winbind_close_sock(ctx);
 				return -1;
@@ -719,20 +719,16 @@ NSS_STATUS winbindd_request_response(struct winbindd_context *ctx,
 				     struct winbindd_response *response)
 {
 	NSS_STATUS status = NSS_STATUS_UNAVAIL;
-	int count = 0;
 	struct winbindd_context *wb_ctx = ctx;
 
 	if (ctx == NULL) {
 		wb_ctx = &wb_global_ctx;
 	}
 
-	while ((status == NSS_STATUS_UNAVAIL) && (count < 10)) {
-		status = winbindd_send_request(wb_ctx, req_type, 0, request);
-		if (status != NSS_STATUS_SUCCESS)
-			return(status);
-		status = winbindd_get_response(wb_ctx, response);
-		count += 1;
-	}
+	status = winbindd_send_request(wb_ctx, req_type, 0, request);
+	if (status != NSS_STATUS_SUCCESS)
+		return (status);
+	status = winbindd_get_response(wb_ctx, response);
 
 	return status;
 }
@@ -743,20 +739,16 @@ NSS_STATUS winbindd_priv_request_response(struct winbindd_context *ctx,
 					  struct winbindd_response *response)
 {
 	NSS_STATUS status = NSS_STATUS_UNAVAIL;
-	int count = 0;
 	struct winbindd_context *wb_ctx = ctx;
 
 	if (ctx == NULL) {
 		wb_ctx = &wb_global_ctx;
 	}
 
-	while ((status == NSS_STATUS_UNAVAIL) && (count < 10)) {
-		status = winbindd_send_request(wb_ctx, req_type, 1, request);
-		if (status != NSS_STATUS_SUCCESS)
-			return(status);
-		status = winbindd_get_response(wb_ctx, response);
-		count += 1;
-	}
+	status = winbindd_send_request(wb_ctx, req_type, 1, request);
+	if (status != NSS_STATUS_SUCCESS)
+		return (status);
+	status = winbindd_get_response(wb_ctx, response);
 
 	return status;
 }
-- 
1.9.1


From a61a88790a0c1006e2418aa5a8ace76cb58456be Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Mon, 6 Jul 2015 21:29:17 +0300
Subject: [PATCH v6 06/10] winbindd: periodically remove timed out clients

Periodically scan winbind client list and close connections
in which either the client is idle, or the request is taking
too long to complete.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397

Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>
Reviewed-by: Jeremy Allison <jra at samba.org>
---
 source3/winbindd/winbindd.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index fb47781..66ea68b 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -47,6 +47,8 @@
 #undef DBGC_CLASS
 #define DBGC_CLASS DBGC_WINBIND
 
+#define SCRUB_CLIENTS_INTERVAL 5
+
 static bool client_is_idle(struct winbindd_cli_state *state);
 static void remove_client(struct winbindd_cli_state *state);
 static void winbindd_setup_max_fds(void);
@@ -1143,6 +1145,20 @@ static void remove_timed_out_clients(void)
 	}
 }
 
+static void winbindd_scrub_clients_handler(struct tevent_context *ev,
+					   struct tevent_timer *te,
+					   struct timeval current_time,
+					   void *private_data)
+{
+	remove_timed_out_clients();
+	if (tevent_add_timer(ev, ev,
+			     timeval_current_ofs(SCRUB_CLIENTS_INTERVAL, 0),
+			     winbindd_scrub_clients_handler, NULL) == NULL) {
+		DEBUG(0, ("winbindd: failed to reschedule client scrubber\n"));
+		exit(1);
+	}
+}
+
 struct winbindd_listen_state {
 	bool privileged;
 	int fd;
@@ -1276,6 +1292,8 @@ static bool winbindd_setup_listeners(void)
 	}
 	tevent_fd_set_auto_close(fde);
 
+	winbindd_scrub_clients_handler(winbind_event_context(), NULL,
+				       timeval_current(), NULL);
 	return true;
 failed:
 	TALLOC_FREE(pub_state);
-- 
1.9.1


From 41d9390e73deac1469dd15f96df5ec00f1b7734a Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Mon, 6 Jul 2015 12:13:15 +0300
Subject: [PATCH v6 07/10] doc: clarify "winbind max clients"

Add clarification about the nature of "winbind max clients" parameter.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397

Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>
Reviewed-by: Volker Lendecke <vl at samba.org>
---
 docs-xml/smbdotconf/winbind/winbindmaxclients.xml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/docs-xml/smbdotconf/winbind/winbindmaxclients.xml b/docs-xml/smbdotconf/winbind/winbindmaxclients.xml
index d37ee25..c11d0b6 100644
--- a/docs-xml/smbdotconf/winbind/winbindmaxclients.xml
+++ b/docs-xml/smbdotconf/winbind/winbindmaxclients.xml
@@ -6,6 +6,12 @@
 	<para>This parameter specifies the maximum number of clients
 	the <citerefentry><refentrytitle>winbindd</refentrytitle>
 	<manvolnum>8</manvolnum></citerefentry> daemon can connect with.
+	The parameter is not a hard limit.
+	The <citerefentry><refentrytitle>winbindd</refentrytitle>
+	<manvolnum>8</manvolnum></citerefentry> daemon configures
+	itself to be able to accept at least that many connections,
+	and if the limit is reached, an attempt is made to disconnect
+	idle clients.
 	</para>
 </description>
 
-- 
1.9.1


From cc2e9843f970cc7d53d1ea028283a82c25c371bb Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Mon, 13 Jul 2015 21:08:16 +0300
Subject: [PATCH v6 08/10] winbindd: add service routines to support a sorted
 client list

Add some routines that support keeping the client list sorted
(by last access time) and traversing the list from oldest to
newest

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397

Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>
---
 source3/winbindd/winbindd_proto.h |  4 ++++
 source3/winbindd/winbindd_util.c  | 22 ++++++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/source3/winbindd/winbindd_proto.h b/source3/winbindd/winbindd_proto.h
index 5ad69e2..9920a3f 100644
--- a/source3/winbindd/winbindd_proto.h
+++ b/source3/winbindd/winbindd_proto.h
@@ -430,8 +430,12 @@ char *fill_domain_username_talloc(TALLOC_CTX *ctx,
 				  const char *user,
 				  bool can_assume);
 struct winbindd_cli_state *winbindd_client_list(void);
+struct winbindd_cli_state *winbindd_client_list_tail(void);
+struct winbindd_cli_state *
+winbindd_client_list_prev(struct winbindd_cli_state *cli);
 void winbindd_add_client(struct winbindd_cli_state *cli);
 void winbindd_remove_client(struct winbindd_cli_state *cli);
+void winbindd_promote_client(struct winbindd_cli_state *cli);
 int winbindd_num_clients(void);
 NTSTATUS lookup_usergroups_cached(struct winbindd_domain *domain,
 				  TALLOC_CTX *mem_ctx,
diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index d73327c..0108504 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -1206,6 +1206,21 @@ struct winbindd_cli_state *winbindd_client_list(void)
 	return _client_list;
 }
 
+/* Return list-tail of all connected clients */
+
+struct winbindd_cli_state *winbindd_client_list_tail(void)
+{
+	return DLIST_TAIL(_client_list);
+}
+
+/* Return previous (read:newer) client in list */
+
+struct winbindd_cli_state *
+winbindd_client_list_prev(struct winbindd_cli_state *cli)
+{
+	return DLIST_PREV(cli);
+}
+
 /* Add a connection to the list */
 
 void winbindd_add_client(struct winbindd_cli_state *cli)
@@ -1222,6 +1237,13 @@ void winbindd_remove_client(struct winbindd_cli_state *cli)
 	_num_clients--;
 }
 
+/* Move a client to head or list */
+
+void winbindd_promote_client(struct winbindd_cli_state *cli)
+{
+	DLIST_PROMOTE(_client_list, cli);
+}
+
 /* Return number of open clients */
 
 int winbindd_num_clients(void)
-- 
1.9.1


From 0665b9c7c08544cfd4d918032b8ec24a9b2874fa Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Mon, 13 Jul 2015 21:33:41 +0300
Subject: [PATCH v6 09/10] winbindd: keep client list sorted by access time

Keep client list sorted by last access time, newest
to oldest.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397

Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>
---
 source3/winbindd/winbindd.c      | 5 ++---
 source3/winbindd/winbindd_util.c | 2 ++
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index 66ea68b..620fd3f 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -701,7 +701,8 @@ static void process_request(struct winbindd_cli_state *state)
 
 	state->cmd_name = "unknown request";
 	state->recv_fn = NULL;
-	state->last_access = time(NULL);
+	/* client is newest */
+	winbindd_promote_client(state);
 
 	/* Process command */
 
@@ -930,8 +931,6 @@ static void new_connection(int listen_sock, bool privileged)
 		return;
 	}
 
-	state->last_access = time(NULL);	
-
 	state->privileged = privileged;
 
 	req = wb_req_read_send(state, winbind_event_context(), state->sock,
diff --git a/source3/winbindd/winbindd_util.c b/source3/winbindd/winbindd_util.c
index 0108504..233b5c9 100644
--- a/source3/winbindd/winbindd_util.c
+++ b/source3/winbindd/winbindd_util.c
@@ -1225,6 +1225,7 @@ winbindd_client_list_prev(struct winbindd_cli_state *cli)
 
 void winbindd_add_client(struct winbindd_cli_state *cli)
 {
+	cli->last_access = time(NULL);
 	DLIST_ADD(_client_list, cli);
 	_num_clients++;
 }
@@ -1241,6 +1242,7 @@ void winbindd_remove_client(struct winbindd_cli_state *cli)
 
 void winbindd_promote_client(struct winbindd_cli_state *cli)
 {
+	cli->last_access = time(NULL);
 	DLIST_PROMOTE(_client_list, cli);
 }
 
-- 
1.9.1


From c86308226661692c83faa2ac6c598270865342cd Mon Sep 17 00:00:00 2001
From: Uri Simchoni <urisimchoni at gmail.com>
Date: Mon, 13 Jul 2015 21:42:57 +0300
Subject: [PATCH v6 10/10] winbindd: shorten client list scan

Counting on the client list being sorted by last access time,
the list scan for removing timed-out clients is shortened - once
the list is scanned oldest to newest, and once a non-timed-out
client is found, the scan can stop.

Also, finding the oldest idle client for removing an idle client
is simplified - oldest idle client is last idle client.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=11397

Signed-off-by: Uri Simchoni <urisimchoni at gmail.com>
---
 source3/winbindd/winbindd.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/source3/winbindd/winbindd.c b/source3/winbindd/winbindd.c
index 620fd3f..c878ce2 100644
--- a/source3/winbindd/winbindd.c
+++ b/source3/winbindd/winbindd.c
@@ -1086,16 +1086,13 @@ static bool client_is_idle(struct winbindd_cli_state *state) {
 static bool remove_idle_client(void)
 {
 	struct winbindd_cli_state *state, *remove_state = NULL;
-	time_t last_access = 0;
 	int nidle = 0;
 
 	for (state = winbindd_client_list(); state; state = state->next) {
 		if (client_is_idle(state)) {
 			nidle++;
-			if (!last_access || state->last_access < last_access) {
-				last_access = state->last_access;
-				remove_state = state;
-			}
+			/* list is sorted by access time */
+			remove_state = state;
 		}
 	}
 
@@ -1117,14 +1114,14 @@ static bool remove_idle_client(void)
 
 static void remove_timed_out_clients(void)
 {
-	struct winbindd_cli_state *state, *next = NULL;
+	struct winbindd_cli_state *state, *prev = NULL;
 	time_t curr_time = time(NULL);
 	int timeout_val = lp_winbind_request_timeout();
 
-	for (state = winbindd_client_list(); state; state = next) {
+	for (state = winbindd_client_list_tail(); state; state = prev) {
 		time_t expiry_time;
 
-		next = state->next;
+		prev = winbindd_client_list_prev(state);
 		expiry_time = state->last_access + timeout_val;
 
 		if (curr_time > expiry_time) {
@@ -1140,6 +1137,10 @@ static void remove_timed_out_clients(void)
 					(unsigned int)state->pid));
 			}
 			remove_client(state);
+		} else {
+			/* list is sorted, previous clients in
+			   list are newer */
+			break;
 		}
 	}
 }
-- 
1.9.1



More information about the samba-technical mailing list