libteam April 2018

libteam@lists.fedorahosted.org

4 participants
5 discussions

[ISSUE] Failed to find "enabled" option.
by Xin Long 16 Sep '19

16 Sep '19

I found this err in "loadbalance" and "lacp" runner when adding ports. It's caused by trying to set "enabled" option in .port_link_changed() or .port_changed(). When a new port is added, the first 'port changed event' process is earlier than CMD TEAM_CMD_OPTIONS_GET, in this CMD, all the options are synchronized from kernel. It means there's no 'enabled' option yet when calling port_link_changed in the first 'port changed event' process. In lb_event_watch_port_link_changed and lacp_event_watch_port_changed, they call teamd_port_check_enable to set 'enabled' option. this err is triggered. I'm not sure why teamd_port_check_enable needs to check if 'enabled' option exists. I checked the ab's .port_link_changed(), it just sets it by calling team_set_port_enabled(), instead of checking 'enabled' option first. can we just use team_set_port_enabled to set it directly in .port(_link)_changed OR improve teamd_port_check_enable to avoid this err ? Thanks.

3 5

[PATCH] libteam: don't crash when trying to print unregistered device name
by Antti Tiainen 03 May '18

03 May '18

team_port_str() will crash when trying to print port name that was just unregistered, if dellink event is handled before port removal event. This is regression from Commit 046fb6ba0aec ("libteam: resynchronize ifinfo after lost RTNLGRP_LINK notifications"), which made it free all removed interfaces after ifinfo handlers are called. Put the ifinfo_destroy_removed() back to dellink/newlink handlers as it was before that commit. Clean up the ifinfo list after change handlers only if it refreshed the entire ifinfo list after lost events. There's still a rare possibility that dellink event is missed due to full socket receive buffer, which would cause ifinfo refresh and clearing removed interfaces. For this, add NULL check to team_port_str() so it doesn't try to print port device name in this situation. Signed-off-by: Antti Tiainen <atiainen(a)forcepoint.com> --- include/team.h | 1 + libteam/ifinfo.c | 7 ++++++- libteam/libteam.c | 2 +- libteam/stringify.c | 3 ++- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/include/team.h b/include/team.h index 9ae517d..b31c8d8 100644 --- a/include/team.h +++ b/include/team.h @@ -223,6 +223,7 @@ enum { TEAM_PORT_CHANGE = 0x1, TEAM_OPTION_CHANGE = 0x2, TEAM_IFINFO_CHANGE = 0x4, + TEAM_IFINFO_REFRESH = 0x8, TEAM_ANY_CHANGE = TEAM_PORT_CHANGE | TEAM_OPTION_CHANGE | TEAM_IFINFO_CHANGE, diff --git a/libteam/ifinfo.c b/libteam/ifinfo.c index 5c32a9c..46d56a2 100644 --- a/libteam/ifinfo.c +++ b/libteam/ifinfo.c @@ -258,6 +258,8 @@ static void obj_input_newlink(struct nl_object *obj, void *arg, bool event) uint32_t ifindex; int err; + ifinfo_destroy_removed(th); + link = (struct rtnl_link *) obj; ifindex = rtnl_link_get_ifindex(link); @@ -294,6 +296,8 @@ static void event_handler_obj_input_dellink(struct nl_object *obj, void *arg) uint32_t ifindex; int err; + ifinfo_destroy_removed(th); + link = (struct rtnl_link *) obj; ifindex = rtnl_link_get_ifindex(link); @@ -412,7 +416,8 @@ int get_ifinfo_list(struct team_handle *th) } } - ret = check_call_change_handlers(th, TEAM_IFINFO_CHANGE); + ret = check_call_change_handlers(th, TEAM_IFINFO_CHANGE | + TEAM_IFINFO_REFRESH); if (ret < 0) err(th, "get_ifinfo_list: check_call_change_handers failed"); return ret; diff --git a/libteam/libteam.c b/libteam/libteam.c index 77a06dd..ce0467e 100644 --- a/libteam/libteam.c +++ b/libteam/libteam.c @@ -236,7 +236,7 @@ int check_call_change_handlers(struct team_handle *th, break; } } - if (call_type_mask & TEAM_IFINFO_CHANGE) { + if (call_type_mask & TEAM_IFINFO_REFRESH) { ifinfo_destroy_removed(th); ifinfo_clear_changed(th); } diff --git a/libteam/stringify.c b/libteam/stringify.c index 38f4788..f1faf90 100644 --- a/libteam/stringify.c +++ b/libteam/stringify.c @@ -344,7 +344,8 @@ static bool __team_port_str(struct team_port *port, team_is_port_removed(port) ? "-" : team_is_port_changed(port) ? "*" : " ", ifindex, - team_get_ifinfo_ifname(ifinfo), + ifinfo ? team_get_ifinfo_ifname(ifinfo) : + "(removed)", team_is_port_link_up(port) ? "up": "down", team_get_port_speed(port), team_get_port_duplex(port) ? "FD" : "HD"); -- 2.16.1

4 4

[PATCHv2] libteam: do not destroy the ifinfo of current unregistered slave dev
by Xin Long 29 Apr '18

29 Apr '18

When a dev is being unregistered and the corresp team port is being removed, the ifinfo should be delayed to destroy when doing dellink next time, because later on the dbg log still needs it to print the ifname that is saved in this ifinfo: ... <port_list> -1450: veth1: down 0Mbit HD <-- </port_list> ... The similar process is also used on the team_port's destruction. However, after Commit 046fb6ba0aec ("libteam: resynchronize ifinfo after lost RTNLGRP_LINK notifications"), it would destroy all those ifinfo with REMOVED flag, including the one of current unregistered slave dev. It would cause a crash later when printing the dbg log. An easy way to reproduce: # ip link add veth1 type veth peer name veth1_peer # sleep 5 && ip link del veth1 & # teamd -g -c '{"device":"team0","runner":{"name":"activebackup"}, "ports":{"veth1":{}}}' This patch is to check !ifinfo->port before calling ifinfo_destroy in ifinfo_destroy_removed and clear_changed in ifinfo_destroy_removed. After this fix, the ifinfo of current unregistered slave dev will not be destroyed this time until next time when processing IFINFO_CHANGE flag for another ifinfo's change in check_call_change_handlers. v1->v2: - improve the changelog, including some typos Jiri noticed. Fixes: 046fb6ba0aec ("libteam: resynchronize ifinfo after lost RTNLGRP_LINK notifications") Signed-off-by: Xin Long <lucien.xin(a)gmail.com> --- libteam/ifinfo.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libteam/ifinfo.c b/libteam/ifinfo.c index 5c32a9c..d47c2bf 100644 --- a/libteam/ifinfo.c +++ b/libteam/ifinfo.c @@ -211,7 +211,8 @@ void ifinfo_clear_changed(struct team_handle *th) struct team_ifinfo *ifinfo; list_for_each_node_entry(ifinfo, &th->ifinfo_list, list) - clear_changed(ifinfo); + if (!ifinfo->port) + clear_changed(ifinfo); } static struct team_ifinfo *ifinfo_find_create(struct team_handle *th, @@ -245,7 +246,7 @@ void ifinfo_destroy_removed(struct team_handle *th) struct team_ifinfo *ifinfo, *tmp; list_for_each_node_entry_safe(ifinfo, tmp, &th->ifinfo_list, list) { - if (is_changed(ifinfo, CHANGED_REMOVED)) + if (is_changed(ifinfo, CHANGED_REMOVED) && !ifinfo->port) ifinfo_destroy(ifinfo); } } -- 2.1.0

