libteam July 2018

libteam@lists.fedorahosted.org

5 participants
6 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] teamd: add port_master_ifindex_changed for teamd_event_watch_ops
by Hangbin Liu 08 Aug '18

08 Aug '18

When multi interfaces up, there is a possibility that the link states notification came later after interface up, which cause the master_ifindex not update timely. Then the teamd_port_present() checking in ab_link_watch_handler() -> teamd_for_each_tdport() -> teamd_get_next_tdport() will failed and we will not set the active port. Fix it by adding a new teamd_event_watch_ops port_master_ifindex_changed for ab mode. Reproducer: \#/bin/bash WAIT=2 COUNT=0 start_team() { num=$1 teamd -o -n -U -d -t team$num -c '{"runner": {"name": "activebackup"},"link_watch": {"name": "ethtool"}}' -gg teamdctl team$num port add eth$num } while :; do echo "-----------------------------------------------------------" let "COUNT++" echo "Loop $COUNT" if teamdctl team1 state | grep -q "active port: eth1" && \ teamdctl team2 state | grep -q "active port: eth2"; then echo "Pass" else echo "FAIL" exit 1 fi teamd -k -t team1 teamd -k -t team2 sleep "$WAIT" start_team 1 & start_team 2 & sleep "$WAIT" done Result: Usually in 10 rounds, we will get the failure, \# teamdctl team1 state setup: runner: activebackup ports: eth1 link watches: link summary: up instance[link_watch_0]: name: ethtool link: up down count: 0 runner: active port: Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com> --- teamd/teamd.h | 5 +++++ teamd/teamd_events.c | 19 +++++++++++++++++++ teamd/teamd_ifinfo_watch.c | 5 +++++ teamd/teamd_runner_activebackup.c | 8 ++++++++ 4 files changed, 37 insertions(+) diff --git a/teamd/teamd.h b/teamd/teamd.h index 5dbfb9b..3934fc2 100644 --- a/teamd/teamd.h +++ b/teamd/teamd.h @@ -189,6 +189,9 @@ struct teamd_event_watch_ops { struct teamd_port *tdport, void *priv); int (*port_ifname_changed)(struct teamd_context *ctx, struct teamd_port *tdport, void *priv); + int (*port_master_ifindex_changed)(struct teamd_context *ctx, + struct teamd_port *tdport, + void *priv); int (*option_changed)(struct teamd_context *ctx, struct team_option *option, void *priv); char *option_changed_match_name; @@ -208,6 +211,8 @@ int teamd_event_ifinfo_hwaddr_changed(struct teamd_context *ctx, struct team_ifinfo *ifinfo); int teamd_event_ifinfo_ifname_changed(struct teamd_context *ctx, struct team_ifinfo *ifinfo); +int teamd_event_ifinfo_master_ifindex_changed(struct teamd_context *ctx, + struct team_ifinfo *ifinfo); int teamd_event_ifinfo_admin_state_changed(struct teamd_context *ctx, struct team_ifinfo *ifinfo); int teamd_events_init(struct teamd_context *ctx); diff --git a/teamd/teamd_events.c b/teamd/teamd_events.c index 1a95974..65aa46a 100644 --- a/teamd/teamd_events.c +++ b/teamd/teamd_events.c @@ -167,6 +167,25 @@ int teamd_event_ifinfo_ifname_changed(struct teamd_context *ctx, return 0; } +int teamd_event_ifinfo_master_ifindex_changed(struct teamd_context *ctx, + struct team_ifinfo *ifinfo) +{ + struct event_watch_item *watch; + uint32_t ifindex = team_get_ifinfo_ifindex(ifinfo); + struct teamd_port *tdport = teamd_get_port(ctx, ifindex); + int err; + + list_for_each_node_entry(watch, &ctx->event_watch_list, list) { + if (watch->ops->port_master_ifindex_changed && tdport) { + err = watch->ops->port_master_ifindex_changed(ctx, tdport, + watch->priv); + if (err) + return err; + } + } + return 0; +} + int teamd_event_ifinfo_admin_state_changed(struct teamd_context *ctx, struct team_ifinfo *ifinfo) { diff --git a/teamd/teamd_ifinfo_watch.c b/teamd/teamd_ifinfo_watch.c index f334ff6..6a19532 100644 --- a/teamd/teamd_ifinfo_watch.c +++ b/teamd/teamd_ifinfo_watch.c @@ -59,6 +59,11 @@ static int ifinfo_change_handler_func(struct team_handle *th, void *priv, if (err) return err; } + if (team_is_ifinfo_master_ifindex_changed(ifinfo)) { + err = teamd_event_ifinfo_master_ifindex_changed(ctx, ifinfo); + if (err) + return err; + } } return 0; } diff --git a/teamd/teamd_runner_activebackup.c b/teamd/teamd_runner_activebackup.c index 8a3447f..f92d341 100644 --- a/teamd/teamd_runner_activebackup.c +++ b/teamd/teamd_runner_activebackup.c @@ -520,6 +520,13 @@ static int ab_event_watch_port_link_changed(struct teamd_context *ctx, return ab_link_watch_handler(ctx, priv); } +static int ab_event_watch_port_master_ifindex_changed(struct teamd_context *ctx, + struct teamd_port *tdport, + void *priv) +{ + return ab_link_watch_handler(ctx, priv); +} + static int ab_event_watch_prio_option_changed(struct teamd_context *ctx, struct team_option *option, void *priv) @@ -532,6 +539,7 @@ static const struct teamd_event_watch_ops ab_event_watch_ops = { .port_hwaddr_changed = ab_event_watch_port_hwaddr_changed, .port_added = ab_event_watch_port_added, .port_link_changed = ab_event_watch_port_link_changed, + .port_master_ifindex_changed = ab_event_watch_port_master_ifindex_changed, .option_changed = ab_event_watch_prio_option_changed, .option_changed_match_name = "priority", }; -- 2.5.5

