libteam March 2019

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: add a default value 100 for link_watch.interval
by Hangbin Liu 17 Apr '19

17 Apr '19

As we don't have a default value for link_watch.interval. If user forgot to set this parameter, teamd will failed to init port priv and exit at the end. e.g. teamd -g -c '{"runner":{"name":"activebackup"},"link_watch":{"name":"arp_ping","target_host":"198.51.100.1"}}' teamdctl team0 port add p5p1 teamdctl team0 port add p5p2 teamd debug log shows: p5p2: Got link watch from global config. p5p2: Using sticky "0". Failed to get "interval" link-watch option. Failed to load options. Failed to init port priv. Callback named "lw_periodic" not found. Callback named "lw_socket" not found. Loop callback failed with: Invalid argument Failed loop callback: libteam_events, 0x5624c28b9410 select() failed. Exiting... Removed loop callback: usock_acc_conn, 0x5624c28bab60 Removed loop callback: usock, 0x5624c28b9410 Removed loop callback: workq, 0x5624c28b9410 Removed loop callback: libteam_events, 0x5624c28b9410 Removed loop callback: daemon, 0x5624c28b9410 Failed: Bad file descriptor Fix it by adding a default value for link_watch.interval. Reported-by: LiLiang <liali(a)redhat.com> Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com> --- man/teamd.conf.5 | 10 ++++++++++ teamd/teamd_lw_psr.c | 11 ++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/man/teamd.conf.5 b/man/teamd.conf.5 index 9bdf46a..6d5374a 100644 --- a/man/teamd.conf.5 +++ b/man/teamd.conf.5 @@ -308,6 +308,11 @@ Default: .TP .BR "link_watch.interval "| " ports.PORTIFNAME.link_watch.interval " (int) Value is a positive number in milliseconds. It is the interval between ARP requests being sent. +.RS 7 +.PP +Default: +.BR "100" +.RE .TP .BR "link_watch.init_wait "| " ports.PORTIFNAME.link_watch.init_wait " (int) Value is a positive number in milliseconds. It is the delay between link watch initialization and the first ARP request being sent. @@ -371,6 +376,11 @@ Default: .TP .BR "link_watch.interval "| " ports.PORTIFNAME.link_watch.interval " (int) Value is a positive number in milliseconds. It is the interval between sending NS packets. +.RS 7 +.PP +Default: +.BR "100" +.RE .TP .BR "link_watch.init_wait "| " ports.PORTIFNAME.link_watch.init_wait " (int) Value is a positive number in milliseconds. It is the delay between link watch initialization and the first NS packet being sent. diff --git a/teamd/teamd_lw_psr.c b/teamd/teamd_lw_psr.c index c0772db..3ddf295 100644 --- a/teamd/teamd_lw_psr.c +++ b/teamd/teamd_lw_psr.c @@ -29,6 +29,7 @@ static const struct timespec lw_psr_default_init_wait = { 0, 1 }; #define LW_PSR_DEFAULT_MISSED_MAX 3 +#define LW_PSR_DEFAULT_INTERVAL 100 #define LW_PERIODIC_CB_NAME "lw_periodic" static int lw_psr_callback_periodic(struct teamd_context *ctx, int events, void *priv) @@ -77,9 +78,13 @@ static int lw_psr_load_options(struct teamd_context *ctx, int tmp; err = teamd_config_int_get(ctx, &tmp, "@.interval", cpcookie); - if (err) { - teamd_log_err("Failed to get \"interval\" link-watch option."); - return -EINVAL; + if (!err) { + if (tmp < 0) { + teamd_log_err("\"interval\" must not be negative number."); + return -EINVAL; + } + } else { + tmp = LW_PSR_DEFAULT_INTERVAL; } teamd_log_dbg("interval \"%d\".", tmp); ms_to_timespec(&psr_ppriv->interval, tmp); -- 2.19.2

3 11

[PATCH] teamd: remove port if adding fails
by Hangbin Liu 17 Apr '19

17 Apr '19

When we add a port to team via teamdctl, some drivers do not support changing mac address after dev opened, which would lead to the failure of port_obj_create(). Currently we only destroy the port object if adding port fails. But the port is still enslaved to team in kernel. The interface shows it's a team_slave via ip link cmd, but teamdctl state shows nothing. This may make users confused. Fix it by removing the port if adding fails. Reported-by: Vladimir Benes <vbenes(a)redhat.com> Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com> --- teamd/teamd_per_port.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/teamd/teamd_per_port.c b/teamd/teamd_per_port.c index 09d1dc7..7f0a45d 100644 --- a/teamd/teamd_per_port.c +++ b/teamd/teamd_per_port.c @@ -42,6 +42,8 @@ struct port_obj { }; #define _port(port_obj) (&(port_obj)->port) +static int teamd_port_remove(struct teamd_context *ctx, + struct teamd_port *tdport); int teamd_port_priv_create_and_get(void **ppriv, struct teamd_port *tdport, const struct teamd_port_priv *pp, @@ -203,6 +205,7 @@ static int port_obj_create(struct teamd_context *ctx, teamd_event_port_removed: teamd_event_port_removed(ctx, tdport); list_del: + teamd_port_remove(ctx, tdport); port_obj_destroy(ctx, port_obj); port_obj_free(port_obj); return err; -- 2.19.2

2 5

[PATCH] teamd: lacp: update port state according to partner's sync bit
by Hangbin Liu 08 Mar '19

08 Mar '19