2 4

[PATCH] libteam: do not destroy the ifinfo of current unregistered dev enslaved into team
by Xin Long 21 Apr '18

21 Apr '18

When a dev is being unregistered and the corresp team port is being removed, the ifinfo should be delayed to destroy when doing dellink next time, because later on the dbg log still needs it to print the ifname saved in this ifinfo: ... <port_list> -1450: veth1: down 0Mbit HD <-- </port_list> ... The smilar process is also used on the team_port's destruction. However, after Commit 046fb6ba0aec ("libteam: resynchronize ifinfo after lost RTNLGRP_LINK notifications"), it would destroy all those ifinfo with REMVOED flag, including the one of current unregistered dev enslaved into team. It would cause a crash later when trying to print the dbg log. A simple way to reproduce: # ip link add veth1 type veth peer name veth1_peer # sleep 5 && ip link del veth1 & # teamd -g -c '{"device":"team0","runner":{"name":"activebackup"}, "ports":{"veth1":{}}}' This patch is to avoid it by simply checking !ifinfo->port before calling ifinfo_destroy in ifinfo_destroy_removed(), as well as in ifinfo_clear_changed(), so that the ifinfo of current unregistered dev enslaved into team will be delayed to destroy next time. Fixes: 046fb6ba0aec ("libteam: resynchronize ifinfo after lost RTNLGRP_LINK notifications") Signed-off-by: Xin Long <lucien.xin(a)gmail.com> --- libteam/ifinfo.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libteam/ifinfo.c b/libteam/ifinfo.c index 5c32a9c..d47c2bf 100644 --- a/libteam/ifinfo.c +++ b/libteam/ifinfo.c @@ -211,7 +211,8 @@ void ifinfo_clear_changed(struct team_handle *th) struct team_ifinfo *ifinfo; list_for_each_node_entry(ifinfo, &th->ifinfo_list, list) - clear_changed(ifinfo); + if (!ifinfo->port) + clear_changed(ifinfo); } static struct team_ifinfo *ifinfo_find_create(struct team_handle *th, @@ -245,7 +246,7 @@ void ifinfo_destroy_removed(struct team_handle *th) struct team_ifinfo *ifinfo, *tmp; list_for_each_node_entry_safe(ifinfo, tmp, &th->ifinfo_list, list) { - if (is_changed(ifinfo, CHANGED_REMOVED)) + if (is_changed(ifinfo, CHANGED_REMOVED) && !ifinfo->port) ifinfo_destroy(ifinfo); } } -- 2.1.0

