libteam May 2020

libteam@lists.fedorahosted.org

5 participants
7 discussions

Re: [PATCH] teamd: Disregard current state when considering port enablement
by Jiri Pirko 09 Sep '20

09 Sep '20

Wed, Nov 13, 2019 at 02:26:47PM CET, petrm(a)mellanox.com wrote: >On systems where carrier is gained very quickly, there is a race between >teamd and the kernel that sometimes leads to all team slaves being stuck in >enabled=false state. > >When a port is enslaved to a team device, the kernel sends a netlink >message marking the port as enabled. teamd's lb_event_watch_port_added() >calls team_set_port_enabled(false), because link is down at that point. The >kernel responds with a message marking the port as disabled. At this point, >there are two outstanding messages: the initial one marking port as >enabled, and the second one marking it as disabled. teamd has not processed >either of these. > >Next teamd gets the netlink message that sets enabled=true, and updates its >internal cache accordingly. If at this point ethtool link-watch wakes up, >teamd considers (in teamd_port_check_enable()) enabling the port. After >consulting the cache, it concludes the port is already up, and neglects to >do so. Only then does teamd get the netlink message informing it of setting >enabled=false. > >The problem is that the teamd cache is not synchronous with respect to the >kernel state. If the carrier takes a while to come up (as is normally the >case), this is not a problem, because teamd caches up quickly enough. But >this may not always be the case, and particularly on a simulated system, >the carrier is gained almost immediately. > >Fix this by not suppressing the enablement message. > >Signed-off-by: Petr Machata <petrm(a)mellanox.com> applied. Thanks!

2 2

[libteam PATCH] teamd/lacp: silence ignore none LACP frames
by Hangbin Liu 11 Jun '20

11 Jun '20

According to 802.3, Annex 43B, section 4, aside from LACP, the Slow Protocol linktype is also to be used by other protocols, like 0x02 for LAMP, 0x03 for OAM. So let's only check LACP frames. For none LACP protocols, just silence ignore. Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com> --- teamd/teamd_runner_lacp.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c index 11d02f1..9437f05 100644 --- a/teamd/teamd_runner_lacp.c +++ b/teamd/teamd_runner_lacp.c @@ -96,17 +96,27 @@ static void lacpdu_init(struct lacpdu *lacpdu) static bool lacpdu_check(struct lacpdu *lacpdu) { + /* + * According to Annex 43B, section 4, aside from LACP, the Slow + * Protocol linktype is also to be used by other protocols, like + * 0x02 for LAMP, 0x03 for OAM. So for none LACP protocols, just + * silence ignore. + */ + if (lacpdu->subtype != 0x01) + return false; + /* * According to 43.4.12 version_number, tlv_type and reserved fields * should not be checked. */ - if (lacpdu->subtype != 0x01 || - lacpdu->actor_info_len != 0x14 || + if (lacpdu->actor_info_len != 0x14 || lacpdu->partner_info_len != 0x14 || lacpdu->collector_info_len != 0x10 || - lacpdu->terminator_info_len != 0x00) + lacpdu->terminator_info_len != 0x00) { + teamd_log_warn("malformed LACP PDU came."); return false; + } return true; } @@ -1088,10 +1098,8 @@ static int lacpdu_recv(struct lacp_port *lacp_port) if (!teamd_port_present(lacp_port->ctx, lacp_port->tdport)) return 0; - if (!lacpdu_check(&lacpdu)) { - teamd_log_warn("malformed LACP PDU came."); + if (!lacpdu_check(&lacpdu)) return 0; - } /* Check if we have correct info about the other side */ if (memcmp(&lacpdu.actor, &lacp_port->partner, -- 2.25.4

2 2

[PATCH libteam] teamd: fix ctx->hwaddr value assignment
by Hangbin Liu 22 May '20

22 May '20

We should use memcpy to copy the data to ctx->hwaddr. Use ctx->hwaddr = hwaddr will change the ctx->hwaddr to a wrong address and cause the team hwaddr unable to be updated. Reported-by: Li Liang <liali(a)redhat.com> Suggested-by: Xin Long <lucien.xin(a)gmail.com> Fixes: 9ca6bf9 ("teamd: update ctx->hwaddr after setting team dev to new hwaddr") Signed-off-by: Hangbin Liu <liuhangbin(a)gmail.com> --- teamd/teamd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/teamd/teamd.c b/teamd/teamd.c index f955b19..9360cbf 100644 --- a/teamd/teamd.c +++ b/teamd/teamd.c @@ -896,7 +896,7 @@ static int teamd_hwaddr_check_change(struct teamd_context *ctx, teamd_log_err("Failed to set team device hardware address."); return err; } - ctx->hwaddr = hwaddr; + memcpy(ctx->hwaddr, hwaddr, hwaddr_len); ctx->hwaddr_len = hwaddr_len; return 0; } -- 2.25.4

