Skip setting the same hwaddr to a lag port if not needed
diff mbox series

Message ID 1587145415-16372-1-git-send-email-pavel.contrib@gmail.com
State New
Headers show
Series
  • Skip setting the same hwaddr to a lag port if not needed
Related show

Commit Message

Pavel Shirshov April 17, 2020, 5:43 p.m. UTC
Avoid setting the same mac address to a LAG port,
if the LAG port already has the right one.

Signed-off-by: Pavel Shirshov <pavel.contrib@gmail.com>
---
 teamd/teamd.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Jiri Pirko April 25, 2020, 6 a.m. UTC | #1
Fri, Apr 17, 2020 at 07:43:35PM CEST, pavel.contrib@gmail.com wrote:
>Avoid setting the same mac address to a LAG port,
>if the LAG port already has the right one.

What's the benefit added by this patch?


>
>Signed-off-by: Pavel Shirshov <pavel.contrib@gmail.com>
>---
> teamd/teamd.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
>diff --git a/teamd/teamd.c b/teamd/teamd.c
>index 8cdc16d..f955b19 100644
>--- a/teamd/teamd.c
>+++ b/teamd/teamd.c
>@@ -837,7 +837,14 @@ static int teamd_set_hwaddr(struct teamd_context *ctx)
> 		err = -EINVAL;
> 		goto free_hwaddr;
> 	}
>-	err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len);
>+
>+	if (memcmp(hwaddr, ctx->hwaddr, hwaddr_len))
>+		err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len);
>+	else {
>+		err = 0;
>+		teamd_log_dbg(ctx, "Skip setting same hwaddr string: \"%s\".", hwaddr_str);
>+	}
>+
> 	if (!err)
> 		ctx->hwaddr_explicit = true;
> free_hwaddr:
>-- 
>2.7.4
>
Pavel Shirshov April 27, 2020, 7:58 p.m. UTC | #2
Hi Jiri,


Thank you, for reviewing the patch.


This patch has the following benefits:

   1. We avoid changing settings of a link. Which prevents the kernel from
   sending multiply Netlink messages. That will benefit Netlink message
   listeners. They will not need to react to the Netlink messages.
   2. It makes libteam faster. We don't need to use any syscalls and go
   deep into libteam functions when we don't need to. One simple check and
   libteam can avoid a lot of work.

In our case, when we have hundreds of ports, and up to a hundred LAGs, this
patch saves us significant time.


Please let me know if you have additional questions.


Thanks

On Fri, Apr 24, 2020 at 11:01 PM Jiri Pirko <jiri@resnulli.us> wrote:

> Fri, Apr 17, 2020 at 07:43:35PM CEST, pavel.contrib@gmail.com wrote:
> >Avoid setting the same mac address to a LAG port,
> >if the LAG port already has the right one.
>
> What's the benefit added by this patch?
>
>
> >
> >Signed-off-by: Pavel Shirshov <pavel.contrib@gmail.com>
> >---
> > teamd/teamd.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> >diff --git a/teamd/teamd.c b/teamd/teamd.c
> >index 8cdc16d..f955b19 100644
> >--- a/teamd/teamd.c
> >+++ b/teamd/teamd.c
> >@@ -837,7 +837,14 @@ static int teamd_set_hwaddr(struct teamd_context
> *ctx)
> >               err = -EINVAL;
> >               goto free_hwaddr;
> >       }
> >-      err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len);
> >+
> >+      if (memcmp(hwaddr, ctx->hwaddr, hwaddr_len))
> >+              err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr,
> hwaddr_len);
> >+      else {
> >+              err = 0;
> >+              teamd_log_dbg(ctx, "Skip setting same hwaddr string:
> \"%s\".", hwaddr_str);
> >+      }
> >+
> >       if (!err)
> >               ctx->hwaddr_explicit = true;
> > free_hwaddr:
> >--
> >2.7.4
> >
>
Jiri Pirko May 4, 2020, 1:40 p.m. UTC | #3
Mon, Apr 27, 2020 at 09:58:45PM CEST, pavel.contrib@gmail.com wrote:
>Hi Jiri,
>
>
>Thank you, for reviewing the patch.
>
>
>This patch has the following benefits:
>
>   1. We avoid changing settings of a link. Which prevents the kernel from
>   sending multiply Netlink messages. That will benefit Netlink message
>   listeners. They will not need to react to the Netlink messages.
>   2. It makes libteam faster. We don't need to use any syscalls and go
>   deep into libteam functions when we don't need to. One simple check and
>   libteam can avoid a lot of work.
>
>In our case, when we have hundreds of ports, and up to a hundred LAGs, this
>patch saves us significant time.


Could you please add this info into the patch description?

Thanks!


