diff mbox series

libcurl channel: enable connection timeout option

Message ID 20210420183028.219884-1-sava.jakovljev@teufel.de
State Changes Requested
Headers show
Series libcurl channel: enable connection timeout option | expand

Commit Message

Sava Jakovljev April 20, 2021, 6:30 p.m. UTC
* Add new configuration option for Suricatta which enables users to set
  connection timeout in seconds.
* Update documentation.
---
 corelib/channel_curl.c              |  9 +++++++++
 doc/source/swupdate.rst             | 11 +++++++++--
 examples/configuration/swupdate.cfg |  8 ++++++--
 include/channel_curl.h              |  3 +++
 suricatta/server_hawkbit.c          | 19 +++++++++++++++----
 5 files changed, 42 insertions(+), 8 deletions(-)

--
2.26.3

Comments

Sava Jakovljev April 20, 2021, 6:33 p.m. UTC | #1
Hi Stefano,

What do you think about exposing other libcurl configuration options, like 
SPEED_LOW_TIME_SEC and others? I didn't want to do it in this patch, but 
maybe it would be good if we would make configuration of used curl channel 
configurable.

Best regards,
Sava Jakovljev

On Tuesday, April 20, 2021 at 8:30:52 PM UTC+2 Sava Jakovljev wrote:

