[PATCH] s4 standard process model: Limit number of processes forked.
Gary Lockyer
gary at catalyst.net.nz
Mon Nov 26 03:48:56 UTC 2018
Honour the 'max smbd processes' smbd.conf parameter. When set to a non
zero value the number of processes forked to handle new ldap and
netlogon connections, will be capped at the value specified. A value of
zero, the default indicates no limits on the number of processes
started. Once the process limit is reached, new connections are closed
immediately.
Review and comments appreciated.
CI results: https://gitlab.com/samba-team/devel/samba/pipelines/37846294
Ngā mihi
Gary
-------------- next part --------------
From 4359854dde9b883a735fcaba19baa92bb4a363dd Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Tue, 18 Sep 2018 11:21:40 +1200
Subject: [PATCH 1/3] s4 smbd standard tests: limit forked processes
Tests to confirm the standard process model honours the smbd.conf
variable "max smbd processes", when forking a new process on accept.
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
python/samba/tests/process_limits.py | 71 ++++++++++++++++++++++++++++
selftest/knownfail.d/process_limit | 1 +
selftest/target/README | 7 +++
selftest/target/Samba.pm | 1 +
selftest/target/Samba4.pm | 40 ++++++++++++++++
source4/selftest/tests.py | 9 ++++
6 files changed, 129 insertions(+)
create mode 100644 python/samba/tests/process_limits.py
create mode 100644 selftest/knownfail.d/process_limit
diff --git a/python/samba/tests/process_limits.py b/python/samba/tests/process_limits.py
new file mode 100644
index 00000000000..1b06568c99e
--- /dev/null
+++ b/python/samba/tests/process_limits.py
@@ -0,0 +1,71 @@
+# Tests for limiting processes forked on accept by the standard process model
+#
+# Copyright (C) Andrew Bartlett <abartlet at samba.org> 2018
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+from __future__ import print_function
+"""Tests limits on processes forked by fork on accept in the standard process
+ model.
+ NOTE: This test runs in an environment with an artificially low setting for
+ smbd max processes
+"""
+
+
+import os
+from samba.tests import TestCase
+from samba.samdb import SamDB
+from ldb import LdbError, ERR_OPERATIONS_ERROR
+
+
+class StandardModelProcessLimitTests(TestCase):
+
+ def setUp(self):
+ super(StandardModelProcessLimitTests, self).setUp()
+
+ def tearDown(self):
+ super(StandardModelProcessLimitTests, self).tearDown()
+
+ def simple_bind(self):
+ creds = self.insta_creds(template=self.get_credentials())
+ creds.set_bind_dn("%s\\%s" % (creds.get_domain(),
+ creds.get_username()))
+
+ return SamDB(url="ldaps://%s" % os.environ["SERVER"],
+ lp=self.get_loadparm(),
+ credentials=creds)
+
+ def test_process_limits(self):
+ creds = self.insta_creds(template=self.get_credentials())
+ creds.set_bind_dn("%s\\%s" % (creds.get_domain(),
+ creds.get_username()))
+
+ connections = []
+ try:
+ # Open a series of LDAP connections, the maximum number of
+ # active connections should be 2, so the third should fail.
+ # But as it is possible that there may be other processes holding
+ # connections, need to allow for earlier connection failures.
+ for _ in range(3):
+ connections.append(self.simple_bind())
+ self.fail(
+ "Processes not limited, able to make more than 2 connections")
+ except LdbError as e:
+ (errno, estr) = e.args
+ if errno != ERR_OPERATIONS_ERROR:
+ raise
+ if not estr.endswith("NT_STATUS_CONNECTION_DISCONNECTED"):
+ raise
+ pass
diff --git a/selftest/knownfail.d/process_limit b/selftest/knownfail.d/process_limit
new file mode 100644
index 00000000000..db34dae23ca
--- /dev/null
+++ b/selftest/knownfail.d/process_limit
@@ -0,0 +1 @@
+^samba.tests.process_limits.samba.tests.process_limits.StandardModelProcessLimitTests.test_process_limits
diff --git a/selftest/target/README b/selftest/target/README
index 3fd283ed266..5afed596e36 100644
--- a/selftest/target/README
+++ b/selftest/target/README
@@ -107,3 +107,10 @@ preforkrestartdc testenv
Used to test killing and restarting processes under the pre-fork model. Due to
the destructive nature of the tests, it's not recommended to use this testenv
for anything else.
+
+proclimitdc testenv
+-------------------
+Used to test process limits on the standard model. It sets the number of
+allowed processes artificially low, to test that new connections are refused
+correctly. Due to the limited number of connections accepted, it's not
+recommended to use this testenv for anything else.
diff --git a/selftest/target/Samba.pm b/selftest/target/Samba.pm
index f5c490fb05b..4820f987fd7 100644
--- a/selftest/target/Samba.pm
+++ b/selftest/target/Samba.pm
@@ -425,6 +425,7 @@ sub get_interface($)
$interfaces{"offlinebackupdc"} = 44;
$interfaces{"customdc"} = 45;
$interfaces{"prockilldc"} = 46;
+ $interfaces{"proclimitdc"} = 47;
# update lib/socket_wrapper/socket_wrapper.c
# #define MAX_WRAPPED_INTERFACES 64
diff --git a/selftest/target/Samba4.pm b/selftest/target/Samba4.pm
index c2e9fdb8276..0f921bda89c 100755
--- a/selftest/target/Samba4.pm
+++ b/selftest/target/Samba4.pm
@@ -2207,6 +2207,8 @@ sub check_env($$)
renamedc => ["backupfromdc"],
offlinebackupdc => ["backupfromdc"],
labdc => ["backupfromdc"],
+ preforkrestartdc => [],
+ proclimitdc => [],
none => [],
);
@@ -2667,6 +2669,44 @@ sub setup_preforkrestartdc
return $env;
}
+#
+# ad_dc test environment used solely to test standard process model connection
+# process limits. As the limit is set artificially low it should not be used
+# for other tests.
+sub setup_proclimitdc
+{
+ my ($self, $path) = @_;
+
+ # If we didn't build with ADS, pretend this env was never available
+ if (not $self->{target3}->have_ads()) {
+ return "UNKNOWN";
+ }
+
+ my $env = $self->provision_ad_dc(
+ $path,
+ "proclimitdc",
+ "PROCLIMITDOM",
+ "proclimit.samba.example.com",
+ "max smbd processes = 2");
+ unless ($env) {
+ return undef;
+ }
+
+ $env->{NSS_WRAPPER_MODULE_SO_PATH} = undef;
+ $env->{NSS_WRAPPER_MODULE_FN_PREFIX} = undef;
+
+ if (not defined($self->check_or_start($env, "standard"))) {
+ return undef;
+ }
+
+ my $upn_array = ["$env->{REALM}.upn"];
+ my $spn_array = ["$env->{REALM}.spn"];
+
+ $self->setup_namespaces($env, $upn_array, $spn_array);
+
+ return $env;
+}
+
# Sets up a DC that's solely used to do a domain backup from. We then use the
# backupfrom-DC to create the restore-DC - this proves that the backup/restore
# process will create a Samba DC that will actually start up.
diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py
index 6a1e1448ef9..e663fcbc340 100755
--- a/source4/selftest/tests.py
+++ b/source4/selftest/tests.py
@@ -1242,3 +1242,12 @@ planoldpythontestsuite("preforkrestartdc:local",
extra_args=['-U"$USERNAME%$PASSWORD"'],
name="samba.tests.prefork_restart",
py3_compatible=True)
+planoldpythontestsuite("proclimitdc:local",
+ "samba.tests.process_limits",
+ extra_path=[
+ os.path.join(srcdir(), 'python/samba/tests')],
+ extra_args=['-U"$USERNAME%$PASSWORD"'],
+ environ={'CLIENT_IP': '127.0.0.11',
+ 'SOCKET_WRAPPER_DEFAULT_IFACE': 11},
+ name="samba.tests.process_limits",
+ py3_compatible=True)
--
2.17.1
From ee0454fa7b83be458561bfc8070d36656b61f892 Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Fri, 7 Sep 2018 07:04:48 +1200
Subject: [PATCH 2/3] s4 smdb standard: Limit processes forked on accept.
Limit the number of processes started by the standard model on accept.
For those services that support fork on accept, the standard model forks
a new process for each new connection. This patch limits the number of
processes to the value specified in 'max smbd processes', a value of
zero indicates that there is no limit on the number of processes that
can be forked.
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
.../smbdotconf/tuning/maxsmbdprocesses.xml | 7 ++++
selftest/knownfail.d/process_limit | 1 -
source4/smbd/process_standard.c | 33 +++++++++++++++++--
3 files changed, 38 insertions(+), 3 deletions(-)
delete mode 100644 selftest/knownfail.d/process_limit
diff --git a/docs-xml/smbdotconf/tuning/maxsmbdprocesses.xml b/docs-xml/smbdotconf/tuning/maxsmbdprocesses.xml
index a194a26a13c..700e1ad8db5 100644
--- a/docs-xml/smbdotconf/tuning/maxsmbdprocesses.xml
+++ b/docs-xml/smbdotconf/tuning/maxsmbdprocesses.xml
@@ -10,6 +10,13 @@
conditions, each user will have an <citerefentry><refentrytitle>smbd</refentrytitle>
<manvolnum>8</manvolnum></citerefentry> associated with him or her to handle connections to all
shares from a given host.</para>
+
+ <para>For a Samba ADDC running the standard process model this option
+ limits the number of processes forked to handle requests.
+ Currently new processes are only forked for ldap and netlogon
+ requests.
+ </para>
+
</description>
<value type="default">0</value>
diff --git a/selftest/knownfail.d/process_limit b/selftest/knownfail.d/process_limit
deleted file mode 100644
index db34dae23ca..00000000000
--- a/selftest/knownfail.d/process_limit
+++ /dev/null
@@ -1 +0,0 @@
-^samba.tests.process_limits.samba.tests.process_limits.StandardModelProcessLimitTests.test_process_limits
diff --git a/source4/smbd/process_standard.c b/source4/smbd/process_standard.c
index d25538716ba..7e2f672e6de 100644
--- a/source4/smbd/process_standard.c
+++ b/source4/smbd/process_standard.c
@@ -32,6 +32,9 @@
#include "lib/util/debug.h"
#include "source3/lib/messages_dgm.h"
+static unsigned connections_active = 0;
+static unsigned smbd_max_processes = 0;
+
struct standard_child_state {
const char *name;
pid_t pid;
@@ -143,8 +146,7 @@ static void standard_child_pipe_handler(struct tevent_context *ev,
if (errno == 0) {
errno = ECHILD;
}
- TALLOC_FREE(state);
- return;
+ goto done;
}
if (WIFEXITED(status)) {
status = WEXITSTATUS(status);
@@ -157,7 +159,17 @@ static void standard_child_pipe_handler(struct tevent_context *ev,
DBG_ERR("Child %d (%s) terminated with signal %d\n",
(int)state->pid, state->name, status);
}
+done:
TALLOC_FREE(state);
+ if (smbd_max_processes > 0) {
+ if (connections_active < 1) {
+ DBG_ERR("Number of active connections "
+ "less than 1 (%d)\n",
+ connections_active);
+ connections_active = 1;
+ }
+ connections_active--;
+ }
return;
}
@@ -282,6 +294,21 @@ static void standard_accept_connection(
return;
}
+ if (smbd_max_processes > 0) {
+ if (connections_active >= smbd_max_processes) {
+ DBG_ERR("(%d) connections already active, "
+ "maximum is (%d). Dropping request\n",
+ connections_active,
+ smbd_max_processes);
+ /*
+ * Drop the connection as we're overloaded at the moment
+ */
+ talloc_free(sock2);
+ return;
+ }
+ connections_active++;
+ }
+
state = setup_standard_child_pipe(ev, NULL);
if (state == NULL) {
return;
@@ -486,6 +513,8 @@ static void standard_new_task(struct tevent_context *ev,
service_details->inhibit_fork_on_accept;
proc_ctx->forked_on_accept = false;
+ smbd_max_processes = lpcfg_max_smbd_processes(lp_ctx);
+
/* setup this new task. Cluster ID is PID based for this process model */
task = new_task(ev, lp_ctx, cluster_id(pid, 0), private_data, proc_ctx);
/*
--
2.17.1
From 0ce9cd589012ca4fc085b44d7e6f67356ef3b8ea Mon Sep 17 00:00:00 2001
From: Gary Lockyer <gary at catalyst.net.nz>
Date: Wed, 19 Sep 2018 15:52:15 +1200
Subject: [PATCH 3/3] WHATSNEW: standard process limits
Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
---
WHATSNEW.txt | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/WHATSNEW.txt b/WHATSNEW.txt
index fc43edc8e86..2a6f56bf132 100644
--- a/WHATSNEW.txt
+++ b/WHATSNEW.txt
@@ -75,6 +75,15 @@ delay between restart attempts up until it reaches "prefork maximum backoff".
Using the default sequence the restart delays (in seconds) are:
0, 10, 20, ..., 120, 120, ...
+standard process model
+----------------------
+
+When using the standard process model samba forks a new process to handle ldap
+and netlogon connections. Samba now honours the 'max smbd processes' smbd.conf
+parameter. The default value of 0, indicates there is no limit. The limit
+is applied individually to netlogon and ldap. When the process limit is
+exceeded Samba drops new connections immediately.
+
REMOVED FEATURES
================
--
2.17.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20181126/b0463c0a/signature.sig>
More information about the samba-technical
mailing list