libteam February 2019

libteam@lists.fedorahosted.org

1 participants
7 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: 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

Re: A question about team port enable/disable
by Hangbin Liu 21 Feb '19

21 Feb '19

Hi Jiri, On Thu, Feb 21, 2019 at 09:44:07AM +0100, Jiri Pirko wrote: > Thu, Feb 21, 2019 at 09:26:17AM CET, liuhangbin(a)gmail.com wrote: > >> Just confirmed the issue is not cause by this code. I will give an update after get more info. > >> Sorry to bother you. > >> > > > >Update: This issue was caused by teaming PFs and VFs . > > > >The customer has a topology like > > > >eth0 (PF link that's running LACP) > > --> vf0 (VF that use loadbalance mode) > >eth1 (physical link that's running LACP) > > --> vf1 (VF that use loadbalance mode) > > > >After eth0/vf0 down and up. eth0 enters lacp negotiate mode and not > >active yet. But vf0 is up immediately. > >So vf0 will be active(cause it use loadbalance mode) and starts > >sending packets. In the end, these packets > >are sent from eth0 as it's PF device. > > It is unclear to me how your topology looks exactly. Anyway, if you run > lacp, you don't really care about physical link up/down. So that should > be okay for the vf, even if it gets link up. Sorry I didn't make it clear. Hope this topo helps. Host vf0 -- team1 -- vf1 | | eth0 -- team0 -- eth1 | | --|----------------|------ switch eth0 and eth1 are two PF devices, with team0(LACP mode) on them. vf0 and vf1 are two VF devices, with team1(loadbalance mode) on them. Here is the process: 1) The switch port eth0 conected down/unplugged. Then eth0/vf0 also down. 2) eth1/vf1 enabled and start to send/reply messages. 3) After a while, the switch port up/plugged. eth0/vf0 up at the same time. 4) eth0 goto lacp negotiate state 5) vf0 up and enabled directly as it use loadbalance mode 6) vf0 start to reply packages, while eth0 and the switch port are still in lacp negotiation. So the switch port is not ready to forward packets. The packets send from vf0 are dropped. > > > > >To avoid vf0 active before eth0 finishing LACP negotiation. The > >customer use 'teamnl -p eth0 team0 getoption enabled' to detect > >eth0's state and setup vf0 manually. This method works. But I'm not > > Setup how? I don't understand the scenario, sorry :/ Could you attach > some configs or something? I dont have the script files. From the info they supply, I guess it looks like: ``` #/bin/bash while true; do teamnl -p eth0 team0 getoption enabled && break || continue done teamnl team1 setoption activeport 4 ``` If you need, I can ask for the script they use. Thanks Hangbin

1 0

Re: A question about team port enable/disable
by Jiri Pirko 21 Feb '19

21 Feb '19

Sat, Dec 15, 2018 at 11:45:25AM CET, yanmiaobest(a)gmail.com wrote: >Hi Jiri > >This is Miao, I am currently working on a project to implement a new team policy >based on the kernel team support. Basically, I have lots of 'ports', >each of them >has a ID. I'd like to use the ID to calculate the hash and distribute >the traffic to >different team slave ports, similar to round-robin. > >And when looking into team code, I see the team kernel driver has >the following comments: > >/* > * Enable/disable port by adding to enabled port hashlist and setting > * port->index (Might be racy so reader could see incorrect ifindex when > * processing a flying packet, but that is not a problem). Write guarded > * by team->lock. > */ > >The team datapath runs lockless, and libteam can enable/disble port >one the fly when traffic happens. For exmaple, I have 4 port and I >want, port0->slave0, >port1->slave1, port2->slave2 and port3->slave3, when slave1 device is link >down state, it becomes port0->slave0, port1->slave2, port2->slave3, >port3->slave0, >using the ID%num as the hash function. > >But it seems if datapath runs lockless, it could see the wrong ifindex during >configuration as stated in the comment because the team drvier needs to >reconstruct the active ports list. So my question is why this is not a problem >when datapathsees a wrong ifindex, wouldn't that break the defined team >policy ? Thank you very much for your help. Yes, it would. For a packet from time to time. Is it a problem? > >Regards, >Miao

2 3

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

libteam February 2019