teamd: return 0 if tdport doesn't exist in teamd_config_port_set

Commit Message

Xin Long July 1, 2019, 7:05 a.m. UTC

This issue can be reproduced by doing:

  # ip link add dummy1 type dummy
  # teamd -t team0 -c '{"runner": {"name": "activebackup"}}' -d
  # teamdctl team0 port config update dummy1 '{"prio": -10}'
  # ip link set dummy0 master team0

and the error shows:

  libteamdctl: usock: Error message received: "ConfigUpdateFail"
  libteamdctl: usock: Error message content: "Failed to update config."
  command call failed (Invalid argument)

It's a regression caused by Commit c8b356a3cd36 ("teamd: config: update
local prio to kernel") where it requires the tdport has to exist when
a tdport config is being updated. However teamd supports for the port
config going first before the port being enslaved.

This issue breaks how NM-team starts a team device. Here to fix it by
returning 0 even if the tdport doesn't exist in teamd_config_port_set.
While at it, also improve some error log and code type.

Reported-by: Radek Vykydal <[e-mail]>
Fixes: c8b356a3cd36 ("teamd: config: update local prio to kernel")
Signed-off-by: Xin Long <[e-mail]>
---
 teamd/teamd_config.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

Comments

Jiri Pirko July 2, 2019, 10:07 a.m. UTC | #1

Mon, Jul 01, 2019 at 09:05:50AM CEST, [e-mail] wrote:
>This issue can be reproduced by doing:
>
>  # ip link add dummy1 type dummy
>  # teamd -t team0 -c '{"runner": {"name": "activebackup"}}' -d
>  # teamdctl team0 port config update dummy1 '{"prio": -10}'
>  # ip link set dummy0 master team0
>
>and the error shows:
>
>  libteamdctl: usock: Error message received: "ConfigUpdateFail"
>  libteamdctl: usock: Error message content: "Failed to update config."
>  command call failed (Invalid argument)
>
>It's a regression caused by Commit c8b356a3cd36 ("teamd: config: update
>local prio to kernel") where it requires the tdport has to exist when
>a tdport config is being updated. However teamd supports for the port
>config going first before the port being enslaved.
>
>This issue breaks how NM-team starts a team device. Here to fix it by
>returning 0 even if the tdport doesn't exist in teamd_config_port_set.
>While at it, also improve some error log and code type.

No, please do it in a separate patch/patches. One change per patch.
Thanks!

Patch

diff --git a/teamd/teamd_config.c b/teamd/teamd_config.c
index 34fef1f..610ad5f 100644
--- a/teamd/teamd_config.c
+++ b/teamd/teamd_config.c
@@ -164,20 +164,22 @@  static int teamd_config_port_set(struct teamd_context *ctx, const char *port_nam
 
 	tdport = teamd_get_port_by_ifname(ctx, port_name);
 	if (!tdport)
-		return -ENODEV;
+		return 0;
 
 	config = json_object_get(port_obj, "prio");
-	if (json_is_integer(config)) {
-		tmp = json_integer_value(config);
-		err = team_set_port_priority(ctx->th, tdport->ifindex, tmp);
-		if (err) {
-			teamd_log_err("%s: Failed to set \"priority\".",
-				      tdport->ifname);
-			return err;
-		}
+	if (!json_is_integer(config)) {
+		teamd_log_err("%s: Failed to get integer for \"priority\".",
+			      tdport->ifname);
+		return -ENOENT;
 	}
 
-	return 0;
+	tmp = json_integer_value(config);
+	err = team_set_port_priority(ctx->th, tdport->ifindex, tmp);
+	if (err)
+		teamd_log_err("%s: Failed to update \"priority\" to kernel",
+			      tdport->ifname);
+
+	return err;
 }
 
 int teamd_config_port_update(struct teamd_context *ctx, const char *port_name,
@@ -206,16 +208,14 @@  int teamd_config_port_update(struct teamd_context *ctx, const char *port_name,
 	/* replace existing object content */
 	json_object_clear(port_obj);
 	err = json_object_update(port_obj, port_new_obj);
-	if (err)
+	if (err) {
 		teamd_log_err("%s: Failed to update existing config "
 			      "port object", port_name);
-	else {
-		err = teamd_config_port_set(ctx, port_name, port_new_obj);
-		if (err)
-			teamd_log_err("%s: Failed to update config to kernel",
-				      port_name);
+		goto new_port_decref;
 	}
 
+	err = teamd_config_port_set(ctx, port_name, port_new_obj);
+
 new_port_decref:
 	json_decref(port_new_obj);
 	return err;