[PATCH] Do not fail teamd_add_ports() when one port is missing

[Pawel Wieczorkiewicz]

When multiple ports are specified in the config and one of
them is not present (e.g. usb2eth device) during team instance
start up time, instead of failing the whole setup, just report
failure with a missing port and carry on with the rest.

Signed-off-by: Pawel Wieczorkiewicz <pwieczorkiewicz(a)suse.de&gt;
---
 teamd/teamd_per_port.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/teamd/teamd_per_port.c b/teamd/teamd_per_port.c
index 0e62091..b85a425 100644
--- a/teamd/teamd_per_port.c
+++ b/teamd/teamd_per_port.c
@@ -340,6 +340,11 @@ int teamd_port_add_ifname(struct teamd_context *ctx, const char
*port_name)
 	teamd_log_dbg("%s: Adding port (found ifindex \"%d\").",
 		      port_name, ifindex);
 	err = team_port_add(ctx->th, ifindex);
+	if (err == -ENODEV) {
+		teamd_log_err("%s: Failed to add missing port.", port_name);
+		return 0;
+	}
+
 	if (err)
 		teamd_log_err("%s: Failed to add port.", port_name);
 	return err;
-- 
2.1.4

[Jiri Pirko]

Mon, Sep 07, 2015 at 06:00:31PM CEST, pwieczorkiewicz(a)suse.de wrote:
> When multiple ports are specified in the config and one of
> them is not present (e.g. usb2eth device) during team instance
> start up time, instead of failing the whole setup, just report
> failure with a missing port and carry on with the rest.
>
> Signed-off-by: Pawel Wieczorkiewicz <pwieczorkiewicz(a)suse.de&gt;
> ---
>  teamd/teamd_per_port.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/teamd/teamd_per_port.c b/teamd/teamd_per_port.c
> index 0e62091..b85a425 100644
> --- a/teamd/teamd_per_port.c
> +++ b/teamd/teamd_per_port.c
> @@ -340,6 +340,11 @@ int teamd_port_add_ifname(struct teamd_context *ctx, const char
> *port_name)
>  	teamd_log_dbg("%s: Adding port (found ifindex \"%d\").",
>  		      port_name, ifindex);
>  	err = team_port_add(ctx->th, ifindex);
> +	if (err == -ENODEV) {
> +		teamd_log_err("%s: Failed to add missing port.", port_name);
> +		return 0;
> +	}
> + 
This is not correct. In case user is adding port via ctl
(teamd_ctl_method_port_add), this would make it return success for
non-existent device.

What is your use-case? You device ports in config file?

[Pawel Wieczorkiewicz]

On Tue, 8 Sep 2015 08:11:05 +0200
Jiri Pirko <jiri(a)resnulli.us&gt; wrote:

>  >@@ -340,6 +340,11 @@ int teamd_port_add_ifname(struct
> teamd_context
>  >*ctx, const char *port_name)
>  > 	teamd_log_dbg("%s: Adding port (found ifindex \"%d\").",
>  > 		      port_name, ifindex);
>  > 	err = team_port_add(ctx->th, ifindex);
>  >+	if (err == -ENODEV) {
>  >+		teamd_log_err("%s: Failed to add missing port.",
>  >port_name);
>  >+		return 0;
>  >+	}
>  >+
>  
>  This is not correct. In case user is adding port via ctl
>  (teamd_ctl_method_port_add), this would make it return success for
>  non-existent device.
>   
Yes, you are right. I chose wrong function (a general one). The better
one could be IMO:

diff --git a/teamd/teamd.c b/teamd/teamd.c
index a7c0f77..8102c79 100644
--- a/teamd/teamd.c
+++ b/teamd/teamd.c
@@ -846,6 +846,8 @@ static int teamd_add_ports(struct teamd_context
*ctx) 
        teamd_config_for_each_key(key, ctx, "$.ports") {
                err = teamd_port_add_ifname(ctx, key);
+               if (err == -ENODEV)
+                       continue;
                if (err)
                        return err;
        }

>  What is your use-case? You device ports in config file? 
The point here is to avoid a general failure (no team device up) when
one of the ports specified in the config is missing. I.e. config
contains three eth ports, one is missing and we start team0 up. Instead
of failing completely it would be better to report port add failure and
setup device with available ports.

When some PortAdd method is used to add non-existing port to existing
teamd instance, indeed an error should be reported.

-- 
Best Regards,
Pawel Wieczorkiewicz <pwieczorkiewicz(a)suse.de&gt;, Linux System Developer

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu,
Graham Norton, HRB 21284 (AG Nürnberg) Maxfeldstraße 5 / 90409
Nürnberg / Germany / Phone: [phone] - 613

[Jiri Pirko]

Tue, Sep 08, 2015 at 10:50:06AM CEST, pwieczorkiewicz(a)suse.de wrote:
> On Tue, 8 Sep 2015 08:11:05 +0200
> Jiri Pirko <jiri(a)resnulli.us&gt; wrote:
>
> > >@@ -340,6 +340,11 @@ int teamd_port_add_ifname(struct teamd_context
> > >*ctx, const char *port_name)
> > > 	teamd_log_dbg("%s: Adding port (found ifindex \"%d\").",
> > > 		      port_name, ifindex);
> > > 	err = team_port_add(ctx->th, ifindex);
> > >+	if (err == -ENODEV) {
> > >+		teamd_log_err("%s: Failed to add missing port.",
> > >port_name);
> > >+		return 0;
> > >+	}
> > >+
> > 
> > This is not correct. In case user is adding port via ctl
> > (teamd_ctl_method_port_add), this would make it return success for
> > non-existent device.
> > 
>
> Yes, you are right. I chose wrong function (a general one). The better
> one could be IMO:
>
> diff --git a/teamd/teamd.c b/teamd/teamd.c
> index a7c0f77..8102c79 100644
> --- a/teamd/teamd.c
> +++ b/teamd/teamd.c
> @@ -846,6 +846,8 @@ static int teamd_add_ports(struct teamd_context
> *ctx) 
>         teamd_config_for_each_key(key, ctx, "$.ports") {
>                 err = teamd_port_add_ifname(ctx, key);
> +               if (err == -ENODEV)
> +                       continue;
>                 if (err)
>                         return err;
>         }
>
> > What is your use-case? You device ports in config file?
>
> The point here is to avoid a general failure (no team device up) when
> one of the ports specified in the config is missing. I.e. config
> contains three eth ports, one is missing and we start team0 up. Instead
> of failing completely it would be better to report port add failure and
> setup device with available ports. 
Okay. But please, log a warning in case err == -ENODEV. Thanks.


> When some PortAdd method is used to add non-existing port to existing
> teamd instance, indeed an error should be reported.
>
> -- 
> Best Regards,
> Pawel Wieczorkiewicz <pwieczorkiewicz(a)suse.de&gt;, Linux System Developer
>
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu,
> Graham Norton, HRB 21284 (AG Nürnberg) Maxfeldstraße 5 / 90409
> Nürnberg / Germany / Phone: [phone] - 613