| Message ID | CAJMWnCjTzhhqe4FgkaSfOrUoTLbLruoLkAinXE9-FkZF7kBsgg@mail.gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Expose retry-sleep setting to the downloader | expand |
Hi João, On 13.04.23 14:29, João Loureiro wrote: > Even though the downloader already counts with a retry-sleep logic that > adds a delay between failed retries, that option is not exposed to the user > via arguments. > > Currently 0s is the default delay between failed attempts. In my experience, > when dealing with unstable connections, it is desirable to have some delay > between retries, to increase the chance of having a successful connection. > > This patch sets the default delay to default 5s, and also gives the possibility > of the user to customize it via downloader's args. > > Signed-off-by: João Loureiro <joaofl@gmail.com> > --- > diff --git a/corelib/downloader.c b/corelib/downloader.c > index 6bff9c7..12dcc77 100644 > --- a/corelib/downloader.c > +++ b/corelib/downloader.c > @@ -24,6 +24,11 @@ > */ > #define DL_LOWSPEED_TIME 300 > +/* > + * Time to wait before retrying. > + */ > +#define DL_RETRY_SLEEP 5 > + > #define DL_DEFAULT_RETRIES 3 > static struct option long_options[] = { > @@ -81,6 +86,8 @@ static int download_settings(void *elem, void > __attribute__ ((__unused__)) *dat > get_field(LIBCFG_PARSER, elem, "retries", > &opt->retries); > + get_field(LIBCFG_PARSER, elem, "retry-sleep", > + &opt->retry_sleep); > get_field(LIBCFG_PARSER, elem, "timeout", > &opt->low_speed_timeout); > @@ -95,15 +102,17 @@ void download_print_help(void) > "\t -u, --url <url> * <url> is a link to the .swu update image\n" > "\t -r, --retries number of retries (resumed download) if connection\n" > "\t is broken (0 means indefinitely retries) (default: %d)\n" > + "\t -s, --retry-sleep 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", > - DL_DEFAULT_RETRIES, DL_LOWSPEED_TIME); > + DL_DEFAULT_RETRIES, DL_LOWSPEED_TIME, DL_RETRY_SLEEP); > } > static channel_data_t channel_options = { > .source = SOURCE_DOWNLOADER, > .debug = false, > .retries = DL_DEFAULT_RETRIES, > + .retry_sleep = DL_RETRY_SLEEP, > .low_speed_timeout = DL_LOWSPEED_TIME, > .headers_to_send = NULL, > .max_download_speed = 0, /* Unlimited download speed is default. */ > @@ -126,12 +135,15 @@ int start_download(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:r:a:", > + while ((choice = getopt_long(argc, argv, "t:u:s:r:a:", > long_options, NULL)) != -1) { > switch (choice) { > case 't': > channel_options.low_speed_timeout = strtoul(optarg, NULL, 10); > break; > + case 's': > + channel_options.retry_sleep = strtoul(optarg, NULL, 10); > + break; > case 'u': > SETSTRING(channel_options.url, optarg); > break; > diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst > index 802b16a..70ab4d9 100644 > --- a/doc/source/swupdate.rst > +++ b/doc/source/swupdate.rst > @@ -581,22 +581,27 @@ Example: ``swupdate -d "-u example.com"`` > Mandatory arguments are marked with '\*': > -+-------------+----------+--------------------------------------------+ > -| Parameter | Type | Description | > -+=============+==========+============================================+ > -| -u <url> | string | \* This is the URL where new software is | > -| | | pulled. URL is a link to a valid .swu image| > -+-------------+----------+--------------------------------------------+ > -| -r <retries>| integer | Number of retries before a download is | > -| | | considered broken. With "-r 0", SWUpdate | > -| | | will not stop until a valid software is | > -| | | loaded. | > -+-------------+----------+--------------------------------------------+ > -| -t <timeout>| integer | Timeout for connection lost | > -| | | downloader or Webserver | > -+-------------+----------+--------------------------------------------+ > -| -a <usr:pwd>| string | Send user and password for Basic Auth | > -+-------------+----------+--------------------------------------------+ > ++--------------------+----------+--------------------------------------------+ > +| Parameter | Type | Description | > ++====================+==========+============================================+ > +| -u <url> | string | \* This is the URL where new software is | > +| | | pulled. URL is a link to a valid .swu image| > ++--------------------+----------+--------------------------------------------+ > +| -r <retries> | integer | Number of retries before a download is | > +| | | considered broken. With "-r 0", SWUpdate | > +| | | will not stop until a valid software is | > +| | | loaded. | > ++--------------------+----------+--------------------------------------------+ > +| -s <retry-sleep> | integer | Number of retries before a download is | > +| | | considered broken. With "-r 0", SWUpdate | > +| | | will not stop until a valid software is | > +| | | loaded. | > ++--------------------+----------+--------------------------------------------+ > +| -t <timeout> | integer | Timeout for connection lost | > +| | | downloader or Webserver | > ++--------------------+----------+--------------------------------------------+ > +| -a <usr:pwd> | string | Send user and password for Basic Auth | > ++--------------------+----------+--------------------------------------------+ > Suricatta command line parameters > ................................. > This is not a patch, it is a mess ! Your e-mailer has mangled the original patch and it is useless. It cannot be neither reviewed nor applied. Please use git send-email to send the patch to the ML, thanks ! Some general remarks about the feature: - if this is added, it should be for all modules, that is it should work with SUricatta, too. - It is not enough to add it as command line parameter, it should be available via configuration file, too. Best regards, Stefano Babic
diff --git a/corelib/downloader.c b/corelib/downloader.c index 6bff9c7..12dcc77 100644 --- a/corelib/downloader.c +++ b/corelib/downloader.c @@ -24,6 +24,11 @@ */ #define DL_LOWSPEED_TIME 300 +/* + * Time to wait before retrying. + */ +#define DL_RETRY_SLEEP 5 + #define DL_DEFAULT_RETRIES 3 static struct option long_options[] = { @@ -81,6 +86,8 @@ static int download_settings(void *elem, void __attribute__ ((__unused__)) *dat get_field(LIBCFG_PARSER, elem, "retries", &opt->retries); + get_field(LIBCFG_PARSER, elem, "retry-sleep", + &opt->retry_sleep); get_field(LIBCFG_PARSER, elem, "timeout", &opt->low_speed_timeout); @@ -95,15 +102,17 @@ void download_print_help(void) "\t -u, --url <url> * <url> is a link to the .swu update image\n" "\t -r, --retries number of retries (resumed download) if connection\n" "\t is broken (0 means indefinitely retries) (default: %d)\n" + "\t -s, --retry-sleep 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", - DL_DEFAULT_RETRIES, DL_LOWSPEED_TIME); + DL_DEFAULT_RETRIES, DL_LOWSPEED_TIME, DL_RETRY_SLEEP); } static channel_data_t channel_options = { .source = SOURCE_DOWNLOADER, .debug = false, .retries = DL_DEFAULT_RETRIES, + .retry_sleep = DL_RETRY_SLEEP, .low_speed_timeout = DL_LOWSPEED_TIME, .headers_to_send = NULL, .max_download_speed = 0, /* Unlimited download speed is default. */ @@ -126,12 +135,15 @@ int start_download(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:r:a:", + while ((choice = getopt_long(argc, argv, "t:u:s:r:a:", long_options, NULL)) != -1) { switch (choice) { case 't': channel_options.low_speed_timeout = strtoul(optarg, NULL, 10); break; + case 's': + channel_options.retry_sleep = strtoul(optarg, NULL, 10); + break; case 'u': SETSTRING(channel_options.url, optarg); break; diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst index 802b16a..70ab4d9 100644 --- a/doc/source/swupdate.rst +++ b/doc/source/swupdate.rst @@ -581,22 +581,27 @@ Example: ``swupdate -d "-u example.com"`` Mandatory arguments are marked with '\*': -+-------------+----------+--------------------------------------------+ -| Parameter | Type | Description | -+=============+==========+============================================+ -| -u <url> | string | \* This is the URL where new software is | -| | | pulled. URL is a link to a valid .swu image| -+-------------+----------+--------------------------------------------+ -| -r <retries>| integer | Number of retries before a download is | -| | | considered broken. With "-r 0", SWUpdate | -| | | will not stop until a valid software is | -| | | loaded. | -+-------------+----------+--------------------------------------------+ -| -t <timeout>| integer | Timeout for connection lost | -| | | downloader or Webserver | -+-------------+----------+--------------------------------------------+ -| -a <usr:pwd>| string | Send user and password for Basic Auth | -+-------------+----------+--------------------------------------------+ ++--------------------+----------+--------------------------------------------+ +| Parameter | Type | Description | ++====================+==========+============================================+ +| -u <url> | string | \* This is the URL where new software is | +| | | pulled. URL is a link to a valid .swu image| ++--------------------+----------+--------------------------------------------+ +| -r <retries> | integer | Number of retries before a download is | +| | | considered broken. With "-r 0", SWUpdate | +| | | will not stop until a valid software is | +| | | loaded. | ++--------------------+----------+--------------------------------------------+ +| -s <retry-sleep> | integer | Number of retries before a download is | +| | | considered broken. With "-r 0", SWUpdate | +| | | will not stop until a valid software is | +| | | loaded. | ++--------------------+----------+--------------------------------------------+ +| -t <timeout> | integer | Timeout for connection lost | +| | | downloader or Webserver | ++--------------------+----------+--------------------------------------------+ +| -a <usr:pwd> | string | Send user and password for Basic Auth | ++--------------------+----------+--------------------------------------------+ Suricatta command line parameters ................................. -- You received this message because you are subscribed to the Google Groups "swupdate" group. To unsubscribe from this group and stop receiving emails from it, send an email to swupdate+unsubscribe@googlegroups.com.
Even though the downloader already counts with a retry-sleep logic that adds a delay between failed retries, that option is not exposed to the user via arguments. Currently 0s is the default delay between failed attempts. In my experience, when dealing with unstable connections, it is desirable to have some delay between retries, to increase the chance of having a successful connection. This patch sets the default delay to default 5s, and also gives the possibility of the user to customize it via downloader's args. Signed-off-by: João Loureiro <joaofl@gmail.com> --- To view this discussion on the web visit https://groups.google.com/d/msgid/swupdate/CAJMWnCjTzhhqe4FgkaSfOrUoTLbLruoLkAinXE9-FkZF7kBsgg%40mail.gmail.com.