[patchv2] teamd: remove port if adding fails
diff mbox series

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

Commit Message

Hangbin Liu March 13, 2019, 7:04 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(). The call path looks like below for LACP mode:

- port_obj_create()
  - port_priv_init_all()
    - lacp_port_added()
      - lacp_port_set_mac()

Currently, we only destroy the port object if adding port fails. But the
port is still enslaved to team in kernel. IP link command shows the port
is a team_slave, but teamdctl state shows nothing. This may make users
confused.

Fix it by removing the port if adding fails.

v2: also calls teamd_port_remove in port_obj_remove()

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

Comments

Hangbin Liu April 1, 2019, 9:26 a.m. UTC | #1
On Wed, Mar 13, 2019 at 03:04:29PM +0800, Hangbin Liu 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(). The call path looks like below for LACP mode:
> 
> - port_obj_create()
>   - port_priv_init_all()
>     - lacp_port_added()
>       - lacp_port_set_mac()
> 
> Currently, we only destroy the port object if adding port fails. But the
> port is still enslaved to team in kernel. IP link command shows the port
> is a team_slave, but teamdctl state shows nothing. This may make users
> confused.
> 
> Fix it by removing the port if adding fails.
> 
> v2: also calls teamd_port_remove in port_obj_remove()
> 
> Reported-by: Vladimir Benes <vbenes@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

Hi Jiri,

Would you please help review this patch?

Thanks
Hangbin
Jiri Pirko April 17, 2019, 7:36 a.m. UTC | #2
Wed, Mar 13, 2019 at 08:04:29AM 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(). The call path looks like below for LACP mode:
>
>- port_obj_create()
>  - port_priv_init_all()
>    - lacp_port_added()
>      - lacp_port_set_mac()
>
>Currently, we only destroy the port object if adding port fails. But the
>port is still enslaved to team in kernel. IP link command shows the port
>is a team_slave, but teamdctl state shows nothing. This may make users
>confused.
>
>Fix it by removing the port if adding fails.
>
>v2: also calls teamd_port_remove in port_obj_remove()
>
>Reported-by: Vladimir Benes <vbenes@redhat.com>
>Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>

applied.

Patch
diff mbox series

diff --git a/teamd/teamd_per_port.c b/teamd/teamd_per_port.c
index 09d1dc7..f98a90d 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;
@@ -214,6 +217,7 @@  static void port_obj_remove(struct teamd_context *ctx,
 	struct teamd_port *tdport = _port(port_obj);
 
 	teamd_event_port_removed(ctx, tdport);
+	teamd_port_remove(ctx, tdport);
 	port_obj_destroy(ctx, port_obj);
 	port_obj_free(port_obj);
 }