[PATCH] Hawkbit: Do not overide polling interval when 0 is received

[Francois Baldassari]

The Hawkbit implementation provided in Suricatta receives a polling
interval from the server. In the event its value is 0, it should use a
default value.

Currently, this default value is hardcoded to 45. However, users are
able to specify an alternative value in the swupdate.cfg file.

This change keeps the polling value unchanged when a 0 is received so
that the swupdate.cfg value (if it exists) is not overriden by the
hardcoded one (45).

Signed-off-by: François Baldassari <fran...@memfault.com>
---
suricatta/server_hawkbit.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
index 5a456e1..8214d2b 100644
--- a/suricatta/server_hawkbit.c
+++ b/suricatta/server_hawkbit.c
@@ -496,10 +496,12 @@ server_op_res_t server_set_polling_interval_json(json_object *json_root)
if (server_hawkbit.update_state == STATE_WAIT)
polling_interval /= 10;

- server_hawkbit.polling_interval =
- polling_interval == 0 ? CHANNEL_DEFAULT_POLLING_INTERVAL : polling_interval;
- DEBUG("Set polling interval to %ds as announced by server.",
- server_hawkbit.polling_interval);
+ if (polling_interval != 0) {
+ server_hawkbit.polling_interval = polling_interval;
+ DEBUG("Set polling interval to %ds as announced by server.",
+ server_hawkbit.polling_interval);
+ }
+
return SERVER_OK;
}

--
2.32.0 (Apple Git-132)

[Stefano Babic]

Hi Francois,

On 10.06.22 06:33, Francois Baldassari wrote:
> The Hawkbit implementation provided in Suricatta receives a polling
> interval from the server. In the event its value is 0, it should use a
> default value.
>
> Currently, this default value is hardcoded to 45. However, users are
> able to specify an alternative value in the swupdate.cfg file.
>
> This change keeps the polling value unchanged when a 0 is received so
> that the swupdate.cfg value (if it exists) is not overriden by the
> hardcoded one (45).
>

Probably it is better to understand which is the goal. Theoretically,
the Hawkbit server is the master. It expects that devices get the
polling time and use it.

However, the server does not complain. This is (ab)used by SWUpdate in
several conditions via the value in swupdate.cfg, for example at the
startup: if the device is new and runs for the first time after been
sold, it is good to start soon the update without waiting for the
polling cycle, that in most cases can be hours.

But after that, the server becomes the master - and polling cycle == 0
is really forbidden. You cannot set it with Hawkbit's WebGUI (I have not
tried with Management Interface), and any attempt to set it to 0, the
polling cycle will fallback to 30 seconds. That means it is quite
impossible to set it to 0, and this is for Hawkbit's developers an
error. The current code just checks for this error condition, and sets a
default fallback for a case that shouldn't happen. How have you seen
this ? Can you better describe how you set the polling time on Hawkbit ?

And what about if polling is set to 0 in swupdate.cfg ?

Best regards,
Stefano Babic

> Signed-off-by: François Baldassari <fran...@memfault.com>
> ---
> suricatta/server_hawkbit.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c
> index 5a456e1..8214d2b 100644
> --- a/suricatta/server_hawkbit.c
> +++ b/suricatta/server_hawkbit.c
> @@ -496,10 +496,12 @@ server_op_res_t server_set_polling_interval_json(json_object *json_root)
> if (server_hawkbit.update_state == STATE_WAIT)
> polling_interval /= 10;
>
> - server_hawkbit.polling_interval =
> - polling_interval == 0 ? CHANNEL_DEFAULT_POLLING_INTERVAL : polling_interval;
> - DEBUG("Set polling interval to %ds as announced by server.",
> - server_hawkbit.polling_interval);
> + if (polling_interval != 0) {
> + server_hawkbit.polling_interval = polling_interval;
> + DEBUG("Set polling interval to %ds as announced by server.",
> + server_hawkbit.polling_interval);
> + }
> +
> return SERVER_OK;
> }
>


--

=====================================================================
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================

[François Baldassari]

Hi Stefano,

On Fri, Jun 10, 2022 at 1:58 AM Stefano Babic <sba...@denx.de> wrote:
>
> Hi Francois,
>
> On 10.06.22 06:33, Francois Baldassari wrote:
> > The Hawkbit implementation provided in Suricatta receives a polling
> > interval from the server. In the event its value is 0, it should use a
> > default value.
> >
> > Currently, this default value is hardcoded to 45. However, users are
> > able to specify an alternative value in the swupdate.cfg file.
> >
> > This change keeps the polling value unchanged when a 0 is received so
> > that the swupdate.cfg value (if it exists) is not overriden by the
> > hardcoded one (45).
> >
>
> Probably it is better to understand which is the goal. Theoretically,
> the Hawkbit server is the master. It expects that devices get the
> polling time and use it.

Good question. My goal is to create a way to override the polling
interval locally for development. Currently, SWUpdate has local
configuration entries for the polling interval in swupdate.cfg, but it
is immediately overridden by the server. My approach is to have the
server set it to "0" which tells the device it is in control (and
hence it will use whatever was configured in swupdate.cfg).

