libteam July 2019

libteam@lists.fedorahosted.org

5 participants
10 discussions

spammy log messages with loadbalance runner
by Daniel Arena 25 Oct '19

25 Oct '19

Hello, I have a loadbalance runner for 2 interfaces setup through NetworkManager on fedora 30. I noticed in the system journal the teamd messages have been quite spammy with stats updates/rebalance information from teamd. See example log lines below. I tried looking at `man teamd.conf` but only found the debug_level setting, which is defaulted to 0. Is there some other way to turn these messages off? I tried adjusting NetworkManager's log levels also, but even after shutting off all logging, the messages from teamd still get through. Is this actually an issue for NetworkManager, maybe it should be capturing these log messages and filtering them with NetworkManager's log levels? My teamd config looks like this: {"runner": { "name": "loadbalance", "tx_hash": ["eth","ipv4", "ipv6"], "tx_balancer": {"name": "basic"} }, "ports":{"eno1": {}, "eno2": {}} } Jul 03 15:41:57 server1 teamd_nm-team[32311]: stats update for port eno2: "3257841132330". Jul 03 15:41:57 server1 teamd_nm-team[32311]: stats update for port eno1: "6585330648". Jul 03 15:41:57 server1 teamd_nm-team[32311]: stats update for hash "21": "1242456011". Jul 03 15:41:57 server1 teamd_nm-team[32311]: stats update for hash "16": "1270449453". Jul 03 15:41:57 server1 teamd_nm-team[32311]: stats update for hash "9": "3761543798". Jul 03 15:41:57 server1 teamd_nm-team[32311]: stats update for hash "1": "626647946". Jul 03 15:41:57 server1 teamd_nm-team[32311]: Remapped hash "21" (delta 447647) to port eno2. Jul 03 15:41:57 server1 teamd_nm-team[32311]: Remapped hash "16" (delta 5301) to port eno1. Jul 03 15:41:57 server1 teamd_nm-team[32311]: Port eno2 rebalanced, delta: 447647 Jul 03 15:41:57 server1 teamd_nm-team[32311]: Port eno1 rebalanced, delta: 6205 Thanks, Dan

4 7

[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] man teamd.conf: update some parameter default values
by Hangbin Liu 30 Jul '19

30 Jul '19