2 1

[PATCH] Send LACP PDU right after the Actor state has been changed
by Pavel Shirshov 15 May '20

15 May '20

According to the LACP standard "LACP daemon must send LACP PDU packets with updates when the Actor's state changes or when it is apparent from the Partner's LACPDUs that the Partner does not know the Actor's current state." The current libteam implementation sends periodic updates only, so in LACP slow rate mode the update will be transmitted within 30 seconds which doesn't follow the LACP standard. To fix the issue: 1. Revert the patch: b2de61b39696 ("Fix sending duplicate LACP frames at the start of establishing a logical channel.") 2. Recalculate LACP actor state before the comparison what the LACP Partner thinks about the LACP Actor state. Signed-off-by: Pavel Shirshov <pavel.contrib(a)gmail.com> --- teamd/teamd_runner_lacp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c index ec01237..11d02f1 100644 --- a/teamd/teamd_runner_lacp.c +++ b/teamd/teamd_runner_lacp.c @@ -996,8 +996,7 @@ static int lacp_port_set_state(struct lacp_port *lacp_port, return err; lacp_port_actor_update(lacp_port); - if (lacp_port->periodic_on) - return 0; + return lacpdu_send(lacp_port); } @@ -1110,9 +1109,10 @@ static int lacpdu_recv(struct lacp_port *lacp_port) if (err) return err; + lacp_port_actor_update(lacp_port); + /* Check if the other side has correct info about us */ - if (!lacp_port->periodic_on && - memcmp(&lacpdu.partner, &lacp_port->actor, + if (memcmp(&lacpdu.partner, &lacp_port->actor, sizeof(struct lacpdu_info))) { err = lacpdu_send(lacp_port); if (err) -- 2.7.4

2 1

link aggregation plus active-backup
by d tbsky 11 May '20

11 May '20

Hi: I have 2 server connected with a two port 10g nic, and and one port gigabit nic. (total three ports). I want to do lacp with the two 10g ports. but since the ports come from the same nic, I want to use the gigabit nic as failover backup. can I make lacp team0 with the two 10g port, then make active-backup team1 with team0 and gigabit nic? I wonder if this kind of setup is doable and stable. thanks a lot for your help!!

3 3

[PATCH] Skip setting the same hwaddr to a lag port if not needed
by Pavel Shirshov 11 May '20

11 May '20

Avoid setting the same mac address to a LAG port, if the LAG port already has the right one. Benefits: 1. Make libteam avoid changing settings of a link, which prevents the kernel from sending multiply Netlink messages. Netlink message listeners don't need to react to the Netlink messages. 2. Libteam works faster. It doesn't need to use any syscalls and go deep into libteam functions in the case when nothing is changed. A straightforward check and libteam avoid much work. In the case, when libteam user has hundreds of ports, and up to a hundred LAGs, this patch saves them significant CPU time. Signed-off-by: Pavel Shirshov <pavel.contrib(a)gmail.com> --- teamd/teamd.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/teamd/teamd.c b/teamd/teamd.c index 8cdc16d..f955b19 100644 --- a/teamd/teamd.c +++ b/teamd/teamd.c @@ -837,7 +837,14 @@ static int teamd_set_hwaddr(struct teamd_context *ctx) err = -EINVAL; goto free_hwaddr; } - err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len); + + if (memcmp(hwaddr, ctx->hwaddr, hwaddr_len)) + err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len); + else { + err = 0; + teamd_log_dbg(ctx, "Skip setting same hwaddr string: \"%s\".", hwaddr_str); + } + if (!err) ctx->hwaddr_explicit = true; free_hwaddr: -- 2.7.4

2 1

[PATCH] Skip setting the same hwaddr to a lag port if not needed
by Pavel Shirshov 04 May '20

04 May '20

Avoid setting the same mac address to a LAG port, if the LAG port already has the right one. Signed-off-by: Pavel Shirshov <pavel.contrib(a)gmail.com> --- teamd/teamd.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/teamd/teamd.c b/teamd/teamd.c index 8cdc16d..f955b19 100644 --- a/teamd/teamd.c +++ b/teamd/teamd.c @@ -837,7 +837,14 @@ static int teamd_set_hwaddr(struct teamd_context *ctx) err = -EINVAL; goto free_hwaddr; } - err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len); + + if (memcmp(hwaddr, ctx->hwaddr, hwaddr_len)) + err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len); + else { + err = 0; + teamd_log_dbg(ctx, "Skip setting same hwaddr string: \"%s\".", hwaddr_str); + } + if (!err) ctx->hwaddr_explicit = true; free_hwaddr: -- 2.7.4

2 4

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

libteam May 2020