3 8

[PATCHv2] utils: check to_stdout return correctly in bond2team
by Xin Long 31 Jul '18

31 Jul '18

to_stdout is a function, not a string, so fix the check on its return in bond2team. v1->v2: improve the coding style as Flavio suggested. Fixes: d5a1c8ee9e36 ("utils: add bond2team conversion tool") Signed-off-by: Xin Long <lucien.xin(a)gmail.com> --- utils/bond2team | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/utils/bond2team b/utils/bond2team index f8d46ef..fc81c4b 100755 --- a/utils/bond2team +++ b/utils/bond2team @@ -623,7 +623,7 @@ team_ifcfg_write() team_ifcfg_deliver() { pr_dbg "${FUNCNAME} $*" - if [ ! to_stdout ]; then + if ! to_stdout; then return 0 fi -- 2.1.0

3 2

[PATCH] binding/python: use SWIG_FromCharPtrAndSize for Python3 support
by Xin Long 31 Jul '18

31 Jul '18

PyString_FromStringAndSize is replaced with PyUnicode_FromStringAndSize in Python3, and SWIG_FromCharPtrAndSize will choose the right one with the check "#if PY_VERSION_HEX >= 0x0300000". Fixes: 4cb7829debd7 ("add support for hw address manipulation") Signed-off-by: Xin Long <lucien.xin(a)gmail.com> --- binding/python/team/capi.i.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/binding/python/team/capi.i.in b/binding/python/team/capi.i.in index 5ce4659..c7a5842 100644 --- a/binding/python/team/capi.i.in +++ b/binding/python/team/capi.i.in @@ -40,7 +40,7 @@ int team_hwaddr_set(struct team_handle *th, uint32_t ifindex, const char *addr, if ($1) free($1); } %typemap(argout) (char *addr, unsigned int addr_len) { - $result = SWIG_Python_AppendOutput($result, PyString_FromStringAndSize($1,$2)); + $result = SWIG_Python_AppendOutput($result, SWIG_FromCharPtrAndSize($1,$2)); } %apply char *OUTPUT {char *addr}; int team_hwaddr_get(struct team_handle *th, uint32_t ifindex, char *addr, unsigned int addr_len); -- 2.1.0

2 1

Re: [PATCH] utils: check to_stdout return correctly in bond2team
by Xin Long 26 Jul '18

26 Jul '18

On Wed, Jul 25, 2018 at 9:51 PM, Flavio Leitner <fbl(a)redhat.com> wrote: > On Wed, Jul 25, 2018 at 04:24:40PM +0800, Xin Long wrote: >> to_stdout is a function, not a string, so fix the check on >> its return in bond2team. >> >> Fixes: d5a1c8ee9e36 ("utils: add bond2team conversion tool") >> Signed-off-by: Xin Long <lucien.xin(a)gmail.com> >> --- >> utils/bond2team | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/utils/bond2team b/utils/bond2team >> index f8d46ef..1a3d7d6 100755 >> --- a/utils/bond2team >> +++ b/utils/bond2team >> @@ -623,9 +623,7 @@ team_ifcfg_write() >> team_ifcfg_deliver() >> { >> pr_dbg "${FUNCNAME} $*" >> - if [ ! to_stdout ]; then >> - return 0 >> - fi >> + to_stdout || return 0 >> >> if [ -z "${OUTPUT_DIR}" ]; then >> show_output_files > > Although that fixes the problem, it's not the coding style. > Would this work for you? Sure, will post v2. > > diff --git a/utils/bond2team b/utils/bond2team > index f8d46ef..fc81c4b 100755 > --- a/utils/bond2team > +++ b/utils/bond2team > @@ -623,7 +623,7 @@ team_ifcfg_write() > team_ifcfg_deliver() > { > pr_dbg "${FUNCNAME} $*" > - if [ ! to_stdout ]; then > + if ! to_stdout; then > return 0 > fi > > > -- > Flavio

1 0

[PATCH] utils: check to_stdout return correctly in bond2team
by Xin Long 25 Jul '18

25 Jul '18

to_stdout is a function, not a string, so fix the check on its return in bond2team. Fixes: d5a1c8ee9e36 ("utils: add bond2team conversion tool") Signed-off-by: Xin Long <lucien.xin(a)gmail.com> --- utils/bond2team | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/utils/bond2team b/utils/bond2team index f8d46ef..1a3d7d6 100755 --- a/utils/bond2team +++ b/utils/bond2team @@ -623,9 +623,7 @@ team_ifcfg_write() team_ifcfg_deliver() { pr_dbg "${FUNCNAME} $*" - if [ ! to_stdout ]; then - return 0 - fi + to_stdout || return 0 if [ -z "${OUTPUT_DIR}" ]; then show_output_files -- 2.1.0

1 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

libteam July 2018