Skip to content

Commit

Permalink
teamd: fix possible race in master ifname callback
Browse files Browse the repository at this point in the history
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: 5f35166 ("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>
  • Loading branch information
jpirko committed Feb 4, 2020
1 parent 471fb50 commit c723737
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 8 deletions.
2 changes: 2 additions & 0 deletions teamd/teamd.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
13 changes: 10 additions & 3 deletions teamd/teamd_link_watch.c
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
Expand Down
24 changes: 19 additions & 5 deletions teamd/teamd_per_port.c
Original file line number Diff line number Diff line change
Expand Up @@ -389,26 +389,40 @@ 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;
}

*enabled = team_get_option_value_bool(option);
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;
Expand Down

0 comments on commit c723737

Please sign in to comment.