Update default runner.name to roundrobin, default runner.tx_hash array to ["eth", "ipv4", "ipv6"], default runner.fast_rate to false. Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com> --- man/teamd.conf.5 | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/man/teamd.conf.5 b/man/teamd.conf.5 index 9090b4a..350ffc9 100644 --- a/man/teamd.conf.5 +++ b/man/teamd.conf.5 @@ -42,6 +42,9 @@ To do passive load balancing, runner only sets up BPF hash function which will d .PP .BR "lacp "\(em Implements 802.3ad LACP protocol. Can use same Tx port selection possibilities as loadbalance runner. +.PP +Default: +.BR "roundrobin" .RE .TP .BR "notify_peers.count " (int) @@ -182,6 +185,10 @@ Uses source and destination SCTP ports. .PP .BR "l4 "\(em Uses source and destination TCP and UDP and SCTP ports. +.PP +Default: +.B +["eth", "ipv4", "ipv6"] .RE .TP .BR "runner.tx_balancer.name " (string) @@ -217,6 +224,11 @@ Default: Option specifies the rate at which our link partner is asked to transmit LACPDU packets. If this is .BR "true" then packets will be sent once per second. Otherwise they will be sent every 30 seconds. +.RS 7 +.PP +Default: +.BR "false" +.RE .TP .BR "runner.tx_hash " (array) Same as for load balance runner. -- 2.19.2

2 1

[PATCH] teamd: improve the error output for non-integer port prio
by Xin Long 02 Jul '19

02 Jul '19

This patch is to improve the error log when users pass a non-integer value to set port's prio. After that, we can remove the error output for teamd_config_port_set failure from teamd_config_port_update. Signed-off-by: Xin Long <lucien.xin(a)gmail.com> --- teamd/teamd_config.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/teamd/teamd_config.c b/teamd/teamd_config.c index 1bf85ac..610ad5f 100644 --- a/teamd/teamd_config.c +++ b/teamd/teamd_config.c @@ -167,17 +167,19 @@ static int teamd_config_port_set(struct teamd_context *ctx, const char *port_nam return 0; config = json_object_get(port_obj, "prio"); - if (json_is_integer(config)) { - tmp = json_integer_value(config); - err = team_set_port_priority(ctx->th, tdport->ifindex, tmp); - if (err) { - teamd_log_err("%s: Failed to set \"priority\".", - tdport->ifname); - return err; - } + if (!json_is_integer(config)) { + teamd_log_err("%s: Failed to get integer for \"priority\".", + tdport->ifname); + return -ENOENT; } - return 0; + tmp = json_integer_value(config); + err = team_set_port_priority(ctx->th, tdport->ifindex, tmp); + if (err) + teamd_log_err("%s: Failed to update \"priority\" to kernel", + tdport->ifname); + + return err; } int teamd_config_port_update(struct teamd_context *ctx, const char *port_name, @@ -206,16 +208,14 @@ int teamd_config_port_update(struct teamd_context *ctx, const char *port_name, /* replace existing object content */ json_object_clear(port_obj); err = json_object_update(port_obj, port_new_obj); - if (err) + if (err) { teamd_log_err("%s: Failed to update existing config " "port object", port_name); - else { - err = teamd_config_port_set(ctx, port_name, port_new_obj); - if (err) - teamd_log_err("%s: Failed to update config to kernel", - port_name); + goto new_port_decref; } + err = teamd_config_port_set(ctx, port_name, port_new_obj); + new_port_decref: json_decref(port_new_obj); return err; -- 2.1.0

2 1

[PATCHv2] teamd: return 0 if tdport doesn't exist in teamd_config_port_set
by Xin Long 02 Jul '19

02 Jul '19

2 1

[PATCH] teamd: add port_master_ifindex_changed for link_watch_port_watch_ops
by Xin Long 02 Jul '19

02 Jul '19

This is a follow up of Commit b6f63db7f3c8 ("teamd: add port_master_ifindex_changed for teamd_event_watch_ops") to fix the same issue in it. The issue is a race between option change event and ifinfo (master_ifindex) change event, so wherever a option change function tries to iterate tdport by teamd_for_each_tdport, the same thing should be done in its master_ifindex change function, including link_watch_port_watch_ops. The fix has been verified in one customer's env where it appeared no arp requests coming out on the only state-up tdport prior to this patch. Reported-by: Michal Tesar <mtesar(a)redhat.com> Fixes: b6f63db7f3c8 ("teamd: add port_master_ifindex_changed for teamd_event_watch_ops") Signed-off-by: Xin Long <lucien.xin(a)gmail.com> --- teamd/teamd_link_watch.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/teamd/teamd_link_watch.c b/teamd/teamd_link_watch.c index 019ec5d..62f8267 100644 --- a/teamd/teamd_link_watch.c +++ b/teamd/teamd_link_watch.c @@ -450,10 +450,19 @@ static int link_watch_enabled_option_changed(struct teamd_context *ctx, return link_watch_refresh_forced_send(ctx); } + +static int link_watch_port_master_ifindex_changed(struct teamd_context *ctx, + struct teamd_port *tdport, + void *priv) +{ + return link_watch_refresh_forced_send(ctx); +} + static const struct teamd_event_watch_ops link_watch_port_watch_ops = { .port_added = link_watch_event_watch_port_added, .port_removed = link_watch_event_watch_port_removed, .port_link_changed = link_watch_event_watch_port_link_changed, + .port_master_ifindex_changed = link_watch_port_master_ifindex_changed, .option_changed = link_watch_enabled_option_changed, .option_changed_match_name = "enabled", }; -- 2.1.0

2 1

[patch libteam] initscripts: fix if/fi align
by Hangbin Liu 02 Jul '19

02 Jul '19

Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com> --- teamd/redhat/initscripts/network-scripts/ifup-Team | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/teamd/redhat/initscripts/network-scripts/ifup-Team b/teamd/redhat/initscripts/network-scripts/ifup-Team index a245dac..1e38b8f 100755 --- a/teamd/redhat/initscripts/network-scripts/ifup-Team +++ b/teamd/redhat/initscripts/network-scripts/ifup-Team @@ -36,7 +36,7 @@ if [ -n "${TEAM_CONFIG}" ]; then if [ ! -x /usr/bin/teamd ]; then net_log $"Team support not available: teamd not found" exit 1 - fi + fi /usr/bin/teamd ${TEAMD_CMDLINE} -d -D -t ${DEVICE} -c "${TEAM_CONFIG}" -p /var/run/teamd-${DEVICE}.pid || exit 1 # Bring up all existing port devices now for device in $(LANG=C egrep -l "^[[:space:]]*TEAM_MASTER=\"?${DEVICE}\"?" /etc/sysconfig/network-scripts/ifcfg-*) ; do -- 2.19.2

2 1

[patch libteam] initscripts: need add the port before update config
by Hangbin Liu 02 Jul '19

02 Jul '19

After c8b356a3cd36 ("teamd: config: update local prio to kernel"), we update team port prio not only on local, but also on kernel side. So we need to add the port to team first before update TEAM_PORT_CONFIG. Or the teamd_config_port_update() would be failed because we could not find the port in teamd_get_port_by_ifname() during teamd_config_port_set(). Reported-by: LiLiang <liali(a)redhat.com> Fixes: c8b356a3cd36 ("teamd: config: update local prio to kernel") Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com> --- teamd/redhat/initscripts/network-scripts/ifup-TeamPort | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/teamd/redhat/initscripts/network-scripts/ifup-TeamPort b/teamd/redhat/initscripts/network-scripts/ifup-TeamPort index 8c3cc3a..954b47a 100755 --- a/teamd/redhat/initscripts/network-scripts/ifup-TeamPort +++ b/teamd/redhat/initscripts/network-scripts/ifup-TeamPort @@ -51,8 +51,8 @@ if [ -n "${TEAM_MASTER}" ]; then exit 0 fi /sbin/ip link set dev ${DEVICE} down + /usr/bin/teamdctl ${TEAM_MASTER} port add ${DEVICE} || exit 1 if [ -n "${TEAM_PORT_CONFIG}" ]; then /usr/bin/teamdctl ${TEAM_MASTER} port config update ${DEVICE} "${TEAM_PORT_CONFIG}" || exit 1 fi - /usr/bin/teamdctl ${TEAM_MASTER} port add ${DEVICE} || exit 1 fi -- 2.19.2

3 2

[PATCH] teamd: return 0 if tdport doesn't exist in teamd_config_port_set
by Xin Long 02 Jul '19

02 Jul '19

This issue can be reproduced by doing: # ip link add dummy1 type dummy # teamd -t team0 -c '{"runner": {"name": "activebackup"}}' -d # teamdctl team0 port config update dummy1 '{"prio": -10}' # ip link set dummy0 master team0 and the error shows: libteamdctl: usock: Error message received: "ConfigUpdateFail" libteamdctl: usock: Error message content: "Failed to update config." command call failed (Invalid argument) It's a regression caused by Commit c8b356a3cd36 ("teamd: config: update local prio to kernel") where it requires the tdport has to exist when a tdport config is being updated. However teamd supports for the port config going first before the port being enslaved. This issue breaks how NM-team starts a team device. Here to fix it by returning 0 even if the tdport doesn't exist in teamd_config_port_set. While at it, also improve some error log and code type. Reported-by: Radek Vykydal <rvykydal(a)redhat.com> Fixes: c8b356a3cd36 ("teamd: config: update local prio to kernel") Signed-off-by: Xin Long <lucien.xin(a)gmail.com> --- teamd/teamd_config.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/teamd/teamd_config.c b/teamd/teamd_config.c index 34fef1f..610ad5f 100644 --- a/teamd/teamd_config.c +++ b/teamd/teamd_config.c @@ -164,20 +164,22 @@ static int teamd_config_port_set(struct teamd_context *ctx, const char *port_nam tdport = teamd_get_port_by_ifname(ctx, port_name); if (!tdport) - return -ENODEV; + return 0; config = json_object_get(port_obj, "prio"); - if (json_is_integer(config)) { - tmp = json_integer_value(config); - err = team_set_port_priority(ctx->th, tdport->ifindex, tmp); - if (err) { - teamd_log_err("%s: Failed to set \"priority\".", - tdport->ifname); - return err; - } + if (!json_is_integer(config)) { + teamd_log_err("%s: Failed to get integer for \"priority\".", + tdport->ifname); + return -ENOENT; } - return 0; + tmp = json_integer_value(config); + err = team_set_port_priority(ctx->th, tdport->ifindex, tmp); + if (err) + teamd_log_err("%s: Failed to update \"priority\" to kernel", + tdport->ifname); + + return err; } int teamd_config_port_update(struct teamd_context *ctx, const char *port_name, @@ -206,16 +208,14 @@ int teamd_config_port_update(struct teamd_context *ctx, const char *port_name, /* replace existing object content */ json_object_clear(port_obj); err = json_object_update(port_obj, port_new_obj); - if (err) + if (err) { teamd_log_err("%s: Failed to update existing config " "port object", port_name); - else { - err = teamd_config_port_set(ctx, port_name, port_new_obj); - if (err) - teamd_log_err("%s: Failed to update config to kernel", - port_name); + goto new_port_decref; } + err = teamd_config_port_set(ctx, port_name, port_new_obj); + new_port_decref: json_decref(port_new_obj); return err; -- 2.1.0

2 1

[PATCH] teamd: fix a json object memleak in get_port_obj()
by Xin Long 02 Jul '19

02 Jul '19

The issue can be noticed simply by doing: # ip link add aeth0 type dummy # teamd -d -t ateam0 -c '{"link_watch" : {"name" : "ethtool"}}' # teamdctl ateam0 port add aeth0 # while true; do # teamdctl ateam0 config dump actual > /dev/null # date "+%Y-%m-%dT%H:%M:%S $(ps -auwwx | grep teamd |grep -v grep)" # done After 30 mins: USER PID %CPU %MEM VSZ ... COMMAND 2019-06-06T06:25:55 root 26574 0.0 0.0 66572 ... teamd ... ... 2019-06-06T06:55:55 root 26574 1.2 0.6 89012 ... teamd ... MEM used by team grew from 0.0% to 0.6%, VSZ from 66572 to 89012. get_port_obj() calld in teamd_config_actual_dump() should not have used json_object_set(), which can json_incref the child object causing later json_decref(parent object) not to be able to free it. So change to use json_object_set_new(), a function for setting newly created objects, and it won't json_incref the child object that will be freed when its parent object gets freed later. Fixes: c6eba4866e18 ("teamd: add possibility to add/change port config") Reported-by: Michael Colombo <mcolombo(a)redhat.com> Signed-off-by: Xin Long <lucien.xin(a)gmail.com> --- teamd/teamd_config.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/teamd/teamd_config.c b/teamd/teamd_config.c index 69b25de..34fef1f 100644 --- a/teamd/teamd_config.c +++ b/teamd/teamd_config.c @@ -84,7 +84,7 @@ static int get_port_obj(json_t **pport_obj, json_t *config_json, ports_obj = json_object(); if (!ports_obj) return -ENOMEM; - err = json_object_set(config_json, "ports", ports_obj); + err = json_object_set_new(config_json, "ports", ports_obj); if (err) { json_decref(ports_obj); return -ENOMEM; @@ -95,7 +95,7 @@ static int get_port_obj(json_t **pport_obj, json_t *config_json, port_obj = json_object(); if (!port_obj) return -ENOMEM; - err = json_object_set(ports_obj, port_name, port_obj); + err = json_object_set_new(ports_obj, port_name, port_obj); if (err) { json_decref(port_obj); return -ENOMEM; -- 2.1.0

2 1

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

libteam July 2019