Skip to content

Commit

Permalink
[teamd] prevent re-entrance of port priv change handler (sonic-net#2723)
Browse files Browse the repository at this point in the history
When adding a lag member dynamically after system boots up, teamd
port priv change handler could re-entrant itself and causing adding
operation to fail.

While handling PORT_CHANGE event, teamd_per_port.c port priv change
handler was called, it will then call runner_lacp to add port to lag,
the later causes IFINFO_CHANGE to be notified and calls the priv change
handler again, this re-entrance would cause runner_lacp port_added to
be called again and messes up with the previous adding sequence. Then
fails the lag member adding operation.

Prevent per port priv change handler re-entrance solves the problem.

Signed-off-by: Ying Xie <ying.xie@microsoft.com>
  • Loading branch information
yxieca authored and tiantianlv committed Apr 10, 2019
1 parent 54c805e commit fca6b90
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
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

1 change: 1 addition & 0 deletions src/libteam/series
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@
0006-Fix-ifinfo_link_with_port-race-condition-with-newlink.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

0 comments on commit fca6b90

Please sign in to comment.