libteam August 2018

libteam@lists.fedorahosted.org

3 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 libteam] teamd/teamd_runner_lacp: lacp_port_set_state
by MK Lim 05 Nov '18

05 Nov '18

if new state is PORT_STATE_DEFAULTED, set lacp_port->partner.state to 0x02 This ensures LACPDU is sent to update the far side when local local port transitions from default to current Currently, local port moves from default to current without sending LACPDU until 30sec later. This causes traffic disruption because far side has not completed negotiation Using 0x02 also cause LACPDU to be sent every 1 sec as long as port remains in defaulted state and LACP mode used is "active" --- teamd/teamd_runner_lacp.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c index 7b8f0a7..555aa06 100644 --- a/teamd/teamd_runner_lacp.c +++ b/teamd/teamd_runner_lacp.c @@ -965,7 +965,12 @@ static int lacp_port_set_state(struct lacp_port *lacp_port, case PORT_STATE_DEFAULTED: teamd_loop_callback_disable(lacp_port->ctx, LACP_TIMEOUT_CB_NAME, lacp_port); - /* fall through */ + memset(&lacp_port->partner, 0, sizeof(lacp_port->partner)); + lacp_port->partner.state |= INFO_STATE_LACP_TIMEOUT; + err = lacp_port_partner_update(lacp_port); + if (err) + return err; + break; case PORT_STATE_DISABLED: memset(&lacp_port->partner, 0, sizeof(lacp_port->partner)); err = lacp_port_partner_update(lacp_port); -- 2.15.1 (Apple Git-101)

3 4

[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

Re: [patch libteam] Pass device ID through to super class init.
by Jiri Pirko 08 Aug '18

08 Aug '18

Wed, Aug 08, 2018 at 11:04:13AM CEST, Paul.D.Smith(a)metaswitch.com wrote: >Jiri, > >I following the instructions as far as, and including, using the 'git format-patch' command but my version of Git does not have the 'git send-email' command so I just cut-n-pasted the patch into an e-mail by hand. Okay. That's the problem. "git send-email" is usually distributed in a separate package. In Fedora it is in "git-email" package. > >The motivation for the change is that we found that the code just didn't work; my understanding is that regardless of which team device we were attempting to program, the superclass init() always attempted to program the '0' team device. > >I'll drop you more info next week when my colleague, who hit this when developing our code and can probably give you a more detailed answer, returns from holiday. Okay. So it is a bugfix. Yeah, please provide this info in the patch description. Also, if you find the offending commit tha actually caused this issue, it is fine to add line similar to this: Fixes: d5a1c8ee9e36 ("utils: add bond2team conversion tool") Please see "git log" and find for "Fixes" to see how is it used. Thanks! > >Regards, >Paul DS. > > >-----Original Message----- >From: Jiri Pirko <jiri(a)resnulli.us> >Sent: 08 August 2018 09:39 >To: Paul D. Smith <Paul.D.Smith(a)metaswitch.com> >Cc: libteam(a)lists.fedorahosted.org >Subject: Re: [patch libteam] Pass device ID through to super class init. > >Mon, Aug 06, 2018 at 02:46:30PM CEST, Paul.D.Smith(a)metaswitch.com wrote: >>From 34bccf0221af3f063686d58704d1255eab5f15b9 Mon Sep 17 00:00:00 2001 >>From: "Paul D.Smith" <paul.d.smith(a)metaswitch.com> >>Date: Mon, 6 Aug 2018 13:41:28 +0100 >>Subject: [patch libteam] Pass device ID through to super class init. > >These 4 lines does not supposed to be there. Odd. Did you use "git format-patch" and "git send-email"? > >Also, please provide a bit of description of the change. At least some motivation for the change. > >Thanks! > > >> >>Signed-off-by: Paul D.Smith <paul.d.smith(a)metaswitch.com> >>--- >>binding/python/team/core.py | 2 +- >>1 file changed, 1 insertion(+), 1 deletion(-) >> >>diff --git a/binding/python/team/core.py b/binding/python/team/core.py >>index 54161bf..e784930 100644 >>--- a/binding/python/team/core.py >>+++ b/binding/python/team/core.py >>@@ -348,7 +348,7 @@ class Team(TeamNetDevice): >> if not th: >> raise TeamLibError("Failed to allocate team handle.") >> >>- super(Team, self).__init__(th) >>+ super(Team, self).__init__(th, teamdev) >> >> if isinstance(teamdev, str): >> err = 0 >>-- >>2.9.2

1 0

Re: [patch libteam] Pass device ID through to super class init.
by Jiri Pirko 08 Aug '18

08 Aug '18

Mon, Aug 06, 2018 at 02:46:30PM CEST, Paul.D.Smith(a)metaswitch.com wrote: >From 34bccf0221af3f063686d58704d1255eab5f15b9 Mon Sep 17 00:00:00 2001 >From: "Paul D.Smith" <paul.d.smith(a)metaswitch.com> >Date: Mon, 6 Aug 2018 13:41:28 +0100 >Subject: [patch libteam] Pass device ID through to super class init. These 4 lines does not supposed to be there. Odd. Did you use "git format-patch" and "git send-email"? Also, please provide a bit of description of the change. At least some motivation for the change. Thanks! > >Signed-off-by: Paul D.Smith <paul.d.smith(a)metaswitch.com> >--- >binding/python/team/core.py | 2 +- >1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/binding/python/team/core.py b/binding/python/team/core.py >index 54161bf..e784930 100644 >--- a/binding/python/team/core.py >+++ b/binding/python/team/core.py >@@ -348,7 +348,7 @@ class Team(TeamNetDevice): > if not th: > raise TeamLibError("Failed to allocate team handle.") > >- super(Team, self).__init__(th) >+ super(Team, self).__init__(th, teamdev) > > if isinstance(teamdev, str): > err = 0 >-- >2.9.2

1 0

Re: [patch libteam] DEFAULTED partner state 0x02
by Jiri Pirko 08 Aug '18

08 Aug '18

Wed, Aug 08, 2018 at 10:23:11AM CEST, mengkoon(a)live.com wrote: >Hi, > > >When a port transitioned to "defaulted" state, the partner information is cleared to 0, including partner.state, and subsequently, LACPDU is only transmitted once every 30 seconds as long as port remains in default state > > >When a LACPDU arrives from the other side of the link, the local port transitions to "current", and there are 2 scenarios: > > > 1. If the received lacpdu.actor have its TIMEOUT state set, a LACPDU is transmitted, and the LACP negotiation is completed in short time. > 2. If the received lacpdu.actor does not have its TIMEOUT state set, no LACPDU is transmitted until the current 30sec interval is up. The LACP negotiation isn't completed, so the far end device port did not transition into "current" > > >Scenario 2 causes disruption to traffic teamd moved its port to "current" before the other side, and in worst case, up to 30sec earlier. > > >The patch below seems to fix it by setting the TIMEOUT on lacp_port>partner.state when port moves into "default" status. Which means the lacp_port>partner.state = 0x02 instead of 0x00 > > >This also has an effect of making teamd transmit LACPDU every 1 sec while port is in default state. > > >Or do you know if there's a better way to resolve this? > > >thank you > > >regards The format of your patch description is wrong. Please do "git log" to see how it is supposed to look. Please do wrap lines after 72 cols. > >mk > > >--- > teamd/teamd_runner_lacp.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > >diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c >index 7b8f0a7..555aa06 100644 >--- a/teamd/teamd_runner_lacp.c >+++ b/teamd/teamd_runner_lacp.c >@@ -965,7 +965,12 @@ static int lacp_port_set_state(struct lacp_port *lacp_port, > case PORT_STATE_DEFAULTED: > teamd_loop_callback_disable(lacp_port->ctx, > LACP_TIMEOUT_CB_NAME, lacp_port); >- /* fall through */ >+ memset(&lacp_port->partner, 0, sizeof(lacp_port->partner)); >+ lacp_port->partner.state |= INFO_STATE_LACP_TIMEOUT; >+ err = lacp_port_partner_update(lacp_port); >+ if (err) >+ return err; >+ break; Your email client crippled the patch. Please use "git format-patch" and "git send-email". Please see "SubmittingPatches" file for the details. > case PORT_STATE_DISABLED: > memset(&lacp_port->partner, 0, sizeof(lacp_port->partner)); > err = lacp_port_partner_update(lacp_port); >-- >

1 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

libteam August 2018