teamd: update ctx->hwaddr after setting ctx->ifindex to new hwaddr
diff mbox series

Message ID 20191119103100.9687-1-liuhangbin@gmail.com
State New
Headers show
Series
  • teamd: update ctx->hwaddr after setting ctx->ifindex to new hwaddr
Related show

Commit Message

Hangbin Liu Nov. 19, 2019, 10:31 a.m. UTC
When we add the first slave to team port, we will update ctx->ifindex
with new hwaddr in function

teamd_event_watch_port_added()
  - teamd_hwaddr_check_change(),

But we didn't update the ctx->hwaddr, which will cause the first added
slave set to team's init hwaddr again later. e.g. in the following functions

lacp_port_set_mac()
lb_event_watch_port_added()
ab_hwaddr_policy_same_all_port_added().

The tdport's hwaddr will be reset based on ctx->hwaddr. Fix it by updating
ctx->hwaddr when set ctx->ifindex to new hwaddr.

Note: function teamd_set_hwaddr() is not considered as it will set
ctx->hwaddr_explicit = true.

Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 teamd/teamd.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Jiri Pirko Nov. 28, 2019, 4:59 p.m. UTC | #1
Tue, Nov 19, 2019 at 11:31:00AM CET, liuhangbin@gmail.com wrote:
>When we add the first slave to team port, we will update ctx->ifindex
>with new hwaddr in function

I don't understand this sentence :(


>
>teamd_event_watch_port_added()
>  - teamd_hwaddr_check_change(),
>
>But we didn't update the ctx->hwaddr, which will cause the first added
>slave set to team's init hwaddr again later. e.g. in the following functions

I don't understand this either :/


>
>lacp_port_set_mac()
>lb_event_watch_port_added()
>ab_hwaddr_policy_same_all_port_added().
>
>The tdport's hwaddr will be reset based on ctx->hwaddr. Fix it by updating
>ctx->hwaddr when set ctx->ifindex to new hwaddr.

"set ifindex to new hwaddr"

I think I know (from the code) what you want to do, but the description
didn't help me. On contratry, it makes me confused.

Could you please fix the description?

>
>Note: function teamd_set_hwaddr() is not considered as it will set
>ctx->hwaddr_explicit = true.
>
>Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
>---
> teamd/teamd.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/teamd/teamd.c b/teamd/teamd.c
>index 6c47312..9622da1 100644
>--- a/teamd/teamd.c
>+++ b/teamd/teamd.c
>@@ -867,7 +867,7 @@ static int teamd_add_ports(struct teamd_context *ctx)
> static int teamd_hwaddr_check_change(struct teamd_context *ctx,
> 				     struct teamd_port *tdport)
> {
>-	const char *hwaddr;
>+	char *hwaddr;
> 	unsigned char hwaddr_len;
> 	int err;
> 
>@@ -885,6 +885,8 @@ static int teamd_hwaddr_check_change(struct teamd_context *ctx,
> 		teamd_log_err("Failed to set team device hardware address.");
> 		return err;
> 	}
>+	ctx->hwaddr = hwaddr;
>+	ctx->hwaddr_len = hwaddr_len;
> 	return 0;
> }
> 
>-- 
>2.19.2
>

Patch
diff mbox series

diff --git a/teamd/teamd.c b/teamd/teamd.c
index 6c47312..9622da1 100644
--- a/teamd/teamd.c
+++ b/teamd/teamd.c
@@ -867,7 +867,7 @@  static int teamd_add_ports(struct teamd_context *ctx)
 static int teamd_hwaddr_check_change(struct teamd_context *ctx,
 				     struct teamd_port *tdport)
 {
-	const char *hwaddr;
+	char *hwaddr;
 	unsigned char hwaddr_len;
 	int err;
 
@@ -885,6 +885,8 @@  static int teamd_hwaddr_check_change(struct teamd_context *ctx,
 		teamd_log_err("Failed to set team device hardware address.");
 		return err;
 	}
+	ctx->hwaddr = hwaddr;
+	ctx->hwaddr_len = hwaddr_len;
 	return 0;
 }