2 2

[PATCH] configure.ac: Empty LDFLAGS before checking for libnl3
by Timothy Redaelli 19 Apr '18

19 Apr '18

Currently since CFLAGS are dropped if you have LDFLAGS=-pie (default on RHEL) the rtnl_link_get_phys_port_id, rtnl_link_set_carrier and rtnl_link_get_carrier tests always fails: /usr/bin/ld: /tmp/ccv5GdFD.o: relocation R_X86_64_PC32 against undefined symbol `rtnl_link_get_carrier@@libnl_3' can not be used when making a shared object; recompile with -fPIC This commits empty LDFLAGS before launching the 3 tests and restores it after the tests. Signed-off-by: Timothy Redaelli <tredaelli(a)redhat.com> --- configure.ac | 3 +++ 1 file changed, 3 insertions(+) diff --git a/configure.ac b/configure.ac index 60657bb..f27c15c 100644 --- a/configure.ac +++ b/configure.ac @@ -39,8 +39,10 @@ LT_INIT PKG_CHECK_MODULES([LIBNL], [libnl-3.0 libnl-genl-3.0 libnl-route-3.0 libnl-cli-3.0]) TMP_CFLAGS="$CFLAGS" + TMP_LDFLAGS="$LDFLAGS" TMP_LIBS="$LIBS" CFLAGS="$CPPFLAGS $LIBNL_CFLAGS" + LDFLAGS="" LIBS="$LIBS $LIBNL_LIBS" AC_CHECK_LIB([nl-route-3], [rtnl_link_get_phys_port_id], AC_DEFINE(HAVE_RTNL_LINK_GET_PHYS_ID, [1], [Define to 1 if you have rtnl_link_get_phys_port_id function.])) @@ -49,6 +51,7 @@ PKG_CHECK_MODULES([LIBNL], [libnl-3.0 libnl-genl-3.0 libnl-route-3.0 libnl-cli-3 AC_CHECK_LIB([nl-route-3], [rtnl_link_get_carrier], AC_DEFINE(HAVE_RTNL_LINK_GET_CARRIER, [1], [Define to 1 if you have rtnl_link_get_carrier.])) CFLAGS="$TMP_CFLAGS" + LDFLAGS="$TMP_LDFLAGS" LIBS="$TMP_LIBS" PKG_CHECK_MODULES([LIBDAEMON], [libdaemon]) -- 2.14.3

2 1

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

libteam April 2018