netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [conntrack-tools PATCH 1/4] conntrackd: evaluate configuration earlier
@ 2017-06-06 10:58 Arturo Borrero Gonzalez
  2017-06-06 10:58 ` [conntrack-tools PATCH 2/4] conntrackd: make the daemon run in RT mode by default Arturo Borrero Gonzalez
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Arturo Borrero Gonzalez @ 2017-06-06 10:58 UTC (permalink / raw)
  To: netfilter-devel

Run the evaluation step sooner in the conntrackd startup routine.
Don't close log or unlink lockfile at this stage.

Signed-off-by: Arturo Borrero Gonzalez <arturo@debian.org>
---
 src/main.c |   20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/main.c b/src/main.c
index fb20f1d..4b6d17d 100644
--- a/src/main.c
+++ b/src/main.c
@@ -338,6 +338,15 @@ int main(int argc, char *argv[])
 		exit(EXIT_FAILURE);
 	}
 
+	/*
+	 * Evaluate configuration
+	 */
+	if (evaluate() == -1) {
+		dlog(LOG_ERR, "conntrackd cannot start, please review your "
+		     "configuration");
+		exit(EXIT_FAILURE);
+	}
+
 	if (type == REQUEST) {
 		if (do_local_request(action, &conf.local, local_step) == -1) {
 			dlog(LOG_ERR, "can't connect: is conntrackd "
@@ -383,17 +392,6 @@ int main(int argc, char *argv[])
 	}
 
 	/*
-	 * Evaluate configuration
-	 */
-	if (evaluate() == -1) {
-		dlog(LOG_ERR, "conntrackd cannot start, please review your "
-		     "configuration");
-		close_log();
-		unlink(CONFIG(lockfile));
-		exit(EXIT_FAILURE);
-	}
-
-	/*
 	 * initialization process
 	 */
 


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [conntrack-tools PATCH 2/4] conntrackd: make the daemon run in RT mode by default
  2017-06-06 10:58 [conntrack-tools PATCH 1/4] conntrackd: evaluate configuration earlier Arturo Borrero Gonzalez
@ 2017-06-06 10:58 ` Arturo Borrero Gonzalez
  2017-06-06 11:10   ` Pablo Neira Ayuso
  2017-06-06 10:58 ` [conntrack-tools PATCH 3/4] conntrackd: cleanup if failed forking Arturo Borrero Gonzalez
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Arturo Borrero Gonzalez @ 2017-06-06 10:58 UTC (permalink / raw)
  To: netfilter-devel

In order to prevent netlink buffer overrun, conntrackd is recommended to run
at max priority.
Make conntrackd to use a RT (SHED_RR) scheduler by default at max priority.
This is common among other HA daemons. For example corosync uses SCHED_RR
by default.
This change should help ease the configuration of conntrackd.

Note that a sched priority that high makes the nice value useless, so deprecate
both options now.

The code is moved to the init() routine. In case of error setting the
scheduler, the system default will be used. Report a message to the user
and continue working.

Signed-off-by: Arturo Borrero Gonzalez <arturo@debian.org>
---
 conntrackd.conf.5                |   46 +++-----------------------------------
 doc/helper/conntrackd.conf       |   21 -----------------
 doc/stats/conntrackd.conf        |   19 ----------------
 doc/sync/alarm/conntrackd.conf   |   21 -----------------
 doc/sync/ftfw/conntrackd.conf    |   21 -----------------
 doc/sync/notrack/conntrackd.conf |   21 -----------------
 include/conntrackd.h             |    5 ----
 src/main.c                       |   28 -----------------------
 src/read_config_yy.y             |   21 +++++------------
 src/run.c                        |   18 +++++++++++++++
 10 files changed, 28 insertions(+), 193 deletions(-)

diff --git a/conntrackd.conf.5 b/conntrackd.conf.5
index 94de327..1e56a1f 100644
--- a/conntrackd.conf.5
+++ b/conntrackd.conf.5
@@ -480,14 +480,8 @@ By default runtime support is disabled.
 
 .TP
 .BI "Nice <value>"
-Set the \fBnice(1)\fP value of the daemon, this value goes from -20 (most
-favorable scheduling) to 19 (least favorable). Using a very low value reduces
-the chances to lose state-change events.
-
-Example: Nice -20
-
-Default is 0 but this example sets it to most favourable scheduling as
-this is generally a good idea.
+Deprecated. This option will be removed in the future.
+Conntrackd now uses by default a RT scheduler.
 
 .TP
 .BI "HashSize <value>"
@@ -731,29 +725,8 @@ Example:
 .fi
 
 .SS SCHEDULER