I am open to alternative approaches and am willing to implement them.
One option might be to create a new entry in swupdate.cfg that say
"ignore server polling interval". Let me know what you think.

>
> However, the server does not complain. This is (ab)used by SWUpdate in
> several conditions via the value in swupdate.cfg, for example at the
> startup: if the device is new and runs for the first time after been
> sold, it is good to start soon the update without waiting for the
> polling cycle, that in most cases can be hours.
>
> But after that, the server becomes the master - and polling cycle == 0
> is really forbidden. You cannot set it with Hawkbit's WebGUI (I have not
> tried with Management Interface), and any attempt to set it to 0, the
> polling cycle will fallback to 30 seconds. That means it is quite
> impossible to set it to 0, and this is for Hawkbit's developers an
> error. The current code just checks for this error condition, and sets a
> default fallback for a case that shouldn't happen. How have you seen
> this ? Can you better describe how you set the polling time on Hawkbit ?

I am working with an alternative implementation of the Hawkbit DDI
where it *is* possible
to return 0.

>
> And what about if polling is set to 0 in swupdate.cfg ?

In that case we should either fallback on the 45 seconds default, or
stop polling altogether.

[Stefano Babic]

Hi Francois,

On 14.06.22 02:17, François Baldassari wrote:
> Hi Stefano,
>
> On Fri, Jun 10, 2022 at 1:58 AM Stefano Babic <sba...@denx.de> wrote:
>>
>> Hi Francois,
>>
>> On 10.06.22 06:33, Francois Baldassari wrote:
>>> The Hawkbit implementation provided in Suricatta receives a polling
>>> interval from the server. In the event its value is 0, it should use a
>>> default value.
>>>
>>> Currently, this default value is hardcoded to 45. However, users are
>>> able to specify an alternative value in the swupdate.cfg file.
>>>
>>> This change keeps the polling value unchanged when a 0 is received so
>>> that the swupdate.cfg value (if it exists) is not overriden by the
>>> hardcoded one (45).
>>>
>>
>> Probably it is better to understand which is the goal. Theoretically,
>> the Hawkbit server is the master. It expects that devices get the
>> polling time and use it.
>
> Good question. My goal is to create a way to override the polling
> interval locally for development. Currently, SWUpdate has local
> configuration entries for the polling interval in swupdate.cfg, but it
> is immediately overridden by the server.

Yes, the server is the master - this is specified in Hawkbit's doc
and/or said by Hawkbit's developers. They expect that the client is
behaving according to Hawkbit's configuration.

> My approach is to have the
> server set it to "0" which tells the device it is in control

mmmhhh...do you have already asked Hawkbit's ML for this ? I doubt this
will be mainlined because it is a system configuration for all connected
devices.

> (and
> hence it will use whatever was configured in swupdate.cfg).
>
> I am open to alternative approaches and am willing to implement them.
> One option might be to create a new entry in swupdate.cfg that say
> "ignore server polling interval". Let me know what you think.

This attribute is doable.

The only concern is about some abuse. SWUpdate currently change and
reduce the polling time just at the start up - I told you the use case.
If SWUpdate does not follow anymore the server, there could be thousands
of devices polling very quick the master, producing some sort of DOS.
One can object that this is already possible without SWUpdate, and it is
true as well.

Another and already implemented option is using an external trigger, see
as example swupdate-ipc. With --enable and --trigger, SWUpdate is
polling via command.

>
>>
>> However, the server does not complain. This is (ab)used by SWUpdate in
>> several conditions via the value in swupdate.cfg, for example at the
>> startup: if the device is new and runs for the first time after been
>> sold, it is good to start soon the update without waiting for the
>> polling cycle, that in most cases can be hours.
>>
>> But after that, the server becomes the master - and polling cycle == 0
>> is really forbidden. You cannot set it with Hawkbit's WebGUI (I have not
>> tried with Management Interface), and any attempt to set it to 0, the
>> polling cycle will fallback to 30 seconds. That means it is quite
>> impossible to set it to 0, and this is for Hawkbit's developers an
>> error. The current code just checks for this error condition, and sets a
>> default fallback for a case that shouldn't happen. How have you seen
>> this ? Can you better describe how you set the polling time on Hawkbit ?
>
> I am working with an alternative implementation of the Hawkbit DDI
> where it *is* possible
> to return 0.

What does it mean ? Is this change then pushed to Hawkbit ? Or are you
reimplementing Hawkbit's REST API to be compatible ? In that case, which
server, which license, ?

>
>>
>> And what about if polling is set to 0 in swupdate.cfg ?
>
> In that case we should either fallback on the 45 seconds default, or
> stop polling altogether.

Stop polling is wrong. There are already way (enable / disable) to stop
the polling. You can already inform SWUpdate to stop, or you can enable
or trigger a polling.

Best regards,
Stefano Babic

HRB 165235 Munich, Office: Kirchenstr.5, 82194 Groebenzell, Germany