teamd: fix possible race in master ifname callback

jpirko · jpirko · commit c7237377dead · 2020-02-04T18:16:04.000+01:00
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>
diff --git 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
@@ -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
@@ -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;
