From: MaximumG <maxime.gerges@live.fr>
Date: Mon, 28 Jan 2019 07:46:45 +0000 (+0100)
Subject: Add support for timeout on sqlippool allocate_clear
X-Git-Tag: release_3_0_18~35^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a71f63b5acc35ebbc647ad3be52602178ec9f131;p=thirdparty%2Ffreeradius-server.git

Add support for timeout on sqlippool allocate_clear

Add allocate_clear_timeout support to sqlippol
---

diff --git a/raddb/mods-available/sqlippool b/raddb/mods-available/sqlippool
index 40356b637d9..a628fbedd87 100644
--- a/raddb/mods-available/sqlippool
+++ b/raddb/mods-available/sqlippool
@@ -27,6 +27,12 @@ sqlippool {
 	# IP lease duration. (Leases expire even if Acct Stop packet is lost)
 	lease_duration = 3600
 
+	#
+	# Timeout between each consecutive 'allocate_clear' queries (default: 1s)
+	# This will avoid having too many deadlock issues, especially on MySQL backend.
+	#
+	allocate_clear_timeout = 1
+
 	#
 	#  As of 3.0.16, the 'ipv6 = yes' configuration is deprecated.
 	#  You should use the "attribute_name" configuration item
diff --git a/src/modules/rlm_sqlippool/rlm_sqlippool.c b/src/modules/rlm_sqlippool/rlm_sqlippool.c
index 3eb4f12a95d..e43ea0af41a 100644
--- a/src/modules/rlm_sqlippool/rlm_sqlippool.c
+++ b/src/modules/rlm_sqlippool/rlm_sqlippool.c
@@ -55,6 +55,7 @@ typedef struct rlm_sqlippool_t {
 	time_t		last_clear;		//!< So we only do it once a second.
 	char const	*allocate_begin;	//!< SQL query to begin.
 	char const	*allocate_clear;	//!< SQL query to clear an IP.
+	uint32_t    allocate_clear_timeout; //!< Number of second between two allocate_clear SQL query
 	char const	*allocate_find;		//!< SQL query to find an unused IP.
 	char const	*allocate_update;	//!< SQL query to mark an IP as used.
 	char const	*allocate_commit;	//!< SQL query to commit.
@@ -140,6 +141,9 @@ static CONF_PARSER module_config[] = {
 	{ "allocate-clear", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT | PW_TYPE_DEPRECATED, rlm_sqlippool_t, allocate_clear), NULL },
 	{ "allocate_clear", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT , rlm_sqlippool_t, allocate_clear), ""  },
 
+	{ "allocate-clear-timeout", FR_CONF_OFFSET(PW_TYPE_INTEGER | PW_TYPE_DEPRECATED, rlm_sqlippool_t, allocate_clear_timeout), NULL },
+	{ "allocate_clear_timeout", FR_CONF_OFFSET(PW_TYPE_INTEGER, rlm_sqlippool_t, allocate_clear_timeout), "1" },
+
 	{ "allocate-find", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT | PW_TYPE_DEPRECATED, rlm_sqlippool_t, allocate_find), NULL },
 	{ "allocate_find", FR_CONF_OFFSET(PW_TYPE_STRING | PW_TYPE_XLAT | PW_TYPE_REQUIRED, rlm_sqlippool_t, allocate_find), ""  },
 
@@ -523,6 +527,7 @@ static rlm_rcode_t CC_HINT(nonnull) mod_post_auth(void *instance, REQUEST *reque
 	VALUE_PAIR *vp;
 	rlm_sql_handle_t *handle;
 	time_t now;
+	uint32_t diff_time;
 
 	/*
 	 *	If there is already an attribute in the reply do nothing
@@ -554,10 +559,13 @@ static rlm_rcode_t CC_HINT(nonnull) mod_post_auth(void *instance, REQUEST *reque
 	 *	race conditions for the check, but so what.  The
 	 *	actual work is protected by a transaction.  The idea
 	 *	here is that if we're allocating 100 IPs a second,
-	 *	we're only do 1 CLEAR per second.
+	 *	we're only do 1 CLEAR per allocate_clear_timeout.
+	 *  This will avoid having several queries to deadlock and blocking all
+	 *  the sqlippool module.
 	 */
 	now = time(NULL);
-	if (inst->allocate_clear && *inst->allocate_clear && (inst->last_clear < now)) {
+	diff_time = difftime(now, inst->last_clear);
+	if (inst->allocate_clear && *inst->allocate_clear && diff_time >= inst->allocate_clear_timeout) {
 		inst->last_clear = now;
 
 		DO_PART(allocate_begin);