[PATCH 0/3] downloader curl options: add max-download-speed

[Dominique Martinet]

suricatta had a bunch of options not handled for downloader, so these
few patches level the field a bit:
- for command line option, just add --max-download-speed
- for config parsing, use common channel_settings()
- while here ustrtoull bit me during testing so make it a bit more
strict; I wasn't sure if we wanted a hard error but keeping the same
value is probably better for compatiblity... Thinking again most
callers don't check for errors so it'd probably be better to keep the
original value for compatibility instead?
I don't have strong feelings either way

Dominique Martinet (2):
util: ustrtoull: error on bad suffixes
downloader: allow common channel settings

Shin-ya Koga (1):
downloader: add -n/--max-download-speed option

core/util.c | 9 +++++++++
corelib/downloader.c | 16 ++++++++++------
doc/source/swupdate.rst | 8 ++++++++
examples/configuration/swupdate.cfg | 4 ++++
4 files changed, 31 insertions(+), 6 deletions(-)

--
2.39.2

[Dominique Martinet]

The previous patch added max-download-speed so just add this one to
the config example, but using channel_settings() gets a few others
for free if someone ever requires setting them.

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---
corelib/downloader.c | 5 +----
examples/configuration/swupdate.cfg | 4 ++++
2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/corelib/downloader.c b/corelib/downloader.c
index 04dea8b935ad..aae0066784b4 100644
--- a/corelib/downloader.c
+++ b/corelib/downloader.c
@@ -107,12 +107,9 @@ static int download_settings(void *elem, void __attribute__ ((__unused__)) *dat
opt->auth = NULL;
}

- get_field(LIBCFG_PARSER, elem, "retries",
- &opt->retries);
- get_field(LIBCFG_PARSER, elem, "retrywait",
- &opt->retry_sleep);
get_field(LIBCFG_PARSER, elem, "timeout",
&opt->low_speed_timeout);
+ channel_settings(elem, &channel_options);

return 0;
}
diff --git a/examples/configuration/swupdate.cfg b/examples/configuration/swupdate.cfg
index b46c92d924ca..24aae3285f90 100644
--- a/examples/configuration/swupdate.cfg
+++ b/examples/configuration/swupdate.cfg
@@ -104,11 +104,15 @@ logcolors : {
# authentication : string
# credentials needed to get software if server
# enables Basic Auth to allow this downloading
+# max-download-speed : string
+# Specify maximum download speed to use. Value can be expressed as
+# B/s, kB/s, M/s, G/s. Example: 512k
download :
{
authentication = "user:password";
retries = 3;
timeout = 1800;
+ max-download-speed = "1M";
retrywait = 5;
url = "http://example.com/software.swu";
userid = 1000;
--
2.39.2

[Dominique Martinet]

From: Shin-ya Koga <shiny...@atmark-techno.com>

Mirror suricatta (-u)'s --max-download-speed option for downloader (-d),
users sometime need to limit download speed on bad connections.

Note ustrtoull parses K/M/G suffixes for us.

Signed-off-by: Shin-ya Koga <shiny...@atmark-techno.com>

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---

corelib/downloader.c | 11 +++++++++--
doc/source/swupdate.rst | 8 ++++++++
2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/corelib/downloader.c b/corelib/downloader.c
index a3d74e60f274..04dea8b935ad 100644
--- a/corelib/downloader.c
+++ b/corelib/downloader.c
@@ -36,6 +36,7 @@ static struct option long_options[] = {
{"retrywait", required_argument, NULL, 'w'},
{"timeout", required_argument, NULL, 't'},
{"authentication", required_argument, NULL, 'a'},
+ {"max-download-speed", required_argument, NULL, 'n'},
{NULL, 0, NULL, 0}};

static channel_data_t channel_options = {
@@ -202,7 +203,9 @@ void download_print_help(void)
"\t is broken (0 means indefinitely retries) (default: %d)\n"
"\t -w, --retrywait timeout to wait before retrying retries (default: %d)\n"
"\t -t, --timeout timeout to check if a connection is lost (default: %d)\n"
- "\t -a, --authentication authentication information as username:password\n",
+ "\t -a, --authentication authentication information as username:password\n"
+ "\t -n, --max-download-speed <limit> Set download speed limit.\n"
+ "\t Example: -n 100k; -n 1M; -n 100; -n 1G (default: no limit)\n",
DL_DEFAULT_RETRIES, DL_LOWSPEED_TIME, CHANNEL_DEFAULT_RESUME_DELAY);
}

@@ -220,7 +223,7 @@ int start_download_server(const char *fname, int argc, char *argv[])
/* reset to optind=1 to parse download's argument vector */
optind = 1;
int choice = 0;
- while ((choice = getopt_long(argc, argv, "t:u:w:r:a:",
+ while ((choice = getopt_long(argc, argv, "t:u:w:r:a:n:",
long_options, NULL)) != -1) {
switch (choice) {
case 't':
@@ -238,6 +241,10 @@ int start_download_server(const char *fname, int argc, char *argv[])
case 'r':
channel_options.retries = strtoul(optarg, NULL, 10);
break;
+ case 'n':
+ channel_options.max_download_speed =
+ (unsigned int)ustrtoull(optarg, NULL, 10);
+ break;
case '?':
default:
return -EINVAL;
diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst
index ff00ca02a144..489604c73a2b 100644
--- a/doc/source/swupdate.rst
+++ b/doc/source/swupdate.rst
@@ -605,6 +605,14 @@ Mandatory arguments are marked with '\*':
+----------------+----------+--------------------------------------------+
| -a <usr:pwd> | string | Send user and password for Basic Auth |
+----------------+----------+--------------------------------------------+
+| -n <value> | string | Maximum download speed to be used. |
+| | | Value be specified in kB/s, B/s, MB/s |
+| | | or GB/s. Examples: |
+| | | -n 100k : Set limit to 100 kB/s. |
+| | | -n 500 : Set limit to 500 B/s. |
+| | | -n 2M : Set limit to 1 M/s. |
+| | | -n 1G : Set limit to 1 G/s. |
++----------------+----------+--------------------------------------------+

Suricatta command line parameters
.................................
--
2.39.2

[Dominique Martinet]

The previous patch added max-download-speed so just add this one to
the config example, but using channel_settings() gets a few others
for free if someone ever requires setting them.

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---

(Sending this ugly v2 as part of previous thread, please tell me if
you'd rather I resend the whole thing)

v1->v2: restore "retries" get_field().

Koga-san noticed the "chan->retries" setting in channel_settings() is
actually named "retry", not "retries", so we need to keep both.

In theory we could have channel_settings check both (and timeout too
while we're here), but it's just as simple to keep it in downloader.c to
avoid further confusions.

corelib/downloader.c | 3 +--
examples/configuration/swupdate.cfg | 4 ++++
2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/corelib/downloader.c b/corelib/downloader.c
index 04dea8b935ad..151648e4cd72 100644
--- a/corelib/downloader.c
+++ b/corelib/downloader.c
@@ -109,10 +109,9 @@ static int download_settings(void *elem, void __attribute__ ((__unused__)) *dat

get_field(LIBCFG_PARSER, elem, "retries",

[Stefano Babic]

Reviewed-by: Stefano Babic <stefan...@swupdate.org>

[Stefano Babic]

On 20.05.24 08:37, Dominique Martinet wrote:

Reviewed-by: Stefano Babic <stefan...@swupdate.org>