>
>
>Please let me know if you have additional questions.
>
>
>Thanks
>
>On Fri, Apr 24, 2020 at 11:01 PM Jiri Pirko <jiri@resnulli.us> wrote:
>
>> Fri, Apr 17, 2020 at 07:43:35PM CEST, pavel.contrib@gmail.com wrote:
>> >Avoid setting the same mac address to a LAG port,
>> >if the LAG port already has the right one.
>>
>> What's the benefit added by this patch?
>>
>>
>> >
>> >Signed-off-by: Pavel Shirshov <pavel.contrib@gmail.com>
>> >---
>> > teamd/teamd.c | 9 ++++++++-
>> > 1 file changed, 8 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/teamd/teamd.c b/teamd/teamd.c
>> >index 8cdc16d..f955b19 100644
>> >--- a/teamd/teamd.c
>> >+++ b/teamd/teamd.c
>> >@@ -837,7 +837,14 @@ static int teamd_set_hwaddr(struct teamd_context
>> *ctx)
>> >               err = -EINVAL;
>> >               goto free_hwaddr;
>> >       }
>> >-      err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len);
>> >+
>> >+      if (memcmp(hwaddr, ctx->hwaddr, hwaddr_len))
>> >+              err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr,
>> hwaddr_len);
>> >+      else {
>> >+              err = 0;
>> >+              teamd_log_dbg(ctx, "Skip setting same hwaddr string:
>> \"%s\".", hwaddr_str);
>> >+      }
>> >+
>> >       if (!err)
>> >               ctx->hwaddr_explicit = true;
>> > free_hwaddr:
>> >--
>> >2.7.4
>> >
>>
Pavel Shirshov May 4, 2020, 7:59 p.m. UTC | #4
Hi Jiri,

Sure. Done. I've sent the patch to the list.

Thanks

On Mon, May 4, 2020 at 6:40 AM Jiri Pirko <jiri@resnulli.us> wrote:

> Mon, Apr 27, 2020 at 09:58:45PM CEST, pavel.contrib@gmail.com wrote:
> >Hi Jiri,
> >
> >
> >Thank you, for reviewing the patch.
> >
> >
> >This patch has the following benefits:
> >
> >   1. We avoid changing settings of a link. Which prevents the kernel from
> >   sending multiply Netlink messages. That will benefit Netlink message
> >   listeners. They will not need to react to the Netlink messages.
> >   2. It makes libteam faster. We don't need to use any syscalls and go
> >   deep into libteam functions when we don't need to. One simple check and
> >   libteam can avoid a lot of work.
> >
> >In our case, when we have hundreds of ports, and up to a hundred LAGs,
> this
> >patch saves us significant time.
>
>
> Could you please add this info into the patch description?
>
> Thanks!
>
>
> >
> >
> >Please let me know if you have additional questions.
> >
> >
> >Thanks
> >
> >On Fri, Apr 24, 2020 at 11:01 PM Jiri Pirko <jiri@resnulli.us> wrote:
> >
> >> Fri, Apr 17, 2020 at 07:43:35PM CEST, pavel.contrib@gmail.com wrote:
> >> >Avoid setting the same mac address to a LAG port,
> >> >if the LAG port already has the right one.
> >>
> >> What's the benefit added by this patch?
> >>
> >>
> >> >
> >> >Signed-off-by: Pavel Shirshov <pavel.contrib@gmail.com>
> >> >---
> >> > teamd/teamd.c | 9 ++++++++-
> >> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >> >
> >> >diff --git a/teamd/teamd.c b/teamd/teamd.c
> >> >index 8cdc16d..f955b19 100644
> >> >--- a/teamd/teamd.c
> >> >+++ b/teamd/teamd.c
> >> >@@ -837,7 +837,14 @@ static int teamd_set_hwaddr(struct teamd_context
> >> *ctx)
> >> >               err = -EINVAL;
> >> >               goto free_hwaddr;
> >> >       }
> >> >-      err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr,
> hwaddr_len);
> >> >+
> >> >+      if (memcmp(hwaddr, ctx->hwaddr, hwaddr_len))
> >> >+              err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr,
> >> hwaddr_len);
> >> >+      else {
> >> >+              err = 0;
> >> >+              teamd_log_dbg(ctx, "Skip setting same hwaddr string:
> >> \"%s\".", hwaddr_str);
> >> >+      }
> >> >+
> >> >       if (!err)
> >> >               ctx->hwaddr_explicit = true;
> >> > free_hwaddr:
> >> >--
> >> >2.7.4
> >> >
> >>
>

Patch
diff mbox series

diff --git a/teamd/teamd.c b/teamd/teamd.c
index 8cdc16d..f955b19 100644
--- a/teamd/teamd.c
+++ b/teamd/teamd.c
@@ -837,7 +837,14 @@  static int teamd_set_hwaddr(struct teamd_context *ctx)
 		err = -EINVAL;
 		goto free_hwaddr;
 	}
-	err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len);
+
+	if (memcmp(hwaddr, ctx->hwaddr, hwaddr_len))
+		err = team_hwaddr_set(ctx->th, ctx->ifindex, hwaddr, hwaddr_len);
+	else {
+		err = 0;
+		teamd_log_dbg(ctx, "Skip setting same hwaddr string: \"%s\".", hwaddr_str);
+	}
+
 	if (!err)
 		ctx->hwaddr_explicit = true;
 free_hwaddr: