[PATCH] ipc: runtime configurable socket paths

[rei...@gmail.com]

From: Xabier Marquiegui <xmarq...@ainguraiiot.com>

if multiple instances of swupdate are to be run on a single system
socket paths need to be configurable at runtime. This modification
allows the user to select socket paths at runtime using environment
variables SOCKET_CTRL_PATH and SOCKET_PROGRESS_PATH.

Signed-off-by: Xabier Marquiegui <rei...@gmail.com>
---
doc/source/swupdate.rst | 11 +++++++++++
ipc/network_ipc.c | 5 +++++
ipc/progress_ipc.c | 5 +++++
3 files changed, 21 insertions(+)

diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst
index 802b16a..c0744a7 100644
--- a/doc/source/swupdate.rst
+++ b/doc/source/swupdate.rst
@@ -204,6 +204,17 @@ copies. Not all handlers support to stream directly into the target.
Streaming with zero-copy is enabled by setting the flag "installed-directly"
in the description of the single image.

+Multiple swupdate instances supported
+-------------------------------------
+
+Multiple swupdate instances can run in a single system. In order to make this possible
+it is necessary to customize the control and progress socket paths. This can be achieved
+at runtime by setting the environment variables ``SOCKET_CTRL_PATH`` and
+``SOCKET_PROGRESS_PATH``.
+
+These runtime variables will override the default value and the compile-time values set for
+these socket files.
+
Configuration and build
=======================

diff --git a/ipc/network_ipc.c b/ipc/network_ipc.c
index 6c8c03a..25167cd 100644
--- a/ipc/network_ipc.c
+++ b/ipc/network_ipc.c
@@ -27,6 +27,11 @@ static char* SOCKET_CTRL_PATH = NULL;
#define SOCKET_CTRL_DEFAULT "sockinstctrl"