> * Add new configuration option for Suricatta which enables users to set
> connection timeout in seconds.
> * Update documentation.
> ---
> corelib/channel_curl.c | 9 +++++++++
> doc/source/swupdate.rst | 11 +++++++++--
> examples/configuration/swupdate.cfg | 8 ++++++--
> include/channel_curl.h | 3 +++
> suricatta/server_hawkbit.c | 19 +++++++++++++++----
> 5 files changed, 42 insertions(+), 8 deletions(-)
>
> diff --git a/corelib/channel_curl.c b/corelib/channel_curl.c
> index d2c1ab0..ff5f6d1 100644
> --- a/corelib/channel_curl.c
> +++ b/corelib/channel_curl.c
> @@ -534,6 +534,13 @@ channel_op_res_t channel_set_options(channel_t *this, 
> channel_data_t *channel_da
> "this is most probably not what you want. "
> "Adapted it to %us instead.\n", SPEED_LOW_TIME_SEC);
> }
> + if (channel_data->connection_timeout <= 0) {
> + channel_data->connection_timeout = DEFAULT_CONNECTION_TIMEOUT;
> + DEBUG("cURL's connection timeout is disabled or invalid."
> + "This is most probably not what you want. "
> + "Adapted it to %us instead.\n", DEFAULT_CONNECTION_TIMEOUT);
> + }
> +
> channel_curl_t *channel_curl = this->priv;
> channel_op_res_t result = CHANNEL_OK;
> if ((curl_easy_setopt(channel_curl->handle, CURLOPT_URL,
> @@ -544,6 +551,8 @@ channel_op_res_t channel_set_options(channel_t *this, 
> channel_data_t *channel_da
> SPEED_LOW_BYTES_SEC) != CURLE_OK) ||
> (curl_easy_setopt(channel_curl->handle, CURLOPT_LOW_SPEED_TIME,
> channel_data->low_speed_timeout) != CURLE_OK) ||
> + (curl_easy_setopt(channel_curl->handle, CURLOPT_CONNECTTIMEOUT,
> + channel_data->connection_timeout) != CURLE_OK) ||
> (curl_easy_setopt(channel_curl->handle, CURLOPT_HTTPHEADER,
> channel_curl->header) != CURLE_OK) ||
> (curl_easy_setopt(channel_curl->handle, CURLOPT_MAXREDIRS, -1) !=
> diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst
> index d20e250..0b8e0c1 100644
> --- a/doc/source/swupdate.rst
> +++ b/doc/source/swupdate.rst
> @@ -184,7 +184,7 @@ Images fully streamed
> ---------------------
>
> In case of remote update, SWUpdate extracts relevant images from the stream
> -and copies them into the directory pointed to by the environment variable
> +and copies them into the directory pointed to by the environment variable
> ``TMPDIR`` (if unset, to ``/tmp``) before calling the handlers.
> This guarantee that an update is initiated only if all parts are present 
> and
> correct. However, on some systems with less resources, the amount of RAM
> @@ -643,6 +643,13 @@ Mandatory arguments are marked with '\*':
> | | | Suricatta is started with initial state of |
> | | | STATE_WAIT ("-c 6"), this value is ignored.|
>
> +-------------------------+----------+--------------------------------------------+
> +| -s <seconds> | integer | Connection timeout to use in seconds. |
> +| | | Default value is 15 seconds. |
> +| | | NOTE: it is not possible for Suricatta to |
> +| | | respond to external program API requests |
> +| | | during this period - adapt this value to |
> +| | | your use case! |
>
> ++-------------------------+----------+--------------------------------------------+
>
>
> systemd Integration
> @@ -697,7 +704,7 @@ files are also handed over on a "regular" start of 
> SWUpdate via
> ``systemctl start swupdate.service``.
>
> Note that the socket paths in the two ``ListenStream=`` directives
> -have to match the socket paths ``CONFIG_SOCKET_CTRL_PATH`` and
> +have to match the socket paths ``CONFIG_SOCKET_CTRL_PATH`` and
> ``CONFIG_SOCKET_PROGRESS_PATH`` in SWUpdate's configuration.
> Here, the default socket path configuration is depicted.
>
> diff --git a/examples/configuration/swupdate.cfg 
> b/examples/configuration/swupdate.cfg
> index c0547c5..49f9b00 100644
> --- a/examples/configuration/swupdate.cfg
> +++ b/examples/configuration/swupdate.cfg
> @@ -15,7 +15,7 @@
> # enable sending logs to syslog daemon
> # public-key-file : string
> # file with public key for
> -# image verification
> +# image verification
> # mtd-blacklist : list integers
> # MTD devices where SWUpdate
> # must not try to check for UBI filesystem.
> @@ -120,7 +120,7 @@ identify : (
> { name = "swCompatibility"; value = "unknown";}
> );
>
> -#
> +#
> # suricatta section: setup for backend
> #
> # Currently, they refer to the Hawkbit agent.
> @@ -169,6 +169,9 @@ identify : (
> # initial-report-resend-period : integer
> # Specify period between re-tryint to send initial state, specified with 
> "-c" option,
> # when connection to Hawkbit is not available. Default value is 10 seconds.
> +# connection-timeout : integer
> +# Specify server connection timeout. If no connection has been 
> established in this
> +# period, libcurl will consider connection unsuccessful. Default value is 
> 15 seconds.
>
> suricatta :
> {
> @@ -184,6 +187,7 @@ suricatta :
> groupid = 1000;
> enable = true;
> initial-report-resend-period = 30;
> + connection-timeout = 10;
> /*
> cafile = "/etc/ssl/cafile";
> sslkey = "/etc/ssl/sslkey";
> diff --git a/include/channel_curl.h b/include/channel_curl.h
> index 67cc971..c5ff881 100644
> --- a/include/channel_curl.h
> +++ b/include/channel_curl.h
> @@ -14,6 +14,8 @@
> #include <stdbool.h>
> #include "swupdate_status.h"
>
> +#define DEFAULT_CONNECTION_TIMEOUT 15
> +
> /* Curl Channel Implementation Private Header File.
> *
> * This is a "private" header for testability, i.e., the declarations and
> @@ -60,6 +62,7 @@ typedef struct {
> unsigned int method;
> unsigned int retries;
> unsigned int low_speed_timeout;
> + unsigned int connection_timeout;
> channel_body_t format;
> bool debug;
> bool usessl;
> diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
> index f0d10c0..1ae1a07 100644
> --- a/suricatta/server_hawkbit.c
> +++ b/suricatta/server_hawkbit.c
> @@ -51,6 +51,7 @@ static struct option long_options[] = {
> {"disable-token-for-dwl", no_argument, NULL, '1'},
> {"cache", required_argument, NULL, '2'},
> {"initial-report-resend-period", required_argument, NULL, 'm'},
> + {"connection-timeout", required_argument, NULL, 's'},
> {NULL, 0, NULL, 0}};
>
> static unsigned short mandatory_argument_count = 0;
> @@ -130,7 +131,8 @@ static channel_data_t channel_data_defaults = {.debug 
> = false,
> .format = CHANNEL_PARSE_JSON,
> .nocheckanswer = false,
> .nofollow = false,
> - .strictssl = true
> + .strictssl = true,
> + .connection_timeout = 0
> };
>
> static struct timeval server_time;
> @@ -1599,10 +1601,12 @@ void server_print_help(void)
> "\t --disable-token-for-dwl Do not send authentication header when 
> downlloading SWU.\n"
> "\t --cache <file> Use cache file as starting SWU\n"
> "\t -m, --initial-report-resend-period <seconds> Time to wait prior to 
> retry "
> - "sending initial state with '-c' option (default: %ds).\n",
> + "sending initial state with '-c' option (default: %ds).\n"
> + "\t -s, --connection-timeout Set the server connection timeout (default: 
> %ds).\n",
> CHANNEL_DEFAULT_POLLING_INTERVAL, CHANNEL_DEFAULT_RESUME_TRIES,
> CHANNEL_DEFAULT_RESUME_DELAY,
> - INITIAL_STATUS_REPORT_WAIT_DELAY);
> + INITIAL_STATUS_REPORT_WAIT_DELAY,
> + DEFAULT_CONNECTION_TIMEOUT);
> }
>
> static int server_hawkbit_settings(void *elem, void __attribute__ 
> ((__unused__)) *data)
> @@ -1636,6 +1640,9 @@ static int server_hawkbit_settings(void *elem, void 
> __attribute__ ((__unused__)
> get_field(LIBCFG_PARSER, elem, "usetokentodwl",
> &server_hawkbit.usetokentodwl);
>
> + get_field(LIBCFG_PARSER, elem, "connection-timeout",
> + &channel_data_defaults.connection_timeout);
> +
> GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "targettoken", tmp);
> if (strlen(tmp))
> SETSTRING(server_hawkbit.targettoken, tmp);
> @@ -1683,7 +1690,7 @@ server_op_res_t server_start(char *fname, int argc, 
> char *argv[])
> /* reset to optind=1 to parse suricatta's argument vector */
> optind = 1;
> opterr = 0;
> - while ((choice = getopt_long(argc, argv, "t:i:c:u:p:xr:y::w:k:g:f:2:m:",
> + while ((choice = getopt_long(argc, argv, 
> "t:i:c:u:p:xr:y::w:k:g:f:2:m:s:",
> long_options, NULL)) != -1) {
> switch (choice) {
> case 't':
> @@ -1772,6 +1779,10 @@ server_op_res_t server_start(char *fname, int argc, 
> char *argv[])
> server_hawkbit.initial_report_resend_period =
> (unsigned int)strtoul(optarg, NULL, 10);
> break;
> + case 's':
> + channel_data_defaults.connection_timeout =
> + (unsigned int)strtoul(optarg, NULL, 10);
> + break;
> /* Ignore not recognized options, they can be already parsed by the caller 
> */
> case '?':
> break;
> --
> 2.26.3
>
>
diff mbox series

Patch

diff --git a/corelib/channel_curl.c b/corelib/channel_curl.c
index d2c1ab0..ff5f6d1 100644
--- a/corelib/channel_curl.c
+++ b/corelib/channel_curl.c
@@ -534,6 +534,13 @@  channel_op_res_t channel_set_options(channel_t *this, channel_data_t *channel_da
 			  "this is most probably not what you want. "
 			  "Adapted it to %us instead.\n", SPEED_LOW_TIME_SEC);
 	}
+	if (channel_data->connection_timeout <= 0) {
+		channel_data->connection_timeout = DEFAULT_CONNECTION_TIMEOUT;
+		DEBUG("cURL's connection timeout is disabled or invalid."
+			  "This is most probably not what you want. "
+			  "Adapted it to %us instead.\n", DEFAULT_CONNECTION_TIMEOUT);
+	}
+
 	channel_curl_t *channel_curl = this->priv;
 	channel_op_res_t result = CHANNEL_OK;
 	if ((curl_easy_setopt(channel_curl->handle, CURLOPT_URL,
@@ -544,6 +551,8 @@  channel_op_res_t channel_set_options(channel_t *this, channel_data_t *channel_da
 			      SPEED_LOW_BYTES_SEC) != CURLE_OK) ||
 	    (curl_easy_setopt(channel_curl->handle, CURLOPT_LOW_SPEED_TIME,
 			      channel_data->low_speed_timeout) != CURLE_OK) ||
+	    (curl_easy_setopt(channel_curl->handle, CURLOPT_CONNECTTIMEOUT,
+			      channel_data->connection_timeout) != CURLE_OK) ||
 	    (curl_easy_setopt(channel_curl->handle, CURLOPT_HTTPHEADER,
 			      channel_curl->header) != CURLE_OK) ||
 	    (curl_easy_setopt(channel_curl->handle, CURLOPT_MAXREDIRS, -1) !=
diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst
index d20e250..0b8e0c1 100644
--- a/doc/source/swupdate.rst
+++ b/doc/source/swupdate.rst
@@ -184,7 +184,7 @@  Images fully streamed
 ---------------------

 In case of remote update, SWUpdate extracts relevant images from the stream
-and copies them into the directory pointed to by the environment variable
+and copies them into the directory pointed to by the environment variable
 ``TMPDIR`` (if unset, to ``/tmp``) before calling the handlers.
 This guarantee that an update is initiated only if all parts are present and
 correct. However, on some systems with less resources, the amount of RAM
@@ -643,6 +643,13 @@  Mandatory arguments are marked with '\*':
 |                         |          | Suricatta is started with initial state of |
 |                         |          | STATE_WAIT ("-c 6"), this value is ignored.|
 +-------------------------+----------+--------------------------------------------+
+| -s <seconds>            | integer  | Connection timeout to use in seconds.      |
+|                         |          | Default value is 15 seconds.               |
+|                         |          | NOTE: it is not possible for Suricatta to  |
+|                         |          | respond to external program API requests   |
+|                         |          | during this period - adapt this value to   |
+|                         |          | your use case!                             |
++-------------------------+----------+--------------------------------------------+


 systemd Integration
@@ -697,7 +704,7 @@  files are also handed over on a "regular" start of SWUpdate via
 ``systemctl start swupdate.service``.

 Note that the socket paths in the two ``ListenStream=`` directives
-have to match the socket paths ``CONFIG_SOCKET_CTRL_PATH`` and
+have to match the socket paths ``CONFIG_SOCKET_CTRL_PATH`` and
 ``CONFIG_SOCKET_PROGRESS_PATH`` in SWUpdate's configuration.
 Here, the default socket path configuration is depicted.

diff --git a/examples/configuration/swupdate.cfg b/examples/configuration/swupdate.cfg
index c0547c5..49f9b00 100644
--- a/examples/configuration/swupdate.cfg
+++ b/examples/configuration/swupdate.cfg
@@ -15,7 +15,7 @@ 
 #	 		  enable sending logs to syslog daemon
 # public-key-file	: string
 #			  file with public key for
-#			  image verification
+#			  image verification
 # mtd-blacklist		: list integers
 #			  MTD devices where SWUpdate
 #			  must not try to check for UBI filesystem.
@@ -120,7 +120,7 @@  identify : (
 	{ name = "swCompatibility"; value = "unknown";}
 );

-#
+#
 # suricatta section: setup for backend
 #
 # Currently, they refer to the Hawkbit agent.
@@ -169,6 +169,9 @@  identify : (
 # initial-report-resend-period  : integer
 #             Specify period between re-tryint to send initial state, specified with "-c" option,
 #             when connection to Hawkbit is not available. Default value is 10 seconds.
+# connection-timeout : integer
+#			  Specify server connection timeout. If no connection has been established in this
+#			  period, libcurl will consider connection unsuccessful. Default value is 15 seconds.

 suricatta :
 {
@@ -184,6 +187,7 @@  suricatta :
 	groupid		= 1000;
 	enable		= true;
 	initial-report-resend-period = 30;
+	connection-timeout = 10;
 /*
 	cafile		= "/etc/ssl/cafile";
 	sslkey		= "/etc/ssl/sslkey";
diff --git a/include/channel_curl.h b/include/channel_curl.h
index 67cc971..c5ff881 100644
--- a/include/channel_curl.h
+++ b/include/channel_curl.h
@@ -14,6 +14,8 @@ 
 #include <stdbool.h>
 #include "swupdate_status.h"

+#define DEFAULT_CONNECTION_TIMEOUT	15
+
 /* Curl Channel Implementation Private Header File.
  *
  * This is a "private" header for testability, i.e., the declarations and
@@ -60,6 +62,7 @@  typedef struct {
 	unsigned int method;
 	unsigned int retries;
 	unsigned int low_speed_timeout;
+	unsigned int connection_timeout;
 	channel_body_t format;
 	bool debug;
 	bool usessl;
diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
index f0d10c0..1ae1a07 100644
--- a/suricatta/server_hawkbit.c
+++ b/suricatta/server_hawkbit.c
@@ -51,6 +51,7 @@  static struct option long_options[] = {
     {"disable-token-for-dwl", no_argument, NULL, '1'},
     {"cache", required_argument, NULL, '2'},
     {"initial-report-resend-period", required_argument, NULL, 'm'},
+	{"connection-timeout", required_argument, NULL, 's'},
     {NULL, 0, NULL, 0}};

 static unsigned short mandatory_argument_count = 0;
@@ -130,7 +131,8 @@  static channel_data_t channel_data_defaults = {.debug = false,
 					       .format = CHANNEL_PARSE_JSON,
 					       .nocheckanswer = false,
 					       .nofollow = false,
-					       .strictssl = true
+					       .strictssl = true,
+						   .connection_timeout = 0
 						};

 static struct timeval server_time;
@@ -1599,10 +1601,12 @@  void server_print_help(void)
 	    "\t  --disable-token-for-dwl Do not send authentication header when downlloading SWU.\n"
 	    "\t  --cache <file>      Use cache file as starting SWU\n"
 		"\t  -m, --initial-report-resend-period <seconds> Time to wait prior to retry "
-		"sending initial state with '-c' option (default: %ds).\n",
+		"sending initial state with '-c' option (default: %ds).\n"
+		"\t  -s, --connection-timeout Set the server connection timeout (default: %ds).\n",
 	    CHANNEL_DEFAULT_POLLING_INTERVAL, CHANNEL_DEFAULT_RESUME_TRIES,
 	    CHANNEL_DEFAULT_RESUME_DELAY,
-	    INITIAL_STATUS_REPORT_WAIT_DELAY);
+	    INITIAL_STATUS_REPORT_WAIT_DELAY,
+		DEFAULT_CONNECTION_TIMEOUT);
 }

 static int server_hawkbit_settings(void *elem, void  __attribute__ ((__unused__)) *data)
@@ -1636,6 +1640,9 @@  static int server_hawkbit_settings(void *elem, void  __attribute__ ((__unused__)
 	get_field(LIBCFG_PARSER, elem, "usetokentodwl",
 		&server_hawkbit.usetokentodwl);

+	get_field(LIBCFG_PARSER, elem, "connection-timeout",
+		&channel_data_defaults.connection_timeout);
+
 	GET_FIELD_STRING_RESET(LIBCFG_PARSER, elem, "targettoken", tmp);
 	if (strlen(tmp))
 		SETSTRING(server_hawkbit.targettoken, tmp);
@@ -1683,7 +1690,7 @@  server_op_res_t server_start(char *fname, int argc, char *argv[])
 	/* reset to optind=1 to parse suricatta's argument vector */
 	optind = 1;
 	opterr = 0;
-	while ((choice = getopt_long(argc, argv, "t:i:c:u:p:xr:y::w:k:g:f:2:m:",
+	while ((choice = getopt_long(argc, argv, "t:i:c:u:p:xr:y::w:k:g:f:2:m:s:",
 				     long_options, NULL)) != -1) {
 		switch (choice) {
 		case 't':
@@ -1772,6 +1779,10 @@  server_op_res_t server_start(char *fname, int argc, char *argv[])
 			server_hawkbit.initial_report_resend_period =
 				(unsigned int)strtoul(optarg, NULL, 10);
 			break;
+		case 's':
+			channel_data_defaults.connection_timeout =
+				(unsigned int)strtoul(optarg, NULL, 10);
+			break;
 		/* Ignore not recognized options, they can be already parsed by the caller */
 		case '?':
 			break;