1) According to 6.4.12 of IEEE 802.1AX-2008, Figure 6-18, if the partner's sync bit is not set, the lacp port state should be set to EXPIRED. Function lacpdu_recv() makes the state of the lacp port to PORT_STATE_CURRENT when the teamd receives any valid lacp packet. Fix it by checking the parter's sync bit. 2) According to 6.4.15 of IEEE 802.1AX-2008, Figure 6-22, the state that the port is selected moves MUX state from DETACHED to ATTACHED. But ATTACHED sate does not mean that the port can send and receive user frames. COLLECTING_DISTRIBUTION state is the sate that the port can send and receive user frames. To move MUX state from ATTACHED to COLLECTING_DISTRIBUTION, the partner state should be sync as well as the port selected. In function lacp_port_actor_update(), only INFO_STATE_SYNCHRONIZATION should be set to the actor.state when the port is selected. INFO_STATE_COLLECTING and INFO_STATE_DISTRIBUTING should be set to false with ATTACHED mode and set to true when INFO_STATE_SYNCHRONIZATION of partner.state is set. In function lacp_port_should_be_{enabled, disabled}(), we also need to check the INFO_STATE_SYNCHRONIZATION bit of partner.state. Reported-by: Cisco Systems Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com> --- teamd/teamd_runner_lacp.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c index 7b8f0a7..5e763ad 100644 --- a/teamd/teamd_runner_lacp.c +++ b/teamd/teamd_runner_lacp.c @@ -333,7 +333,8 @@ static int lacp_port_should_be_enabled(struct lacp_port *lacp_port) struct lacp *lacp = lacp_port->lacp; if (lacp_port_selected(lacp_port) && - lacp_port->agg_lead == lacp->selected_agg_lead) + lacp_port->agg_lead == lacp->selected_agg_lead && + lacp_port->partner.state & INFO_STATE_SYNCHRONIZATION) return true; return false; } @@ -343,7 +344,8 @@ static int lacp_port_should_be_disabled(struct lacp_port *lacp_port) struct lacp *lacp = lacp_port->lacp; if (!lacp_port_selected(lacp_port) || - lacp_port->agg_lead != lacp->selected_agg_lead) + lacp_port->agg_lead != lacp->selected_agg_lead || + !(lacp_port->partner.state & INFO_STATE_SYNCHRONIZATION)) return true; return false; } @@ -914,9 +916,13 @@ static void lacp_port_actor_update(struct lacp_port *lacp_port) if (lacp_port->lacp->cfg.fast_rate) state |= INFO_STATE_LACP_TIMEOUT; if (lacp_port_selected(lacp_port) && - lacp_port_agg_selected(lacp_port)) - state |= INFO_STATE_SYNCHRONIZATION | - INFO_STATE_COLLECTING | INFO_STATE_DISTRIBUTING; + lacp_port_agg_selected(lacp_port)) { + state |= INFO_STATE_SYNCHRONIZATION; + state &= ~(INFO_STATE_COLLECTING | INFO_STATE_DISTRIBUTING); + if (lacp_port->partner.state & INFO_STATE_SYNCHRONIZATION) + state |= INFO_STATE_COLLECTING | + INFO_STATE_DISTRIBUTING; + } if (lacp_port->state == PORT_STATE_EXPIRED) state |= INFO_STATE_EXPIRED; if (lacp_port->state == PORT_STATE_DEFAULTED) @@ -1095,7 +1101,10 @@ static int lacpdu_recv(struct lacp_port *lacp_port) return err; } - err = lacp_port_set_state(lacp_port, PORT_STATE_CURRENT); + if (lacp_port->partner.state & INFO_STATE_SYNCHRONIZATION) + err = lacp_port_set_state(lacp_port, PORT_STATE_CURRENT); + else + err = lacp_port_set_state(lacp_port, PORT_STATE_EXPIRED); if (err) return err; -- 2.19.2

2 8

[patch libteam] man: fix runner.min_ports default value
by Hangbin Liu 08 Mar '19

08 Mar '19

It should be 1 instead of 0. Reported-by: LiLiang <liali(a)redhat.com> Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com> --- man/teamd.conf.5 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/teamd.conf.5 b/man/teamd.conf.5 index 9bdf46a..5b0f3e9 100644 --- a/man/teamd.conf.5 +++ b/man/teamd.conf.5 @@ -240,7 +240,7 @@ Specifies the minimum number of ports that must be active before asserting carri .RS 7 .PP Default: -.BR "0" +.BR "1" .RE .TP .BR "runner.agg_select_policy " (string) -- 2.19.2

2 1

[patch libteam] teamd: lw: nsna_ping: only send ns on enabled port
by Hangbin Liu 08 Mar '19

08 Mar '19

We forget to check forced_send when using nsna_ping link_watch. Ns is sent from all ports, which cause switch mac flapping. Some reply packets are delivered to disabled port and dropped directly. Fix it by checking forced_send and only send ns on enabled port. Reported-by: LiLiang <liali(a)redhat.com> Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com> --- teamd/teamd_lw_nsna_ping.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/teamd/teamd_lw_nsna_ping.c b/teamd/teamd_lw_nsna_ping.c index 127d950..82371c4 100644 --- a/teamd/teamd_lw_nsna_ping.c +++ b/teamd/teamd_lw_nsna_ping.c @@ -203,6 +203,9 @@ static int lw_nsnap_send(struct lw_psr_port_priv *psr_ppriv) struct sockaddr_in6 sendto_addr; struct ns_packet nsp; + if (!(psr_ppriv->common.forced_send)) + return 0; + err = teamd_getsockname_hwaddr(psr_ppriv->sock, &ll_my, sizeof(nsp.hwaddr)); if (err) -- 2.19.2

2 1

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

libteam March 2019