[PATCH v3] teamd: stop iterating callbacks when a loop restart is requested
by Lubomir Rintel
A crash was observed:
Added loop callback: dbus_watch, 0x560d279e4530
Added loop callback: dbus_watch, 0x560d279e4580
...
Removed loop callback: dbus_watch, 0x560d279e4580
Removed loop callback: dbus_watch, 0x560d279e4530
Aug 31 11:54:11 holaprdel kernel: traps: teamd[557] general protection fault ip:560d26469a55 sp:7ffd43ca9650 error:0 in teamd[560d26463000+16000]
Traceback (from a different run than above):
Core was generated by `/usr/bin/teamd -o -n -U -D -N -t team0 -gg'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00005600ac090a55 in teamd_run_loop_do_callbacks (ctx=0x5600ad9bac70, fds=0x7fff40861250, lcb_list=0x5600ad9bad58) at /usr/src/debug/libteam-1.31-14.el9.x86_64/teamd/teamd.c:321
321 list_for_each_node_entry_safe(lcb, tmp, lcb_list, list) {
(gdb) bt
#0 0x00005600ac090a55 in teamd_run_loop_do_callbacks (ctx=0x5600ad9bac70, fds=0x7fff40861250, lcb_list=0x5600ad9bad58) at /usr/src/debug/libteam-1.31-14.el9.x86_64/teamd/teamd.c:321
#1 teamd_run_loop_run (ctx=0x5600ad9bac70) at /usr/src/debug/libteam-1.31-14.el9.x86_64/teamd/teamd.c:415
#2 0x00005600ac08d8cb in teamd_start (p_ret=<synthetic pointer>, ctx=<optimized out>) at /usr/src/debug/libteam-1.31-14.el9.x86_64/teamd/teamd.c:1557
#3 main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/libteam-1.31-14.el9.x86_64/teamd/teamd.c:1876
(gdb) print *lcb
Cannot access memory at address 0x9dd29944fb7e97fc
(gdb) print *tmp
Cannot access memory at address 0x9dd29944fb7e9804
(gdb)
What has happened is that libdbus called the remove_watch callback twice
in a single go, causing two callbacks being destroyed in one iteration
of teamd_run_loop_do_callbacks()'s list_for_each_node_entry_safe().
This basically turns the _safe() variant of the macro unhelpful, as tmp
points to stale data anyway.
Let's use the unsafe variant then, and terminate the loop once
teamd_loop_callback_del() asks for main loop's attention via
teamd_run_loop_restart(). If there are other callbacks pending an action,
they will get their turn in the next main loop iteration.
Signed-off-by: Lubomir Rintel <lkundrak(a)v3.sk>
---
Changes since v2:
- Break long lines in a comment (comments are apparently hard,
hope it's good this time!).
Changes since v1:
- Fixed formatting of a comment block, reported by checkpatch.pl
teamd/teamd.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/teamd/teamd.c b/teamd/teamd.c
index b310140..a89b702 100644
--- a/teamd/teamd.c
+++ b/teamd/teamd.c
@@ -309,16 +309,28 @@ static void teamd_run_loop_set_fds(struct list_item *lcb_list,
}
}
+static int teamd_check_ctrl(struct teamd_context *ctx)
+{
+ int ctrl_fd = ctx->run_loop.ctrl_pipe_r;
+ struct timeval tv;
+ fd_set rfds;
+
+ FD_ZERO(&rfds);
+ FD_SET(ctrl_fd, &rfds);
+ tv.tv_sec = tv.tv_usec = 0;
+
+ return select(ctrl_fd + 1, &rfds, NULL, NULL, &tv);
+}
+
static int teamd_run_loop_do_callbacks(struct list_item *lcb_list, fd_set *fds,
struct teamd_context *ctx)
{
struct teamd_loop_callback *lcb;
- struct teamd_loop_callback *tmp;
int i;
int events;
int err;
- list_for_each_node_entry_safe(lcb, tmp, lcb_list, list) {
+ list_for_each_node_entry(lcb, lcb_list, list) {
for (i = 0; i < 3; i++) {
if (!(lcb->fd_event & (1 << i)))
continue;
@@ -339,6 +351,16 @@ static int teamd_run_loop_do_callbacks(struct list_item *lcb_list, fd_set *fds,
teamd_log_dbg(ctx, "Failed loop callback: %s, %p",
lcb->name, lcb->priv);
}
+
+ /*
+ * If there's a control byte ready, it's possible that
+ * one or more entries have been removed from the
+ * callback list and restart has been requested. In any
+ * case, let the main loop deal with it first so that
+ * we know we're safe to proceed.
+ */
+ if (teamd_check_ctrl(ctx))
+ return 0;
}
}
return 0;
--
2.37.3
1 year
[PATCH] teamd: do no remove the ports on shutdown with -N
by Lubomir Rintel
With -N, teamd currently leaves the team device in place on shutdown,
as it's supposed to, but it removes all the ports. This severely limits
usefullness of the option, because it's still impossible to replace
the daemon with another one without disrupting connectivity.
One use case where this is important is the handover from initrd to real
root, when a team device was used to provide connectivity to a network
root filesystem:
Systemd's isolation of switch-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 not flush the ports when -N is used.
Suggested-by: Jiri Pirko <jiri(a)resnulli.us>
Signed-off-by: Lubomir Rintel <lkundrak(a)v3.sk>
---
teamd/teamd_per_port.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/teamd/teamd_per_port.c b/teamd/teamd_per_port.c
index d429753..9689df4 100644
--- a/teamd/teamd_per_port.c
+++ b/teamd/teamd_per_port.c
@@ -217,7 +217,6 @@ static void port_obj_remove(struct teamd_context *ctx,
struct teamd_port *tdport = _port(port_obj);
teamd_event_port_removed(ctx, tdport);
- teamd_port_remove(ctx, tdport);
port_obj_destroy(ctx, port_obj);
port_obj_free(port_obj);
}
--
2.37.3
1 year
[PATCH] dbus: allows root to access o.fd.DBus.Introspectable interface
by Lubomir Rintel
This is useful for, well, introspection. Before:
# busctl tree org.libteam.teamd.team0
Failed to introspect object / of service org.libteam.teamd.team0: Access denied
No objects discovered.
After:
# busctl tree org.libteam.teamd.team0
└─/org
└─/org/libteam
└─/org/libteam/teamd
[root@bzdocha libteam]# busctl introspect org.libteam.teamd.team0 /org/libteam/teamd
NAME TYPE SIGNATURE RESULT/VALUE FLAGS
org.libteam.teamd interface - - -
.ConfigDump method - - -
.ConfigDumpActual method - - -
.PortAdd method s - -
.PortConfigDump method s - -
.PortConfigUpdate method ss - -
.PortRemove method s - -
.StateDump method - - -
.StateItemValueGet method s - -
.StateItemValueSet method ss - -
---
teamd/dbus/teamd.conf | 1 +
1 file changed, 1 insertion(+)
diff --git a/teamd/dbus/teamd.conf b/teamd/dbus/teamd.conf
index 20366ea..6497a93 100644
--- a/teamd/dbus/teamd.conf
+++ b/teamd/dbus/teamd.conf
@@ -5,6 +5,7 @@
<policy user="root">
<allow own_prefix="org.libteam.teamd"/>
<allow send_interface="org.libteam.teamd"/>
+ <allow send_interface="org.freedesktop.DBus.Introspectable"/>
</policy>
<policy context="default">
<deny own_prefix="org.libteam.teamd"/>
--
2.38.1
1 year