char *get_ctrl_socket(void) {
+ const char *socket_ctrl_path = getenv("SOCKET_CTRL_PATH");
+ if (socket_ctrl_path) {
+ return (char *)socket_ctrl_path;
+ }
+
if (!SOCKET_CTRL_PATH || !strlen(SOCKET_CTRL_PATH)) {
const char *tmpdir = getenv("TMPDIR");
if (!tmpdir)
diff --git a/ipc/progress_ipc.c b/ipc/progress_ipc.c
index 745dc44..e97c13b 100644
--- a/ipc/progress_ipc.c
+++ b/ipc/progress_ipc.c
@@ -25,6 +25,11 @@ char *SOCKET_PROGRESS_PATH = NULL;
#define SOCKET_PROGRESS_DEFAULT "swupdateprog"

char *get_prog_socket(void) {
+ const char *socket_progress_path = getenv("SOCKET_PROGRESS_PATH");
+ if (socket_progress_path) {
+ return (char *)socket_progress_path;
+ }
+
if (!SOCKET_PROGRESS_PATH || !strlen(SOCKET_PROGRESS_PATH)) {
const char *tmpdir = getenv("TMPDIR");
if (!tmpdir)
--
2.34.1

[Dominique MARTINET]

rei...@gmail.com wrote on Mon, Oct 31, 2022 at 10:50:09AM +0100:
> From: Xabier Marquiegui <xmarq...@ainguraiiot.com>
>
> if multiple instances of swupdate are to be run on a single system
> socket paths need to be configurable at runtime. This modification
> allows the user to select socket paths at runtime using environment
> variables SOCKET_CTRL_PATH and SOCKET_PROGRESS_PATH.
>
> Signed-off-by: Xabier Marquiegui <rei...@gmail.com>

(your author and sign off mail addresses do not match)

> diff --git a/doc/source/swupdate.rst b/doc/source/swupdate.rst
> index 802b16a..c0744a7 100644
> --- a/doc/source/swupdate.rst
> +++ b/doc/source/swupdate.rst
> @@ -204,6 +204,17 @@ copies. Not all handlers support to stream directly into the target.
> Streaming with zero-copy is enabled by setting the flag "installed-directly"
> in the description of the single image.
>
> +Multiple swupdate instances supported
> +-------------------------------------
> +
> +Multiple swupdate instances can run in a single system. In order to make this possible
> +it is necessary to customize the control and progress socket paths. This can be achieved
> +at runtime by setting the environment variables ``SOCKET_CTRL_PATH`` and
> +``SOCKET_PROGRESS_PATH``.
> +
> +These runtime variables will override the default value and the compile-time values set for
> +these socket files.

This already works if you set TMPDIR manually, but it is a path full of
thorns: you _really_ need to make sure swupdate will not actually
perform multiple updates in parallel as you will get a broken system.

I do not think we should recommend, or even allow this explicitly. Run
swupdate once and use swupdate-client or the ipc to trigger updates as
required.

--
Dominique

[Xabier M.]

Thank you for your input Dominique. This is my first patch request on a mailing list. I'll fix the author address if we agree that the patch will move forward.

The way I see it you are right, but SWupdate can also break a system in hundreds of other ways if the developer implementing it doesn't know what is doing.

Let me explain the use case better, and let me know your opinion:

Not all swupdate instances need to be used for full system upgrades. One may be used for full system upgrades, but the other one may be used to allow installing small files to the system without a reboot after completion. Using two separate processes enables the use of different keys and web servers, which can be a useful way of controlling what users can install small files and what users can do full system upgrades.

Being able to select the socket filename allows for a better organized system. It's more elegant to have all socket files in the same path (which is commonly in /run, not /tmp), and keep doing temporary work on /tmp.

Thanks,

Xabier 

[Stefano Babic]

Hi Dominique, Xabier,

Correct : this is the way to run multiple instance. Setting the sockets
isn't enough: when SWUpdate runs, it creates directories into TMPDIR
where it puts temporary files, like sw-description before parsing. If
you run multiple instances, you have to set up (in systemd) multiple run
units and each of them sets a different TMPDIR.

So why do we need this patch ? Use case is already covered as Dominique
explained.

> but it is a path full of
> thorns: you _really_ need to make sure swupdate will not actually
> perform multiple updates in parallel as you will get a broken system.
>

This is true in a multiple of cases, the integrator must know what is
doing. Anyway, there are use cases for it. I described in details the
use case with compound images in [1]. In that use case, the two
instances run at the same time and there is no way that an instance can
damage the other one (even with wrong SWUs, of course). Other use cases
raise when for security reasons, one instance is allowed to manage just
a subset of the hardware - linux rights, cgroups, etc. will help to do this.

Of course, these setup can become complicate and special care must be taken.

> I do not think we should recommend, or even allow this explicitly. Run
> swupdate once and use swupdate-client or the ipc to trigger updates as
> required.
>

It depends on the setup and use cases.

Best regards,
Stefano

[1]
https://www.slideshare.net/StefanoBabic/evolution-of-otaupdateintheiotworld

[Stefano Babic]

Hi Xabier,

On 01.11.22 07:55, Xabier M. wrote:
> Thank you for your input Dominique. This is my first patch request on a
> mailing list. I'll fix the author address if we agree that the patch
> will move forward.
>
> The way I see it you are right, but SWupdate can also break a system in
> hundreds of other ways if the developer implementing it doesn't know
> what is doing.
>

Absolutely correct : a sw engineer should know what is doing.

> Let me explain the use case better, and let me know your opinion:
>
> Not all swupdate instances need to be used for full system upgrades. One
> may be used for full system upgrades, but the other one may be used to
> allow installing small files to the system without a reboot after
> completion. Using two separate processes enables the use of different
> keys and web servers, which can be a useful way of controlling what
> users can install small files and what users can do full system upgrades.
>
> Being able to select the socket filename allows for a better organized
> system. It's more elegant to have all socket files in the same path
> (which is commonly in /run, not /tmp), and keep doing temporary work on
> /tmp.
>
> Thanks,
>
> Xabier
>
> El mar, 1 nov 2022 2:44, Dominique MARTINET
> <dominique...@atmark-techno.com

> <mailto:dominique...@atmark-techno.com>> escribió:
>
> rei...@gmail.com <mailto:rei...@gmail.com> wrote on Mon, Oct 31,

> 2022 at 10:50:09AM +0100:
> > From: Xabier Marquiegui <xmarq...@ainguraiiot.com

> <mailto:xmarq...@ainguraiiot.com>>

> >
> > if multiple instances of swupdate are to be run on a single system
> > socket paths need to be configurable at runtime. This modification
> > allows the user to select socket paths at runtime using environment
> > variables SOCKET_CTRL_PATH and SOCKET_PROGRESS_PATH.
> >
> > Signed-off-by: Xabier Marquiegui <rei...@gmail.com

> <mailto:rei...@gmail.com>>

> --
> 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+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/swupdate/CANN7TUqcHNLXvUT1cQjJk3M40C8_ROqjrOQE9XJr2WKotAVP6w%40mail.gmail.com <https://groups.google.com/d/msgid/swupdate/CANN7TUqcHNLXvUT1cQjJk3M40C8_ROqjrOQE9XJr2WKotAVP6w%40mail.gmail.com?utm_medium=email&utm_source=footer>.

[Stefano Babic]

Hi Xabier,

On 01.11.22 07:55, Xabier M. wrote:

> Thank you for your input Dominique. This is my first patch request on a
> mailing list. I'll fix the author address if we agree that the patch
> will move forward.
>
> The way I see it you are right, but SWupdate can also break a system in
> hundreds of other ways if the developer implementing it doesn't know
> what is doing.
>
> Let me explain the use case better, and let me know your opinion:
>
> Not all swupdate instances need to be used for full system upgrades. One
> may be used for full system upgrades, but the other one may be used to
> allow installing small files to the system without a reboot after
> completion. Using two separate processes enables the use of different
> keys and web servers, which can be a useful way of controlling what
> users can install small files and what users can do full system upgrades.
>

Ok

> Being able to select the socket filename allows for a better organized
> system.

This is not enough to guarantee that two updates at once do not
conflict, and this is the reason why sockets' path is derived from TMPDIR.

> It's more elegant to have all socket files in the same path
> (which is commonly in /run, not /tmp),

It does not work because SWUpdate is using this path. Then set
/run/swupdate-1, /run/swupdate-2 as TMPDIR for each instance.

> and keep doing temporary work on
> /tmp.
>
> Thanks,

Best regards,
Stefano

>
> Xabier
>
> El mar, 1 nov 2022 2:44, Dominique MARTINET
> <dominique...@atmark-techno.com

> <mailto:dominique...@atmark-techno.com>> escribió:
>
> rei...@gmail.com <mailto:rei...@gmail.com> wrote on Mon, Oct 31,

> 2022 at 10:50:09AM +0100:
> > From: Xabier Marquiegui <xmarq...@ainguraiiot.com

> <mailto:xmarq...@ainguraiiot.com>>

> >
> > if multiple instances of swupdate are to be run on a single system
> > socket paths need to be configurable at runtime. This modification
> > allows the user to select socket paths at runtime using environment
> > variables SOCKET_CTRL_PATH and SOCKET_PROGRESS_PATH.
> >
> > Signed-off-by: Xabier Marquiegui <rei...@gmail.com

> <mailto:rei...@gmail.com>>

[Xabier M.]

Thank you very much for the detailed answer Dominique.

I now understand that I missed the main key point that sw-description is a fixed filename that will be placed in TMPDIR. This could in fact cause inconveniences if two system upgrades are attempted at the same time, but in my experience swu gets equally hung up if I push two swu files through two browsers to a single swupdate instance.

It is also true that two instances of SWUpdate can be run by customizing TMPDIR, which makes the patch description not very accurate, but I still think that using tmpdir to store work files that are related to a specific swu upload, and socket files that are related to the actual service is not the best way to organize resources form a filesystem point of view. Not a big deal, just cosmetic.

[Stefano Babic]

Hi Xabier,

On 01.11.22 11:22, Xabier M. wrote:
> Thank you very much for the detailed answer Dominique.
>
> I now understand that I missed the main key point that sw-description is
> a fixed filename that will be placed in TMPDIR. This could in fact cause
> inconveniences if two system upgrades are attempted at the same time,
> but in my experience swu gets equally hung up if I push two swu files
> through two browsers to a single swupdate instance.

This is not true. If you get this, it should be analyzed and checked.
SWUpdate rejects a second update if an update is already running. If I
run an update opening two browsers, and I start an update from one of
them, a second update from the second browser has no effect (browser
reports "server returns zero"). And both browsers will show the update
progress.

>
> It is also true that two instances of SWUpdate can be run by customizing
> TMPDIR, which makes the patch description not very accurate, but I still
> think that using tmpdir to store work files that are related to a
> specific swu upload, and socket files that are related to the actual
> service is not the best way to organize resources form a filesystem
> point of view.
> Not a big deal, just cosmetic.

Again, it is not just cosmetic. Each SWUpdate instance needs a set (not
just a socket), not just the sockets. When SWUPdate needs some temporary
file, it evaluates env(TMPDIR) before doing. Check how many times this
is done in code.

If you add runtime path for sockets, we need then the same for the path
where to store scripts, where to store files in case streaming is
disabled, etc. It is then very easy to mix up all these setup: using
TMPDIR we have just one variable, adding granularity (but do we need it
?) requires a lot of different configuration.

Regards,
Stefano

>
> El mar, 1 nov 2022 a las 10:45, Stefano Babic (<sba...@denx.de

> <mailto:sba...@denx.de>>) escribió:

> > <mailto:dominique...@atmark-techno.com

> <mailto:dominique...@atmark-techno.com>>> escribió:
> >
> > rei...@gmail.com <mailto:rei...@gmail.com>

> <mailto:rei...@gmail.com <mailto:rei...@gmail.com>> wrote on Mon,

> Oct 31,
> >     2022 at 10:50:09AM +0100:
> >      > From: Xabier Marquiegui <xmarq...@ainguraiiot.com
> <mailto:xmarq...@ainguraiiot.com>

> >     <mailto:xmarq...@ainguraiiot.com

> <mailto:xmarq...@ainguraiiot.com>>>
> >      >
> >      > if multiple instances of swupdate are to be run on a
> single system
> >      > socket paths need to be configurable at runtime. This
> modification
> >      > allows the user to select socket paths at runtime using
> environment
> >      > variables SOCKET_CTRL_PATH and SOCKET_PROGRESS_PATH.
> >      >
> >      > Signed-off-by: Xabier Marquiegui <rei...@gmail.com
> <mailto:rei...@gmail.com>

> >     <mailto:rei...@gmail.com <mailto:rei...@gmail.com>>>

> <mailto:swupdate%2Bunsu...@googlegroups.com>
> > <mailto:swupdate+u...@googlegroups.com
> <mailto:swupdate%2Bunsu...@googlegroups.com>>.

> > To view this discussion on the web visit
> >

> https://groups.google.com/d/msgid/swupdate/CANN7TUqcHNLXvUT1cQjJk3M40C8_ROqjrOQE9XJr2WKotAVP6w%40mail.gmail.com <https://groups.google.com/d/msgid/swupdate/CANN7TUqcHNLXvUT1cQjJk3M40C8_ROqjrOQE9XJr2WKotAVP6w%40mail.gmail.com> <https://groups.google.com/d/msgid/swupdate/CANN7TUqcHNLXvUT1cQjJk3M40C8_ROqjrOQE9XJr2WKotAVP6w%40mail.gmail.com?utm_medium=email&utm_source=footer <https://groups.google.com/d/msgid/swupdate/CANN7TUqcHNLXvUT1cQjJk3M40C8_ROqjrOQE9XJr2WKotAVP6w%40mail.gmail.com?utm_medium=email&utm_source=footer>>.

>
> --
> 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+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/swupdate/CANN7TUrbtChQ8%3DoMuUMNcFn_iuFKWaB99U1mTKkK7mLTP8s9Hg%40mail.gmail.com <https://groups.google.com/d/msgid/swupdate/CANN7TUrbtChQ8%3DoMuUMNcFn_iuFKWaB99U1mTKkK7mLTP8s9Hg%40mail.gmail.com?utm_medium=email&utm_source=footer>.

[Xabier M.]

Hi Stefano,

I really appreciate the discussion, your time, and your patience.

>This is not true. If you get this, it should be analyzed and checked.
>SWUpdate rejects a second update if an update is already running. If I
>run an update opening two browsers, and I start an update from one of
>them, a second update from the second browser has no effect (browser
>reports "server returns zero"). And both browsers will show the update
>progress.

I have seen this happen twice in a row this morning. I will have to take some time to analyze more in detail under what circumstances this is happening so that we can assess if it's something related to my specific setup, or if it's something that's worth reviewing from a swupdate point of view. Right now I'm overloaded with other tasks, so I will postpone this indefinitely.

Also, I realize this thread should end here because it's a separate issue, but let me share the information I have at this moment, in case it's useful in any way:

Linux 4.9.330 aarch64

systemd 250 (250.5+)

swupdate d542b7fa7c8b7ddb75e7cb3335f5a2590df2edab

Procedure:

Open Two firefox windows and a file manager between both windows

Drag swu file to one firefox window

As fast as possible drag the same swu file to another window

Result: the browsers become unresponsive. The software update never completes. If I close the browsers and open a clean one the swupdate mongoose website is not accessible. If I restart the swupdate service everything goes back to normal.

Thank you all for helping me learn and grow. I hope someday I can contribute with something that's useful :)

[Stefano Babic]

Hi Xabier,

This is the default behavior - the integrated Webserver does not check
if data are not sent anymore, and it lets the update opened. Yo can
change this behavior adding a timeout to the webserver. If Webserver
does not get anymore data, it closes the connection and the update is
aborted, letting a second update to run.

To enable this, you need a swupdate.cfg with :

webserver :
{
timeout = 20;
};

> If I restart the swupdate service
> everything goes back to normal.

This is the default behavior, there is no check.

>
> Thank you all for helping me learn and grow. I hope someday I can
> contribute with something that's useful :)
>

Best regards,
Stefano Babic

> El mar, 1 nov 2022 a las 11:52, Stefano Babic (<sba...@denx.de

> > <mailto:sba...@denx.de <mailto:sba...@denx.de>>>) escribió:

> >     <mailto:swupdate%2Bunsu...@googlegroups.com
> <mailto:swupdate%252Buns...@googlegroups.com>>
> >      > <mailto:swupdate+u...@googlegroups.com
> <mailto:swupdate%2Bunsu...@googlegroups.com>
> >     <mailto:swupdate%2Bunsu...@googlegroups.com
> <mailto:swupdate%252Buns...@googlegroups.com>>>.

> >      > To view this discussion on the web visit
> >      >
> >

> https://groups.google.com/d/msgid/swupdate/CANN7TUqcHNLXvUT1cQjJk3M40C8_ROqjrOQE9XJr2WKotAVP6w%40mail.gmail.com <https://groups.google.com/d/msgid/swupdate/CANN7TUqcHNLXvUT1cQjJk3M40C8_ROqjrOQE9XJr2WKotAVP6w%40mail.gmail.com> <https://groups.google.com/d/msgid/swupdate/CANN7TUqcHNLXvUT1cQjJk3M40C8_ROqjrOQE9XJr2WKotAVP6w%40mail.gmail.com <https://groups.google.com/d/msgid/swupdate/CANN7TUqcHNLXvUT1cQjJk3M40C8_ROqjrOQE9XJr2WKotAVP6w%40mail.gmail.com>> <https://groups.google.com/d/msgid/swupdate/CANN7TUqcHNLXvUT1cQjJk3M40C8_ROqjrOQE9XJr2WKotAVP6w%40mail.gmail.com?utm_medium=email&utm_source=footer <https://groups.google.com/d/msgid/swupdate/CANN7TUqcHNLXvUT1cQjJk3M40C8_ROqjrOQE9XJr2WKotAVP6w%40mail.gmail.com?utm_medium=email&utm_source=footer> <https://groups.google.com/d/msgid/swupdate/CANN7TUqcHNLXvUT1cQjJk3M40C8_ROqjrOQE9XJr2WKotAVP6w%40mail.gmail.com?utm_medium=email&utm_source=footer <https://groups.google.com/d/msgid/swupdate/CANN7TUqcHNLXvUT1cQjJk3M40C8_ROqjrOQE9XJr2WKotAVP6w%40mail.gmail.com?utm_medium=email&utm_source=footer>>>.

> >
> > --
> > 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+u...@googlegroups.com

> <mailto:swupdate%2Bunsu...@googlegroups.com>
> > <mailto:swupdate+u...@googlegroups.com
> <mailto:swupdate%2Bunsu...@googlegroups.com>>.
> > To view this discussion on the web visit
> >

> https://groups.google.com/d/msgid/swupdate/CANN7TUrbtChQ8%3DoMuUMNcFn_iuFKWaB99U1mTKkK7mLTP8s9Hg%40mail.gmail.com <https://groups.google.com/d/msgid/swupdate/CANN7TUrbtChQ8%3DoMuUMNcFn_iuFKWaB99U1mTKkK7mLTP8s9Hg%40mail.gmail.com> <https://groups.google.com/d/msgid/swupdate/CANN7TUrbtChQ8%3DoMuUMNcFn_iuFKWaB99U1mTKkK7mLTP8s9Hg%40mail.gmail.com?utm_medium=email&utm_source=footer <https://groups.google.com/d/msgid/swupdate/CANN7TUrbtChQ8%3DoMuUMNcFn_iuFKWaB99U1mTKkK7mLTP8s9Hg%40mail.gmail.com?utm_medium=email&utm_source=footer>>.

>
> --
> 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+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/swupdate/CANN7TUo558%3D%2Bi7ie-LJ_SFb%2B%2Bfn_FPkWaga%3DKTzQYgz%3Dc06wbA%40mail.gmail.com <https://groups.google.com/d/msgid/swupdate/CANN7TUo558%3D%2Bi7ie-LJ_SFb%2B%2Bfn_FPkWaga%3DKTzQYgz%3Dc06wbA%40mail.gmail.com?utm_medium=email&utm_source=footer>.