teamd: remove port if adding fails
diff mbox series

Message ID 20190227080557.23348-1-liuhangbin@gmail.com
State New
Headers show
Series
  • teamd: remove port if adding fails
Related show

Commit Message

Hangbin Liu Feb. 27, 2019, 8:05 a.m. UTC
When we add a port to team via teamdctl, some drivers do not support changing
mac address after dev opened, which would lead to the failure of
port_obj_create().

Currently we only destroy the port object if adding port fails. But the port
is still enslaved to team in kernel. The interface shows it's a team_slave
via ip link cmd, but teamdctl state shows nothing. This may make users
confused.

Fix it by removing the port if adding fails.

Reported-by: Vladimir Benes <vbenes@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 teamd/teamd_per_port.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Jiri Pirko March 8, 2019, 8:53 a.m. UTC | #1
Wed, Feb 27, 2019 at 09:05:57AM CET, liuhangbin@gmail.com wrote:
>When we add a port to team via teamdctl, some drivers do not support changing
>mac address after dev opened, which would lead to the failure of
>port_obj_create().

Where exactly this failure happens?


>
>Currently we only destroy the port object if adding port fails. But the port
>is still enslaved to team in kernel. The interface shows it's a team_slave
>via ip link cmd, but teamdctl state shows nothing. This may make users
>confused.
>
>Fix it by removing the port if adding fails.
>
>Reported-by: Vladimir Benes <vbenes@redhat.com>
>Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>---
> teamd/teamd_per_port.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/teamd/teamd_per_port.c b/teamd/teamd_per_port.c
>index 09d1dc7..7f0a45d 100644
>--- a/teamd/teamd_per_port.c
>+++ b/teamd/teamd_per_port.c
>@@ -42,6 +42,8 @@ struct port_obj {
> };
> 
> #define _port(port_obj) (&(port_obj)->port)
>+static int teamd_port_remove(struct teamd_context *ctx,
>+			     struct teamd_port *tdport);
> 
> int teamd_port_priv_create_and_get(void **ppriv, struct teamd_port *tdport,
> 				   const struct teamd_port_priv *pp,
>@@ -203,6 +205,7 @@ static int port_obj_create(struct teamd_context *ctx,
> teamd_event_port_removed:
> 	teamd_event_port_removed(ctx, tdport);
> list_del:
>+	teamd_port_remove(ctx, tdport);

This is not in sync with port_obj_remove() :/


> 	port_obj_destroy(ctx, port_obj);
> 	port_obj_free(port_obj);
> 	return err;
>-- 
>2.19.2
>
Hangbin Liu March 8, 2019, 10:53 a.m. UTC | #2
On Fri, Mar 08, 2019 at 09:53:49AM +0100, Jiri Pirko wrote:
> Wed, Feb 27, 2019 at 09:05:57AM CET, liuhangbin@gmail.com wrote:
> >When we add a port to team via teamdctl, some drivers do not support changing
> >mac address after dev opened, which would lead to the failure of
> >port_obj_create().
> 
> Where exactly this failure happens?

for LACP mode:
port_obj_create()
  - port_priv_init_all()
    - lacp_port_added()
      - lacp_port_set_mac()
> 
> >@@ -203,6 +205,7 @@ static int port_obj_create(struct teamd_context *ctx,
> > teamd_event_port_removed:
> > 	teamd_event_port_removed(ctx, tdport);
> > list_del:
> >+	teamd_port_remove(ctx, tdport);
> 
> This is not in sync with port_obj_remove() :/

Oh, I will add this in port_obj_remove()

Thanks
Hangbin

Patch
diff mbox series

diff --git a/teamd/teamd_per_port.c b/teamd/teamd_per_port.c
index 09d1dc7..7f0a45d 100644
--- a/teamd/teamd_per_port.c
+++ b/teamd/teamd_per_port.c
@@ -42,6 +42,8 @@  struct port_obj {
 };
 
 #define _port(port_obj) (&(port_obj)->port)
+static int teamd_port_remove(struct teamd_context *ctx,
+			     struct teamd_port *tdport);
 
 int teamd_port_priv_create_and_get(void **ppriv, struct teamd_port *tdport,
 				   const struct teamd_port_priv *pp,
@@ -203,6 +205,7 @@  static int port_obj_create(struct teamd_context *ctx,
 teamd_event_port_removed:
 	teamd_event_port_removed(ctx, tdport);
 list_del:
+	teamd_port_remove(ctx, tdport);
 	port_obj_destroy(ctx, port_obj);
 	port_obj_free(port_obj);
 	return err;