rpms/bind
7
0
Fork 1
Code Issues Pull Requests Releases Activity

Replace downstream change with upstream proposal

Browse Source 
bind-dyndb-ldap requires sending from custom spawned thread to main
named threads. Change queue type to locked variant, which would not
crash when isc_send_task() is called from dyndb worker thread.

Related: rhbz#2048235
This commit is contained in:
Petr Menšík 2022-03-01 16:43:39 +01:00
parent 36d2b49469
commit ee4347d7db
4 changed files with 221 additions and 300 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

116
bind-9.16-hp-workers-rh2048235-dyndb.patch
View File

@ -1,116 +0,0 @@
From b6320643194a3b435f2a6ae4569ace19db6ac795 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemensik@redhat.com>
Date: Fri, 11 Feb 2022 14:19:10 +0100
Subject: [PATCH] Propagate extra hp threads to dyndb plugins
Plugin may require extra threads for processing its own source data. If
that plugin needs to call isc_task_send() from those threads, it needs
extra hp threads initialized. Allow plugin to read how many such threads
it can use. Allows plugin to fail initialization if not enough workers
were initialized.
---
bin/named/server.c | 9 +++++----
lib/dns/dyndb.c | 4 +++-
lib/dns/include/dns/dyndb.h | 22 +++++++++++++---------
3 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/bin/named/server.c b/bin/named/server.c
index 721464db8e..27e23fd52e 100644
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -5581,10 +5581,11 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
if (dctx == NULL) {
const void *hashinit = isc_hash_get_initializer();
- CHECK(dns_dyndb_createctx(mctx, hashinit, named_g_lctx,
- view, named_g_server->zonemgr,
- named_g_server->task,
- named_g_timermgr, &dctx));
+ CHECK(dns_dyndb_createctx(
+ mctx, hashinit, named_g_lctx, view,
+ named_g_server->zonemgr, named_g_server->task,
+ named_g_timermgr, named_g_hp_extra_workers,
+ &dctx));
}
CHECK(configure_dyndb(dyndb, mctx, dctx));
diff --git a/lib/dns/dyndb.c b/lib/dns/dyndb.c
index 1caf90bab7..7d2784c5fa 100644
--- a/lib/dns/dyndb.c
+++ b/lib/dns/dyndb.c
@@ -408,7 +408,8 @@ dns_dyndb_cleanup(bool exiting) {
isc_result_t
dns_dyndb_createctx(isc_mem_t *mctx, const void *hashinit, isc_log_t *lctx,
dns_view_t *view, dns_zonemgr_t *zmgr, isc_task_t *task,
- isc_timermgr_t *tmgr, dns_dyndbctx_t **dctxp) {
+ isc_timermgr_t *tmgr, unsigned int extra_workers,
+ dns_dyndbctx_t **dctxp) {
dns_dyndbctx_t *dctx;
REQUIRE(dctxp != NULL && *dctxp == NULL);
@@ -429,6 +430,7 @@ dns_dyndb_createctx(isc_mem_t *mctx, const void *hashinit, isc_log_t *lctx,
dctx->hashinit = hashinit;
dctx->lctx = lctx;
dctx->memdebug = &isc_mem_debugging;
+ dctx->extra_workers = extra_workers;
isc_mem_attach(mctx, &dctx->mctx);
dctx->magic = DNS_DYNDBCTX_MAGIC;
diff --git a/lib/dns/include/dns/dyndb.h b/lib/dns/include/dns/dyndb.h
index bda744a8d9..a93947d340 100644
--- a/lib/dns/include/dns/dyndb.h
+++ b/lib/dns/include/dns/dyndb.h
@@ -35,14 +35,15 @@ ISC_LANG_BEGINDECLS
*/
struct dns_dyndbctx {
unsigned int magic;
- const void *hashinit;
- isc_mem_t *mctx;
- isc_log_t *lctx;
- dns_view_t *view;
- dns_zonemgr_t *zmgr;
- isc_task_t *task;
+ const void * hashinit;
+ isc_mem_t * mctx;
+ isc_log_t * lctx;
+ dns_view_t * view;
+ dns_zonemgr_t * zmgr;
+ isc_task_t * task;
isc_timermgr_t *timermgr;
- unsigned int *memdebug;
+ unsigned int * memdebug;
+ unsigned int extra_workers;
};
#define DNS_DYNDBCTX_MAGIC ISC_MAGIC('D', 'd', 'b', 'c')
@@ -58,7 +59,7 @@ struct dns_dyndbctx {
*/
#ifndef DNS_DYNDB_VERSION
#define DNS_DYNDB_VERSION 1
-#define DNS_DYNDB_AGE 0
+#define DNS_DYNDB_AGE 1
#endif /* ifndef DNS_DYNDB_VERSION */
typedef isc_result_t
@@ -135,7 +136,8 @@ dns_dyndb_cleanup(bool exiting);
isc_result_t
dns_dyndb_createctx(isc_mem_t *mctx, const void *hashinit, isc_log_t *lctx,
dns_view_t *view, dns_zonemgr_t *zmgr, isc_task_t *task,
- isc_timermgr_t *tmgr, dns_dyndbctx_t **dctxp);
+ isc_timermgr_t *tmgr, unsigned int extra_workers,
+ dns_dyndbctx_t **dctxp);
/*%
* Create a dyndb initialization context structure, with
* pointers to structures in the server that the dyndb module will
@@ -143,6 +145,8 @@ dns_dyndb_createctx(isc_mem_t *mctx, const void *hashinit, isc_log_t *lctx,
* etc). This structure is expected to last only until all dyndb
* modules have been loaded and initialized; after that it will be
* destroyed with dns_dyndb_destroyctx().
+ * extra workers specifies how many extra worker threads can plugin
+ * use to send tasks to named threads by isc_task_send()
*
* Returns:
*\li #ISC_R_SUCCESS
--
2.34.1

180
bind-9.16-hp-workers-rh2048235.patch
View File

@ -1,180 +0,0 @@
From 140ae14a4d6374530c9ab19a29c246abbf6d60b8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemensik@redhat.com>
Date: Fri, 11 Feb 2022 13:29:03 +0100
Subject: [PATCH] Add extra hp workers parameter
May be needed by dyndb plugins to prepare extra worker threads. If they
use isc_task_send, their threads need to have initialized enough thread
identifiers in isc_hp_init. New parameters allow that.
Allows specification of both relative and absolute number of extra
workers.
Adds new function isc_managers_create2() with extra parameter, call
original function from many tests and tools unchanged.
---
bin/named/include/named/globals.h | 2 ++
bin/named/include/named/main.h | 2 +-
bin/named/main.c | 29 +++++++++++++++++++++++++----
bin/named/named.rst | 9 +++++++++
lib/isc/include/isc/managers.h | 5 +++++
lib/isc/managers.c | 11 ++++++++++-
6 files changed, 52 insertions(+), 6 deletions(-)
diff --git a/bin/named/include/named/globals.h b/bin/named/include/named/globals.h
index 82b632ef04..d46e62b94d 100644
--- a/bin/named/include/named/globals.h
+++ b/bin/named/include/named/globals.h
@@ -53,6 +53,8 @@ EXTERN unsigned int named_g_udpdisp INIT(0);
EXTERN isc_taskmgr_t *named_g_taskmgr INIT(NULL);
EXTERN dns_dispatchmgr_t *named_g_dispatchmgr INIT(NULL);
EXTERN unsigned int named_g_cpus_detected INIT(1);
+EXTERN unsigned int named_g_hp_extra_workers INIT(0);
+EXTERN unsigned int named_g_hp_extra_percent INIT(100);
#ifdef ENABLE_AFL
EXTERN bool named_g_run_done INIT(false);
diff --git a/bin/named/include/named/main.h b/bin/named/include/named/main.h
index 816aeae7a0..9618251f4d 100644
--- a/bin/named/include/named/main.h
+++ b/bin/named/include/named/main.h
@@ -23,7 +23,7 @@
/*
* Commandline arguments for named; also referenced in win32/ntservice.c
*/
-#define NAMED_MAIN_ARGS "46A:c:d:D:E:fFgL:M:m:n:N:p:sS:t:T:U:u:vVx:X:"
+#define NAMED_MAIN_ARGS "46A:c:d:D:E:fFgh:H:L:M:m:n:N:p:sS:t:T:U:u:vVx:X:"
ISC_PLATFORM_NORETURN_PRE void
named_main_earlyfatal(const char *format, ...)
diff --git a/bin/named/main.c b/bin/named/main.c
index 9ad2d0e277..fbe4c8d011 100644
--- a/bin/named/main.c
+++ b/bin/named/main.c
@@ -351,7 +351,8 @@ usage(void) {
"username] [-U listeners]\n"
" [-X lockfile] [-m "
"{usage|trace|record|size|mctx}]\n"
- " [-M fill|nofill]\n"
+ " [-M fill|nofill] [-h extra_workers] "
+ "[-H extra_percent]\n"
"usage: named [-v|-V]\n");
}
@@ -792,6 +793,21 @@ parse_command_line(int argc, char *argv[]) {
named_g_foreground = true;
named_g_logstderr = true;
break;
+ case 'h':
+ named_g_hp_extra_workers =
+ parse_int(isc_commandline_argument,
+ "number of extra workers");
+ break;
+ case 'H':
+ named_g_hp_extra_percent =
+ parse_int(isc_commandline_argument,
+ "percent of extra workers");
+ if (named_g_hp_extra_percent < 100) {
+ named_main_earlyfatal(
+ "percent of extra workers "
+ "cannot be less than 100");
+ }
+ break;
case 'L':
named_g_logfile = isc_commandline_argument;
break;
@@ -904,6 +920,10 @@ create_managers(void) {
if (named_g_cpus == 0) {
named_g_cpus = named_g_cpus_detected;
}
+ if (named_g_hp_extra_percent > 100) {
+ named_g_hp_extra_workers +=
+ named_g_cpus * (named_g_hp_extra_percent - 100) / 100;
+ }
isc_log_write(
named_g_lctx, NAMED_LOGCATEGORY_GENERAL, NAMED_LOGMODULE_SERVER,
ISC_LOG_INFO, "found %u CPU%s, using %u worker thread%s",
@@ -924,11 +944,12 @@ create_managers(void) {
"using %u UDP listener%s per interface", named_g_udpdisp,
named_g_udpdisp == 1 ? "" : "s");
- result = isc_managers_create(named_g_mctx, named_g_cpus, 0, &named_g_nm,
- &named_g_taskmgr);
+ result = isc_managers_create2(named_g_mctx, named_g_cpus, 0,
+ named_g_hp_extra_workers, &named_g_nm,
+ &named_g_taskmgr);
if (result != ISC_R_SUCCESS) {
UNEXPECTED_ERROR(__FILE__, __LINE__,
- "isc_managers_create() failed: %s",
+ "isc_managers_create2() failed: %s",
isc_result_totext(result));
return (ISC_R_UNEXPECTED);
}
diff --git a/bin/named/named.rst b/bin/named/named.rst
index 39c445f5f9..b5bf95398c 100644
--- a/bin/named/named.rst
+++ b/bin/named/named.rst
@@ -75,6 +75,15 @@ Options
``-g``
This option runs the server in the foreground and forces all logging to ``stderr``.
+``-h #workers``
+ This option requests extra worker threads initialized by hazard pointers. Used only for
+ dyndb plugins.
+
+``-H workers%``
+ This option requests extra worker threads initialized by hazard pointers. Value is
+ computed as percent of number of cpus set by ``-n`` parameter. Minimal value is 100%.
+ Used only for dyndb plugins.
+
``-L logfile``
This option sets the log to the file ``logfile`` by default, instead of the system log.
diff --git a/lib/isc/include/isc/managers.h b/lib/isc/include/isc/managers.h
index 0ed17ff31d..712ea3e3ed 100644
--- a/lib/isc/include/isc/managers.h
+++ b/lib/isc/include/isc/managers.h
@@ -25,5 +25,10 @@ isc_result_t
isc_managers_create(isc_mem_t *mctx, size_t workers, size_t quantum,
isc_nm_t **netmgrp, isc_taskmgr_t **taskmgrp);
+isc_result_t
+isc_managers_create2(isc_mem_t *mctx, size_t workers, size_t quantum,
+ size_t hp_extra, isc_nm_t **netmgrp,
+ isc_taskmgr_t **taskmgrp);
+
void
isc_managers_destroy(isc_nm_t **netmgrp, isc_taskmgr_t **taskmgrp);
diff --git a/lib/isc/managers.c b/lib/isc/managers.c
index a285e80d8a..daf19c5596 100644
--- a/lib/isc/managers.c
+++ b/lib/isc/managers.c
@@ -21,15 +21,24 @@
isc_result_t
isc_managers_create(isc_mem_t *mctx, size_t workers, size_t quantum,
isc_nm_t **netmgrp, isc_taskmgr_t **taskmgrp) {
+ return isc_managers_create2(mctx, workers, quantum, 0, netmgrp,
+ taskmgrp);
+}
+
+isc_result_t
+isc_managers_create2(isc_mem_t *mctx, size_t workers, size_t quantum,
+ size_t hp_extra, isc_nm_t **netmgrp,
+ isc_taskmgr_t **taskmgrp) {
isc_result_t result;
isc_taskmgr_t *taskmgr = NULL;
isc_nm_t *netmgr = NULL;
+ size_t hp_workers = 4 * (workers + hp_extra);
/*
* We have ncpus network threads, ncpus old network threads - make
* it 4x just to be on the safe side.
*/
- isc_hp_init(4 * workers);
+ isc_hp_init(hp_workers);
REQUIRE(netmgrp != NULL && *netmgrp == NULL);
isc__netmgr_create(mctx, workers, &netmgr);
--
2.34.1

214
bind-9.16-locked-isc-queue.patch Normal file
View File

@ -0,0 +1,214 @@
From 0df59049fe13ef89d362fa7f109f289b297441dc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Tue, 22 Feb 2022 23:40:39 +0100
Subject: [PATCH] Provide alternative isc_queue implementation based on locked
list
The current implementation of isc_queue uses Michael-Scott lock-free
queue that in turn uses hazard pointers. It was discovered that the way
we use the isc_queue, such complicated mechanism isn't really needed,
because most of the time, we either execute the work directly when on
nmthread (in case of UDP) or schedule the work from the matching
nmthreads.
Provide alternative implementation for the isc_queue based on locked
ISC_LIST.
PatchNumber: 11
PatchNumber: 11
---
bin/named/main.c | 1 -
configure.ac | 12 ++++
lib/isc/include/isc/queue.h | 3 +-
lib/isc/queue.c | 121 ++++++++++++++++++++++++++++++++++++
4 files changed, 134 insertions(+), 3 deletions(-)
diff --git a/bin/named/main.c b/bin/named/main.c
index 9ad2d0e..8870933 100644
--- a/bin/named/main.c
+++ b/bin/named/main.c
@@ -34,7 +34,6 @@
#include <isc/dir.h>
#include <isc/file.h>
#include <isc/hash.h>
-#include <isc/hp.h>
#include <isc/httpd.h>
#include <isc/managers.h>
#include <isc/netmgr.h>
diff --git a/configure.ac b/configure.ac
index 79d33d1..26241a0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2263,8 +2263,20 @@ AS_CASE([$with_cmocka],
AC_SUBST([CMOCKA_CFLAGS])
AC_SUBST([CMOCKA_LIBS])
+#
+# Use lock-free Michael-Scott's queue implementation or locked-list queue
+#
+# [pairwise: --enable-lock-free-queue, --disable-lock-free-queue]
+AC_ARG_ENABLE([lock-free-queue],
+ [AS_HELP_STRING([--enable-lock-free-queue],[enable lock-free queue implementation (default is enabled)])],
+ [],[enable_lock_free_queue=yes])
+AS_CASE([$enable_lock_free_queue],
+ [no],[],
+ [yes],[AC_DEFINE([USE_LOCK_FREE_QUEUE],[1],[Define to 1 to enable lock-free queue])])
+
AC_DEFINE([SKIPPED_TEST_EXIT_CODE], [0], [Exit code for skipped tests])
+
#
# Check for kyua execution engine if CMocka was requested
# and bail out if execution engine was not found
diff --git a/lib/isc/include/isc/queue.h b/lib/isc/include/isc/queue.h
index 0927075..568bf18 100644
--- a/lib/isc/include/isc/queue.h
+++ b/lib/isc/include/isc/queue.h
@@ -39,8 +39,7 @@ uintptr_t
isc_queue_dequeue(isc_queue_t *queue);
/*%<
* Remove an object pointer from the head of the queue and return the
- * pointer. If the queue is empty, return `nulluintptr` (the uintptr_t
- * representation of NULL).
+ * pointer. If the queue is empty, return `NULL`.
*
* Requires:
* \li 'queue' is not null.
diff --git a/lib/isc/queue.c b/lib/isc/queue.c
index d7ea824..c4cb404 100644
--- a/lib/isc/queue.c
+++ b/lib/isc/queue.c
@@ -28,6 +28,10 @@
static uintptr_t nulluintptr = (uintptr_t)NULL;
+#if USE_LOCK_FREE_QUEUE
+
+#define BUFFER_SIZE 1024
+
typedef struct node {
atomic_uint_fast32_t deqidx;
atomic_uintptr_t items[BUFFER_SIZE];
@@ -232,3 +236,120 @@ isc_queue_destroy(isc_queue_t *queue) {
alloced = queue->alloced_ptr;
isc_mem_putanddetach(&queue->mctx, alloced, sizeof(*queue) + ALIGNMENT);
}
+
+#else /* USE_LOCK_FREE_QUEUE */
+
+typedef struct node node_t;
+
+struct node {
+ uintptr_t item;
+ ISC_LINK(node_t) link;
+};
+
+struct isc_queue {
+ isc_mem_t *mctx;
+ isc_mutex_t lock;
+ int max_threads;
+ ISC_LIST(node_t) nodes;
+ void *alloced_ptr;
+};
+
+static node_t *
+node_new(isc_mem_t *mctx, uintptr_t item) {
+ node_t *node = isc_mem_get(mctx, sizeof(*node));
+ *node = (node_t){
+ .item = item,
+ };
+
+ ISC_LINK_INIT(node, link);
+
+ return (node);
+}
+
+static void
+node_destroy(isc_mem_t *mctx, node_t *node) {
+ isc_mem_put(mctx, node, sizeof(*node));
+}
+
+isc_queue_t *
+isc_queue_new(isc_mem_t *mctx, int max_threads) {
+ isc_queue_t *queue = NULL;
+ void *qbuf = NULL;
+ uintptr_t qptr;
+
+ qbuf = isc_mem_get(mctx, sizeof(*queue) + ALIGNMENT);
+ qptr = (uintptr_t)qbuf;
+ queue = (isc_queue_t *)(qptr + (ALIGNMENT - (qptr % ALIGNMENT)));
+
+ if (max_threads == 0) {
+ max_threads = MAX_THREADS;
+ }
+
+ *queue = (isc_queue_t){
+ .max_threads = max_threads,
+ .alloced_ptr = qbuf,
+ };
+
+ ISC_LIST_INIT(queue->nodes);
+
+ isc_mutex_init(&queue->lock);
+ isc_mem_attach(mctx, &queue->mctx);
+
+ return (queue);
+}
+
+void
+isc_queue_enqueue(isc_queue_t *queue, uintptr_t item) {
+ node_t *node = node_new(queue->mctx, item);
+ REQUIRE(item != nulluintptr);
+
+ LOCK(&queue->lock);
+ ISC_LIST_ENQUEUE(queue->nodes, node, link);
+ UNLOCK(&queue->lock);
+}
+
+uintptr_t
+isc_queue_dequeue(isc_queue_t *queue) {
+ node_t *node = NULL;
+ uintptr_t item = nulluintptr;
+ REQUIRE(queue != NULL);
+
+ LOCK(&queue->lock);
+ node = ISC_LIST_HEAD(queue->nodes);
+ if (node != NULL) {
+ ISC_LIST_DEQUEUE(queue->nodes, node, link);
+ item = node->item;
+ }
+ UNLOCK(&queue->lock);
+
+ if (node != NULL) {
+ node_destroy(queue->mctx, node);
+ }
+
+ return (item);
+}
+
+void
+isc_queue_destroy(isc_queue_t *queue) {
+ node_t *node = NULL;
+ void *alloced = NULL;
+
+ REQUIRE(queue != NULL);
+
+ LOCK(&queue->lock);
+ node = ISC_LIST_HEAD(queue->nodes);
+ while (node != NULL) {
+ node_t *next = ISC_LIST_NEXT(node, link);
+ ISC_LIST_DEQUEUE(queue->nodes, node, link);
+ node_destroy(queue->mctx, node);
+ node = next;
+ }
+ UNLOCK(&queue->lock);
+
+ isc_mutex_destroy(&queue->lock);
+
+ alloced = queue->alloced_ptr;
+ isc_mem_putanddetach(&queue->mctx, alloced, sizeof(*queue) + ALIGNMENT);
+}
+
+#endif /* USE_LOCK_FREE_QUEUE */
--
2.34.1

11
bind.spec
View File

@ -53,7 +53,7 @@ Summary: The Berkeley Internet Name Domain (BIND) DNS (Domain Name System) serv
Name: bind
License: MPLv2.0
Version: 9.16.26
Release: 1%{?dist}
Release: 2%{?dist}
Epoch: 32
Url: https://www.isc.org/downloads/bind/
#
@ -103,9 +103,8 @@ Patch24: bind-9.9.1-P2-dlz-libdb.patch
# https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/2689
Patch25:bind-9.11-rh1666814.patch
# https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/5795
Patch26:bind-9.16-hp-workers-rh2048235.patch
Patch27:bind-9.16-hp-workers-rh2048235-dyndb.patch
# https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/5905
Patch26: bind-9.16-locked-isc-queue.patch
%{?systemd_ordering}
Requires: coreutils
@ -465,6 +464,7 @@ export LIBDIR_SUFFIX
--includedir=%{_includedir}/bind9 \
--with-tuning=large \
--with-libidn2 \
--disable-lock-free-queue \
%if %{with GEOIP2}
--with-maxminddb \
%endif
@ -1115,6 +1115,9 @@ fi;
%endif
%changelog
* Tue Mar 01 2022 Petr Menšík <pemensik@redhat.com> - 32:9.16.26-2
- Switch to locked queue (#2048235)
* Thu Feb 17 2022 Petr Menšík <pemensik@redhat.com> - 32:9.16.26-1
- Update to 9.16.26 (#2055120)