From: Jiri Pirko jiri@mellanox.com
In teamd the messages from kernel are processed in order: options ifinfo ports
However, kernel sends the messages in a different order. See: team_upper_dev_link() which generates ifinfo notification __team_port_change_port_added() which generates ports notification __team_options_change_check() which generates options notification
So there is a chance that the teamd processed only the port without options and right after that it processes ifinfo.
Fix this by introducing teamd_port_enabled_check() which does not log error and ignore the port in case this call fails. This is safe as this is going to be handled by future link_watch_enabled_option_changed() call.
Signed-off-by: Jiri Pirko jiri@mellanox.com Fixes: 5f351666409f ("teamd: add port_master_ifindex_changed for link_watch_port_watch_ops") Tested-by: Stepan Blyshchak stepanb@mellanox.com --- teamd/teamd.h | 2 ++ teamd/teamd_link_watch.c | 13 ++++++++++--- teamd/teamd_per_port.c | 24 +++++++++++++++++++----- 3 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/teamd/teamd.h b/teamd/teamd.h index fb2872efd0ac..f94c918cfe52 100644 --- a/teamd/teamd.h +++ b/teamd/teamd.h @@ -325,6 +325,8 @@ int teamd_port_remove_all(struct teamd_context *ctx); void teamd_port_obj_remove_all(struct teamd_context *ctx); int teamd_port_enabled(struct teamd_context *ctx, struct teamd_port *tdport, bool *enabled); +int teamd_port_enabled_check(struct teamd_context *ctx, + struct teamd_port *tdport, bool *enabled); int teamd_port_prio(struct teamd_context *ctx, struct teamd_port *tdport); int teamd_port_check_enable(struct teamd_context *ctx, struct teamd_port *tdport, diff --git a/teamd/teamd_link_watch.c b/teamd/teamd_link_watch.c index 5c626eb3e636..cae6549d613a 100644 --- a/teamd/teamd_link_watch.c +++ b/teamd/teamd_link_watch.c @@ -423,9 +423,16 @@ static int link_watch_refresh_forced_send(struct teamd_context *ctx) int err;
teamd_for_each_tdport(tdport, ctx) { - err = teamd_port_enabled(ctx, tdport, &port_enabled); - if (err) - return err; + err = teamd_port_enabled_check(ctx, tdport, &port_enabled); + if (err) { + /* Looks like the options are not ready for this port. + * This can happen when called from + * link_watch_port_master_ifindex_changed(). Skip this + * for now, let it be handled by future call of + * link_watch_enabled_option_changed(). + */ + continue; + } __set_forced_send_for_port(tdport, port_enabled); if (port_enabled) enabled_port_count++; diff --git a/teamd/teamd_per_port.c b/teamd/teamd_per_port.c index 327aefbe9a44..166da576ef7c 100644 --- a/teamd/teamd_per_port.c +++ b/teamd/teamd_per_port.c @@ -389,19 +389,21 @@ int teamd_port_remove_ifname(struct teamd_context *ctx, const char *port_name) return teamd_port_remove(ctx, tdport); }
-int teamd_port_enabled(struct teamd_context *ctx, struct teamd_port *tdport, - bool *enabled) +int __teamd_port_enabled(struct teamd_context *ctx, struct teamd_port *tdport, + bool *enabled, bool may_fail) { struct team_option *option;
option = team_get_option(ctx->th, "np", "enabled", tdport->ifindex); if (!option) { - teamd_log_err("%s: Failed to find "enabled" option.", - tdport->ifname); + if (!may_fail) + teamd_log_err("%s: Failed to find "enabled" option.", + tdport->ifname); return -ENOENT; } if (team_get_option_type(option) != TEAM_OPTION_TYPE_BOOL) { - teamd_log_err("Unexpected type of "enabled" option."); + if (!may_fail) + teamd_log_err("Unexpected type of "enabled" option."); return -EINVAL; }
@@ -409,6 +411,18 @@ int teamd_port_enabled(struct teamd_context *ctx, struct teamd_port *tdport, return 0; }
+int teamd_port_enabled(struct teamd_context *ctx, struct teamd_port *tdport, + bool *enabled) +{ + return __teamd_port_enabled(ctx, tdport, enabled, false); +} + +int teamd_port_enabled_check(struct teamd_context *ctx, + struct teamd_port *tdport, bool *enabled) +{ + return __teamd_port_enabled(ctx, tdport, enabled, true); +} + int teamd_port_prio(struct teamd_context *ctx, struct teamd_port *tdport) { int prio;
On Mon, Feb 3, 2020 at 7:07 PM Jiri Pirko jiri@resnulli.us wrote:
From: Jiri Pirko jiri@mellanox.com
In teamd the messages from kernel are processed in order: options ifinfo ports
However, kernel sends the messages in a different order. See: team_upper_dev_link() which generates ifinfo notification __team_port_change_port_added() which generates ports notification __team_options_change_check() which generates options notification
So there is a chance that the teamd processed only the port without options and right after that it processes ifinfo.
Fix this by introducing teamd_port_enabled_check() which does not log error and ignore the port in case this call fails. This is safe as this is going to be handled by future link_watch_enabled_option_changed() call.
Signed-off-by: Jiri Pirko jiri@mellanox.com Fixes: 5f351666409f ("teamd: add port_master_ifindex_changed for link_watch_port_watch_ops") Tested-by: Stepan Blyshchak stepanb@mellanox.com
Reviewed-by: Xin Long lucien.xin@gmail.com
teamd/teamd.h | 2 ++ teamd/teamd_link_watch.c | 13 ++++++++++--- teamd/teamd_per_port.c | 24 +++++++++++++++++++----- 3 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/teamd/teamd.h b/teamd/teamd.h index fb2872efd0ac..f94c918cfe52 100644 --- a/teamd/teamd.h +++ b/teamd/teamd.h @@ -325,6 +325,8 @@ int teamd_port_remove_all(struct teamd_context *ctx); void teamd_port_obj_remove_all(struct teamd_context *ctx); int teamd_port_enabled(struct teamd_context *ctx, struct teamd_port *tdport, bool *enabled); +int teamd_port_enabled_check(struct teamd_context *ctx,
struct teamd_port *tdport, bool *enabled);int teamd_port_prio(struct teamd_context *ctx, struct teamd_port *tdport); int teamd_port_check_enable(struct teamd_context *ctx, struct teamd_port *tdport, diff --git a/teamd/teamd_link_watch.c b/teamd/teamd_link_watch.c index 5c626eb3e636..cae6549d613a 100644 --- a/teamd/teamd_link_watch.c +++ b/teamd/teamd_link_watch.c @@ -423,9 +423,16 @@ static int link_watch_refresh_forced_send(struct teamd_context *ctx) int err;
teamd_for_each_tdport(tdport, ctx) {
err = teamd_port_enabled(ctx, tdport, &port_enabled);if (err)return err;
err = teamd_port_enabled_check(ctx, tdport, &port_enabled);if (err) {/* Looks like the options are not ready for this port.* This can happen when called from* link_watch_port_master_ifindex_changed(). Skip this* for now, let it be handled by future call of* link_watch_enabled_option_changed().*/continue;} __set_forced_send_for_port(tdport, port_enabled); if (port_enabled) enabled_port_count++;diff --git a/teamd/teamd_per_port.c b/teamd/teamd_per_port.c index 327aefbe9a44..166da576ef7c 100644 --- a/teamd/teamd_per_port.c +++ b/teamd/teamd_per_port.c @@ -389,19 +389,21 @@ int teamd_port_remove_ifname(struct teamd_context *ctx, const char *port_name) return teamd_port_remove(ctx, tdport); }
-int teamd_port_enabled(struct teamd_context *ctx, struct teamd_port *tdport,
bool *enabled)+int __teamd_port_enabled(struct teamd_context *ctx, struct teamd_port *tdport,
bool *enabled, bool may_fail){ struct team_option *option;
option = team_get_option(ctx->th, "np", "enabled", tdport->ifindex); if (!option) {
teamd_log_err("%s: Failed to find \"enabled\" option.",tdport->ifname);
if (!may_fail)teamd_log_err("%s: Failed to find \"enabled\" option.",tdport->ifname); return -ENOENT; } if (team_get_option_type(option) != TEAM_OPTION_TYPE_BOOL) {
teamd_log_err("Unexpected type of \"enabled\" option.");
if (!may_fail)teamd_log_err("Unexpected type of \"enabled\" option."); return -EINVAL; }@@ -409,6 +411,18 @@ int teamd_port_enabled(struct teamd_context *ctx, struct teamd_port *tdport, return 0; }
+int teamd_port_enabled(struct teamd_context *ctx, struct teamd_port *tdport,
bool *enabled)+{
return __teamd_port_enabled(ctx, tdport, enabled, false);+}
+int teamd_port_enabled_check(struct teamd_context *ctx,
struct teamd_port *tdport, bool *enabled)+{
return __teamd_port_enabled(ctx, tdport, enabled, true);+}
int teamd_port_prio(struct teamd_context *ctx, struct teamd_port *tdport) { int prio; -- 2.21.0
Mon, Feb 03, 2020 at 04:54:35PM CET, lucien.xin@gmail.com wrote:
On Mon, Feb 3, 2020 at 7:07 PM Jiri Pirko jiri@resnulli.us wrote:
From: Jiri Pirko jiri@mellanox.com
In teamd the messages from kernel are processed in order: options ifinfo ports
However, kernel sends the messages in a different order. See: team_upper_dev_link() which generates ifinfo notification __team_port_change_port_added() which generates ports notification __team_options_change_check() which generates options notification
So there is a chance that the teamd processed only the port without options and right after that it processes ifinfo.
Fix this by introducing teamd_port_enabled_check() which does not log error and ignore the port in case this call fails. This is safe as this is going to be handled by future link_watch_enabled_option_changed() call.
Signed-off-by: Jiri Pirko jiri@mellanox.com Fixes: 5f351666409f ("teamd: add port_master_ifindex_changed for link_watch_port_watch_ops") Tested-by: Stepan Blyshchak stepanb@mellanox.com
Reviewed-by: Xin Long lucien.xin@gmail.com
applied, thanks!
libteam@lists.fedorahosted.org
-
Jiri Pirko -
Xin Long