3:44 a.m.
Thu, Sep 01, 2022 at 12:55:46PM CEST, lkundrak(a)v3.sk wrote:
Add a possibility to terminate while leaving the team and the port
intact. This is useful when running from NetworkManager which keeps
tracks of the ports themselves.
There's one important use case where this is essential: handover from
initrd to real root. Systemd's isolation of swithch-root.target stops
NetworkManager.service and then terminates its kids, including teamd.
The real NetworkManager.service would eventually catch up and restart
it, but there's a short period when team ports are removed which is not
great if we're booting off that device. Also, it may be that ports come
up in different order, causing team to get a different MAC address,
which will invalidate the DHCP lease we got beforehands and screwing up
L3 addressing.
Let's also not add new option for this purpose, to save poor
NetworkManager from unpleasantness of determining whether teamd supports
it. The worst that could happen, with new NetworkManager and old teamd,
is that things will behave the the way they always did.
Related NetworkManager change:
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requ...
Signed-off-by: Lubomir Rintel <lkundrak(a)v3.sk>
---
man/teamd.8 | 1 +
teamd/teamd.c | 16 +++++++++++-----
teamd/teamd.h | 2 +-
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/man/teamd.8 b/man/teamd.8
index 3d27eae..f99285c 100644
--- a/man/teamd.8
+++ b/man/teamd.8
@@ -80,6 +80,7 @@ Take over the device if it already exists.
.TP
.B "\-N, \-\-no-quit-destroy"
This option also ensures that the team device is not removed after teamd finishes.
+Repeating the option causes the ports to stay in place.
.TP
.BI "\-t " devicename ", \-\-team-dev " devicename
Use the specified team device name (overrides "device" key in the
configuration).
diff --git a/teamd/teamd.c b/teamd/teamd.c
index b310140..8379f23 100644
--- a/teamd/teamd.c
+++ b/teamd/teamd.c
@@ -112,6 +112,7 @@ static void print_help(const struct teamd_context *ctx) {
" already exists\n"
" -o --take-over Take over the device if it already
exists\n"
" -N --no-quit-destroy Do not destroy the device on quit\n"
+ " Also leave ports in place if used multiple
times\n"
" -t --team-dev=DEVNAME Use the specified team device\n"
" -n --no-ports Start without ports\n"
" -D --dbus-enable Enable D-Bus interface\n"
@@ -204,7 +205,7 @@ static int parse_command_line(struct teamd_context *ctx,
ctx->take_over = true;
break;
case 'N':
- ctx->no_quit_destroy = true;
+ ctx->no_quit_destroy++;
break;
case 't':
free(ctx->team_devname);
@@ -346,10 +347,13 @@ static int teamd_run_loop_do_callbacks(struct list_item *lcb_list,
fd_set *fds,
static int teamd_flush_ports(struct teamd_context *ctx)
{
- if (!ctx->no_quit_destroy)
+ switch (ctx->no_quit_destroy) {
+ case 0:
return teamd_port_remove_all(ctx);
- else
+ case 1:
teamd_port_obj_remove_all(ctx);
This is not correct. You have to do a proper cleanup. Avoiding this call
avoids the cleanup.
Actually, I think this is rather a matter of fixing a bug in
no_quit_destroy implementation. Because the reason for calling
teamd_port_obj_remove_all() is to actually do a cleanup in case of
no_quit_destroy, but incidentally the teamd_port_remove() is called
during it. Perhaps, only remove of call to teamd_port_remove(ctx, tdport)
from port_obj_remove() would fix your problem.
>+ /* If 2 or higher, don't remove any ports at all. */
>+ }
> return 0;
> }
>
>@@ -370,8 +374,10 @@ static int teamd_run_loop_run(struct teamd_context *ctx)
> */
>
> while (true) {
>- if (quit_in_progress && !teamd_has_ports(ctx))
>- return ctx->run_loop.err;
>+ if (quit_in_progress) {
>+ if (!teamd_has_ports(ctx) || ctx->no_quit_destroy > 1)
>+ return ctx->run_loop.err;
>+ }
>
> for (i = 0; i < 3; i++)
> FD_ZERO(&fds[i]);
>diff --git a/teamd/teamd.h b/teamd/teamd.h
>index f94c918..262634c 100644
>--- a/teamd/teamd.h
>+++ b/teamd/teamd.h
>@@ -102,7 +102,7 @@ struct teamd_context {
> char * log_output;
> bool force_recreate;
> bool take_over;
>- bool no_quit_destroy;
>+ unsigned int no_quit_destroy;
> bool init_no_ports;
> bool pre_add_ports;
> char * config_file;
>--
>2.37.2
>