[teamd] do not process lacpdu before the port ifinfo is set (#2815)
Port libteam patch which fixes the race condition we observed during warm reboot. Remove early patches: 0006, 0008, 0009. Signed-off-by: Ying Xie <ying.xie@microsoft.com>
This commit is contained in:
parent
675a89959a
commit
e4a663a606
@ -1,45 +0,0 @@
|
|||||||
From 9a27e9b85afbbf6235e61c2426481e49a2139219 Mon Sep 17 00:00:00 2001
|
|
||||||
From: Shu0T1an ChenG <shuche@microsoft.com>
|
|
||||||
Date: Tue, 15 Jan 2019 12:23:02 -0800
|
|
||||||
Subject: [PATCH] Fix ifinfo_link_with_port race condition with newlink
|
|
||||||
|
|
||||||
The race condition could happen like this:
|
|
||||||
When an interface is enslaved into the port channel immediately after
|
|
||||||
it is created, the order of creating the ifinfo and linking the ifinfo to
|
|
||||||
the port is not guaranteed.
|
|
||||||
|
|
||||||
The team handler will listen to both netlink message to track new links
|
|
||||||
get created to allocate the ifinfo and add the ifinfo into its linked list,
|
|
||||||
and the team port change message to link the new port with ifinfo found
|
|
||||||
in its linkedin list. However, when the ifinfo is not yet created, the error
|
|
||||||
message "Failed to link port with ifinfo" is thrown with member port failed
|
|
||||||
to be added into the team handler's port list.
|
|
||||||
|
|
||||||
This fix adds a condition to check if ifinfo_link_with_port is linking ifinfo
|
|
||||||
to a port or to the team interface itself. If it is a port, ifinfo_find_create
|
|
||||||
function is used to fix the race condition.
|
|
||||||
|
|
||||||
Signed-off-by: Shu0T1an ChenG <shuche@microsoft.com>
|
|
||||||
---
|
|
||||||
libteam/ifinfo.c | 5 ++++-
|
|
||||||
1 file changed, 4 insertions(+), 1 deletion(-)
|
|
||||||
|
|
||||||
diff --git a/libteam/ifinfo.c b/libteam/ifinfo.c
|
|
||||||
index 44de4ca..444e0cd 100644
|
|
||||||
--- a/libteam/ifinfo.c
|
|
||||||
+++ b/libteam/ifinfo.c
|
|
||||||
@@ -429,7 +429,10 @@ int ifinfo_link_with_port(struct team_handle *th, uint32_t ifindex,
|
|
||||||
{
|
|
||||||
struct team_ifinfo *ifinfo;
|
|
||||||
|
|
||||||
- ifinfo = ifinfo_find(th, ifindex);
|
|
||||||
+ if (port)
|
|
||||||
+ ifinfo = ifinfo_find_create(th, ifindex);
|
|
||||||
+ else
|
|
||||||
+ ifinfo = ifinfo_find(th, ifindex);
|
|
||||||
if (!ifinfo)
|
|
||||||
return -ENOENT;
|
|
||||||
if (ifinfo->linked)
|
|
||||||
--
|
|
||||||
2.1.4
|
|
||||||
|
|
@ -1,34 +0,0 @@
|
|||||||
From 7dff9798c2c92eb75b0120737efb81febcdb80c1 Mon Sep 17 00:00:00 2001
|
|
||||||
From: Ying Xie <ying.xie@microsoft.com>
|
|
||||||
Date: Sun, 24 Mar 2019 21:49:59 +0000
|
|
||||||
Subject: [PATCH] [teamd] register change handler for TEAM_IFINFO_CHANGE as
|
|
||||||
well
|
|
||||||
|
|
||||||
There has been a race condition in the libnal/teamd interation, causing
|
|
||||||
TEAM_PORT_CHANGE to report a port with empty device name. Which then
|
|
||||||
causes the teamd unable to add the lag members into the lag.
|
|
||||||
|
|
||||||
Registering to the TEAM_IFINFO_CHANGE would give teamd another chance to
|
|
||||||
add member again.
|
|
||||||
|
|
||||||
Signed-off-by: Ying Xie <ying.xie@microsoft.com>
|
|
||||||
---
|
|
||||||
teamd/teamd_per_port.c | 2 +-
|
|
||||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
|
||||||
|
|
||||||
diff --git a/teamd/teamd_per_port.c b/teamd/teamd_per_port.c
|
|
||||||
index 09d1dc7..137da57 100644
|
|
||||||
--- a/teamd/teamd_per_port.c
|
|
||||||
+++ b/teamd/teamd_per_port.c
|
|
||||||
@@ -274,7 +274,7 @@ static int port_priv_change_handler_func(struct team_handle *th, void *priv,
|
|
||||||
|
|
||||||
static const struct team_change_handler port_priv_change_handler = {
|
|
||||||
.func = port_priv_change_handler_func,
|
|
||||||
- .type_mask = TEAM_PORT_CHANGE,
|
|
||||||
+ .type_mask = TEAM_PORT_CHANGE | TEAM_IFINFO_CHANGE,
|
|
||||||
};
|
|
||||||
|
|
||||||
int teamd_per_port_init(struct teamd_context *ctx)
|
|
||||||
--
|
|
||||||
2.7.4
|
|
||||||
|
|
@ -1,90 +0,0 @@
|
|||||||
From fb00b070482dc587eec7b4e34acec094be1af00d Mon Sep 17 00:00:00 2001
|
|
||||||
From: Ying Xie <ying.xie@microsoft.com>
|
|
||||||
Date: Fri, 29 Mar 2019 20:54:49 +0000
|
|
||||||
Subject: [PATCH 10/11] [teamd] prevent private change handler reentrance
|
|
||||||
|
|
||||||
While handling PORT_CHANGE, teamd could casue an INTERFACE_CHANGE in the
|
|
||||||
same context. Which will interfere with the PORT_CHANGE handling and
|
|
||||||
causing it to fail.
|
|
||||||
|
|
||||||
Lock is not needed because the re-entrance happened in the same thread
|
|
||||||
context.
|
|
||||||
|
|
||||||
This issue was noticed while dynamically adding a port into a lag.
|
|
||||||
|
|
||||||
Signed-off-by: Ying Xie <ying.xie@microsoft.com>
|
|
||||||
---
|
|
||||||
teamd/teamd.c | 2 ++
|
|
||||||
teamd/teamd.h | 2 ++
|
|
||||||
teamd/teamd_per_port.c | 13 +++++++++++--
|
|
||||||
3 files changed, 15 insertions(+), 2 deletions(-)
|
|
||||||
|
|
||||||
diff --git a/teamd/teamd.c b/teamd/teamd.c
|
|
||||||
index e28aa7d..140b98b 100644
|
|
||||||
--- a/teamd/teamd.c
|
|
||||||
+++ b/teamd/teamd.c
|
|
||||||
@@ -1255,6 +1255,8 @@ static int teamd_init(struct teamd_context *ctx)
|
|
||||||
{
|
|
||||||
int err;
|
|
||||||
|
|
||||||
+ ctx->reentrant = false;
|
|
||||||
+
|
|
||||||
ctx->th = team_alloc();
|
|
||||||
if (!ctx->th) {
|
|
||||||
teamd_log_err("Team alloc failed.");
|
|
||||||
diff --git a/teamd/teamd.h b/teamd/teamd.h
|
|
||||||
index 622c365..7cd3266 100644
|
|
||||||
--- a/teamd/teamd.h
|
|
||||||
+++ b/teamd/teamd.h
|
|
||||||
@@ -160,6 +160,8 @@ struct teamd_context {
|
|
||||||
int pipe_r;
|
|
||||||
int pipe_w;
|
|
||||||
} workq;
|
|
||||||
+
|
|
||||||
+ bool reentrant;
|
|
||||||
};
|
|
||||||
|
|
||||||
struct teamd_port {
|
|
||||||
diff --git a/teamd/teamd_per_port.c b/teamd/teamd_per_port.c
|
|
||||||
index 137da57..8b4a457 100644
|
|
||||||
--- a/teamd/teamd_per_port.c
|
|
||||||
+++ b/teamd/teamd_per_port.c
|
|
||||||
@@ -250,6 +250,10 @@ static int port_priv_change_handler_func(struct team_handle *th, void *priv,
|
|
||||||
struct port_obj *port_obj;
|
|
||||||
int err;
|
|
||||||
|
|
||||||
+ if (ctx->reentrant) {
|
|
||||||
+ return 0;
|
|
||||||
+ }
|
|
||||||
+ ctx->reentrant = true;
|
|
||||||
team_for_each_port(port, th) {
|
|
||||||
uint32_t ifindex = team_get_port_ifindex(port);
|
|
||||||
|
|
||||||
@@ -258,17 +262,22 @@ static int port_priv_change_handler_func(struct team_handle *th, void *priv,
|
|
||||||
if (team_is_port_removed(port))
|
|
||||||
continue;
|
|
||||||
err = port_obj_create(ctx, &port_obj, ifindex, port);
|
|
||||||
- if (err)
|
|
||||||
+ if (err) {
|
|
||||||
+ ctx->reentrant = false;
|
|
||||||
return err;
|
|
||||||
+ }
|
|
||||||
}
|
|
||||||
if (team_is_port_changed(port)) {
|
|
||||||
err = teamd_event_port_changed(ctx, _port(port_obj));
|
|
||||||
- if (err)
|
|
||||||
+ if (err) {
|
|
||||||
+ ctx->reentrant = false;
|
|
||||||
return err;
|
|
||||||
+ }
|
|
||||||
}
|
|
||||||
if (team_is_port_removed(port))
|
|
||||||
port_obj_remove(ctx, port_obj);
|
|
||||||
}
|
|
||||||
+ ctx->reentrant = false;
|
|
||||||
return 0;
|
|
||||||
}
|
|
||||||
|
|
||||||
--
|
|
||||||
2.7.4
|
|
||||||
|
|
@ -0,0 +1,46 @@
|
|||||||
|
From 28ccb37eb388e7e3d214a9b05011f8421f0d65ac Mon Sep 17 00:00:00 2001
|
||||||
|
From: Ying Xie <ying.xie@microsoft.com>
|
||||||
|
Date: Fri, 26 Apr 2019 23:30:38 +0000
|
||||||
|
Subject: [PATCH] teamd: do not process lacpdu before the port ifinfo is set
|
||||||
|
|
||||||
|
Now the port ifinfo will be set in obj_input_newlink when a RTM_NEWLINK
|
||||||
|
event is received.
|
||||||
|
|
||||||
|
But when a port is being added, if a lacpdu gets received on this port
|
||||||
|
before the RTM_NEWLINK event, lacpdu_recv will process the packet with
|
||||||
|
incorrect port ifinfo.
|
||||||
|
|
||||||
|
In Patrick's case, as ifinfo->master_ifindex was 0, it would skip this
|
||||||
|
port in teamd_for_each_tdport, which caused lacp_port->agg_lead not to
|
||||||
|
be updated in lacp_switch_agg_lead. Later the lacp_port actor would go
|
||||||
|
to a unexpected state.
|
||||||
|
|
||||||
|
This patch is to avoid it by checking teamd_port_present in lacpdu_recv
|
||||||
|
so that it would not process lacpdu before the port ifinfo is set.
|
||||||
|
|
||||||
|
Reported-by: Patrick Talbert <ptalbert@redhat.com>
|
||||||
|
Tested-by: Patrick Talbert <ptalbert@redhat.com>
|
||||||
|
Signed-off-by: Xin Long <lucien.xin@gmail.com>
|
||||||
|
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
|
||||||
|
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
|
||||||
|
---
|
||||||
|
teamd/teamd_runner_lacp.c | 3 +++
|
||||||
|
1 file changed, 3 insertions(+)
|
||||||
|
|
||||||
|
diff --git a/teamd/teamd_runner_lacp.c b/teamd/teamd_runner_lacp.c
|
||||||
|
index 78f05dd..c8ae541 100644
|
||||||
|
--- a/teamd/teamd_runner_lacp.c
|
||||||
|
+++ b/teamd/teamd_runner_lacp.c
|
||||||
|
@@ -1132,6 +1132,9 @@ static int lacpdu_process(struct lacp_port *lacp_port, struct lacpdu* lacpdu)
|
||||||
|
{
|
||||||
|
int err;
|
||||||
|
|
||||||
|
+ if (!teamd_port_present(lacp_port->ctx, lacp_port->tdport))
|
||||||
|
+ return 0;
|
||||||
|
+
|
||||||
|
if (!lacpdu_check(lacpdu)) {
|
||||||
|
teamd_log_warn("malformed LACP PDU came.");
|
||||||
|
return 0;
|
||||||
|
--
|
||||||
|
2.7.4
|
||||||
|
|
@ -3,9 +3,7 @@
|
|||||||
0003-teamd-lacp-runner-will-send-lacp-update-right-after-.patch
|
0003-teamd-lacp-runner-will-send-lacp-update-right-after-.patch
|
||||||
0004-libteam-Add-lacp-fallback-support-for-single-member-.patch
|
0004-libteam-Add-lacp-fallback-support-for-single-member-.patch
|
||||||
0005-libteam-Add-warm_reboot-mode.patch
|
0005-libteam-Add-warm_reboot-mode.patch
|
||||||
0006-Fix-ifinfo_link_with_port-race-condition-with-newlink.patch
|
|
||||||
0007-Skip-setting-the-same-hwaddr-to-lag-port-to-avoid-di.patch
|
0007-Skip-setting-the-same-hwaddr-to-lag-port-to-avoid-di.patch
|
||||||
0008-teamd-register-change-handler-for-TEAM_IFINFO_CHANGE.patch
|
|
||||||
0009-teamd-prevent-private-change-handler-reentrance.patch
|
|
||||||
0010-teamd-lacp-update-port-state-according-to-partners-sy.patch
|
0010-teamd-lacp-update-port-state-according-to-partners-sy.patch
|
||||||
0011-libteam-resynchronize-ifinfo-after-lost-RTNLGRP_LINK-.patch
|
0011-libteam-resynchronize-ifinfo-after-lost-RTNLGRP_LINK-.patch
|
||||||
|
0012-teamd-do-not-process-lacpdu-before-the-port-ifinfo-i.patch
|
||||||
|
Reference in New Issue
Block a user