-Select a different scheduler for the daemon, you can select between \fBRR\fP
-and \fBFIFO\fP and the process priority.
-
-See \fBsched_setscheduler(2)\fP for more information. Using a RT scheduler
-reduces the chances to overrun the Netlink buffer.
-
-Example:
-.nf
-	Scheduler {
-		Type FIFO
-		Priority 99
-	}
-.fi
-
-.TP
-.BI "Type <type>"
-Supported values are \fBRR\fP or \fBFIFO\fP.
-
-.TP
-.BI "Priority <value>"
-Value of the scheduler priority.
-
-Minimum is 0, maximum is 99.
+Deprecated. This section will be removed in the future.
+Conntrackd now uses by default a RT scheduler.
 
 .SH STATS
 This top-level section indicates \fBconntrackd(8)\fP to work as a statistic
@@ -907,7 +880,6 @@ Stats {
 }
 General {
 	Systemd on
-	Nice -1
 	HashSize 8192
 	HashLimit 65535
 	Syslog on
@@ -973,11 +945,6 @@ Sync {
 }
 General {
 	Systemd on
-	Nice -20
-	Scheduler {
-		Type FIFO
-		Priority 99
-	}
 	HashSize 32768
 	HashLimit 131072
 	LogFile on
@@ -1036,11 +1003,6 @@ Sync {
 }
 General {
 	Systemd on
-	Nice -20
-	Scheduler {
-		Type FIFO
-		Priority 99
-	}
 	HashSize 32768
 	HashLimit 131072
 	LogFile on
diff --git a/doc/helper/conntrackd.conf b/doc/helper/conntrackd.conf
index 7eae8bc..abc4087 100644
--- a/doc/helper/conntrackd.conf
+++ b/doc/helper/conntrackd.conf
@@ -103,27 +103,6 @@ Helper {
 #
 General {
 	#
-	# Set the nice value of the daemon, this value goes from -20
-	# (most favorable scheduling) to 19 (least favorable). Using a
-	# very low value reduces the chances to lose state-change events.
-	# Default is 0 but this example file sets it to most favourable
-	# scheduling as this is generally a good idea. See man nice(1) for
-	# more information.
-	#
-	Nice -20
-
-	#
-	# Select a different scheduler for the daemon, you can select between
-	# RR and FIFO and the process priority (minimum is 0, maximum is 99).
-	# See man sched_setscheduler(2) for more information. Using a RT
-	# scheduler reduces the chances to overrun the Netlink buffer.
-	#
-	# Scheduler {
-	#	Type FIFO
-	#	Priority 99
-	# }
-
-	#
 	# Logfile: on (/var/log/conntrackd.log), off, or a filename
 	# Default: off
 	#
diff --git a/doc/stats/conntrackd.conf b/doc/stats/conntrackd.conf
index 6a9aec8..e62ad4b 100644
--- a/doc/stats/conntrackd.conf
+++ b/doc/stats/conntrackd.conf
@@ -11,25 +11,6 @@ General {
 	#Systemd on
 
 	#
-	# Set the nice value of the daemon. This value goes from -20
-	# (most favorable scheduling) to 19 (least favorable). Using a
-	# negative value reduces the chances to lose state-change events.
-	# Default is 0. See man nice(1) for more information.
-	#
-	Nice -1
-
-	# 
-	# Select a different scheduler for the daemon, you can select between
-	# RR and FIFO and the process priority (minimum is 0, maximum is 99).
-	# See man sched_setscheduler(2) for more information. Using a RT
-	# scheduler reduces the chances to overrun the Netlink buffer.
-	#
-	# Scheduler {
-	# 	Type FIFO
-	# 	Priority 99
-	# }
-
-	#
 	# Number of buckets in the caches: hash table
 	#
 	HashSize 8192
diff --git a/doc/sync/alarm/conntrackd.conf b/doc/sync/alarm/conntrackd.conf
index 225d1c9..f609310 100644
--- a/doc/sync/alarm/conntrackd.conf
+++ b/doc/sync/alarm/conntrackd.conf
@@ -226,27 +226,6 @@ General {
 	#Systemd on
 
 	#
-	# Set the nice value of the daemon, this value goes from -20
-	# (most favorable scheduling) to 19 (least favorable). Using a
-	# very low value reduces the chances to lose state-change events.
-	# Default is 0 but this example file sets it to most favourable
-	# scheduling as this is generally a good idea. See man nice(1) for
-	# more information.
-	#
-	Nice -20
-
-	#
-	# Select a different scheduler for the daemon, you can select between
-	# RR and FIFO and the process priority (minimum is 0, maximum is 99).
-	# See man sched_setscheduler(2) for more information. Using a RT
-	# scheduler reduces the chances to overrun the Netlink buffer.
-	#
-	# Scheduler {
-	#	Type FIFO
-	#	Priority 99
-	# }
-
-	#
 	# Number of buckets in the cache hashtable. The bigger it is,
 	# the closer it gets to O(1) at the cost of consuming more memory.
 	# Read some documents about tuning hashtables for further reference.
diff --git a/doc/sync/ftfw/conntrackd.conf b/doc/sync/ftfw/conntrackd.conf
index 228674c..f500637 100644
--- a/doc/sync/ftfw/conntrackd.conf
+++ b/doc/sync/ftfw/conntrackd.conf
@@ -249,27 +249,6 @@ General {
 	#Systemd on
 
 	#
-	# Set the nice value of the daemon, this value goes from -20
-	# (most favorable scheduling) to 19 (least favorable). Using a
-	# very low value reduces the chances to lose state-change events.
-	# Default is 0 but this example file sets it to most favourable
-	# scheduling as this is generally a good idea. See man nice(1) for
-	# more information.
-	#
-	Nice -20
-
-	#
-	# Select a different scheduler for the daemon, you can select between
-	# RR and FIFO and the process priority (minimum is 0, maximum is 99).
-	# See man sched_setscheduler(2) for more information. Using a RT
-	# scheduler reduces the chances to overrun the Netlink buffer.
-	#
-	# Scheduler {
-	#	Type FIFO
-	#	Priority 99
-	# }
-
-	#
 	# Number of buckets in the cache hashtable. The bigger it is,
 	# the closer it gets to O(1) at the cost of consuming more memory.
 	# Read some documents about tuning hashtables for further reference.
diff --git a/doc/sync/notrack/conntrackd.conf b/doc/sync/notrack/conntrackd.conf
index 3becd91..718668d 100644
--- a/doc/sync/notrack/conntrackd.conf
+++ b/doc/sync/notrack/conntrackd.conf
@@ -288,27 +288,6 @@ General {
 	#Systemd on
 
 	#
-	# Set the nice value of the daemon, this value goes from -20
-	# (most favorable scheduling) to 19 (least favorable). Using a
-	# very low value reduces the chances to lose state-change events.
-	# Default is 0 but this example file sets it to most favourable
-	# scheduling as this is generally a good idea. See man nice(1) for
-	# more information.
-	#
-	Nice -20
-
-	#
-	# Select a different scheduler for the daemon, you can select between
-	# RR and FIFO and the process priority (minimum is 0, maximum is 99).
-	# See man sched_setscheduler(2) for more information. Using a RT
-	# scheduler reduces the chances to overrun the Netlink buffer.
-	#
-	# Scheduler {
-	#	Type FIFO
-	#	Priority 99
-	# }
-
-	#
 	# Number of buckets in the cache hashtable. The bigger it is,
 	# the closer it gets to O(1) at the cost of consuming more memory.
 	# Read some documents about tuning hashtables for further reference.
diff --git a/include/conntrackd.h b/include/conntrackd.h
index 1a7ea66..1e11615 100644
--- a/include/conntrackd.h
+++ b/include/conntrackd.h
@@ -94,7 +94,6 @@ struct ct_conf {
 	int channel_type_global;
 	struct channel_conf channel[MULTICHANNEL_MAX];
 	struct local_conf local;	/* unix socket facilities */
-	int nice;
 	int limit;
 	int refresh;
 	int cache_timeout;		/* cache entries timeout */
@@ -129,10 +128,6 @@ struct ct_conf {
 		int commit_steps;
 	} general;
 	struct {
-		int type;
-		int prio;
-	} sched;
-	struct {
 		char logfile[FILENAME_MAXLEN];
 		int syslog_facility;
 		size_t buffer_size;
diff --git a/src/main.c b/src/main.c
index 4b6d17d..bab7772 100644
--- a/src/main.c
+++ b/src/main.c
@@ -31,7 +31,6 @@
 #include <string.h>
 #include <stdlib.h>
 #include <unistd.h>
-#include <sched.h>
 #include <limits.h>
 
 struct ct_general_state st;
@@ -112,15 +111,6 @@ set_action_by_table(int i, int argc, char *argv[],
 }
 
 static void
-set_nice_value(int nv)
-{
-	errno = 0;
-	if (nice(nv) == -1 && errno) /* warn only */
-		dlog(LOG_WARNING, "Cannot set nice level %d: %s",
-		     nv, strerror(errno));
-}
-
-static void
 do_chdir(const char *d)
 {
 	if (chdir(d))
@@ -374,24 +364,6 @@ int main(int argc, char *argv[])
 	close(ret);
 
 	/*
-	 * Setting process priority and scheduler
-	 */
-	set_nice_value(CONFIG(nice));
-
-	if (CONFIG(sched).type != SCHED_OTHER) {
-		struct sched_param schedparam = {
-			.sched_priority = CONFIG(sched).prio,
-		};
-
-		ret = sched_setscheduler(0, CONFIG(sched).type, &schedparam);
-		if (ret == -1) {
-			dlog(LOG_ERR, "scheduler configuration failed: %s",
-			     strerror(errno));
-			exit(EXIT_FAILURE);
-		}
-	}
-
-	/*
 	 * initialization process
 	 */
 
diff --git a/src/read_config_yy.y b/src/read_config_yy.y
index 3bb7c5f..ef6b284 100644
--- a/src/read_config_yy.y
+++ b/src/read_config_yy.y
@@ -30,7 +30,6 @@
 #include "cidr.h"
 #include "helper.h"
 #include "stack.h"
-#include <sched.h>
 #include <dlfcn.h>
 #include <libnetfilter_conntrack/libnetfilter_conntrack.h>
 #include <libnetfilter_conntrack/libnetfilter_conntrack_tcp.h>
@@ -963,7 +962,8 @@ netlink_events_reliable : T_NETLINK_EVENTS_RELIABLE T_OFF
 
 nice : T_NICE T_SIGNED_NUMBER
 {
-	conf.nice = $2;
+	dlog(LOG_WARNING, "deprecated nice configuration. "
+	     "Now conntrackd uses RT by default.");
 };
 
 scheduler : T_SCHEDULER '{' scheduler_options '}';
@@ -974,23 +974,14 @@ scheduler_options :
 
 scheduler_line : T_TYPE T_STRING
 {
-	if (strcasecmp($2, "rr") == 0) {
-		conf.sched.type = SCHED_RR;
-	} else if (strcasecmp($2, "fifo") == 0) {
-		conf.sched.type = SCHED_FIFO;
-	} else {
-		dlog(LOG_ERR, "unknown scheduler `%s'", $2);
-		exit(EXIT_FAILURE);
-	}
+	dlog(LOG_WARNING, "deprecated scheduler configuration. "
+	     "Now conntrackd uses RT by default.");
 };
 
 scheduler_line : T_PRIO T_NUMBER
 {
-	conf.sched.prio = $2;
-	if (conf.sched.prio < 0 || conf.sched.prio > 99) {
-		dlog(LOG_ERR, "`Priority' must be [0, 99]\n", $2);
-		exit(EXIT_FAILURE);
-	}
+	dlog(LOG_WARNING, "deprecated scheduler configuration. "
+	     "Now conntrackd uses RT by default.");
 };
 
 event_iterations_limit : T_EVENT_ITER_LIMIT T_NUMBER
diff --git a/src/run.c b/src/run.c
index 1fe6cba..5ad982a 100644
--- a/src/run.c
+++ b/src/run.c
@@ -32,6 +32,7 @@
 #include "internal.h"
 #include "systemd.h"
 
+#include <sched.h>
 #include <errno.h>
 #include <signal.h>
 #include <stdlib.h>
@@ -234,11 +235,28 @@ int evaluate(void)
 	return 0;
 }
 
+
+static void set_scheduler(void)
+{
+	int prio = sched_get_priority_max(SCHED_RR);
+	struct sched_param schedparam = {
+		.sched_priority = prio,
+	};
+
+	if (sched_setscheduler(0, SCHED_RR, &schedparam) < 0)
+		dlog(LOG_WARNING, "scheduler configuration failed: %s. "
+		     "Likely a bug in conntrackd, please report it. "
+		     "Continuing with system default scheduler.",
+		     strerror(errno));
+}
+
 int
 init(void)
 {
 	do_gettimeofday();
 
+	set_scheduler();
+
 	STATE(fds) = create_fds();
 	if (STATE(fds) == NULL) {
 		dlog(LOG_ERR, "can't create file descriptor pool");


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [conntrack-tools PATCH 3/4] conntrackd: cleanup if failed forking
  2017-06-06 10:58 [conntrack-tools PATCH 1/4] conntrackd: evaluate configuration earlier Arturo Borrero Gonzalez
  2017-06-06 10:58 ` [conntrack-tools PATCH 2/4] conntrackd: make the daemon run in RT mode by default Arturo Borrero Gonzalez
@ 2017-06-06 10:58 ` Arturo Borrero Gonzalez
  2017-06-06 16:11   ` Pablo Neira Ayuso
  2017-06-06 10:58 ` [conntrack-tools PATCH 4/4] conntrackd: deprecate unix backlog configuration Arturo Borrero Gonzalez
  2017-06-06 16:11 ` [conntrack-tools PATCH 1/4] conntrackd: evaluate configuration earlier Pablo Neira Ayuso
  3 siblings, 1 reply; 12+ messages in thread
From: Arturo Borrero Gonzalez @ 2017-06-06 10:58 UTC (permalink / raw)
  To: netfilter-devel

Close the logs and lockfile if error while forking.

Signed-off-by: Arturo Borrero Gonzalez <arturo@debian.org>
---
 src/main.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/main.c b/src/main.c
index bab7772..3b19160 100644
--- a/src/main.c
+++ b/src/main.c
@@ -386,6 +386,8 @@ int main(int argc, char *argv[])
 
 		if ((pid = fork()) == -1) {
 			dlog(LOG_ERR, "fork has failed: %s", strerror(errno));
+			close_log();
+			unlink(CONFIG(lockfile));
 			exit(EXIT_FAILURE);
 		} else if (pid) {
 			sd_ct_mainpid(pid);


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [conntrack-tools PATCH 4/4] conntrackd: deprecate unix backlog configuration
  2017-06-06 10:58 [conntrack-tools PATCH 1/4] conntrackd: evaluate configuration earlier Arturo Borrero Gonzalez
  2017-06-06 10:58 ` [conntrack-tools PATCH 2/4] conntrackd: make the daemon run in RT mode by default Arturo Borrero Gonzalez
  2017-06-06 10:58 ` [conntrack-tools PATCH 3/4] conntrackd: cleanup if failed forking Arturo Borrero Gonzalez
@ 2017-06-06 10:58 ` Arturo Borrero Gonzalez
  2017-06-06 11:11   ` Pablo Neira Ayuso
  2017-06-06 16:13   ` Pablo Neira Ayuso
  2017-06-06 16:11 ` [conntrack-tools PATCH 1/4] conntrackd: evaluate configuration earlier Pablo Neira Ayuso
  3 siblings, 2 replies; 12+ messages in thread
From: Arturo Borrero Gonzalez @ 2017-06-06 10:58 UTC (permalink / raw)
  To: netfilter-devel

This configuration option doesn't add any value to users.
Use the magic value of 100 (i.e, the socket will keep 100 pending connections),
which I think is fair enough for what conntrackd can do in the unix socket.

Signed-off-by: Arturo Borrero Gonzalez <arturo@debian.org>
---
 conntrackd.conf.5                |    8 +-------
 doc/helper/conntrackd.conf       |    1 -
 doc/stats/conntrackd.conf        |    1 -
 doc/sync/alarm/conntrackd.conf   |    1 -
 doc/sync/ftfw/conntrackd.conf    |    1 -
 doc/sync/notrack/conntrackd.conf |    1 -
 include/local.h                  |    1 -
 src/local.c                      |    4 +++-
 src/read_config_yy.y             |    2 +-
 9 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/conntrackd.conf.5 b/conntrackd.conf.5
index 1e56a1f..4785f47 100644
--- a/conntrackd.conf.5
+++ b/conntrackd.conf.5
@@ -603,7 +603,6 @@ Example:
 .nf
 	UNIX {
 		Path /var/run/conntrackd.ctl
-		Backlog 20
 	}
 .fi
 
@@ -615,9 +614,7 @@ Example: Path /var/run/conntrackd.ctl
 
 .TP
 .BI "Backlog <value>"
-Number of items in the backlog.
-
-Example: Backlog 20
+Deprecated option.
 
 .SS FILTER
 Event filtering. This clause allows you to filter certain traffic.
@@ -886,7 +883,6 @@ General {
 	LockFile /var/lock/conntrack.lock
 	UNIX {
 		Path /var/run/conntrackd.ctl
-		Backlog 20
 	}
 	NetlinkBufferSize 262142
 	NetlinkBufferSizeMaxGrowth 655355
@@ -952,7 +948,6 @@ General {
 	LockFile /var/lock/conntrack.lock
 	UNIX {
 		Path /var/run/conntrackd.ctl
-		Backlog 20
 	}
 	NetlinkBufferSize 2097152
 	NetlinkBufferSizeMaxGrowth 8388608
@@ -1010,7 +1005,6 @@ General {
 	LockFile /var/lock/conntrack.lock
 	UNIX {
 		Path /var/run/conntrackd.ctl
-		Backlog 20
 	}
 	NetlinkBufferSize 2097152
 	NetlinkBufferSizeMaxGrowth 8388608
diff --git a/doc/helper/conntrackd.conf b/doc/helper/conntrackd.conf
index abc4087..4148544 100644
--- a/doc/helper/conntrackd.conf
+++ b/doc/helper/conntrackd.conf
@@ -124,6 +124,5 @@ General {
 	#
 	UNIX {
 		Path /var/run/conntrackd.ctl
-		Backlog 20
 	}
 }
diff --git a/doc/stats/conntrackd.conf b/doc/stats/conntrackd.conf
index e62ad4b..ba957a1 100644
--- a/doc/stats/conntrackd.conf
+++ b/doc/stats/conntrackd.conf
@@ -43,7 +43,6 @@ General {
 	#
 	UNIX {
 		Path /var/run/conntrackd.ctl
-		Backlog 20
 	}
 
 	#
diff --git a/doc/sync/alarm/conntrackd.conf b/doc/sync/alarm/conntrackd.conf
index f609310..831be15 100644
--- a/doc/sync/alarm/conntrackd.conf
+++ b/doc/sync/alarm/conntrackd.conf
@@ -262,7 +262,6 @@ General {
 	#
 	UNIX {
 		Path /var/run/conntrackd.ctl
-		Backlog 20
 	}
 
 	#
diff --git a/doc/sync/ftfw/conntrackd.conf b/doc/sync/ftfw/conntrackd.conf
index f500637..9da0fb6 100644
--- a/doc/sync/ftfw/conntrackd.conf
+++ b/doc/sync/ftfw/conntrackd.conf
@@ -285,7 +285,6 @@ General {
 	#
 	UNIX {
 		Path /var/run/conntrackd.ctl
-		Backlog 20
 	}
 
 	#
diff --git a/doc/sync/notrack/conntrackd.conf b/doc/sync/notrack/conntrackd.conf
index 718668d..600fc89 100644
--- a/doc/sync/notrack/conntrackd.conf
+++ b/doc/sync/notrack/conntrackd.conf
@@ -324,7 +324,6 @@ General {
 	#
 	UNIX {
 		Path /var/run/conntrackd.ctl
-		Backlog 20
 	}
 
 	#
diff --git a/include/local.h b/include/local.h
index f9121b1..22859d7 100644
--- a/include/local.h
+++ b/include/local.h
@@ -6,7 +6,6 @@
 #endif
 
 struct local_conf {
-	int backlog;
 	int reuseaddr;
 	char path[UNIX_PATH_MAX];
 };
diff --git a/src/local.c b/src/local.c
index 3395b4c..2b67885 100644
--- a/src/local.c
+++ b/src/local.c
@@ -26,6 +26,8 @@
 #include <arpa/inet.h>
 #include <sys/un.h>
 
+#define UNIX_SOCKET_BACKLOG 100
+
 int local_server_create(struct local_server *server, struct local_conf *conf)
 {
 	int fd;
@@ -53,7 +55,7 @@ int local_server_create(struct local_server *server, struct local_conf *conf)
 		return -1;
 	}
 
-	if (listen(fd, conf->backlog) == -1) {
+	if (listen(fd, UNIX_SOCKET_BACKLOG) == -1) {
 		close(fd);
 		unlink(conf->path);
 		return -1;
diff --git a/src/read_config_yy.y b/src/read_config_yy.y
index ef6b284..5dca1f6 100644
--- a/src/read_config_yy.y
+++ b/src/read_config_yy.y
@@ -650,7 +650,7 @@ unix_option : T_PATH T_PATH_VAL
 
 unix_option : T_BACKLOG T_NUMBER
 {
-	conf.local.backlog = $2;
+	dlog(LOG_WARNING, "deprecated unix backlog configuration.");
 };
 
 sync: T_SYNC '{' sync_list '}'


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [conntrack-tools PATCH 2/4] conntrackd: make the daemon run in RT mode by default
  2017-06-06 10:58 ` [conntrack-tools PATCH 2/4] conntrackd: make the daemon run in RT mode by default Arturo Borrero Gonzalez
@ 2017-06-06 11:10   ` Pablo Neira Ayuso
  2017-06-07 20:53     ` Arturo Borrero Gonzalez
  0 siblings, 1 reply; 12+ messages in thread
From: Pablo Neira Ayuso @ 2017-06-06 11:10 UTC (permalink / raw)
  To: Arturo Borrero Gonzalez; +Cc: netfilter-devel

Hi Arturo,

On Tue, Jun 06, 2017 at 12:58:32PM +0200, Arturo Borrero Gonzalez wrote:
> In order to prevent netlink buffer overrun, conntrackd is recommended to run
> at max priority.
> Make conntrackd to use a RT (SHED_RR) scheduler by default at max priority.
> This is common among other HA daemons. For example corosync uses SCHED_RR
> by default.
> This change should help ease the configuration of conntrackd.
> 
> Note that a sched priority that high makes the nice value useless, so deprecate
> both options now.
> 
> The code is moved to the init() routine. In case of error setting the
> scheduler, the system default will be used. Report a message to the user
> and continue working.

I think we should provide a good default if someone doesn't specify
anything. So defaulting to RT is fine to me so we converge to what
other HA software is doing.

But I think we should keep the Nice and Scheduler clauses. Just in
case anyone wants to do this fine grain tunning.

So my proposal is:

1) Remove them from the examples configuration files.
2) Keep these toggles documented in manpage.
3) Provide this default if someone doesn't specify anything.

So the idea is that we provide good defaults.

BTW, an option I would really deprecate is the Checksum, a lot of
experimentation was going on at the time I added this (more than 10
years ago), this should really go away since I don't see a usecase for
this.

Thanks!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [conntrack-tools PATCH 4/4] conntrackd: deprecate unix backlog configuration
  2017-06-06 10:58 ` [conntrack-tools PATCH 4/4] conntrackd: deprecate unix backlog configuration Arturo Borrero Gonzalez
@ 2017-06-06 11:11   ` Pablo Neira Ayuso
  2017-06-06 11:21     ` Pablo Neira Ayuso
  2017-06-06 16:13   ` Pablo Neira Ayuso
  1 sibling, 1 reply; 12+ messages in thread
From: Pablo Neira Ayuso @ 2017-06-06 11:11 UTC (permalink / raw)
  To: Arturo Borrero Gonzalez; +Cc: netfilter-devel

On Tue, Jun 06, 2017 at 12:58:44PM +0200, Arturo Borrero Gonzalez wrote:
> This configuration option doesn't add any value to users.
> Use the magic value of 100 (i.e, the socket will keep 100 pending connections),
> which I think is fair enough for what conntrackd can do in the unix socket.

I don't think conntrackd will ever get more than 100 connection that
are pending to be accepted.

So I'm fine with this 4/4 patch.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [conntrack-tools PATCH 4/4] conntrackd: deprecate unix backlog configuration
  2017-06-06 11:11   ` Pablo Neira Ayuso
@ 2017-06-06 11:21     ` Pablo Neira Ayuso
  0 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2017-06-06 11:21 UTC (permalink / raw)
  To: Arturo Borrero Gonzalez; +Cc: netfilter-devel

On Tue, Jun 06, 2017 at 01:11:53PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Jun 06, 2017 at 12:58:44PM +0200, Arturo Borrero Gonzalez wrote:
> > This configuration option doesn't add any value to users.
> > Use the magic value of 100 (i.e, the socket will keep 100 pending connections),
> > which I think is fair enough for what conntrackd can do in the unix socket.
> 
> I don't think conntrackd will ever get more than 100 connection that
> are pending to be accepted.

And this only refers to unix socket indeed, really we can deprecate
this.

Back to what I said for Nice/Scheduler, I'm not so sure about removing
them.  Actually I remember this was useful when I was testing long
time ago.

Basically what I observed is that RT scheduler + process pinning to
spare CPU makes Netlink reliable (no event message loss). And that is
good to have in place under high load, otherwise nodes get out of
sync.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [conntrack-tools PATCH 1/4] conntrackd: evaluate configuration earlier
  2017-06-06 10:58 [conntrack-tools PATCH 1/4] conntrackd: evaluate configuration earlier Arturo Borrero Gonzalez
                   ` (2 preceding siblings ...)
  2017-06-06 10:58 ` [conntrack-tools PATCH 4/4] conntrackd: deprecate unix backlog configuration Arturo Borrero Gonzalez
@ 2017-06-06 16:11 ` Pablo Neira Ayuso
  3 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2017-06-06 16:11 UTC (permalink / raw)
  To: Arturo Borrero Gonzalez; +Cc: netfilter-devel

On Tue, Jun 06, 2017 at 12:58:27PM +0200, Arturo Borrero Gonzalez wrote:
> Run the evaluation step sooner in the conntrackd startup routine.
> Don't close log or unlink lockfile at this stage.

Applied, thanks Arturo!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [conntrack-tools PATCH 3/4] conntrackd: cleanup if failed forking
  2017-06-06 10:58 ` [conntrack-tools PATCH 3/4] conntrackd: cleanup if failed forking Arturo Borrero Gonzalez
@ 2017-06-06 16:11   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2017-06-06 16:11 UTC (permalink / raw)
  To: Arturo Borrero Gonzalez; +Cc: netfilter-devel

On Tue, Jun 06, 2017 at 12:58:38PM +0200, Arturo Borrero Gonzalez wrote:
> Close the logs and lockfile if error while forking.

Also applied, thanks.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [conntrack-tools PATCH 4/4] conntrackd: deprecate unix backlog configuration
  2017-06-06 10:58 ` [conntrack-tools PATCH 4/4] conntrackd: deprecate unix backlog configuration Arturo Borrero Gonzalez
  2017-06-06 11:11   ` Pablo Neira Ayuso
@ 2017-06-06 16:13   ` Pablo Neira Ayuso
  1 sibling, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2017-06-06 16:13 UTC (permalink / raw)
  To: Arturo Borrero Gonzalez; +Cc: netfilter-devel

On Tue, Jun 06, 2017 at 12:58:44PM +0200, Arturo Borrero Gonzalez wrote:
> This configuration option doesn't add any value to users.
> Use the magic value of 100 (i.e, the socket will keep 100 pending connections),
> which I think is fair enough for what conntrackd can do in the unix socket.

And this one applied, thanks.

So you only have to sort out patch 3/4.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [conntrack-tools PATCH 2/4] conntrackd: make the daemon run in RT mode by default
  2017-06-06 11:10   ` Pablo Neira Ayuso
@ 2017-06-07 20:53     ` Arturo Borrero Gonzalez
  2017-06-12  8:15       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 12+ messages in thread
From: Arturo Borrero Gonzalez @ 2017-06-07 20:53 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Netfilter Development Mailing list

On 6 June 2017 at 13:10, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> But I think we should keep the Nice and Scheduler clauses. Just in
> case anyone wants to do this fine grain tunning.
>

The nice value can be changed at runtime externally: using the
nice/renice commands
Perhaps is a bit redundant to have it included in the conntrackd code.
Also, nice values are somehow overridden by either SCHED_RR (our
default) or SCHED_FIFO.
Not sure if it makes sense to run in RT and then lower priority by
means of nice.

I'm tempted to just remove the nice thing in v2, what do you think?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [conntrack-tools PATCH 2/4] conntrackd: make the daemon run in RT mode by default
  2017-06-07 20:53     ` Arturo Borrero Gonzalez
@ 2017-06-12  8:15       ` Pablo Neira Ayuso
  0 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2017-06-12  8:15 UTC (permalink / raw)
  To: Arturo Borrero Gonzalez; +Cc: Netfilter Development Mailing list

On Wed, Jun 07, 2017 at 10:53:53PM +0200, Arturo Borrero Gonzalez wrote:
> On 6 June 2017 at 13:10, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> >
> > But I think we should keep the Nice and Scheduler clauses. Just in
> > case anyone wants to do this fine grain tunning.
> >
> 
> The nice value can be changed at runtime externally: using the
> nice/renice commands
> Perhaps is a bit redundant to have it included in the conntrackd code.
> Also, nice values are somehow overridden by either SCHED_RR (our
> default) or SCHED_FIFO.
> Not sure if it makes sense to run in RT and then lower priority by
> means of nice.
> 
> I'm tempted to just remove the nice thing in v2, what do you think?

Nice and Sched overlap, yes.

So we can just deprecate Nice and still keep Sched around.

Fine with this?

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-06-12  8:15 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-06-06 10:58 [conntrack-tools PATCH 1/4] conntrackd: evaluate configuration earlier Arturo Borrero Gonzalez
2017-06-06 10:58 ` [conntrack-tools PATCH 2/4] conntrackd: make the daemon run in RT mode by default Arturo Borrero Gonzalez
2017-06-06 11:10   ` Pablo Neira Ayuso
2017-06-07 20:53     ` Arturo Borrero Gonzalez
2017-06-12  8:15       ` Pablo Neira Ayuso
2017-06-06 10:58 ` [conntrack-tools PATCH 3/4] conntrackd: cleanup if failed forking Arturo Borrero Gonzalez
2017-06-06 16:11   ` Pablo Neira Ayuso
2017-06-06 10:58 ` [conntrack-tools PATCH 4/4] conntrackd: deprecate unix backlog configuration Arturo Borrero Gonzalez
2017-06-06 11:11   ` Pablo Neira Ayuso
2017-06-06 11:21     ` Pablo Neira Ayuso
2017-06-06 16:13   ` Pablo Neira Ayuso
2017-06-06 16:11 ` [conntrack-tools PATCH 1/4] conntrackd: evaluate configuration earlier Pablo Neira Ayuso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).