libteam September 2022

libteam@lists.fedorahosted.org

1 participants
8 discussions

Re: [patch libteam] ifindex 0 is invalid so do not process it.
by Jiri Pirko 29 Sep '22

29 Sep '22

Wed, Sep 28, 2022 at 12:00:18PM CEST, pauldsmith(a)microsoft.com wrote: >Signed-off-by: "Paul D.Smith" <pauldsmith(a)microsoft.com> > >On some platforms, libraries underpinning libteam assert that an ifindex >passed to them is not zero so an ifindex of zero cannot be passed down. >This patch alters libteam processing order to ensure that this is the >case. Placed signed off line here where it belongs, adjusted subject a bit and applied. Thanks! >--- > binding/python/team/core.py | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > >diff --git a/binding/python/team/core.py b/binding/python/team/core.py >index 54161bf..b8569f9 100644 >--- a/binding/python/team/core.py >+++ b/binding/python/team/core.py >@@ -101,7 +101,8 @@ class TeamNetDevice(object): > @ifindex.setter > def ifindex(self, ifindex): > self._ifindex = ifindex >- self.ifname = self._conv.dev_ifname(ifindex) >+ if self.ifindex: >+ self.ifname = self._conv.dev_ifname(ifindex) > > def get_hwaddr(self): > err, hwaddr = capi.team_hwaddr_get(self._th, self.ifindex, 6) >-- >2.37.3.windows.1 >

1 0

Re: [PATCH 1/2] teamd: stop iterating callbacks when a loop restart is requested
by Jiri Pirko 27 Sep '22

27 Sep '22

Wed, Aug 31, 2022 at 04:44:02PM CEST, lkundrak(a)v3.sk wrote: >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> >--- > teamd/teamd.c | 24 ++++++++++++++++++++++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > >diff --git a/teamd/teamd.c b/teamd/teamd.c >index b310140..1a6dfb3 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); Okay, I don't like the select on ctrl pipe here so much, but on the other hand, I didn't find any better solution. :) I would like to see the ctrl pipe handling only in the main loop. But cannot find how to do it in non-cumbersome way. >+} >+ > 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,14 @@ 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. */ Nit: Please follow the libteam comment format: /* * something * something */ Also, please stay within 80cols. >+ if (teamd_check_ctrl(ctx)) >+ return 0; > } > } > return 0; >-- >2.37.2 >

1 0

Re: [PATCH] teamd: Include missing headers for strrchr and memcmp
by Jiri Pirko 27 Sep '22

27 Sep '22

Tue, Sep 27, 2022 at 03:20:18AM CEST, raj.khem(a)gmail.com wrote: >Compiler does not see the prototype for these functions otherwise and >build fails e.g. > >| ../../git/teamd/teamd_phys_port_check.c:52:10: error: call to undeclared library function 'strrchr' with type 'char *(const char *, int)'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] >| start = strrchr(link, '/'); >| ^ > >Signed-off-by: Khem Raj <raj.khem(a)gmail.com> Applied, thanks!

1 0

Re: [patch libteam] ifindex 0 is invalid so do not process it.
by Jiri Pirko 27 Sep '22

27 Sep '22

Tue, Jul 05, 2022 at 10:10:55AM CEST, pauldsmith(a)microsoft.com wrote: >From 89914e67018bdb8dfed5db1f7ddf7f39c7d1a493 Mon Sep 17 00:00:00 2001 >From: "Paul D.Smith" paul.d.smith(a)metaswitch.com<mailto:paul.d.smith@metaswitch.com> >Date: Thu, 21 Oct 2021 17:28:01 +0100 >Subject: [PATCH] ifindex zero cannot identify interface. This is very odd. Next time, could you please send git-send-email? Thanks! Applied with manual changes to fix the email formatting. > >--- >binding/python/team/core.py | 3 ++- >1 file changed, 2 insertions(+), 1 deletion(-) > >On some platforms, libraries underpinning libteam assert that an ifindex passed >to them is not zero so an ifindex of zero cannot be passed down. Patch alters >libteam processing order to ensure that this is the case. > >diff --git a/binding/python/team/core.py b/binding/python/team/core.py >index 54161bf..b2fc5e8 100644 >--- a/binding/python/team/core.py >+++ b/binding/python/team/core.py >@@ -101,7 +101,8 @@ class TeamNetDevice(object): > @ifindex.setter > def ifindex(self, ifindex): > self._ifindex = ifindex >- self.ifname = self._conv.dev_ifname(ifindex) >+ if ifindex: >+ self.ifname = self._conv.dev_ifname(ifindex) > > def get_hwaddr(self): > err, hwaddr = capi.team_hwaddr_get(self._th, self.ifindex, 6) >-- >2.34.0.rc0 >

