[ISSUE] Failed to find "enabled" option.
by Xin Long
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.
4 years, 7 months
[PATCH] libteam: don't crash when trying to print unregistered device
name
by Antti Tiainen
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
5 years, 12 months
[PATCHv2] libteam: do not destroy the ifinfo of current unregistered slave dev
by Xin Long
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
5 years, 12 months
[PATCH] libteam: do not destroy the ifinfo of current unregistered dev enslaved into team
by Xin Long
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
6 years
[PATCH] configure.ac: Empty LDFLAGS before checking for libnl3
by Timothy Redaelli
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
6 years