1 1

Re: [PATCH] teamd: make -NN not flush the ports
by Jiri Pirko 27 Sep '22

27 Sep '22

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_reques… > >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 >

1 0

Re: [PATCH 1/2] teamd: stop iterating callbacks when a loop restart is requested
by Jiri Pirko 27 Sep '22

27 Sep '22

Why is this [1/2] ?

1 0

[PATCH] libteamdctl: validate the bus name before using it
by Xin Long 27 Sep '22

27 Sep '22

Using bus name without validating it will cause core dump generated, and it can be reproduced by: # ip link add dummy0.1 type dummy # teamdctl dummy0.1 state dump This is normally a bug in some application using the D-Bus library. D-Bus not built with -rdynamic so unable to print a backtrace Aborted (core dumped) Doing this many times can even create too many core files, customers may complain about it. This is triggered when calling cli_method_call("ConfigDump") in cli_init(), so fix it by returning err in cli->init/cli_dbus_init() if the bus name fails to validate. Note this is safe, as with dbus, we can't use invalid dbus name to create the team dev either. Fixes: d8163e34c25c ("libteamdctl: do test method call instead or Introspect call") Reported-by: Uday Patel <upatel(a)redhat.com> Signed-off-by: Xin Long <lucien.xin(a)gmail.com> --- libteamdctl/cli_dbus.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/libteamdctl/cli_dbus.c b/libteamdctl/cli_dbus.c index dfef5c4..242ef86 100644 --- a/libteamdctl/cli_dbus.c +++ b/libteamdctl/cli_dbus.c @@ -183,12 +183,17 @@ static int cli_dbus_init(struct teamdctl *tdc, const char *team_name, void *priv if (ret == -1) return -errno; + err = -EINVAL; dbus_error_init(&error); + if (!dbus_validate_bus_name(cli_dbus->service_name, &error)) { + err(tdc, "dbus: Could not validate bus name: %s - %s", + error.name, error.message); + goto free_service_name; + } cli_dbus->conn = dbus_bus_get(DBUS_BUS_SYSTEM, &error); if (!cli_dbus->conn) { err(tdc, "dbus: Could not acquire the system bus: %s - %s", error.name, error.message); - err = -EINVAL; goto free_service_name; } err = 0; -- 2.31.1

2 4

Re: Release anytime soon?
by Jiri Pirko 27 Sep '22

27 Sep '22

Mon, Jun 27, 2022 at 07:16:36PM CEST, janos2010(a)farkas.ch wrote: >There was no patch needed for the integration on the libteam side. I've >been testing the built package for several months now, but it's unlikely to >be accepted as a package from the git master branch. > >The concern was that Void prefers an actual upstream release, with active >development, and a versioned tarball as a base point, and to avoid >requiring an internal maintainer. An ideal Void packaging is just a >template to integrity check an upstream tarball, a specification of >build//runtime dependencies for the given package, and preferably no >integration patches. > >1.31 may have worked for the basic functionality, but for all I can see, >most of the commits merged into the master branch seem essential. At least >to avoid the daemon using 100% CPU seems critical enough. > >Or perhaps I'm misunderstanding and you would accept a patch to bump the >version and publish 1.32 as a release? Yes. > >Janos > >On Mon, Jun 27, 2022 at 5:45 PM Jiri Pirko <jiri(a)resnulli.us> wrote: > >> Wed, Oct 06, 2021 at 08:11:55PM CEST, chexum+fedora(a)gmail.com wrote: >> >Hi! >> > >> >I'm using the latest code from github, and I would also like to have it >> packaged for a distribution which does not have libteam yet - >> https://github.com/void-linux/void-packages/pull/33028 >> >> Please send a patch. Pull requests are ignored. See file SubmittingPatches >> >> > >> >They are making a point that it's preferable using a proper upstream >> release. As far as I can see, the commits since 1.31 appear somewhat >> important. On the other hand, even the latest github changes are somewhat >> old, so I can't see anything unstable about it. >> > >> >For the benefit of a cleaner packaging, are there any plans to make a >> numbered release anytime soon? >> > >> >Janos >> >_______________________________________________ >> >libteam mailing list -- libteam(a)lists.fedorahosted.org >> >To unsubscribe send an email to libteam-leave(a)lists.fedorahosted.org >> >Fedora Code of Conduct: >> https://docs.fedoraproject.org/en-US/project/code-of-conduct/ >> >List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines >> >List Archives: >> https://lists.fedorahosted.org/archives/list/libteam@lists.fedorahosted.org >> >Do not reply to spam on the list, report it: >> https://pagure.io/fedora-infrastructure >>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

libteam September 2022